Refactors metric embeds to ease adding embed types

There will soon be two new embed types: cluster embeds and
alert embeds. We want to make it as easier to add those
new classes as possible without duplicate work. This
refactor pulls out some shared examples and allows for
future easy ee overrides.

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