From 7b29ad830ea45e7fdac5990902f0ab45cdeff29c Mon Sep 17 00:00:00 2001 From: Donald Cook Date: Fri, 27 Jan 2023 10:11:31 -0800 Subject: [PATCH 01/19] Add a feature flag to disable at all mention Changelog: changed --- config/feature_flags/development/disable_all_mention.yml | 8 ++++++++ .../status_page/reference_links_examples.rb | 1 + lib/banzai/filter/references/user_reference_filter.rb | 2 +- spec/features/markdown/markdown_spec.rb | 2 ++ .../filter/references/user_reference_filter_spec.rb | 2 ++ spec/mailers/emails/service_desk_spec.rb | 1 + spec/services/notification_service_spec.rb | 2 ++ .../shared_examples/models/mentionable_shared_examples.rb | 4 ++++ 8 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 config/feature_flags/development/disable_all_mention.yml diff --git a/config/feature_flags/development/disable_all_mention.yml b/config/feature_flags/development/disable_all_mention.yml new file mode 100644 index 00000000000000..f638c0391e83a7 --- /dev/null +++ b/config/feature_flags/development/disable_all_mention.yml @@ -0,0 +1,8 @@ +--- +name: disable_all_mention +introduced_by_url: +rollout_issue_url: +milestone: '15.9' +type: development +group: group::project management +default_enabled: false diff --git a/ee/spec/support/shared_examples/status_page/reference_links_examples.rb b/ee/spec/support/shared_examples/status_page/reference_links_examples.rb index a612c1ae3b4655..e0a16ca84ae888 100644 --- a/ee/spec/support/shared_examples/status_page/reference_links_examples.rb +++ b/ee/spec/support/shared_examples/status_page/reference_links_examples.rb @@ -10,6 +10,7 @@ let(:gfm_reference) { reference.to_reference(full: true) } before do + stub_feature_flags(disable_all_mention: false) project.add_guest(author) unless project.team.member?(author) project.update!(visibility_level: project_visibility) object.update!(field => gfm_reference) diff --git a/lib/banzai/filter/references/user_reference_filter.rb b/lib/banzai/filter/references/user_reference_filter.rb index 5983036a8e5267..b994c02f4c15ab 100644 --- a/lib/banzai/filter/references/user_reference_filter.rb +++ b/lib/banzai/filter/references/user_reference_filter.rb @@ -45,7 +45,7 @@ def call # have `gfm` and `gfm-project_member` class names attached for styling. def object_link_filter(text, pattern, link_content: nil, link_reference: false) references_in(text, pattern) do |match, username| - if username == 'all' && !skip_project_check? + if username == 'all' && !skip_project_check? && !Feature.enabled?(:disable_all_mention) link_to_all(link_content: link_content) else cached_call(:banzai_url_for_object, match, path: [User, username.downcase]) do diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 9bb0220004a550..27be779d3db885 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -221,6 +221,7 @@ def doc(html = @html) context 'default pipeline' do before do + stub_feature_flags(disable_all_mention: false) @html = markdown(@feat.raw_markdown) end @@ -316,6 +317,7 @@ def doc(html = @html) blob = double(name: name, path: path, mime_type: 'image/jpeg', data: nil) expect(@wiki).to receive(:find_file).with(path, load_content: false).and_return(Gitlab::Git::WikiFile.new(blob)) allow(@wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' } + stub_feature_flags(disable_all_mention: false) @html = markdown(@feat.raw_markdown, { pipeline: :wiki, wiki: @wiki, page_slug: @wiki_page.slug }) end diff --git a/spec/lib/banzai/filter/references/user_reference_filter_spec.rb b/spec/lib/banzai/filter/references/user_reference_filter_spec.rb index e248f2d9b1c24d..47799f7226bb1e 100644 --- a/spec/lib/banzai/filter/references/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/user_reference_filter_spec.rb @@ -44,6 +44,7 @@ def get_reference(user) before do project.add_developer(project.creator) + stub_feature_flags(disable_all_mention: false) end it_behaves_like 'a reference containing an element node' @@ -161,6 +162,7 @@ def get_reference(user) let(:context) { { author: group_member, project: nil, group: group } } it 'supports a special @all mention' do + stub_feature_flags(disable_all_mention: false) reference = User.reference_prefix + 'all' doc = reference_filter("Hey #{reference}", context) diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index f8ed26b3241707..bd5527a5324207 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -52,6 +52,7 @@ end it 'builds the email correctly' do + stub_feature_flags(disable_all_mention: false) aggregate_failures do is_expected.to have_referable_subject(issue, include_project: false, reply: reply_in_subject) is_expected.to have_body_text(expected_template_html) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0e1afaa837847c..3daab91cf5c6bb 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -817,6 +817,7 @@ let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: note_content) } before_all do + stub_feature_flags(disable_all_mention: false) build_team(project) build_group(project) add_users(project) @@ -928,6 +929,7 @@ let(:note) { create(:note_on_project_snippet, author: author, noteable: snippet, project_id: project.id, note: note_content) } before do + stub_feature_flags(disable_all_mention: false) build_team(project) build_group(project) project.add_maintainer(author) diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index f9612dd61beb62..ea5440b6a138a8 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -209,6 +209,7 @@ RSpec.shared_examples 'mentions in description' do |mentionable_type| context 'when storing user mentions' do before do + stub_feature_flags(disable_all_mention: false) mentionable.store_mentions! end @@ -249,6 +250,7 @@ let!(:mentionable) { note.noteable } before do + stub_feature_flags(disable_all_mention: false) note.update!(note: note_desc) note.store_mentions! add_member(user) @@ -290,6 +292,7 @@ let_it_be(:note_desc) { "#{mentioned_user.to_reference} and #{group.to_reference(full: true)} and @all" } before do + stub_feature_flags(disable_all_mention: false) note.update!(note: note_desc) note.store_mentions! add_member(user) @@ -341,6 +344,7 @@ let(:group_member) { create(:group_member, user: create(:user), group: private_group) } before do + stub_feature_flags(disable_all_mention: false) user_mention = note.user_mentions.first mention_ids = { mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << private_project.id, -- GitLab From f1d1b363a1b3f0724eb6ba1291e68da374f33e93 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Thu, 8 Jun 2023 12:55:41 +0900 Subject: [PATCH 02/19] Update documentation --- .../feature_flags/development/disable_all_mention.yml | 6 +++--- doc/user/discussions/index.md | 10 ++++++++++ doc/user/markdown.md | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/config/feature_flags/development/disable_all_mention.yml b/config/feature_flags/development/disable_all_mention.yml index f638c0391e83a7..103968606a2a99 100644 --- a/config/feature_flags/development/disable_all_mention.yml +++ b/config/feature_flags/development/disable_all_mention.yml @@ -1,8 +1,8 @@ --- name: disable_all_mention -introduced_by_url: -rollout_issue_url: -milestone: '15.9' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/18442 +milestone: '16.1' type: development group: group::project management default_enabled: false diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index f603354198679c..c89d8f231a557e 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -51,12 +51,22 @@ You can quickly see which comments involve you, because mentions for yourself (the user who is signed in) are highlighted in a different color. +### `@all` mention + +> - [Feature flag `disable_all_mention` added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586) in GitLab 16.1. +Ask your GitLab administrator if the feature can be disabled. + Avoid mentioning `@all` in issues and merge requests. It sends an email notification to all members of that project's parent group, not only the participants of the project. It might be interpreted as spam. Notifications and mentions can be disabled in [a group's settings](../group/manage.md#disable-email-notifications). +WARNING: +After disabling the feature, +the existing all mention references in the Markdown texts will _not_ be affected and remain as link texts. +Only the future all mention references start appearing as plain texts. + ### Mention a group in an issue or merge request When you mention a group in a comment, every member of the group gets a to-do item diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 2026a03315044c..2689ce0cb69eaf 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -630,7 +630,7 @@ GitLab Flavored Markdown recognizes the following: |:----------------------------------------------------------------------------|:------------------------------|:----------------------------------------|:-------------------------------| | specific user | `@user_name` | | | | specific group | `@group_name` | | | -| entire team | `@all` | | | +| entire team | [`@all`](discussions/index.md#all-mention) | | | | project | `namespace/project>` | | | | issue | ``#123`` | `namespace/project#123` | `project#123` | | merge request | `!123` | `namespace/project!123` | `project!123` | -- GitLab From 30386285251acc2ee4b92ee38c8d7be6df383fea Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Thu, 8 Jun 2023 15:18:40 +0900 Subject: [PATCH 03/19] Update anonymization specs --- .../incident_comment_entity_spec.rb | 2 +- .../status_page/incident_entity_spec.rb | 2 +- .../status_page/reference_links_examples.rb | 40 ++++++++++++++++++- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/ee/spec/serializers/status_page/incident_comment_entity_spec.rb b/ee/spec/serializers/status_page/incident_comment_entity_spec.rb index 1adb9398dd669d..d4a37934bd97e6 100644 --- a/ee/spec/serializers/status_page/incident_comment_entity_spec.rb +++ b/ee/spec/serializers/status_page/incident_comment_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe StatusPage::IncidentCommentEntity do +RSpec.describe StatusPage::IncidentCommentEntity, feature_category: :incident_management do let_it_be(:note, reload: true) { create(:note, note: ':ok:') } let(:json) { subject.as_json } diff --git a/ee/spec/serializers/status_page/incident_entity_spec.rb b/ee/spec/serializers/status_page/incident_entity_spec.rb index 344551f558066e..ebf450e44807f5 100644 --- a/ee/spec/serializers/status_page/incident_entity_spec.rb +++ b/ee/spec/serializers/status_page/incident_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe StatusPage::IncidentEntity do +RSpec.describe StatusPage::IncidentEntity, feature_category: :incident_management do let_it_be(:user) { create(:user) } let_it_be(:issue, reload: true) do diff --git a/ee/spec/support/shared_examples/status_page/reference_links_examples.rb b/ee/spec/support/shared_examples/status_page/reference_links_examples.rb index e0a16ca84ae888..0c2145dfa21d65 100644 --- a/ee/spec/support/shared_examples/status_page/reference_links_examples.rb +++ b/ee/spec/support/shared_examples/status_page/reference_links_examples.rb @@ -10,7 +10,6 @@ let(:gfm_reference) { reference.to_reference(full: true) } before do - stub_feature_flags(disable_all_mention: false) project.add_guest(author) unless project.team.member?(author) project.update!(visibility_level: project_visibility) object.update!(field => gfm_reference) @@ -103,7 +102,44 @@ double(:reference, name: 'All Project and Group Members', to_reference: '@all') end - include_examples 'mention anonymization' + before do + # Note: since we're testing the same reference (@all), we need to reset `field`. + # + # 1. `_html` attributes are only updated if the target attribute changes + # e.g., `description` must change to trigger an update in `description_html` + # 2. `field`` could be an attribute that cannot be blank so use a random placeholder `:tada:`. + object.update!(field => ":tada:") + end + + context 'when `disable_all_mention` FF is disabled' do + before do + stub_feature_flags(disable_all_mention: false) + + object.update!(field => gfm_reference) + end + + let(:anonymized_name) { 'Incident Responder' } + + it 'anonymizes mention' do + aggregate_failures do + expect(value).to include(anonymized_name) + expect(value).not_to include(' gfm_reference) + end + + it 'shows the mention text' do + expect(value).to include('@all') + end + end end context 'with groups' do -- GitLab From 31eda20f7411e5e0934b150570544422345aa604 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Thu, 8 Jun 2023 15:22:21 +0900 Subject: [PATCH 04/19] Check FF earlier --- lib/banzai/filter/references/user_reference_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/banzai/filter/references/user_reference_filter.rb b/lib/banzai/filter/references/user_reference_filter.rb index b994c02f4c15ab..98a2e9b8e4336f 100644 --- a/lib/banzai/filter/references/user_reference_filter.rb +++ b/lib/banzai/filter/references/user_reference_filter.rb @@ -45,7 +45,7 @@ def call # have `gfm` and `gfm-project_member` class names attached for styling. def object_link_filter(text, pattern, link_content: nil, link_reference: false) references_in(text, pattern) do |match, username| - if username == 'all' && !skip_project_check? && !Feature.enabled?(:disable_all_mention) + if !Feature.enabled?(:disable_all_mention) && username == 'all' && !skip_project_check? link_to_all(link_content: link_content) else cached_call(:banzai_url_for_object, match, path: [User, username.downcase]) do -- GitLab From 6640fb5ceb9ae970c135c17e28e53b040b99ef96 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Thu, 8 Jun 2023 17:20:43 +0900 Subject: [PATCH 05/19] Update mentionable specs --- spec/models/concerns/mentionable_spec.rb | 2 +- .../models/mentionable_shared_examples.rb | 47 ++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index d9e53fb7e9a3e5..08ecffde60bf0d 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -32,7 +32,7 @@ def author end end -RSpec.describe Issue, "Mentionable" do +RSpec.describe Issue, "Mentionable", feature_category: :team_planning do describe '#mentioned_users' do let!(:user) { create(:user, username: 'stranger') } let!(:user2) { create(:user, username: 'john') } diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index ea5440b6a138a8..67f6cb716b6001 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -208,14 +208,13 @@ RSpec.shared_examples 'mentions in description' do |mentionable_type| context 'when storing user mentions' do - before do - stub_feature_flags(disable_all_mention: false) - mentionable.store_mentions! - end - context 'when mentionable description has no mentions' do let(:mentionable) { create(mentionable_type, description: "just some description") } + before do + mentionable.store_mentions! + end + it 'stores no mentions' do expect(mentionable.user_mentions.count).to eq 0 end @@ -229,13 +228,39 @@ let(:mentionable_desc) { "#{user.to_reference} #{user2.to_reference} #{user.to_reference} some description #{group.to_reference(full: true)} and #{user2.to_reference} @all" } let(:mentionable) { create(mentionable_type, description: mentionable_desc) } - it 'stores mentions' do - add_member(user) + context 'when `disable_all_mention` FF is disabled' do + before do + stub_feature_flags(disable_all_mention: false) - expect(mentionable.user_mentions.count).to eq 1 - expect(mentionable.referenced_users).to match_array([user, user2]) - expect(mentionable.referenced_projects(user)).to match_array([mentionable.project].compact) # epic.project is nil, and we want empty [] - expect(mentionable.referenced_groups(user)).to match_array([group]) + mentionable.store_mentions! + end + + it 'stores mentions' do + add_member(user) + + expect(mentionable.user_mentions.count).to eq 1 + expect(mentionable.referenced_users).to match_array([user, user2]) + expect(mentionable.referenced_projects(user)).to match_array([mentionable.project].compact) # epic.project is nil, and we want empty [] + expect(mentionable.referenced_groups(user)).to match_array([group]) + end + end + + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + + mentionable.store_mentions! + end + + it 'stores mentions without a project reference' do + add_member(user) + + expect(mentionable.user_mentions.count).to eq 1 + expect(mentionable.referenced_users).to match_array([user, user2]) + # TODO: why? + expect(mentionable.referenced_projects(user)).to be_empty + expect(mentionable.referenced_groups(user)).to match_array([group]) + end end end end -- GitLab From 09942120f0c0683dada87bc9f6671f9bd46f2d72 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Thu, 8 Jun 2023 18:16:43 +0900 Subject: [PATCH 06/19] Update mentionable specs 2 --- .../models/mentionable_shared_examples.rb | 57 ++++++++++++++----- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index 67f6cb716b6001..62bcf074a4fd24 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -240,8 +240,20 @@ expect(mentionable.user_mentions.count).to eq 1 expect(mentionable.referenced_users).to match_array([user, user2]) - expect(mentionable.referenced_projects(user)).to match_array([mentionable.project].compact) # epic.project is nil, and we want empty [] expect(mentionable.referenced_groups(user)).to match_array([group]) + + # NOTE: https://gitlab.com/gitlab-org/gitlab/-/issues/18442 + # + # We created `Mentions` concern to track every note in which usernames are mentioned + # However, we never got to the point of utilizing the concern and its DB tables. + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/21801 + # + # The following test is checking `@all`, a type of user mention, is recording + # the id of the project for the mentionable that has the `@all` mention. + # It's _surmised_ that the original intent was + # the project id would be useful to store so everyone (@all) in the project - + # could be notified using its mention record only. + expect(mentionable.referenced_projects(user)).to match_array([mentionable.project].compact) # epic.project is nil, and we want empty [] end end @@ -252,13 +264,11 @@ mentionable.store_mentions! end - it 'stores mentions without a project reference' do + it 'stores mentions' do add_member(user) expect(mentionable.user_mentions.count).to eq 1 expect(mentionable.referenced_users).to match_array([user, user2]) - # TODO: why? - expect(mentionable.referenced_projects(user)).to be_empty expect(mentionable.referenced_groups(user)).to match_array([group]) end end @@ -274,18 +284,37 @@ let(:note_desc) { "#{user.to_reference} #{user2.to_reference} #{user.to_reference} and #{group.to_reference(full: true)} and #{user2.to_reference} @all" } let!(:mentionable) { note.noteable } - before do - stub_feature_flags(disable_all_mention: false) - note.update!(note: note_desc) - note.store_mentions! - add_member(user) + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + + note.update!(note: note_desc) + note.store_mentions! + add_member(user) + end + + it 'returns all mentionable mentions' do + expect(mentionable.user_mentions.count).to eq 1 + expect(mentionable.referenced_users).to match_array([user, user2]) + expect(mentionable.referenced_groups(user)).to eq [group] + end end - it 'returns all mentionable mentions' do - expect(mentionable.user_mentions.count).to eq 1 - expect(mentionable.referenced_users).to match_array([user, user2]) - expect(mentionable.referenced_projects(user)).to eq [mentionable.project].compact # epic.project is nil, and we want empty [] - expect(mentionable.referenced_groups(user)).to eq [group] + context 'when `disable_all_mention` FF is disabled' do + before do + stub_feature_flags(disable_all_mention: false) + + note.update!(note: note_desc) + note.store_mentions! + add_member(user) + end + + it 'returns all mentionable mentions' do + expect(mentionable.user_mentions.count).to eq 1 + expect(mentionable.referenced_users).to match_array([user, user2]) + expect(mentionable.referenced_groups(user)).to eq [group] + expect(mentionable.referenced_projects(user)).to eq [mentionable.project].compact # epic.project is nil, and we want empty [] + end end if [:epic, :issue].include?(mentionable_type) -- GitLab From 9ec9c560326ff106bf4e529c62b551d6936d1870 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Fri, 9 Jun 2023 10:36:30 +0900 Subject: [PATCH 07/19] Update merge request fixture --- spec/frontend/fixtures/merge_requests.rb | 4 ++++ spec/frontend/user_popovers_spec.js | 2 ++ 2 files changed, 6 insertions(+) diff --git a/spec/frontend/fixtures/merge_requests.rb b/spec/frontend/fixtures/merge_requests.rb index b6f6d14975670c..a1896a6470b752 100644 --- a/spec/frontend/fixtures/merge_requests.rb +++ b/spec/frontend/fixtures/merge_requests.rb @@ -114,6 +114,10 @@ let(:group) { create(:group) } let(:description) { "@#{group.full_path} @all @#{user.username}" } + before do + stub_feature_flags(disable_all_mention: false) + end + it 'merge_requests/merge_request_with_mentions.html' do render_merge_request(merge_request) end diff --git a/spec/frontend/user_popovers_spec.js b/spec/frontend/user_popovers_spec.js index 3346735055d99e..6f39eb9a118977 100644 --- a/spec/frontend/user_popovers_spec.js +++ b/spec/frontend/user_popovers_spec.js @@ -121,6 +121,8 @@ describe('User Popovers', () => { expect(findPopovers().length).toBe(0); }); + // TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/18442 + // Remove as @all is deprecated. it('does not initialize the popovers for @all references', () => { const [projectLink] = Array.from(document.querySelectorAll('.js-user-link[data-project]')); -- GitLab From c0d244692301628401ea8729ffdb7bc4b0ba0124 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Fri, 9 Jun 2023 10:39:30 +0900 Subject: [PATCH 08/19] Update user_reference_filter_spec.rb Refactor --- .../references/user_reference_filter_spec.rb | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/spec/lib/banzai/filter/references/user_reference_filter_spec.rb b/spec/lib/banzai/filter/references/user_reference_filter_spec.rb index 47799f7226bb1e..276701a298485d 100644 --- a/spec/lib/banzai/filter/references/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/user_reference_filter_spec.rb @@ -39,12 +39,31 @@ def get_reference(user) end end - context 'mentioning @all' do + context 'when `disable_all_mention` FF is enabled' do + let(:reference) { User.reference_prefix + 'all' } + + context 'mentioning @all' do + before do + stub_feature_flags(disable_all_mention: true) + + project.add_developer(project.creator) + end + + it 'ignores reference to @all' do + doc = reference_filter("Hey #{reference}", author: project.creator) + + expect(doc.css('a').length).to eq 0 + end + end + end + + context 'mentioning @all (when `disable_all_mention` FF is disabled)' do let(:reference) { User.reference_prefix + 'all' } before do - project.add_developer(project.creator) stub_feature_flags(disable_all_mention: false) + + project.add_developer(project.creator) end it_behaves_like 'a reference containing an element node' -- GitLab From ae8ac947dec71587d9075476fee7e4798d509ec6 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Fri, 9 Jun 2023 11:35:09 +0900 Subject: [PATCH 09/19] Finish updating mentionable specs --- .../models/concerns/ee/mentionable_spec.rb | 2 +- spec/models/concerns/mentionable_spec.rb | 12 +-- .../models/mentionable_shared_examples.rb | 99 ++++++++++++++++++- 3 files changed, 105 insertions(+), 8 deletions(-) diff --git a/ee/spec/models/concerns/ee/mentionable_spec.rb b/ee/spec/models/concerns/ee/mentionable_spec.rb index b0847fbbd0390d..0a76e046501d2e 100644 --- a/ee/spec/models/concerns/ee/mentionable_spec.rb +++ b/ee/spec/models/concerns/ee/mentionable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe EE::Mentionable do +RSpec.describe EE::Mentionable, feature_category: :team_planning do context Epic do describe '#store_mentions!' do it_behaves_like 'mentions in description', :epic diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 08ecffde60bf0d..ad639a7503a00d 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mentionable do +RSpec.describe Mentionable, feature_category: :shared do before do stub_const('Example', Class.new) Example.class_eval do @@ -187,7 +187,7 @@ def create_issue(description:) end end -RSpec.describe Commit, 'Mentionable' do +RSpec.describe Commit, 'Mentionable', feature_category: :source_code_management do let(:project) { create(:project, :public, :repository) } let(:commit) { project.commit } @@ -256,7 +256,7 @@ def create_issue(description:) end end -RSpec.describe MergeRequest, 'Mentionable' do +RSpec.describe MergeRequest, 'Mentionable', feature_category: :code_review_workflow do describe '#store_mentions!' do it_behaves_like 'mentions in description', :merge_request it_behaves_like 'mentions in notes', :merge_request do @@ -277,7 +277,7 @@ def create_issue(description:) end end -RSpec.describe Snippet, 'Mentionable' do +RSpec.describe Snippet, 'Mentionable', feature_category: :source_code_management do describe '#store_mentions!' do it_behaves_like 'mentions in description', :project_snippet it_behaves_like 'mentions in notes', :project_snippet do @@ -294,7 +294,7 @@ def create_issue(description:) end end -RSpec.describe PersonalSnippet, 'Mentionable' do +RSpec.describe PersonalSnippet, 'Mentionable', feature_category: :source_code_management do describe '#store_mentions!' do it_behaves_like 'mentions in description', :personal_snippet it_behaves_like 'mentions in notes', :personal_snippet do @@ -311,7 +311,7 @@ def create_issue(description:) end end -RSpec.describe DesignManagement::Design do +RSpec.describe DesignManagement::Design, feature_category: :team_planning do describe '#store_mentions!' do it_behaves_like 'mentions in notes', :design do let(:note) { create(:diff_note_on_design) } diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index 62bcf074a4fd24..9874db8dbd7485 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -324,6 +324,9 @@ let(:note_desc) { "#{guest.to_reference} and #{user2.to_reference} and #{user.to_reference}" } before do + note.update!(note: note_desc) + note.store_mentions! + add_member(user) note.resource_parent.add_reporter(user2) note.resource_parent.add_guest(guest) # Bypass :confidential update model validation for testing purposes @@ -339,7 +342,7 @@ end RSpec.shared_examples 'load mentions from DB' do |mentionable_type| - context 'load stored mentions' do + context 'load stored mentions (when `disable_all_mention` is disabled)' do let_it_be(:user) { create(:user) } let_it_be(:mentioned_user) { create(:user) } let_it_be(:group) { create(:group) } @@ -347,6 +350,7 @@ before do stub_feature_flags(disable_all_mention: false) + note.update!(note: note_desc) note.store_mentions! add_member(user) @@ -426,6 +430,99 @@ end end end + + context 'when `disable_all_mention` is enabled' do + context 'load stored mentions' do + let_it_be(:user) { create(:user) } + let_it_be(:mentioned_user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:note_desc) { "#{mentioned_user.to_reference} and #{group.to_reference(full: true)} and @all" } + + before do + stub_feature_flags(disable_all_mention: true) + + note.update!(note: note_desc) + note.store_mentions! + add_member(user) + end + + context 'when stored user mention contains ids of inexistent records' do + before do + user_mention = note.user_mentions.first + mention_ids = { + mentioned_users_ids: user_mention.mentioned_users_ids.to_a << non_existing_record_id, + mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << non_existing_record_id + } + user_mention.update!(mention_ids) + end + + it 'filters out inexistent mentions' do + expect(mentionable.referenced_users).to match_array([mentioned_user]) + expect(mentionable.referenced_projects(user)).to be_empty + expect(mentionable.referenced_groups(user)).to match_array([group]) + end + end + + if [:epic, :issue].include?(mentionable_type) + context 'and note is confidential' do + let_it_be(:guest) { create(:user) } + + let(:note_desc) { "#{guest.to_reference} and #{mentioned_user.to_reference}" } + + before do + note.resource_parent.add_reporter(mentioned_user) + note.resource_parent.add_guest(guest) + # Bypass :confidential update model validation for testing purposes + note.update_attribute(:confidential, true) + note.store_mentions! + end + + it 'stores only mentioned users that has permissions' do + expect(mentionable.referenced_users).to contain_exactly(mentioned_user) + end + end + end + + context 'when private projects and groups are mentioned' do + let(:mega_user) { create(:user) } + let(:private_project) { create(:project, :private) } + let(:project_member) { create(:project_member, user: create(:user), project: private_project) } + let(:private_group) { create(:group, :private) } + let(:group_member) { create(:group_member, user: create(:user), group: private_group) } + + before do + user_mention = note.user_mentions.first + mention_ids = { + mentioned_projects_ids: user_mention.mentioned_projects_ids.to_a << private_project.id, + mentioned_groups_ids: user_mention.mentioned_groups_ids.to_a << private_group.id + } + user_mention.update!(mention_ids) + end + + context 'when user has no access to some mentions' do + it 'filters out inaccessible mentions' do + expect(mentionable.referenced_projects(user)).to be_empty + expect(mentionable.referenced_groups(user)).to match_array([group]) + end + end + + context 'when user has access to the private project and group mentions' do + let(:user) { mega_user } + + before do + add_member(user) + private_project.add_developer(user) + private_group.add_developer(user) + end + + it 'returns all mentions' do + expect(mentionable.referenced_projects(user)).to match_array([private_project]) + expect(mentionable.referenced_groups(user)).to match_array([group, private_group]) + end + end + end + end + end end def add_member(user) -- GitLab From 3e05c85188f3c8d5c24390e95d153aa388b26ef9 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Fri, 9 Jun 2023 12:45:14 +0900 Subject: [PATCH 10/19] Update markdown spec --- spec/features/markdown/markdown_spec.rb | 14 ++++++++++++++ spec/fixtures/markdown.md.erb | 2 +- spec/support/matchers/markdown_matchers.rb | 12 ++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 27be779d3db885..5b5d676b7b1921 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -222,11 +222,24 @@ def doc(html = @html) context 'default pipeline' do before do stub_feature_flags(disable_all_mention: false) + @html = markdown(@feat.raw_markdown) end it_behaves_like 'all pipelines' + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + + @html = markdown(@feat.raw_markdown) + end + + it 'includes custom filters' do + expect(doc).to reference_users_excluding_all + end + end + it 'includes custom filters' do aggregate_failures 'UploadLinkFilter' do expect(doc).to parse_upload_links @@ -317,6 +330,7 @@ def doc(html = @html) blob = double(name: name, path: path, mime_type: 'image/jpeg', data: nil) expect(@wiki).to receive(:find_file).with(path, load_content: false).and_return(Gitlab::Git::WikiFile.new(blob)) allow(@wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' } + # TODO stub_feature_flags(disable_all_mention: false) @html = markdown(@feat.raw_markdown, { pipeline: :wiki, wiki: @wiki, page_slug: @wiki_page.slug }) diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index fa73cd53a660bd..37376713355390 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -173,7 +173,7 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e #### UserReferenceFilter -- All: @all +- All: @all (ignored when the feature flag `disable_all_mention` is enabled) - User: <%= user.to_reference %> - Group: <%= group.to_reference %> - Ignores invalid: <%= User.reference_prefix %>fake_user diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index 8fdece7b26d908..7a82d7674d9528 100644 --- a/spec/support/matchers/markdown_matchers.rb +++ b/spec/support/matchers/markdown_matchers.rb @@ -110,6 +110,18 @@ def have_image(src) end end + # UserReferenceFilter + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/18442 + # When `@all` is completely deprecated, this matcher should be renamed to + # `reference_users` and remove the original matcher `reference_users` + matcher :reference_users_excluding_all do + set_default_markdown_messages + + match do |actual| + expect(actual).to have_selector('a.gfm.gfm-project_member', count: 3) + end + end + # IssueReferenceFilter matcher :reference_issues do set_default_markdown_messages -- GitLab From b543db6cb77815169b942add4461fe25d087413c Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Fri, 9 Jun 2023 14:30:48 +0900 Subject: [PATCH 11/19] Update service desk spec --- spec/mailers/emails/service_desk_spec.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index bd5527a5324207..22b910b3dae103 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -52,7 +52,6 @@ end it 'builds the email correctly' do - stub_feature_flags(disable_all_mention: false) aggregate_failures do is_expected.to have_referable_subject(issue, include_project: false, reply: reply_in_subject) is_expected.to have_body_text(expected_template_html) @@ -299,9 +298,26 @@ let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "Hey @all, just a ping", author: User.support_bot) } let(:template_content) { 'some text %{ NOTE_TEXT }' } - let(:expected_template_html) { 'Hey , just a ping' } - it_behaves_like 'a service desk notification email with template content', 'new_note' + context 'when `disable_all_mention` is disabled' do + let(:expected_template_html) { 'Hey , just a ping' } + + before do + stub_feature_flags(disable_all_mention: false) + end + + it_behaves_like 'a service desk notification email with template content', 'new_note' + end + + context 'when `disable_all_mention` is enabled' do + let(:expected_template_html) { 'Hey @all, just a ping' } + + before do + stub_feature_flags(disable_all_mention: true) + end + + it_behaves_like 'a service desk notification email with template content', 'new_note' + end end end -- GitLab From 7b50c0f67f9dd43dd9331b03c07d40de195e4234 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Mon, 12 Jun 2023 20:35:10 +0900 Subject: [PATCH 12/19] Update notification service spec --- spec/services/notification_service_spec.rb | 136 ++++++++++++++++----- 1 file changed, 106 insertions(+), 30 deletions(-) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 3daab91cf5c6bb..99f3134f06f5ec 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -798,6 +798,7 @@ context 'issue note mention', :deliver_mails_inline do let_it_be(:issue) { create(:issue, project: project, assignees: [assignee]) } let_it_be(:mentioned_issue) { create(:issue, assignees: issue.assignees) } + let_it_be(:user_to_exclude) { create(:user) } let_it_be(:author) { create(:user) } let(:user_mentions) do @@ -817,7 +818,6 @@ let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: note_content) } before_all do - stub_feature_flags(disable_all_mention: false) build_team(project) build_group(project) add_users(project) @@ -893,18 +893,56 @@ end end - context 'when `@all` mention is used' do - let(:note_content) { "@all mentioned" } + context 'when `disable_all_mention` FF is disabled' do + before do + stub_feature_flags(disable_all_mention: false) + end + + context 'when `@all` mention is used' do + let(:note_content) { "@all mentioned" } + + it_behaves_like 'correct team members are notified' + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } - it_behaves_like 'correct team members are notified' + it_behaves_like 'correct team members are notified' + end end - context 'when users are individually mentioned' do - # `user_mentions` is concatenanting individual user mentions - # so that the end result is the same as `@all`. - let(:note_content) { "#{user_mentions} mentioned" } + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + context 'when `@all` mention is used' do + before_all do + # user_to_exclude is in the note's project but is neither mentioned nor participating. + project.add_maintainer(user_to_exclude) + end + + let(:note_content) { "@all mentioned" } - it_behaves_like 'correct team members are notified' + it "does not notify users who are not participating or mentioned" do + reset_delivered_emails! + + notification.new_note(note) + + should_email(note.noteable.author) + should_not_email(user_to_exclude) + end + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } + + it_behaves_like 'correct team members are notified' + end end end end @@ -928,21 +966,20 @@ let(:author) { create(:user) } let(:note) { create(:note_on_project_snippet, author: author, noteable: snippet, project_id: project.id, note: note_content) } - before do - stub_feature_flags(disable_all_mention: false) - build_team(project) - build_group(project) - project.add_maintainer(author) - - # make sure these users can read the project snippet! - project.add_guest(@u_guest_watcher) - project.add_guest(@u_guest_custom) - add_member_for_parent_group(@pg_watcher, project) - reset_delivered_emails! - end - describe '#new_note' do shared_examples 'correct team members are notified' do + before do + build_team(project) + build_group(project) + project.add_maintainer(author) + + # make sure these users can read the project snippet! + project.add_guest(@u_guest_watcher) + project.add_guest(@u_guest_custom) + add_member_for_parent_group(@pg_watcher, project) + reset_delivered_emails! + end + it 'notifies the team members' do notification.new_note(note) # Notify all team members @@ -967,18 +1004,57 @@ end end - context 'when `@all` mention is used' do - let(:note_content) { "@all mentioned" } + context 'when `disable_all_mention` FF is disabled' do + before do + stub_feature_flags(disable_all_mention: false) + end + + context 'when `@all` mention is used' do + let(:note_content) { "@all mentioned" } + + it_behaves_like 'correct team members are notified' + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } - it_behaves_like 'correct team members are notified' + it_behaves_like 'correct team members are notified' + end end - context 'when users are individually mentioned' do - # `user_mentions` is concatenanting individual user mentions - # so that the end result is the same as `@all`. - let(:note_content) { "#{user_mentions} mentioned" } + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + end + + context 'when `@all` mention is used' do + let(:user_to_exclude) { create(:user) } + let(:note_content) { "@all mentioned" } + + before do + project.add_maintainer(author) + project.add_maintainer(user_to_exclude) + + reset_delivered_emails! + end + + it "does not notify users who are not participating or mentioned" do + notification.new_note(note) - it_behaves_like 'correct team members are notified' + should_email(note.noteable.author) + should_not_email(user_to_exclude) + end + end + + context 'when users are individually mentioned' do + # `user_mentions` is concatenanting individual user mentions + # so that the end result is the same as `@all`. + let(:note_content) { "#{user_mentions} mentioned" } + + it_behaves_like 'correct team members are notified' + end end end end -- GitLab From d66e064aa3a6e1b0e1a6234b9f51fdeac0213d24 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Mon, 12 Jun 2023 23:27:58 +0000 Subject: [PATCH 13/19] Apply 1 suggestion(s) to 1 file(s) --- doc/user/discussions/index.md | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index c89d8f231a557e..efb0293289d57e 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -51,22 +51,27 @@ You can quickly see which comments involve you, because mentions for yourself (the user who is signed in) are highlighted in a different color. -### `@all` mention +### Mentioning all members -> - [Feature flag `disable_all_mention` added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586) in GitLab 16.1. -Ask your GitLab administrator if the feature can be disabled. +> [Flag](../../administration/feature_flags.md) named `disable_all_mention` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586) in GitLab 16.1. Disabled by default. + +FLAG: + + + +When this feature flag is enabled, typing `@all` in comments and descriptions +results in plain text instead of a mention. +When you disable this feature, existing `@all` mentions in the Markdown texts are not affected +and remain as links. Only future `@all` mentions appear as plain text. + +Avoid mentioning `@all` in comments and descriptions. +When you do it, you don't only mention the participants of the project, issue, or merge request, +but to all members of that project's parent group. +All these users receive an email notification and a to-do item. It might be interpreted as spam. -Avoid mentioning `@all` in issues and merge requests. It sends an email notification -to all members of that project's parent group, not only the participants of the project. -It might be interpreted as spam. Notifications and mentions can be disabled in [a group's settings](../group/manage.md#disable-email-notifications). -WARNING: -After disabling the feature, -the existing all mention references in the Markdown texts will _not_ be affected and remain as link texts. -Only the future all mention references start appearing as plain texts. - ### Mention a group in an issue or merge request When you mention a group in a comment, every member of the group gets a to-do item -- GitLab From 17d4caee25e7ee8aa71258cf9df0d771b1ab9edf Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Mon, 12 Jun 2023 23:28:18 +0000 Subject: [PATCH 14/19] Apply 1 suggestion(s) to 1 file(s) --- doc/user/markdown.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 2689ce0cb69eaf..401fe0bcb09e15 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -630,7 +630,7 @@ GitLab Flavored Markdown recognizes the following: |:----------------------------------------------------------------------------|:------------------------------|:----------------------------------------|:-------------------------------| | specific user | `@user_name` | | | | specific group | `@group_name` | | | -| entire team | [`@all`](discussions/index.md#all-mention) | | | +| entire team | [`@all`](discussions/index.md#mentioning-all-members) | | | | project | `namespace/project>` | | | | issue | ``#123`` | `namespace/project#123` | `project#123` | | merge request | `!123` | `namespace/project!123` | `project!123` | -- GitLab From 9b9103b6804f8dde33acb3501b5fb70d07058a00 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Tue, 13 Jun 2023 10:17:28 +0900 Subject: [PATCH 15/19] Update flag information --- doc/user/discussions/index.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index efb0293289d57e..c5fc302c8c121c 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -56,8 +56,11 @@ in a different color. > [Flag](../../administration/feature_flags.md) named `disable_all_mention` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586) in GitLab 16.1. Disabled by default. FLAG: - - +On self-managed GitLab, by default the feature is available. +To make it unavailable, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) +named `disable_all_mention`. +On GitLab.com, this feature is available as of GitLab 16.1 +but [will be made unavailble in GitLab 16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/18442) When this feature flag is enabled, typing `@all` in comments and descriptions results in plain text instead of a mention. -- GitLab From 82ce98dae6fb8177cadb9f7883e97b2860cffc43 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Tue, 13 Jun 2023 15:28:58 +0900 Subject: [PATCH 16/19] Finish updating markdown spec --- spec/features/markdown/markdown_spec.rb | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 5b5d676b7b1921..eb86393d59ec41 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -322,6 +322,8 @@ def doc(html = @html) context 'wiki pipeline' do before do + stub_feature_flags(disable_all_mention: false) + @wiki = @feat.wiki @wiki_page = @feat.wiki_page @@ -330,12 +332,31 @@ def doc(html = @html) blob = double(name: name, path: path, mime_type: 'image/jpeg', data: nil) expect(@wiki).to receive(:find_file).with(path, load_content: false).and_return(Gitlab::Git::WikiFile.new(blob)) allow(@wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' } - # TODO - stub_feature_flags(disable_all_mention: false) @html = markdown(@feat.raw_markdown, { pipeline: :wiki, wiki: @wiki, page_slug: @wiki_page.slug }) end + context 'when `disable_all_mention` FF is enabled' do + before do + stub_feature_flags(disable_all_mention: true) + + @wiki = @feat.wiki + @wiki_page = @feat.wiki_page + + name = 'example.jpg' + path = "images/#{name}" + blob = double(name: name, path: path, mime_type: 'image/jpeg', data: nil) + expect(@wiki).to receive(:find_file).with(path, load_content: false).and_return(Gitlab::Git::WikiFile.new(blob)) + allow(@wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' } + + @html = markdown(@feat.raw_markdown, { pipeline: :wiki, wiki: @wiki, page_slug: @wiki_page.slug }) + end + + it 'includes custom filters' do + expect(doc).to reference_users_excluding_all + end + end + it_behaves_like 'all pipelines' it 'includes custom filters' do -- GitLab From 91d3af097adfa6eb4c272a19ef6538a8ca910c3a Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Tue, 13 Jun 2023 15:31:30 +0900 Subject: [PATCH 17/19] Use disabled? instead of enabled? --- lib/banzai/filter/references/user_reference_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/banzai/filter/references/user_reference_filter.rb b/lib/banzai/filter/references/user_reference_filter.rb index 98a2e9b8e4336f..d6b6fdb7149781 100644 --- a/lib/banzai/filter/references/user_reference_filter.rb +++ b/lib/banzai/filter/references/user_reference_filter.rb @@ -45,7 +45,7 @@ def call # have `gfm` and `gfm-project_member` class names attached for styling. def object_link_filter(text, pattern, link_content: nil, link_reference: false) references_in(text, pattern) do |match, username| - if !Feature.enabled?(:disable_all_mention) && username == 'all' && !skip_project_check? + if Feature.disabled?(:disable_all_mention) && username == 'all' && !skip_project_check? link_to_all(link_content: link_content) else cached_call(:banzai_url_for_object, match, path: [User, username.downcase]) do -- GitLab From c8ca65ff4108ef7df08cba05daf6a8a1d1524d8a Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 14 Jun 2023 09:37:42 +0900 Subject: [PATCH 18/19] Apply TW suggestion to avoid promising --- doc/user/discussions/index.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index c5fc302c8c121c..544ce78fd5eafe 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -59,8 +59,8 @@ FLAG: On self-managed GitLab, by default the feature is available. To make it unavailable, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `disable_all_mention`. -On GitLab.com, this feature is available as of GitLab 16.1 -but [will be made unavailble in GitLab 16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/18442) +On GitLab.com, this feature is available. +Disabling this feature on GitLab.com is tracked in [issue 18442](https://gitlab.com/gitlab-org/gitlab/-/issues/18442). When this feature flag is enabled, typing `@all` in comments and descriptions results in plain text instead of a mention. -- GitLab From 883e3cd14bab52e1e5cf31bfbc77f2efb779d0c6 Mon Sep 17 00:00:00 2001 From: Eulyeon Ko Date: Wed, 14 Jun 2023 09:38:36 +0900 Subject: [PATCH 19/19] Use a proper rollout issue --- config/feature_flags/development/disable_all_mention.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/disable_all_mention.yml b/config/feature_flags/development/disable_all_mention.yml index 103968606a2a99..1ce939683f494e 100644 --- a/config/feature_flags/development/disable_all_mention.yml +++ b/config/feature_flags/development/disable_all_mention.yml @@ -1,7 +1,7 @@ --- name: disable_all_mention introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110586 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/18442 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/415280 milestone: '16.1' type: development group: group::project management -- GitLab