Merge branch 'feature-flag-contextual-issue-links' into 'master'

Add GFM reference format for feature flags [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53021

app/models/operations/feature_flag.rb

@@ -6,6 +6,7 @@ module Operations
     include AtomicInternalId
     include IidRoutes
     include Limitable
+    include Referable
 
     self.table_name = 'operations_feature_flags'
     self.limit_scope = :project
@@ -65,6 +66,31 @@ module Operations
           .reorder(:id)
           .references(:operations_scopes)
       end
+
+      def reference_prefix
+        '[feature_flag:'
+      end
+
+      def reference_pattern
+        @reference_pattern ||= %r{
+          #{Regexp.escape(reference_prefix)}(#{::Project.reference_pattern}\/)?(?<feature_flag>\d+)#{Regexp.escape(reference_postfix)}
+        }x
+      end
+
+      def link_reference_pattern
+        @link_reference_pattern ||= super("feature_flags", /(?<feature_flag>\d+)\/edit/)
+      end
+
+      def reference_postfix
+        ']'
+      end
+    end
+
+    def to_reference(from = nil, full: false)
+      project
+        .to_reference_base(from, full: full)
+        .then { |reference_base| reference_base.present? ? "#{reference_base}/" : nil }
+        .then { |reference_base| "#{self.class.reference_prefix}#{reference_base}#{iid}#{self.class.reference_postfix}" }
     end
 
     def related_issues(current_user, preload:)

changelogs/unreleased/feature-flag-contextual-issue-links.yml

+---
+title: Add GFM reference format for feature flags
+merge_request: 53021
+author:
+type: added

config/feature_flags/development/feature_flag_contextual_issue.yml

+---
+name: feature_flag_contextual_issue
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53021
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320741
+milestone: '13.9'
+type: development
+group: group::release
+default_enabled: false

doc/user/markdown.md

@@ -444,6 +444,7 @@ GFM recognizes the following:
 | snippet                         | `$123`                     | `namespace/project$123`                 | `project$123`                  |
 | epic **(ULTIMATE)**             | `&123`                     | `group1/subgroup&123`                   |                                |
 | vulnerability **(ULTIMATE)** (1)| `[vulnerability:123]`      | `[vulnerability:namespace/project/123]` | `[vulnerability:project/123]`  |
+| feature flag                    | `[feature_flag:123]`       | `[feature_flag:namespace/project/123]`  | `[feature_flag:project/123]`   |
 | label by ID                     | `~123`                     | `namespace/project~123`                 | `project~123`                  |
 | one-word label by name          | `~bug`                     | `namespace/project~bug`                 | `project~bug`                  |
 | multi-word label by name        | `~"feature request"`       | `namespace/project~"feature request"`   | `project~"feature request"`    |

lib/banzai/filter/feature_flag_reference_filter.rb

+# frozen_string_literal: true
+
+module Banzai
+  module Filter
+    class FeatureFlagReferenceFilter < IssuableReferenceFilter
+      self.reference_type = :feature_flag
+
+      def self.object_class
+        Operations::FeatureFlag
+      end
+
+      def self.object_sym
+        :feature_flag
+      end
+
+      def parent_records(parent, ids)
+        return self.class.object_class.none unless Feature.enabled?(:feature_flag_contextual_issue, parent)
+
+        parent.operations_feature_flags.where(iid: ids.to_a)
+      end
+
+      def url_for_object(feature_flag, project)
+        ::Gitlab::Routing.url_helpers.edit_project_feature_flag_url(
+          project,
+          feature_flag.iid,
+          only_path: context[:only_path]
+        )
+      end
+
+      def object_link_title(object, matches)
+        object.name
+      end
+    end
+  end
+end

lib/banzai/pipeline/gfm_pipeline.rb

@@ -62,7 +62,8 @@ module Banzai
           Filter::CommitReferenceFilter,
           Filter::LabelReferenceFilter,
           Filter::MilestoneReferenceFilter,
-          Filter::AlertReferenceFilter
+          Filter::AlertReferenceFilter,
+          Filter::FeatureFlagReferenceFilter
         ]
       end
 

lib/banzai/pipeline/single_line_pipeline.rb

@@ -24,7 +24,8 @@ module Banzai
           Filter::SnippetReferenceFilter,
           Filter::CommitRangeReferenceFilter,
           Filter::CommitReferenceFilter,
-          Filter::AlertReferenceFilter
+          Filter::AlertReferenceFilter,
+          Filter::FeatureFlagReferenceFilter
         ]
       end
 

lib/banzai/reference_parser/feature_flag_parser.rb

+# frozen_string_literal: true
+
+module Banzai
+  module ReferenceParser
+    class FeatureFlagParser < BaseParser
+      self.reference_type = :feature_flag
+
+      def references_relation
+        Operations::FeatureFlag
+      end
+
+      private
+
+      def can_read_reference?(user, feature_flag, node)
+        can?(user, :read_feature_flag, feature_flag)
+      end
+    end
+  end
+end

spec/lib/banzai/filter/feature_flag_reference_filter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Banzai::Filter::FeatureFlagReferenceFilter do
+  include FilterSpecHelper
+
+  let_it_be(:project) { create(:project, :public) }
+  let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) }
+  let_it_be(:reference) { feature_flag.to_reference }
+
+  it 'requires project context' do
+    expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
+  end
+
+  %w(pre code a style).each do |elem|
+    it "ignores valid references contained inside '#{elem}' element" do
+      exp = act = "<#{elem}>Feature Flag #{reference}</#{elem}>"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'with internal reference' do
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.edit_project_feature_flag_url(project, feature_flag)
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Feature Flag (#{reference}.)")
+
+      expect(doc.to_html).to match(%r{\(<a.+>#{Regexp.escape(reference)}</a>\.\)})
+    end
+
+    it 'ignores invalid feature flag IIDs' do
+      exp = act = "Check [feature_flag:#{non_existing_record_id}]"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+
+    it 'includes a title attribute' do
+      doc = reference_filter("Feature Flag #{reference}")
+
+      expect(doc.css('a').first.attr('title')).to eq feature_flag.name
+    end
+
+    it 'escapes the title attribute' do
+      allow(feature_flag).to receive(:name).and_return(%{"></a>whatever<a title="})
+      doc = reference_filter("Feature Flag #{reference}")
+
+      expect(doc.text).to eq "Feature Flag #{reference}"
+    end
+
+    it 'includes default classes' do
+      doc = reference_filter("Feature Flag #{reference}")
+
+      expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-feature_flag has-tooltip'
+    end
+
+    it 'includes a data-project attribute' do
+      doc = reference_filter("Feature Flag #{reference}")
+      link = doc.css('a').first
+
+      expect(link).to have_attribute('data-project')
+      expect(link.attr('data-project')).to eq project.id.to_s
+    end
+
+    it 'includes a data-feature-flag attribute' do
+      doc = reference_filter("See #{reference}")
+      link = doc.css('a').first
+
+      expect(link).to have_attribute('data-feature-flag')
+      expect(link.attr('data-feature-flag')).to eq feature_flag.id.to_s
+    end
+
+    it 'supports an :only_path context' do
+      doc = reference_filter("Feature Flag #{reference}", only_path: true)
+      link = doc.css('a').first.attr('href')
+
+      expect(link).not_to match %r(https?://)
+      expect(link).to eq urls.edit_project_feature_flag_url(project, feature_flag.iid, only_path: true)
+    end
+
+    context 'when feature_flag_contextual_issue feture flag is disabled' do
+      before do
+        stub_feature_flags(feature_flag_contextual_issue: false)
+      end
+
+      it 'does not link the reference' do
+        doc = reference_filter("See #{reference}")
+
+        expect(doc.css('a').first).to be_nil
+      end
+    end
+  end
+
+  context 'with cross-project / cross-namespace complete reference' do
+    let_it_be(:namespace) { create(:namespace) }
+    let_it_be(:project2)  { create(:project, :public, namespace: namespace) }
+    let_it_be(:feature_flag) { create(:operations_feature_flag, project: project2) }
+    let_it_be(:reference) { "[feature_flag:#{project2.full_path}/#{feature_flag.iid}]" }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.edit_project_feature_flag_url(project2, feature_flag)
+    end
+
+    it 'produces a valid text in a link' do
+      doc = reference_filter("See (#{reference}.)")
+
+      expect(doc.css('a').first.text).to eql(reference)
+    end
+
+    it 'produces a valid text' do
+      doc = reference_filter("See (#{reference}.)")
+
+      expect(doc.text).to eql("See (#{reference}.)")
+    end
+
+    it 'ignores invalid feature flag IIDs on the referenced project' do
+      exp = act = "Check [feature_flag:#{non_existing_record_id}]"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'with cross-project / same-namespace complete reference' do
+    let_it_be(:namespace) { create(:namespace) }
+    let_it_be(:project)   { create(:project, :public, namespace: namespace) }
+    let_it_be(:project2)  { create(:project, :public, namespace: namespace) }
+    let_it_be(:feature_flag) { create(:operations_feature_flag, project: project2) }
+    let_it_be(:reference) { "[feature_flag:#{project2.full_path}/#{feature_flag.iid}]" }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.edit_project_feature_flag_url(project2, feature_flag)
+    end
+
+    it 'produces a valid text in a link' do
+      doc = reference_filter("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+
+      expect(doc.css('a').first.text).to eql("[feature_flag:#{project2.path}/#{feature_flag.iid}]")
+    end
+
+    it 'produces a valid text' do
+      doc = reference_filter("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+
+      expect(doc.text).to eql("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+    end
+
+    it 'ignores invalid feature flag IIDs on the referenced project' do
+      exp = act = "Check [feature_flag:#{non_existing_record_id}]"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'with cross-project shorthand reference' do
+    let_it_be(:namespace) { create(:namespace) }
+    let_it_be(:project)   { create(:project, :public, namespace: namespace) }
+    let_it_be(:project2)  { create(:project, :public, namespace: namespace) }
+    let_it_be(:feature_flag) { create(:operations_feature_flag, project: project2) }
+    let_it_be(:reference) { "[feature_flag:#{project2.path}/#{feature_flag.iid}]" }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.edit_project_feature_flag_url(project2, feature_flag)
+    end
+
+    it 'produces a valid text in a link' do
+      doc = reference_filter("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+
+      expect(doc.css('a').first.text).to eql("[feature_flag:#{project2.path}/#{feature_flag.iid}]")
+    end
+
+    it 'produces a valid text' do
+      doc = reference_filter("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+
+      expect(doc.text).to eql("See ([feature_flag:#{project2.path}/#{feature_flag.iid}].)")
+    end
+
+    it 'ignores invalid feature flag IDs on the referenced project' do
+      exp = act = "Check [feature_flag:#{non_existing_record_id}]"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'with cross-project URL reference' do
+    let_it_be(:namespace) { create(:namespace, name: 'cross-reference') }
+    let_it_be(:project2)  { create(:project, :public, namespace: namespace) }
+    let_it_be(:feature_flag) { create(:operations_feature_flag, project: project2) }
+    let_it_be(:reference) { urls.edit_project_feature_flag_url(project2, feature_flag) }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.edit_project_feature_flag_url(project2, feature_flag)
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("See (#{reference}.)")
+
+      expect(doc.to_html).to match(%r{\(<a.+>#{Regexp.escape(feature_flag.to_reference(project))}</a>\.\)})
+    end
+
+    it 'ignores invalid feature flag IIDs on the referenced project' do
+      act = "See #{invalidate_reference(reference)}"
+
+      expect(reference_filter(act).to_html).to match(%r{<a.+>#{Regexp.escape(invalidate_reference(reference))}</a>})
+    end
+  end
+
+  context 'with group context' do
+    let_it_be(:group) { create(:group) }
+
+    it 'links to a valid reference' do
+      reference = "[feature_flag:#{project.full_path}/#{feature_flag.iid}]"
+      result = reference_filter("See #{reference}", { project: nil, group: group } )
+
+      expect(result.css('a').first.attr('href')).to eq(urls.edit_project_feature_flag_url(project, feature_flag))
+    end
+
+    it 'ignores internal references' do
+      exp = act = "See [feature_flag:#{feature_flag.iid}]"
+
+      expect(reference_filter(act, project: nil, group: group).to_html).to eq exp
+    end
+  end
+end

spec/lib/banzai/reference_parser/feature_flag_parser_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Banzai::ReferenceParser::FeatureFlagParser do
+  include ReferenceParserHelpers
+
+  subject { described_class.new(Banzai::RenderContext.new(project, user)) }
+
+  let(:link) { empty_html_link }
+
+  describe '#nodes_visible_to_user' do
+    let(:project) { create(:project, :public) }
+    let(:user) { create(:user) }
+    let(:feature_flag) { create(:operations_feature_flag, project: project) }
+
+    context 'when the link has a data-issue attribute' do
+      before do
+        link['data-feature-flag'] = feature_flag.id.to_s
+      end
+
+      it_behaves_like "referenced feature visibility", "issues", "merge_requests" do
+        before do
+          project.add_developer(user) if enable_user?
+        end
+      end
+    end
+  end
+
+  describe '#referenced_by' do
+    let_it_be(:project) { create(:project, :public) }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) }
+
+    describe 'when the link has a data-feature-flag attribute' do
+      context 'using an existing feature flag ID' do
+        it 'returns an Array of feature flags' do
+          link['data-feature-flag'] = feature_flag.id.to_s
+
+          expect(subject.referenced_by([link])).to eq([feature_flag])
+        end
+      end
+
+      context 'using a non-existing feature flag ID' do
+        it 'returns an empty Array' do
+          link['data-feature-flag'] = ''
+
+          expect(subject.referenced_by([link])).to eq([])
+        end
+      end
+    end
+  end
+end

spec/models/operations/feature_flag_spec.rb

@@ -16,6 +16,35 @@ RSpec.describe Operations::FeatureFlag do
     it { is_expected.to have_many(:scopes) }
   end
 
+  describe '.reference_pattern' do
+    subject { described_class.reference_pattern }
+
+    it { is_expected.to match('[feature_flag:123]') }
+    it { is_expected.to match('[feature_flag:gitlab-org/gitlab/123]') }
+  end
+
+  describe '.link_reference_pattern' do
+    subject { described_class.link_reference_pattern }
+
+    it { is_expected.to match("#{Gitlab.config.gitlab.url}/gitlab-org/gitlab/-/feature_flags/123/edit") }
+    it { is_expected.not_to match("#{Gitlab.config.gitlab.url}/gitlab-org/gitlab/issues/123/edit") }
+    it { is_expected.not_to match("gitlab-org/gitlab/-/feature_flags/123/edit") }
+  end
+
+  describe '#to_reference' do
+    let(:namespace) { build(:namespace, path: 'sample-namespace') }
+    let(:project) { build(:project, name: 'sample-project', namespace: namespace) }
+    let(:feature_flag) { build(:operations_feature_flag, iid: 1, project: project) }
+
+    it 'returns feature flag id' do
+      expect(feature_flag.to_reference).to eq '[feature_flag:1]'
+    end
+
+    it 'returns complete path to the feature flag with full: true' do
+      expect(feature_flag.to_reference(full: true)).to eq '[feature_flag:sample-namespace/sample-project/1]'
+    end
+  end
+
   describe 'validations' do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:name) }