diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2017-04-25 18:36:18 +0800 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2017-04-25 18:39:41 +0800 |
commit | 4ded8b1cc019f84615985c36c55797d55e1defc7 (patch) | |
tree | 450c79a50156293c1f05bf9edbf7e28f4ae6fce5 | |
parent | 9a905e1b9f9575bb8d637560cb3c59fd82079d2d (diff) | |
download | gitlab-ce-4ded8b1cc019f84615985c36c55797d55e1defc7.tar.gz |
Skip issuable without a project in IssuableExtractor#extract
Closes #31280
-rw-r--r-- | changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml | 4 | ||||
-rw-r--r-- | lib/banzai/issuable_extractor.rb | 6 | ||||
-rw-r--r-- | spec/factories/issues.rb | 4 | ||||
-rw-r--r-- | spec/lib/banzai/filter/issuable_state_filter_spec.rb | 113 |
4 files changed, 71 insertions, 56 deletions
diff --git a/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml b/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml new file mode 100644 index 00000000000..410480de573 --- /dev/null +++ b/changelogs/unreleased/31280-fix-showing-issues-from-pending-delete-projects.yml @@ -0,0 +1,4 @@ +--- +title: Fix 500 error due to trying to show issues from pending deleting projects +merge_request: 10906 +author: diff --git a/lib/banzai/issuable_extractor.rb b/lib/banzai/issuable_extractor.rb index c5ce360e172..19cd709ca49 100644 --- a/lib/banzai/issuable_extractor.rb +++ b/lib/banzai/issuable_extractor.rb @@ -28,9 +28,13 @@ module Banzai issue_parser = Banzai::ReferenceParser::IssueParser.new(project, user) merge_request_parser = Banzai::ReferenceParser::MergeRequestParser.new(project, user) - issue_parser.issues_for_nodes(nodes).merge( + hash = issue_parser.issues_for_nodes(nodes).merge( merge_request_parser.merge_requests_for_nodes(nodes) ) + + hash.each_with_object({}) do |(node, issuable), result| + result[node] = issuable if issuable.project + end end end end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 0b6977e3b17..f1fd1fd7f73 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -8,6 +8,10 @@ FactoryGirl.define do confidential true end + trait :opened do + state :opened + end + trait :closed do state :closed end diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb index 600f3c123ed..67f13ca1985 100644 --- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb @@ -6,11 +6,27 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do let(:user) { create(:user) } let(:context) { { current_user: user, issuable_state_filter_enabled: true } } + let(:closed_issue) { create_issue(:closed, project) } + let(:project) { create_project } + let(:other_project) { create_project } def create_link(text, data) link_to(text, '', class: 'gfm has-tooltip', data: data) end + def create_project + create(:empty_project, :public) + end + + def create_issue(state, the_project = project) + create(:issue, state, project: the_project) + end + + def create_merge_request(state, the_project = project) + create(:merge_request, state, + source_project: project, target_project: project) + end + it 'ignores non-GFM links' do html = %(See <a href="https://google.com/">Google</a>) doc = filter(html, current_user: user) @@ -19,7 +35,6 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do end it 'ignores non-issuable links' do - project = create(:empty_project, :public) link = create_link('text', project: project, reference_type: 'issue') doc = filter(link, context) @@ -27,27 +42,24 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do end it 'ignores issuable links with empty content' do - issue = create(:issue, :closed) - link = create_link('', issue: issue.id, reference_type: 'issue') + link = create_link('', issue: closed_issue.id, reference_type: 'issue') doc = filter(link, context) expect(doc.css('a').last.text).to eq('') end it 'ignores issuable links with custom anchor' do - issue = create(:issue, :closed) - link = create_link('something', issue: issue.id, reference_type: 'issue') + link = create_link('something', issue: closed_issue.id, reference_type: 'issue') doc = filter(link, context) expect(doc.css('a').last.text).to eq('something') end it 'ignores issuable links to specific comments' do - issue = create(:issue, :closed) - link = create_link("#{issue.to_reference} (comment 1)", issue: issue.id, reference_type: 'issue') + link = create_link("#{closed_issue.to_reference} (comment 1)", issue: closed_issue.id, reference_type: 'issue') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.to_reference} (comment 1)") + expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference} (comment 1)") end it 'ignores merge request links to diffs tab' do @@ -63,26 +75,36 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do end it 'handles cross project references' do - issue = create(:issue, :closed) - project = create(:empty_project) - link = create_link(issue.to_reference(project), issue: issue.id, reference_type: 'issue') - doc = filter(link, context.merge(project: project)) + link = create_link(closed_issue.to_reference(other_project), issue: closed_issue.id, reference_type: 'issue') + doc = filter(link, context.merge(project: other_project)) - expect(doc.css('a').last.text).to eq("#{issue.to_reference(project)} (closed)") + expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference(other_project)} (closed)") end it 'does not append state when filter is not enabled' do - issue = create(:issue, :closed) - link = create_link('text', issue: issue.id, reference_type: 'issue') + link = create_link('text', issue: closed_issue.id, reference_type: 'issue') context = { current_user: user } doc = filter(link, context) expect(doc.css('a').last.text).to eq('text') end + context 'when project is in pending delete' do + before do + project.update(pending_delete: true) + end + + it 'does not append issue state' do + link = create_link('text', issue: closed_issue.id, reference_type: 'issue') + doc = filter(link, context) + + expect(doc.css('a').last.text).to eq('text') + end + end + context 'for issue references' do it 'ignores open issue references' do - issue = create(:issue) + issue = create_issue(:opened) link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue') doc = filter(link, context) @@ -90,7 +112,7 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do end it 'ignores reopened issue references' do - issue = create(:issue, :reopened) + issue = create_issue(:reopened) link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue') doc = filter(link, context) @@ -98,73 +120,54 @@ describe Banzai::Filter::IssuableStateFilter, lib: true do end it 'appends state to closed issue references' do - issue = create(:issue, :closed) - link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue') + link = create_link(closed_issue.to_reference, issue: closed_issue.id, reference_type: 'issue') doc = filter(link, context) - expect(doc.css('a').last.text).to eq("#{issue.to_reference} (closed)") + expect(doc.css('a').last.text).to eq("#{closed_issue.to_reference} (closed)") end end context 'for merge request references' do - it 'ignores open merge request references' do - merge_request = create(:merge_request) + def expect_link_text(merge_request, text) link = create_link( merge_request.to_reference, merge_request: merge_request.id, reference_type: 'merge_request' ) + doc = filter(link, context) - expect(doc.css('a').last.text).to eq(merge_request.to_reference) + expect(doc.css('a').last.text).to eq(text) + end + + it 'ignores open merge request references' do + merge_request = create_merge_request(:opened) + + expect_link_text(merge_request, merge_request.to_reference) end it 'ignores reopened merge request references' do - merge_request = create(:merge_request, :reopened) - link = create_link( - merge_request.to_reference, - merge_request: merge_request.id, - reference_type: 'merge_request' - ) - doc = filter(link, context) + merge_request = create_merge_request(:reopened) - expect(doc.css('a').last.text).to eq(merge_request.to_reference) + expect_link_text(merge_request, merge_request.to_reference) end it 'ignores locked merge request references' do - merge_request = create(:merge_request, :locked) - link = create_link( - merge_request.to_reference, - merge_request: merge_request.id, - reference_type: 'merge_request' - ) - doc = filter(link, context) + merge_request = create_merge_request(:locked) - expect(doc.css('a').last.text).to eq(merge_request.to_reference) + expect_link_text(merge_request, merge_request.to_reference) end it 'appends state to closed merge request references' do - merge_request = create(:merge_request, :closed) - link = create_link( - merge_request.to_reference, - merge_request: merge_request.id, - reference_type: 'merge_request' - ) - doc = filter(link, context) + merge_request = create_merge_request(:closed) - expect(doc.css('a').last.text).to eq("#{merge_request.to_reference} (closed)") + expect_link_text(merge_request, "#{merge_request.to_reference} (closed)") end it 'appends state to merged merge request references' do - merge_request = create(:merge_request, :merged) - link = create_link( - merge_request.to_reference, - merge_request: merge_request.id, - reference_type: 'merge_request' - ) - doc = filter(link, context) + merge_request = create_merge_request(:merged) - expect(doc.css('a').last.text).to eq("#{merge_request.to_reference} (merged)") + expect_link_text(merge_request, "#{merge_request.to_reference} (merged)") end end end |