summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLin Jen-Shin <godfat@godfat.org>2017-04-25 18:36:18 +0800
committerLin Jen-Shin <godfat@godfat.org>2017-04-25 18:39:41 +0800
commit4ded8b1cc019f84615985c36c55797d55e1defc7 (patch)
tree450c79a50156293c1f05bf9edbf7e28f4ae6fce5
parent9a905e1b9f9575bb8d637560cb3c59fd82079d2d (diff)
downloadgitlab-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.yml4
-rw-r--r--lib/banzai/issuable_extractor.rb6
-rw-r--r--spec/factories/issues.rb4
-rw-r--r--spec/lib/banzai/filter/issuable_state_filter_spec.rb113
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