diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 04eb4659469..49fc780f372 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -39,6 +39,10 @@ module CacheMarkdownField context[:markdown_engine] = :common_mark + if Feature.enabled?(:personal_snippet_reference_filters, context[:author]) + context[:user] = self.parent_user + end + context end @@ -132,6 +136,10 @@ module CacheMarkdownField end end + def parent_user + nil + end + included do cattr_reader :cached_markdown_fields do Gitlab::MarkdownCache::FieldData.new diff --git a/app/models/note.rb b/app/models/note.rb index e06df816b2e..e1fc16818b3 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -559,6 +559,10 @@ class Note < ApplicationRecord (!system_note_with_references? || all_referenced_mentionables_allowed?(user)) && system_note_viewable_by?(user) end + def parent_user + noteable.author if for_personal_snippet? + end + private # Using this method followed by a call to `save` may result in ActiveRecord::RecordNotUnique exception diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 197795dccfe..0915278fb65 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -3,6 +3,10 @@ class PersonalSnippet < Snippet include WithUploads + def parent_user + author + end + def skip_project_check? true end diff --git a/app/models/project.rb b/app/models/project.rb index cedb5378c67..5e1822f1a34 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2596,6 +2596,8 @@ class Project < ApplicationRecord namespace != from.namespace when Namespace namespace != from + when User + true end end diff --git a/config/feature_flags/development/personal_snippet_reference_filters.yml b/config/feature_flags/development/personal_snippet_reference_filters.yml new file mode 100644 index 00000000000..6a9aefbb379 --- /dev/null +++ b/config/feature_flags/development/personal_snippet_reference_filters.yml @@ -0,0 +1,7 @@ +--- +name: personal_snippet_reference_filters +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/235155 +group: group::editor +type: development +default_enabled: false \ No newline at end of file diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 38105e2237c..b0a2f6f69d5 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -136,7 +136,7 @@ module Banzai end def call - return doc unless project || group + return doc unless project || group || user ref_pattern = object_class.reference_pattern link_pattern = object_class.link_reference_pattern @@ -280,7 +280,7 @@ module Banzai end def object_link_text(object, matches) - parent = context[:project] || context[:group] + parent = project || group || user text = object.reference_link_text(parent) extras = object_link_text_extras(object, matches) diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 1959336ea1b..cfd4b932568 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -76,6 +76,10 @@ module Banzai context[:group] end + def user + context[:user] + end + def skip_project_check? context[:skip_project_check] end diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 55031183e10..e98bb22d3ea 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -120,6 +120,17 @@ RSpec.describe 'Comments on personal snippets', :js do # but we want to make sure expect(page).not_to have_selector('.atwho-view') end + + it_behaves_like 'personal snippet with references' do + let(:container) { 'div#notes' } + + subject do + fill_in 'note[note]', with: references + click_button 'Comment' + + wait_for_requests + end + end end context 'when editing a note' do diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 39801e265e0..f4c6536d6d3 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -144,4 +144,17 @@ RSpec.describe 'User creates snippet', :js do expect(created_snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) end end + + it_behaves_like 'personal snippet with references' do + let(:container) { '.snippet-header .description' } + let(:md_description) { references } + + subject do + visit new_snippet_path + fill_form + click_button('Create snippet') + + wait_for_requests + end + end end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 5f8c65e429e..440943171c3 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -20,6 +20,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do @title, @description, @cached_markdown_version = args[:title], args[:description], args[:cached_markdown_version] @title_html, @description_html = args[:title_html], args[:description_html] @author, @project = args[:author], args[:project] + @parent_user = args[:parent_user] end attr_accessor :title, :description, :cached_markdown_version @@ -41,8 +42,8 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do let(:cache_version) { Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION << 16 } - def thing_subclass(klass, extra_attribute) - Class.new(klass) { attr_accessor(extra_attribute) } + def thing_subclass(klass, *extra_attributes) + Class.new(klass) { attr_accessor(*extra_attributes) } end shared_examples 'a class with cached markdown fields' do @@ -192,11 +193,33 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'with an author' do - let(:thing) { thing_subclass(klass, :author).new(title: markdown, title_html: html, author: :author_value) } + let(:user) { build(:user) } + let(:thing) { thing_subclass(klass, :author).new(title: markdown, title_html: html, author: user) } it 'sets the author in the context' do is_expected.to have_key(:author) - expect(context[:author]).to eq(:author_value) + expect(context[:author]).to eq(user) + end + end + + context 'with a parent_user' do + let(:user) { build(:user) } + let(:thing) { thing_subclass(klass, :author, :parent_user).new(title: markdown, title_html: html, parent_user: user, author: user) } + + it 'sets the user in the context' do + is_expected.to have_key(:user) + expect(context[:user]).to eq(user) + end + + context 'when the personal_snippet_reference_filters flag is disabled' do + before do + stub_feature_flags(personal_snippet_reference_filters: false) + end + + it 'does not set the user in the context' do + is_expected.not_to have_key(:user) + expect(context[:user]).to be_nil + end end end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 864f7221181..7edd7849bbe 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1373,11 +1373,11 @@ RSpec.describe Note do describe 'banzai_render_context' do let(:project) { build(:project_empty_repo) } + subject(:context) { noteable.banzai_render_context(:title) } + context 'when noteable is a merge request' do let(:noteable) { build :merge_request, target_project: project, source_project: project } - subject(:context) { noteable.banzai_render_context(:title) } - it 'sets the label_url_method in the context' do expect(context[:label_url_method]).to eq(:project_merge_requests_url) end @@ -1386,11 +1386,34 @@ RSpec.describe Note do context 'when noteable is an issue' do let(:noteable) { build :issue, project: project } - subject(:context) { noteable.banzai_render_context(:title) } - it 'sets the label_url_method in the context' do expect(context[:label_url_method]).to eq(:project_issues_url) end end + + context 'when noteable is a personal snippet' do + let(:noteable) { build(:personal_snippet) } + + it 'sets the parent user in the context' do + expect(context[:user]).to eq(noteable.author) + end + end + end + + describe '#parent_user' do + it 'returns the author of a personal snippet' do + note = build(:note_on_personal_snippet) + expect(note.parent_user).to eq(note.noteable.author) + end + + it 'returns nil for project snippet' do + note = build(:note_on_project_snippet) + expect(note.parent_user).to be_nil + end + + it 'returns nil when noteable is not a snippet' do + note = build(:note_on_issue) + expect(note.parent_user).to be_nil + end end end diff --git a/spec/models/personal_snippet_spec.rb b/spec/models/personal_snippet_spec.rb index 2236c9dfed7..234f6e4b4b5 100644 --- a/spec/models/personal_snippet_spec.rb +++ b/spec/models/personal_snippet_spec.rb @@ -24,4 +24,12 @@ RSpec.describe PersonalSnippet do let(:expected_web_url_path) { "-/snippets/#{container.id}" } let(:expected_repo_url_path) { "snippets/#{container.id}" } end + + describe '#parent_user' do + it 'returns the snippet author' do + snippet = build(:personal_snippet) + + expect(snippet.parent_user).to eq(snippet.author) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a465425e735..e6d51e0bfd7 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -680,6 +680,12 @@ RSpec.describe Project do end end end + + context 'when argument is a user' do + it 'returns full path to the project' do + expect(project.to_reference_base(owner)).to eq 'sample-namespace/sample-project' + end + end end describe '#to_human_reference' do diff --git a/spec/support/shared_examples/features/snippets_shared_examples.rb b/spec/support/shared_examples/features/snippets_shared_examples.rb index 0ac0dc72017..8d68b1e4c0a 100644 --- a/spec/support/shared_examples/features/snippets_shared_examples.rb +++ b/spec/support/shared_examples/features/snippets_shared_examples.rb @@ -192,3 +192,83 @@ RSpec.shared_examples 'show and render proper snippet blob' do end end end + +RSpec.shared_examples 'personal snippet with references' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:project_snippet) { create(:project_snippet, :repository, project: project)} + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:commit) { project.commit } + + let(:mr_reference) { merge_request.to_reference(full: true) } + let(:issue_reference) { issue.to_reference(full: true) } + let(:snippet_reference) { project_snippet.to_reference(full: true) } + let(:commit_reference) { commit.reference_link_text(full: true) } + + RSpec.shared_examples 'handles resource links' do + context 'with access to the resource' do + before do + project.add_developer(user) + end + + it 'converts the reference to a link' do + subject + + page.within(container) do + aggregate_failures do + expect(page).to have_link(mr_reference) + expect(page).to have_link(issue_reference) + expect(page).to have_link(snippet_reference) + expect(page).to have_link(commit_reference) + end + end + end + end + + context 'without access to the resource' do + it 'does not convert the reference to a link' do + subject + + page.within(container) do + expect(page).not_to have_link(mr_reference) + expect(page).not_to have_link(issue_reference) + expect(page).not_to have_link(snippet_reference) + expect(page).not_to have_link(commit_reference) + end + end + end + end + + context 'when using references to resources' do + let(:references) do + <<~REFERENCES + MR: #{mr_reference} + + Commit: #{commit_reference} + + Issue: #{issue_reference} + + ProjectSnippet: #{snippet_reference} + REFERENCES + end + + it_behaves_like 'handles resource links' + end + + context 'when using links to resources' do + let(:args) { { host: Gitlab.config.gitlab.url, port: nil } } + let(:references) do + <<~REFERENCES + MR: #{merge_request_url(merge_request, args)} + + Commit: #{project_commit_url(project, commit, args)} + + Issue: #{issue_url(issue, args)} + + ProjectSnippet: #{project_snippet_url(project, project_snippet, args)} + REFERENCES + end + + it_behaves_like 'handles resource links' + end +end