diff options
Diffstat (limited to 'spec/lib/banzai')
5 files changed, 206 insertions, 20 deletions
diff --git a/spec/lib/banzai/filter/inline_observability_filter_spec.rb b/spec/lib/banzai/filter/inline_observability_filter_spec.rb index fb1ba46e76c..81896faced8 100644 --- a/spec/lib/banzai/filter/inline_observability_filter_spec.rb +++ b/spec/lib/banzai/filter/inline_observability_filter_spec.rb @@ -2,25 +2,20 @@ require 'spec_helper' -RSpec.describe Banzai::Filter::InlineObservabilityFilter do +RSpec.describe Banzai::Filter::InlineObservabilityFilter, feature_category: :metrics do include FilterSpecHelper let(:input) { %(<a href="#{url}">example</a>) } let(:doc) { filter(input) } - let(:group) { create(:group) } - let(:user) { create(:user) } - describe '#filter?' do - context 'when the document has an external link' do - let(:url) { 'https://foo.com' } - - it 'leaves regular non-observability links unchanged' do - expect(doc.to_s).to eq(input) - end - end + before do + allow(Gitlab::Observability).to receive(:embeddable_url).and_return('embeddable-url') + stub_config_setting(url: "https://www.gitlab.com") + end - context 'when the document contains an embeddable observability link' do - let(:url) { 'https://observe.gitlab.com/12345' } + describe '#filter?' do + context 'when the document contains a valid observability link' do + let(:url) { "https://www.gitlab.com/groups/some-group/-/observability/test" } it 'leaves the original link unchanged' do expect(doc.at_css('a').to_s).to eq(input) @@ -30,17 +25,34 @@ RSpec.describe Banzai::Filter::InlineObservabilityFilter do node = doc.at_css('.js-render-observability') expect(node).to be_present - expect(node.attribute('data-frame-url').to_s).to eq(url) + expect(node.attribute('data-frame-url').to_s).to eq('embeddable-url') + expect(Gitlab::Observability).to have_received(:embeddable_url).with(url).once end end - context 'when feature flag is disabled' do - let(:url) { 'https://observe.gitlab.com/12345' } + context 'with duplicate URLs' do + let(:url) { "https://www.gitlab.com/groups/some-group/-/observability/test" } + let(:input) { %(<a href="#{url}">example1</a><a href="#{url}">example2</a>) } - before do - stub_feature_flags(observability_group_tab: false) + where(:embeddable_url) do + [ + 'not-nil', + nil + ] end + with_them do + it 'calls Gitlab::Observability.embeddable_url only once' do + allow(Gitlab::Observability).to receive(:embeddable_url).with(url).and_return(embeddable_url) + + filter(input) + + expect(Gitlab::Observability).to have_received(:embeddable_url).with(url).once + end + end + end + + shared_examples 'does not embed observabilty' do it 'leaves the original link unchanged' do expect(doc.at_css('a').to_s).to eq(input) end @@ -51,5 +63,39 @@ RSpec.describe Banzai::Filter::InlineObservabilityFilter do expect(node).not_to be_present end end + + context 'when the embeddable url is nil' do + let(:url) { "https://www.gitlab.com/groups/some-group/-/something-else/test" } + + before do + allow(Gitlab::Observability).to receive(:embeddable_url).and_return(nil) + end + + it_behaves_like 'does not embed observabilty' + end + + context 'when the document has an unrecognised link' do + let(:url) { "https://www.gitlab.com/groups/some-group/-/something-else/test" } + + it_behaves_like 'does not embed observabilty' + + it 'does not build the embeddable url' do + expect(Gitlab::Observability).not_to have_received(:embeddable_url) + end + end + + context 'when feature flag is disabled' do + let(:url) { "https://www.gitlab.com/groups/some-group/-/observability/test" } + + before do + stub_feature_flags(observability_group_tab: false) + end + + it_behaves_like 'does not embed observabilty' + + it 'does not build the embeddable url' do + expect(Gitlab::Observability).not_to have_received(:embeddable_url) + end + end end end diff --git a/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb b/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb index 1fdb29b688e..80061539a0b 100644 --- a/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb @@ -162,6 +162,54 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference} - closed)") end + + it 'shows title for references with +s' do + issue = create_issue(:opened, title: 'Some issue') + link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+s') + doc = filter(link, context) + + expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference}) • Unassigned") + end + + context 'when extended summary props are present' do + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:assignees) { create_list(:user, 3) } + let_it_be(:issue) { create_issue(:opened, title: 'Some issue', milestone: milestone, assignees: assignees) } + let_it_be(:link) do + create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+s') + end + + it 'shows extended summary for references with +s' do + doc = filter(link, context) + + expect(doc.css('a').last.text).to eq( + "#{issue.title} (#{issue.to_reference}) • #{assignees[0].name}, #{assignees[1].name}+ • #{milestone.title}" + ) + end + + describe 'checking N+1' do + let_it_be(:milestone2) { create(:milestone, project: project) } + let_it_be(:assignees2) { create_list(:user, 3) } + + it 'does not have N+1 for extended summary', :use_sql_query_cache do + issue2 = create_issue(:opened, title: 'Another issue', milestone: milestone2, assignees: assignees2) + link2 = create_link(issue2.to_reference, issue: issue2.id, reference_type: 'issue', reference_format: '+s') + + # warm up + filter(link, context) + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + filter(link, context) + end.count + + expect(control_count).to eq 10 + + expect do + filter("#{link} #{link2}", context) + end.not_to exceed_all_query_limit(control_count) + end + end + end end context 'for merge request references' do @@ -235,5 +283,80 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor expect(doc.css('a').last.text).to eq("#{merge_request.title} (#{merge_request.to_reference})") end + + it 'shows title for references with +s' do + merge_request = create_merge_request(:opened, title: 'Some merge request') + + link = create_link( + merge_request.to_reference, + merge_request: merge_request.id, + reference_type: 'merge_request', + reference_format: '+s' + ) + + doc = filter(link, context) + + expect(doc.css('a').last.text).to eq("#{merge_request.title} (#{merge_request.to_reference}) • Unassigned") + end + + context 'when extended summary props are present' do + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:assignees) { create_list(:user, 2) } + let_it_be(:merge_request) do + create_merge_request(:opened, title: 'Some merge request', milestone: milestone, assignees: assignees) + end + + let_it_be(:link) do + create_link( + merge_request.to_reference, + merge_request: merge_request.id, + reference_type: 'merge_request', + reference_format: '+s' + ) + end + + it 'shows extended summary for references with +s' do + doc = filter(link, context) + + expect(doc.css('a').last.text).to eq( + "#{merge_request.title} (#{merge_request.to_reference}) • #{assignees[0].name}, #{assignees[1].name} • " \ + "#{milestone.title}" + ) + end + + describe 'checking N+1' do + let_it_be(:milestone2) { create(:milestone, project: project) } + let_it_be(:assignees2) { create_list(:user, 3) } + + it 'does not have N+1 for extended summary', :use_sql_query_cache do + merge_request2 = create_merge_request( + :closed, + title: 'Some merge request', + milestone: milestone2, + assignees: assignees2 + ) + + link2 = create_link( + merge_request2.to_reference, + merge_request: merge_request2.id, + reference_type: 'merge_request', + reference_format: '+s' + ) + + # warm up + filter(link, context) + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + filter(link, context) + end.count + + expect(control_count).to eq 10 + + expect do + filter("#{link} #{link2}", context) + end.not_to exceed_all_query_limit(control_count) + end + end + end end end diff --git a/spec/lib/banzai/filter/references/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/references/issue_reference_filter_spec.rb index d8a97c6c3dc..aadd726ac40 100644 --- a/spec/lib/banzai/filter/references/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/issue_reference_filter_spec.rb @@ -150,6 +150,15 @@ RSpec.describe Banzai::Filter::References::IssueReferenceFilter, feature_categor expect(link.attr('href')).to eq(issue_url) end + it 'includes a data-reference-format attribute for extended summary URL references' do + doc = reference_filter("Issue #{issue_url}+s") + link = doc.css('a').first + + expect(link).to have_attribute('data-reference-format') + expect(link.attr('data-reference-format')).to eq('+s') + expect(link.attr('href')).to eq(issue_url) + end + it 'supports an :only_path context' do doc = reference_filter("Issue #{written_reference}", only_path: true) link = doc.css('a').first.attr('href') diff --git a/spec/lib/banzai/filter/references/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/references/merge_request_reference_filter_spec.rb index 9853d6f4093..156455221cf 100644 --- a/spec/lib/banzai/filter/references/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/merge_request_reference_filter_spec.rb @@ -128,6 +128,15 @@ RSpec.describe Banzai::Filter::References::MergeRequestReferenceFilter, feature_ expect(link.attr('href')).to eq(merge_request_url) end + it 'includes a data-reference-format attribute for extended summary URL references' do + doc = reference_filter("Merge #{merge_request_url}+s") + link = doc.css('a').first + + expect(link).to have_attribute('data-reference-format') + expect(link.attr('data-reference-format')).to eq('+s') + expect(link.attr('href')).to eq(merge_request_url) + end + it 'supports an :only_path context' do doc = reference_filter("Merge #{reference}", only_path: true) link = doc.css('a').first.attr('href') diff --git a/spec/lib/banzai/filter/references/reference_cache_spec.rb b/spec/lib/banzai/filter/references/reference_cache_spec.rb index 7307daca516..7e5ca00f118 100644 --- a/spec/lib/banzai/filter/references/reference_cache_spec.rb +++ b/spec/lib/banzai/filter/references/reference_cache_spec.rb @@ -76,8 +76,7 @@ RSpec.describe Banzai::Filter::References::ReferenceCache, feature_category: :te cache_single.load_records_per_parent end.count - expect(control_count).to eq 1 - + expect(control_count).to eq 2 # Since this is an issue filter that is not batching issue queries # across projects, we have to account for that. # 1 for original issue, 2 for second route/project, 1 for other issue |