Merge branch 'sy-clean-gfm-filter-examples' into 'master'

Refactor metric embeds to ease additional embed types

See merge request gitlab-org/gitlab!24925

lib/banzai/filter/inline_metrics_filter.rb

@@ -20,6 +20,7 @@ module Banzai
       # the cost of doing a full regex match.
       def xpath_search
         "descendant-or-self::a[contains(@href,'metrics') and \
+          contains(@href,'environments') and \
           starts-with(@href, '#{Gitlab.config.gitlab.url}')]"
       end
 

lib/banzai/filter/inline_metrics_redactor_filter.rb

@@ -9,8 +9,8 @@ module Banzai
 
       METRICS_CSS_CLASS = '.js-render-metrics'
       EMBED_LIMIT = 100
-      URL = Gitlab::Metrics::Dashboard::Url
 
+      Route = Struct.new(:regex, :permission)
       Embed = Struct.new(:project_path, :permission)
 
       # Finds all embeds based on the css class the FE
@@ -59,14 +59,28 @@ module Banzai
             embed = Embed.new
             url = node.attribute('data-dashboard-url').to_s
 
-            set_path_and_permission(embed, url, URL.metrics_regex, :read_environment)
-            set_path_and_permission(embed, url, URL.grafana_regex, :read_project) unless embed.permission
+            permissions_by_route.each do |route|
+              set_path_and_permission(embed, url, route.regex, route.permission) unless embed.permission
+            end
 
             embeds[node] = embed if embed.permission
           end
         end
       end
 
+      def permissions_by_route
+        [
+          Route.new(
+            ::Gitlab::Metrics::Dashboard::Url.metrics_regex,
+            :read_environment
+          ),
+          Route.new(
+            ::Gitlab::Metrics::Dashboard::Url.grafana_regex,
+            :read_project
+          )
+        ]
+      end
+
       # Attempts to determine the path and permission attributes
       # of a url based on expected dashboard url formats and
       # sets the attributes on an Embed object

lib/banzai/pipeline/gfm_pipeline.rb

@@ -29,8 +29,7 @@ module Banzai
           Filter::AudioLinkFilter,
           Filter::ImageLazyLoadFilter,
           Filter::ImageLinkFilter,
-          Filter::InlineMetricsFilter,
-          Filter::InlineGrafanaMetricsFilter,
+          *metrics_filters,
           Filter::TableOfContentsFilter,
           Filter::TableOfContentsTagFilter,
           Filter::AutolinkFilter,
@@ -48,6 +47,13 @@ module Banzai
         ]
       end
 
+      def self.metrics_filters
+        [
+          Filter::InlineMetricsFilter,
+          Filter::InlineGrafanaMetricsFilter
+        ]
+      end
+
       def self.reference_filters
         [
           Filter::UserReferenceFilter,

spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb

@@ -8,40 +8,26 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
   let_it_be(:project) { create(:project) }
   let_it_be(:grafana_integration) { create(:grafana_integration, project: project) }
 
-  let(:input) { %(<a href="#{url}">example</a>) }
+  let(:input) { %(<a href="#{trigger_url}">example</a>) }
   let(:doc) { filter(input) }
-
-  let(:url) { grafana_integration.grafana_url + dashboard_path }
   let(:dashboard_path) do
     '/d/XDaNK6amz/gitlab-omnibus-redis' \
     '?from=1570397739557&to=1570484139557' \
     '&var-instance=All&panelId=14'
   end
 
-  it 'appends a metrics charts placeholder with dashboard url after metrics links' do
-    node = doc.at_css('.js-render-metrics')
-    expect(node).to be_present
-
-    dashboard_url = urls.project_grafana_api_metrics_dashboard_url(
+  let(:trigger_url) { grafana_integration.grafana_url + dashboard_path }
+  let(:dashboard_url) do
+    urls.project_grafana_api_metrics_dashboard_url(
       project,
       embedded: true,
-      grafana_url: url,
+      grafana_url: trigger_url,
       start: "2019-10-06T21:35:39Z",
       end: "2019-10-07T21:35:39Z"
     )
-
-    expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url)
   end
 
-  context 'when the dashboard link is part of a paragraph' do
-    let(:paragraph) { %(This is an <a href="#{url}">example</a> of metrics.) }
-    let(:input) { %(<p>#{paragraph}</p>) }
-
-    it 'appends the charts placeholder after the enclosing paragraph' do
-      expect(unescape(doc.at_css('p').to_s)).to include(paragraph)
-      expect(doc.at_css('.js-render-metrics')).to be_present
-    end
-  end
+  it_behaves_like 'a metrics embed filter'
 
   context 'when grafana is not configured' do
     before do

spec/lib/banzai/filter/inline_metrics_filter_spec.rb

@@ -5,66 +5,31 @@ require 'spec_helper'
 describe Banzai::Filter::InlineMetricsFilter do
   include FilterSpecHelper
 
-  let(:input) { %(<a href="#{url}">example</a>) }
-  let(:doc) { filter(input) }
-
-  context 'when the document has an external link' do
-    let(:url) { 'https://foo.com' }
-
-    it 'leaves regular non-metrics links unchanged' do
-      expect(doc.to_s).to eq(input)
-    end
-  end
-
-  context 'when the document has a metrics dashboard link' do
-    let(:params) { ['foo', 'bar', 12] }
-    let(:url) { urls.metrics_namespace_project_environment_url(*params) }
-
-    it 'leaves the original link unchanged' do
-      expect(doc.at_css('a').to_s).to eq(input)
-    end
-
-    it 'appends a metrics charts placeholder with dashboard url after metrics links' do
-      node = doc.at_css('.js-render-metrics')
-      expect(node).to be_present
-
-      dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params, embedded: true)
-      expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url)
+  let(:params) { ['foo', 'bar', 12] }
+  let(:query_params) { {} }
+
+  let(:trigger_url) { urls.metrics_namespace_project_environment_url(*params, query_params) }
+  let(:dashboard_url) { urls.metrics_dashboard_namespace_project_environment_url(*params, **query_params, embedded: true) }
+
+  it_behaves_like 'a metrics embed filter'
+
+  context 'with query params specified' do
+    let(:query_params) do
+      {
+        dashboard: 'config/prometheus/common_metrics.yml',
+        group: 'System metrics (Kubernetes)',
+        title: 'Core Usage (Pod Average)',
+        y_label: 'Cores per Pod'
+      }
     end
 
-    context 'when the metrics dashboard link is part of a paragraph' do
-      let(:paragraph) { %(This is an <a href="#{url}">example</a> of metrics.) }
-      let(:input) { %(<p>#{paragraph}</p>) }
-
-      it 'appends the charts placeholder after the enclosing paragraph' do
-        expect(doc.at_css('p').to_s).to include(paragraph)
-        expect(doc.at_css('.js-render-metrics')).to be_present
-      end
-    end
-
-    context 'with dashboard params specified' do
-      let(:params) do
-        [
-          'foo',
-          'bar',
-          12,
-          {
-            embedded: true,
-            dashboard: 'config/prometheus/common_metrics.yml',
-            group: 'System metrics (Kubernetes)',
-            title: 'Core Usage (Pod Average)',
-            y_label: 'Cores per Pod'
-          }
-        ]
-      end
+    it_behaves_like 'a metrics embed filter'
+  end
 
-      it 'appends a metrics charts placeholder with dashboard url after metrics links' do
-        node = doc.at_css('.js-render-metrics')
-        expect(node).to be_present
+  it 'leaves links to other dashboards unchanged' do
+    url = urls.namespace_project_grafana_api_metrics_dashboard_url('foo', 'bar')
+    input = %(<a href="#{url}">example</a>)
 
-        dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params)
-        expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url)
-      end
-    end
+    expect(filter(input).to_s).to eq(input)
   end
 end

spec/lib/banzai/filter/inline_metrics_redactor_filter_spec.rb

@@ -18,33 +18,6 @@ describe Banzai::Filter::InlineMetricsRedactorFilter do
   end
 
   context 'with a metrics charts placeholder' do
-    shared_examples_for 'a supported metrics dashboard url' do
-      context 'no user is logged in' do
-        it 'redacts the placeholder' do
-          expect(doc.to_s).to be_empty
-        end
-      end
-
-      context 'the user does not have permission do see charts' do
-        let(:doc) { filter(input, current_user: build(:user)) }
-
-        it 'redacts the placeholder' do
-          expect(doc.to_s).to be_empty
-        end
-      end
-
-      context 'the user has requisite permissions' do
-        let(:user) { create(:user) }
-        let(:doc) { filter(input, current_user: user) }
-
-        it 'leaves the placeholder' do
-          project.add_maintainer(user)
-
-          expect(doc.to_s).to eq input
-        end
-      end
-    end
-
     let(:input) { %(<div class="js-render-metrics" data-dashboard-url="#{url}"></div>) }
 
     it_behaves_like 'a supported metrics dashboard url'

spec/support/shared_examples/banzai/filters/inline_embeds_shared_examples.rb

+# frozen_string_literal: true
+
+# Expects 2 attributes to be defined:
+#   trigger_url - Url expected to trigger the insertion of a placeholder.
+#   dashboard_url - Url expected to be present in the placeholder.
+RSpec.shared_examples 'a metrics embed filter' do
+  let(:input) { %(<a href="#{url}">example</a>) }
+  let(:doc) { filter(input) }
+
+  context 'when the document has an external link' do
+    let(:url) { 'https://foo.com' }
+
+    it 'leaves regular non-metrics links unchanged' do
+      expect(doc.to_s).to eq(input)
+    end
+  end
+
+  context 'when the document contains an embeddable link' do
+    let(:url) { trigger_url }
+
+    it 'leaves the original link unchanged' do
+      expect(unescape(doc.at_css('a').to_s)).to eq(input)
+    end
+
+    it 'appends a metrics charts placeholder' do
+      node = doc.at_css('.js-render-metrics')
+      expect(node).to be_present
+
+      expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url)
+    end
+
+    context 'in a paragraph' do
+      let(:paragraph) { %(This is an <a href="#{url}">example</a> of metrics.) }
+      let(:input) { %(<p>#{paragraph}</p>) }
+
+      it 'appends a metrics charts placeholder after the enclosing paragraph' do
+        expect(unescape(doc.at_css('p').to_s)).to include(paragraph)
+        expect(doc.at_css('.js-render-metrics')).to be_present
+      end
+    end
+  end
+
+  # Nokogiri escapes the URLs, but we don't care about that
+  # distinction for the purposes of these filters
+  def unescape(html)
+    CGI.unescapeHTML(html)
+  end
+end

spec/support/shared_examples/banzai/filters/inline_metrics_redactor_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'a supported metrics dashboard url' do
+  context 'no user is logged in' do
+    it 'redacts the placeholder' do
+      expect(doc.to_s).to be_empty
+    end
+  end
+
+  context 'the user does not have permission do see charts' do
+    let(:doc) { filter(input, current_user: build(:user)) }
+
+    it 'redacts the placeholder' do
+      expect(doc.to_s).to be_empty
+    end
+  end
+
+  context 'the user has requisite permissions' do
+    let(:user) { create(:user) }
+    let(:doc) { filter(input, current_user: user) }
+
+    it 'leaves the placeholder' do
+      project.add_maintainer(user)
+
+      expect(doc.to_s).to eq(input)
+    end
+  end
+end