diff options
Diffstat (limited to 'spec/lib/banzai')
4 files changed, 431 insertions, 33 deletions
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 0933f45e7c3..e14b1362687 100644 --- a/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_reference_expansion_filter_spec.rb @@ -21,6 +21,10 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor create(:issue, state, attributes.merge(project: project)) end + def create_item(issuable_type, state, attributes = {}) + create(issuable_type, state, attributes.merge(project: project)) + end + def create_merge_request(state, attributes = {}) create(:merge_request, state, attributes.merge(source_project: project, target_project: project)) end @@ -115,75 +119,88 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor end end - context 'for issue references' do - it 'ignores open issue references' do - issue = create_issue(:opened) - link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue') + shared_examples 'issue / work item references' do + it 'ignores open references' do + issuable = create_item(issuable_type, :opened) + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type) doc = filter(link, context) - expect(doc.css('a').last.text).to eq(issue.to_reference) + expect(doc.css('a').last.text).to eq(issuable.to_reference) end - it 'appends state to closed issue references' do - link = create_link(closed_issue.to_reference, issue: closed_issue.id, reference_type: 'issue') + it 'appends state to moved references' do + moved_issuable = create_item(issuable_type, :closed, project: project, + moved_to: create_item(issuable_type, :opened)) + link = create_link(moved_issuable.to_reference, "#{issuable_type}": moved_issuable.id, + reference_type: issuable_type) doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference} (closed)") + expect(doc.css('a').last.text).to eq("#{moved_issuable.to_reference} (moved)") end - it 'appends state to moved issue references' do - moved_issue = create(:issue, :closed, project: project, moved_to: create_issue(:opened)) - link = create_link(moved_issue.to_reference, issue: moved_issue.id, reference_type: 'issue') + it 'appends state to closed references' do + issuable = create_item(issuable_type, :closed) + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type) doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{moved_issue.to_reference} (moved)") + expect(doc.css('a').last.text).to eq("#{issuable.to_reference} (closed)") end it 'shows title for references with +' do - issue = create_issue(:opened, title: 'Some issue') - link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+') + issuable = create_item(issuable_type, :opened, title: 'Some issue') + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type, + reference_format: '+') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference})") + expect(doc.css('a').last.text).to eq("#{issuable.title} (#{issuable.to_reference})") end it 'truncates long title for references with +' do - issue = create_issue(:opened, title: 'Some issue ' * 10) - link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+') + issuable = create_item(issuable_type, :opened, title: 'Some issue ' * 10) + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type, + reference_format: '+') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.title.truncate(50)} (#{issue.to_reference})") + expect(doc.css('a').last.text).to eq("#{issuable.title.truncate(50)} (#{issuable.to_reference})") end it 'shows both title and state for closed references with +' do - issue = create_issue(:closed, title: 'Some issue') - link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+') + issuable = create_item(issuable_type, :closed, title: 'Some issue') + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type, + reference_format: '+') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference} - closed)") + expect(doc.css('a').last.text).to eq("#{issuable.title} (#{issuable.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') + issuable = create_item(issuable_type, :opened, title: 'Some issue') + link = create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type, + reference_format: '+s') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference}) • Unassigned") + expect(doc.css('a').last.text).to eq("#{issuable.title} (#{issuable.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(:issuable) do + create_item(issuable_type, :opened, title: 'Some issue', milestone: milestone, + assignees: assignees) + end + let_it_be(:link) do - create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+s') + create_link(issuable.to_reference, "#{issuable_type}": issuable.id, reference_type: issuable_type, + 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}" + "#{issuable.title} (#{issuable.to_reference}) • #{assignees[0].name}, #{assignees[1].name}+ " \ + "• #{milestone.title}" ) end @@ -192,8 +209,10 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor 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') + issuable2 = create_item(issuable_type, :opened, title: 'Another issue', + milestone: milestone2, assignees: assignees2) + link2 = create_link(issuable2.to_reference, "#{issuable_type}": issuable2.id, + reference_type: issuable_type, reference_format: '+s') # warm up filter(link, context) @@ -212,6 +231,18 @@ RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter, feature_categor end end + context 'for work item references' do + let_it_be(:issuable_type) { :work_item } + + it_behaves_like 'issue / work item references' + end + + context 'for issue references' do + let_it_be(:issuable_type) { :issue } + + it_behaves_like 'issue / work item references' + end + context 'for merge request references' do it 'ignores open merge request references' do merge_request = create_merge_request(:opened) diff --git a/spec/lib/banzai/filter/references/work_item_reference_filter_spec.rb b/spec/lib/banzai/filter/references/work_item_reference_filter_spec.rb new file mode 100644 index 00000000000..e59e53891bf --- /dev/null +++ b/spec/lib/banzai/filter/references/work_item_reference_filter_spec.rb @@ -0,0 +1,314 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::References::WorkItemReferenceFilter, feature_category: :team_planning do + include FilterSpecHelper + + let_it_be(:namespace) { create(:namespace, name: 'main-namespace') } + let_it_be(:project) { create(:project, :public, namespace: namespace, path: 'main-project') } + let_it_be(:cross_namespace) { create(:namespace, name: 'cross-namespace') } + let_it_be(:cross_project) { create(:project, :public, namespace: cross_namespace, path: 'cross-project') } + let_it_be(:work_item) { create(:work_item, project: project) } + + def item_url(item) + work_item_path = "/#{item.project.namespace.path}/#{item.project.path}/-/work_items/#{item.iid}" + + "http://#{Gitlab.config.gitlab.host}#{work_item_path}" + end + + it 'subclasses from IssueReferenceFilter' do + expect(described_class.superclass).to eq Banzai::Filter::References::IssueReferenceFilter + end + + shared_examples 'a reference with work item type information' do + it 'contains work-item-type as a data attribute' do + doc = reference_filter("Fixed #{reference}") + + expect(doc.css('a').first.attr('data-work-item-type')).to eq('issue') + end + end + + shared_examples 'a work item reference' do + it_behaves_like 'a reference containing an element node' + + it_behaves_like 'a reference with work item type information' + + it 'links to a valid reference' do + doc = reference_filter("Fixed #{written_reference}") + + expect(doc.css('a').first.attr('href')).to eq work_item_url + end + + it 'links with adjacent text' do + doc = reference_filter("Fixed (#{written_reference}.)") + + expect(doc.text).to match(%r{^Fixed \(.*\.\)}) + end + + it 'includes a title attribute' do + doc = reference_filter("Issue #{written_reference}") + + expect(doc.css('a').first.attr('title')).to eq work_item.title + end + + it 'escapes the title attribute' do + work_item.update_attribute(:title, %("></a>whatever<a title=")) + + doc = reference_filter("Issue #{written_reference}") + + expect(doc.text).not_to include 'whatever' + end + + it 'renders non-HTML tooltips' do + doc = reference_filter("Issue #{written_reference}") + + expect(doc.at_css('a')).not_to have_attribute('data-html') + end + + it 'includes default classes' do + doc = reference_filter("Issue #{written_reference}") + expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-work_item' + end + + it 'includes a data-project attribute' do + doc = reference_filter("Issue #{written_reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq cross_project.id.to_s + end + + it 'includes a data-issue attribute' do + doc = reference_filter("See #{written_reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-work-item') + expect(link.attr('data-work-item')).to eq work_item.id.to_s + end + + it 'includes data attributes for issuable popover' do + doc = reference_filter("See #{written_reference}") + link = doc.css('a').first + + expect(link.attr('data-project-path')).to eq cross_project.full_path + expect(link.attr('data-iid')).to eq work_item.iid.to_s + end + + it 'includes a data-original attribute' do + doc = reference_filter("See #{written_reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-original') + expect(link.attr('data-original')).to eq inner_text + end + + it 'does not escape the data-original attribute' do + skip if written_reference.start_with?('<a') + + inner_html = 'element <code>node</code> inside' + doc = reference_filter(%(<a href="#{written_reference}">#{inner_html}</a>)) + + expect(doc.children.first.attr('data-original')).to eq inner_html + end + + it 'includes a data-reference-format attribute' do + skip if written_reference.start_with?('<a') + + doc = reference_filter("Issue #{written_reference}+") + link = doc.css('a').first + + expect(link).to have_attribute('data-reference-format') + expect(link.attr('data-reference-format')).to eq('+') + expect(link.attr('href')).to eq(work_item_url) + end + + it 'includes a data-reference-format attribute for URL references' do + doc = reference_filter("Issue #{work_item_url}+") + link = doc.css('a').first + + expect(link).to have_attribute('data-reference-format') + expect(link.attr('data-reference-format')).to eq('+') + expect(link.attr('href')).to eq(work_item_url) + end + + it 'includes a data-reference-format attribute for extended summary URL references' do + doc = reference_filter("Issue #{work_item_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(work_item_url) + end + + it 'does not process links containing issue numbers followed by text' do + href = "#{written_reference}st" + doc = reference_filter("<a href='#{href}'></a>") + link = doc.css('a').first.attr('href') + + expect(link).to eq(href) + end + end + + # Example: + # "See #1" + context 'when standard internal reference' do + it 'is handled by IssueReferenceFilter, not WorkItemReferenceFilter' do + doc = reference_filter("Fixed ##{work_item.iid}") + + expect(doc.css('a')).to be_empty + end + end + + # Example: + # "See cross-namespace/cross-project#1" + context 'when cross-project / cross-namespace complete reference' do + let_it_be(:work_item2) { create(:work_item, project: cross_project) } + let_it_be(:reference) { "#{cross_project.full_path}##{work_item2.iid}" } + + it 'is handled by IssueReferenceFilter, not WorkItemReferenceFilter' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a')).to be_empty + end + end + + # Example: + # "See main-namespace/cross-project#1" + context 'when cross-project / same-namespace complete reference' do + let_it_be(:cross_project) { create(:project, :public, namespace: namespace, path: 'cross-project') } + let_it_be(:work_item) { create(:work_item, project: cross_project) } + let_it_be(:reference) { "#{cross_project.full_path}##{work_item.iid}" } + + it 'is handled by IssueReferenceFilter, not WorkItemReferenceFilter' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a')).to be_empty + end + end + + # Example: + # "See cross-project#1" + context 'when cross-project / same-namespace shorthand reference' do + let_it_be(:cross_project) { create(:project, :public, namespace: namespace, path: 'cross-project') } + let_it_be(:work_item) { create(:work_item, project: cross_project) } + let_it_be(:reference) { "#{cross_project.path}##{work_item.iid}" } + + it 'is handled by IssueReferenceFilter, not WorkItemReferenceFilter' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a')).to be_empty + end + end + + # Example: + # "See http://localhost/cross-namespace/cross-project/-/work_items/1" + context 'when cross-project URL reference' do + let_it_be(:work_item, reload: true) { create(:work_item, project: cross_project) } + let_it_be(:work_item_url) { item_url(work_item) } + let_it_be(:reference) { work_item_url } + let_it_be(:written_reference) { reference } + let_it_be(:inner_text) { written_reference } + + it_behaves_like 'a work item reference' + end + + # Example: + # "See http://localhost/cross-namespace/cross-project/-/work_items/1#note_123" + context 'when cross-project URL reference with comment anchor' do + let_it_be(:work_item) { create(:work_item, project: cross_project) } + let_it_be(:work_item_url) { item_url(work_item) } + let_it_be(:reference) { "#{work_item_url}#note_123" } + + it_behaves_like 'a reference containing an element node' + + it_behaves_like 'a reference with work item type information' + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('href')).to eq reference + end + + it 'link with trailing slash' do + doc = reference_filter("Fixed (#{work_item_url}/.)") + + expect(doc.to_html).to match(%r{\(<a.+>#{Regexp.escape(work_item.to_reference(project))}</a>\.\)}) + end + + it 'links with adjacent text' do + doc = reference_filter("Fixed (#{reference}.)") + + expect(doc.to_html).to match(%r{\(<a.+>#{Regexp.escape(work_item.to_reference(project))} \(comment 123\)</a>\.\)}) + end + end + + # Example: + # 'See <a href="cross-namespace/cross-project#1">Reference</a>'' + context 'when cross-project reference in link href' do + let_it_be(:work_item) { create(:work_item, project: cross_project) } + let_it_be(:reference) { work_item.to_reference(project) } + let_it_be(:reference_link) { %(<a href="#{reference}">Reference</a>) } + let_it_be(:work_item_url) { item_url(work_item) } + + it 'is handled by IssueReferenceFilter, not WorkItemReferenceFilter' do + doc = reference_filter("See #{reference_link}") + + expect(doc.css('a').first[:href]).to eq reference + expect(doc.css('a').first[:href]).not_to eq work_item_url + end + end + + # Example: + # 'See <a href=\"http://localhost/cross-namespace/cross-project/-/work_items/1\">Reference</a>'' + context 'when cross-project URL in link href' do + let_it_be(:work_item, reload: true) { create(:work_item, project: cross_project) } + let_it_be(:work_item_url) { item_url(work_item) } + let_it_be(:reference) { work_item_url } + let_it_be(:reference_link) { %(<a href="#{reference}">Reference</a>) } + let_it_be(:written_reference) { reference_link } + let_it_be(:inner_text) { 'Reference' } + + it_behaves_like 'a work item reference' + end + + context 'for group context' do + let_it_be(:group) { create(:group) } + let_it_be(:context) { { project: nil, group: group } } + let_it_be(:work_item_url) { item_url(work_item) } + + it 'links to a valid reference for url cross-namespace' do + reference = "#{work_item_url}#note_123" + + doc = reference_filter("See #{reference}", context) + + link = doc.css('a').first + expect(link.attr('href')).to eq("#{work_item_url}#note_123") + expect(link.text).to include("#{project.full_path}##{work_item.iid}") + end + + it 'links to a valid reference for cross-namespace in link href' do + reference = "#{work_item_url}#note_123" + reference_link = %(<a href="#{reference}">Reference</a>) + + doc = reference_filter("See #{reference_link}", context) + + link = doc.css('a').first + expect(link.attr('href')).to eq("#{work_item_url}#note_123") + expect(link.text).to include('Reference') + end + end + + describe 'performance' do + let(:another_work_item) { create(:work_item, project: project) } + + it 'does not have a N+1 query problem' do + single_reference = "Work item #{work_item.to_reference}" + multiple_references = "Work items #{work_item.to_reference} and #{another_work_item.to_reference}" + + control_count = ActiveRecord::QueryRecorder.new { reference_filter(single_reference).to_html }.count + + expect { reference_filter(multiple_references).to_html }.not_to exceed_query_limit(control_count) + end + end +end diff --git a/spec/lib/banzai/issuable_extractor_spec.rb b/spec/lib/banzai/issuable_extractor_spec.rb index b2c869bd066..5bbd98592e7 100644 --- a/spec/lib/banzai/issuable_extractor_spec.rb +++ b/spec/lib/banzai/issuable_extractor_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Banzai::IssuableExtractor, feature_category: :team_planning do let(:user) { create(:user) } let(:extractor) { described_class.new(Banzai::RenderContext.new(project, user)) } let(:issue) { create(:issue, project: project) } + let(:work_item) { create(:work_item, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } let(:issue_link) do html_to_node( @@ -14,6 +15,12 @@ RSpec.describe Banzai::IssuableExtractor, feature_category: :team_planning do ) end + let(:work_item_link) do + html_to_node( + "<a href='' data-work-item='#{work_item.id}' data-reference-type='work_item' class='gfm'>text</a>" + ) + end + let(:merge_request_link) do html_to_node( "<a href='' data-merge-request='#{merge_request.id}' data-reference-type='merge_request' class='gfm'>text</a>" @@ -27,17 +34,17 @@ RSpec.describe Banzai::IssuableExtractor, feature_category: :team_planning do end it 'returns instances of issuables for nodes with references' do - result = extractor.extract([issue_link, merge_request_link]) + result = extractor.extract([issue_link, work_item_link, merge_request_link]) - expect(result).to eq(issue_link => issue, merge_request_link => merge_request) + expect(result).to eq(issue_link => issue, work_item_link => work_item, merge_request_link => merge_request) end describe 'caching', :request_store do it 'saves records to cache' do - extractor.extract([issue_link, merge_request_link]) + extractor.extract([issue_link, work_item_link, merge_request_link]) second_call_queries = ActiveRecord::QueryRecorder.new do - extractor.extract([issue_link, merge_request_link]) + extractor.extract([issue_link, work_item_link, merge_request_link]) end.count expect(second_call_queries).to eq 0 diff --git a/spec/lib/banzai/reference_parser/work_item_parser_spec.rb b/spec/lib/banzai/reference_parser/work_item_parser_spec.rb new file mode 100644 index 00000000000..dbde01cc94f --- /dev/null +++ b/spec/lib/banzai/reference_parser/work_item_parser_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::ReferenceParser::WorkItemParser, feature_category: :team_planning do + include ReferenceParserHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be_with_reload(:project) { create(:project, :public, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:work_item) { create(:work_item, project: project) } + let_it_be(:link) { empty_html_link } + + subject { described_class.new(Banzai::RenderContext.new(project, user)) } + + describe '#records_for_nodes' do + it 'returns a Hash containing the work items for a list of nodes' do + link['data-work-item'] = work_item.id.to_s + nodes = [link] + + expect(subject.records_for_nodes(nodes)).to eq({ link => work_item }) + end + end + + context 'when checking multiple work items on another project' do + let_it_be(:other_project) { create(:project, :public) } + let_it_be(:other_work_item) { create(:work_item, project: other_project) } + let_it_be(:control_links) do + [work_item_link(other_work_item)] + end + + let_it_be(:actual_links) do + control_links + [work_item_link(create(:work_item, project: other_project))] + end + + def work_item_link(work_item) + Nokogiri::HTML.fragment(%(<a data-work-item="#{work_item.id}"></a>)).children[0] + end + + before do + project.add_developer(user) + end + + it_behaves_like 'no N+1 queries' + end +end |