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 0000000000000000000000000000000000000000..1ce939683f494e5efaaa82d21bdb1d84e5112651 --- /dev/null +++ b/config/feature_flags/development/disable_all_mention.yml @@ -0,0 +1,8 @@ +--- +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/415280 +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 f603354198679ce66d52ff5ff5827d6ca75e874b..544ce78fd5eafe452deaf1925221ce394eb38d95 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -51,9 +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. -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. +### Mentioning all members + +> [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. +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. +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. + Notifications and mentions can be disabled in [a group's settings](../group/manage.md#disable-email-notifications). diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 2026a03315044c4232b23527c366eb83212306c7..401fe0bcb09e15e8a4860018143b50044110c43e 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#mentioning-all-members) | | | | project | `namespace/project>` | | | | issue | ``#123`` | `namespace/project#123` | `project#123` | | merge request | `!123` | `namespace/project!123` | `project!123` | diff --git a/ee/spec/models/concerns/ee/mentionable_spec.rb b/ee/spec/models/concerns/ee/mentionable_spec.rb index b0847fbbd0390d27443aedd9eaa3f2dfb8e98e86..0a76e046501d2e69ecdc1ef1364361b4d0f5783c 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/ee/spec/serializers/status_page/incident_comment_entity_spec.rb b/ee/spec/serializers/status_page/incident_comment_entity_spec.rb index 1adb9398dd669d33b430e630156793abcb26287d..d4a37934bd97e67d3a521ce2648785827b6e3093 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 344551f558066ed61f3dbe044875e9df970cea9c..ebf450e44807f522c1588b56c2080c9783046ad0 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 a612c1ae3b4655c8713c6aaf99cdcf07c079da3a..0c2145dfa21d65815a8217e83bf3b53c544d8bce 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 @@ -102,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 diff --git a/lib/banzai/filter/references/user_reference_filter.rb b/lib/banzai/filter/references/user_reference_filter.rb index 5983036a8e5267cda50e0162524ecfd34db00395..d6b6fdb714978134044f50e38e0d9b5aa057d5b9 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 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 diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 9bb0220004a550a72e9a187ebaff0bce47a754e8..eb86393d59ec4102f6bc0a8b55a09af295f8ff74 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -221,11 +221,25 @@ 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 @@ -308,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 @@ -320,6 +336,27 @@ def doc(html = @html) @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 diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index fa73cd53a660bd3f2a227128d71846f8e62b3dd8..37376713355390bb146e179b8cfedee65472ca36 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/frontend/fixtures/merge_requests.rb b/spec/frontend/fixtures/merge_requests.rb index b6f6d14975670c18510d8208b013eb55a296c0d7..a1896a6470b75225b8fe17baf2095245f7216020 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 3346735055d99e578ecf7c9d1dc00578140b67f8..6f39eb9a11897778cd0ff3bd2bbc5ea92891daee 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]')); 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 e248f2d9b1c24dfb982abfb09f4800919e7301a4..276701a298485d754406d858477ff16929758acc 100644 --- a/spec/lib/banzai/filter/references/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/user_reference_filter_spec.rb @@ -39,10 +39,30 @@ 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 + stub_feature_flags(disable_all_mention: false) + project.add_developer(project.creator) end @@ -161,6 +181,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 f8ed26b324170782dd51b9d46d0b6693c833aa9d..22b910b3dae1036a5295ec869fa8184d6ed2cebc 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -298,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 diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index d9e53fb7e9a3e57ff49c4d676c6ac60038974e09..ad639a7503a00d05da1606c538b935a4e5e9025e 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 @@ -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') } @@ -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/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0e1afaa837847c2e861e976769954c6424249bc2..99f3134f06f5ec9046282d5d0c100c073ee4d2f5 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 @@ -892,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 @@ -927,20 +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 - 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 @@ -965,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 diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index 8fdece7b26d908efdc96ac30160946347fa438b5..7a82d7674d9528aaec9d8656df91813a7e67e812 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 diff --git a/spec/support/shared_examples/models/mentionable_shared_examples.rb b/spec/support/shared_examples/models/mentionable_shared_examples.rb index f9612dd61beb625d6fcbeb33fdcb85e033a4ffab..9874db8dbd748583ff502d579ec03ec452de9247 100644 --- a/spec/support/shared_examples/models/mentionable_shared_examples.rb +++ b/spec/support/shared_examples/models/mentionable_shared_examples.rb @@ -208,13 +208,13 @@ RSpec.shared_examples 'mentions in description' do |mentionable_type| context 'when storing user mentions' do - before do - 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 @@ -228,13 +228,49 @@ 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_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 + + 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' do + add_member(user) + + expect(mentionable.user_mentions.count).to eq 1 + expect(mentionable.referenced_users).to match_array([user, user2]) + expect(mentionable.referenced_groups(user)).to match_array([group]) + end end end end @@ -248,17 +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 - 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) @@ -268,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 @@ -283,13 +342,15 @@ 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) } 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 +402,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, @@ -368,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)