Merge branch 'vij-personal-snippet-gfm-user' into 'master'

Add support for a user parent resource in GFM

See merge request gitlab-org/gitlab!38571

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

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

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

app/models/project.rb

@@ -2596,6 +2596,8 @@ class Project < ApplicationRecord
       namespace != from.namespace
     when Namespace
       namespace != from
+    when User
+      true
     end
   end
 

config/feature_flags/development/personal_snippet_reference_filters.yml

+---
+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

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)

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

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

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

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

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

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

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

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