Commit a73e068c06532480137b8b9861ed88f42578bb12
1 parent
434c0341
Exists in
master
and in
4 other branches
Fixing tests after adding iid for issues/mr
Showing
5 changed files
with
37 additions
and
46 deletions
Show diff stats
app/helpers/issues_helper.rb
@@ -37,23 +37,23 @@ module IssuesHelper | @@ -37,23 +37,23 @@ module IssuesHelper | ||
37 | end | 37 | end |
38 | end | 38 | end |
39 | 39 | ||
40 | - def url_for_issue(issue_id) | 40 | + def url_for_issue(issue_iid) |
41 | return "" if @project.nil? | 41 | return "" if @project.nil? |
42 | 42 | ||
43 | if @project.used_default_issues_tracker? | 43 | if @project.used_default_issues_tracker? |
44 | - url = project_issue_url project_id: @project, id: issue_id | 44 | + url = project_issue_url project_id: @project, id: issue_iid |
45 | else | 45 | else |
46 | url = Gitlab.config.issues_tracker[@project.issues_tracker]["issues_url"] | 46 | url = Gitlab.config.issues_tracker[@project.issues_tracker]["issues_url"] |
47 | - url.gsub(':id', issue_id.to_s) | 47 | + url.gsub(':id', issue_iid.to_s) |
48 | .gsub(':project_id', @project.id.to_s) | 48 | .gsub(':project_id', @project.id.to_s) |
49 | .gsub(':issues_tracker_id', @project.issues_tracker_id.to_s) | 49 | .gsub(':issues_tracker_id', @project.issues_tracker_id.to_s) |
50 | end | 50 | end |
51 | end | 51 | end |
52 | 52 | ||
53 | - def title_for_issue(issue_id) | 53 | + def title_for_issue(issue_iid) |
54 | return "" if @project.nil? | 54 | return "" if @project.nil? |
55 | 55 | ||
56 | - if @project.used_default_issues_tracker? && issue = @project.issues.where(id: issue_id).first | 56 | + if @project.used_default_issues_tracker? && issue = @project.issues.where(iid: issue_iid).first |
57 | issue.title | 57 | issue.title |
58 | else | 58 | else |
59 | "" | 59 | "" |
spec/controllers/merge_requests_controller_spec.rb
@@ -8,13 +8,13 @@ describe Projects::MergeRequestsController do | @@ -8,13 +8,13 @@ describe Projects::MergeRequestsController do | ||
8 | before do | 8 | before do |
9 | sign_in(user) | 9 | sign_in(user) |
10 | project.team << [user, :master] | 10 | project.team << [user, :master] |
11 | - Projects::MergeRequestsController.any_instance.stub(validates_merge_request: true) | 11 | + Projects::MergeRequestsController.any_instance.stub(validates_merge_request: true, ) |
12 | end | 12 | end |
13 | 13 | ||
14 | describe "#show" do | 14 | describe "#show" do |
15 | shared_examples "export merge as" do |format| | 15 | shared_examples "export merge as" do |format| |
16 | it "should generally work" do | 16 | it "should generally work" do |
17 | - get :show, project_id: project.code, id: merge_request.id, format: format | 17 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
18 | 18 | ||
19 | expect(response).to be_success | 19 | expect(response).to be_success |
20 | end | 20 | end |
@@ -22,11 +22,11 @@ describe Projects::MergeRequestsController do | @@ -22,11 +22,11 @@ describe Projects::MergeRequestsController do | ||
22 | it "should generate it" do | 22 | it "should generate it" do |
23 | MergeRequest.any_instance.should_receive(:"to_#{format}") | 23 | MergeRequest.any_instance.should_receive(:"to_#{format}") |
24 | 24 | ||
25 | - get :show, project_id: project.code, id: merge_request.id, format: format | 25 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
26 | end | 26 | end |
27 | 27 | ||
28 | it "should render it" do | 28 | it "should render it" do |
29 | - get :show, project_id: project.code, id: merge_request.id, format: format | 29 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
30 | 30 | ||
31 | expect(response.body).to eq((merge_request.send(:"to_#{format}",user)).to_s) | 31 | expect(response.body).to eq((merge_request.send(:"to_#{format}",user)).to_s) |
32 | end | 32 | end |
@@ -34,7 +34,7 @@ describe Projects::MergeRequestsController do | @@ -34,7 +34,7 @@ describe Projects::MergeRequestsController do | ||
34 | it "should not escape Html" do | 34 | it "should not escape Html" do |
35 | MergeRequest.any_instance.stub(:"to_#{format}").and_return('HTML entities &<>" ') | 35 | MergeRequest.any_instance.stub(:"to_#{format}").and_return('HTML entities &<>" ') |
36 | 36 | ||
37 | - get :show, project_id: project.code, id: merge_request.id, format: format | 37 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
38 | 38 | ||
39 | expect(response.body).to_not include('&') | 39 | expect(response.body).to_not include('&') |
40 | expect(response.body).to_not include('>') | 40 | expect(response.body).to_not include('>') |
@@ -48,7 +48,7 @@ describe Projects::MergeRequestsController do | @@ -48,7 +48,7 @@ describe Projects::MergeRequestsController do | ||
48 | let(:format) { :diff } | 48 | let(:format) { :diff } |
49 | 49 | ||
50 | it "should really only be a git diff" do | 50 | it "should really only be a git diff" do |
51 | - get :show, project_id: project.code, id: merge_request.id, format: format | 51 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
52 | 52 | ||
53 | expect(response.body).to start_with("diff --git") | 53 | expect(response.body).to start_with("diff --git") |
54 | end | 54 | end |
@@ -59,24 +59,13 @@ describe Projects::MergeRequestsController do | @@ -59,24 +59,13 @@ describe Projects::MergeRequestsController do | ||
59 | let(:format) { :patch } | 59 | let(:format) { :patch } |
60 | 60 | ||
61 | it "should really be a git email patch with commit" do | 61 | it "should really be a git email patch with commit" do |
62 | - get :show, project_id: project.code, id: merge_request.id, format: format | 62 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
63 | 63 | ||
64 | expect(response.body[0..100]).to start_with("From #{merge_request.commits.last.id}") | 64 | expect(response.body[0..100]).to start_with("From #{merge_request.commits.last.id}") |
65 | end | 65 | end |
66 | 66 | ||
67 | - # TODO: fix or remove | ||
68 | - #it "should contain as many patches as there are commits" do | ||
69 | - #get :show, project_id: project.code, id: merge_request.id, format: format | ||
70 | - | ||
71 | - #patch_count = merge_request.commits.count | ||
72 | - #merge_request.commits.each_with_index do |commit, patch_num| | ||
73 | - #expect(response.body).to match(/^From #{commit.id}/) | ||
74 | - #expect(response.body).to match(/^Subject: \[PATCH #{patch_num}\/#{patch_count}\]/) | ||
75 | - #end | ||
76 | - #end | ||
77 | - | ||
78 | it "should contain git diffs" do | 67 | it "should contain git diffs" do |
79 | - get :show, project_id: project.code, id: merge_request.id, format: format | 68 | + get :show, project_id: project.code, id: merge_request.iid, format: format |
80 | 69 | ||
81 | expect(response.body).to match(/^diff --git/) | 70 | expect(response.body).to match(/^diff --git/) |
82 | end | 71 | end |
spec/helpers/gitlab_markdown_helper_spec.rb
@@ -20,7 +20,7 @@ describe GitlabMarkdownHelper do | @@ -20,7 +20,7 @@ describe GitlabMarkdownHelper do | ||
20 | 20 | ||
21 | describe "#gfm" do | 21 | describe "#gfm" do |
22 | it "should return unaltered text if project is nil" do | 22 | it "should return unaltered text if project is nil" do |
23 | - actual = "Testing references: ##{issue.id}" | 23 | + actual = "Testing references: ##{issue.iid}" |
24 | 24 | ||
25 | gfm(actual).should_not == actual | 25 | gfm(actual).should_not == actual |
26 | 26 | ||
@@ -175,14 +175,14 @@ describe GitlabMarkdownHelper do | @@ -175,14 +175,14 @@ describe GitlabMarkdownHelper do | ||
175 | 175 | ||
176 | describe "referencing an issue" do | 176 | describe "referencing an issue" do |
177 | let(:object) { issue } | 177 | let(:object) { issue } |
178 | - let(:reference) { "##{issue.id}" } | 178 | + let(:reference) { "##{issue.iid}" } |
179 | 179 | ||
180 | include_examples 'referenced object' | 180 | include_examples 'referenced object' |
181 | end | 181 | end |
182 | 182 | ||
183 | describe "referencing a merge request" do | 183 | describe "referencing a merge request" do |
184 | let(:object) { merge_request } | 184 | let(:object) { merge_request } |
185 | - let(:reference) { "!#{merge_request.id}" } | 185 | + let(:reference) { "!#{merge_request.iid}" } |
186 | 186 | ||
187 | include_examples 'referenced object' | 187 | include_examples 'referenced object' |
188 | end | 188 | end |
@@ -230,7 +230,7 @@ describe GitlabMarkdownHelper do | @@ -230,7 +230,7 @@ describe GitlabMarkdownHelper do | ||
230 | end | 230 | end |
231 | 231 | ||
232 | describe "referencing multiple objects" do | 232 | describe "referencing multiple objects" do |
233 | - let(:actual) { "!#{merge_request.id} -> #{commit.id} -> ##{issue.id}" } | 233 | + let(:actual) { "!#{merge_request.iid} -> #{commit.id} -> ##{issue.iid}" } |
234 | 234 | ||
235 | it "should link to the merge request" do | 235 | it "should link to the merge request" do |
236 | expected = project_merge_request_path(project, merge_request) | 236 | expected = project_merge_request_path(project, merge_request) |
@@ -299,7 +299,7 @@ describe GitlabMarkdownHelper do | @@ -299,7 +299,7 @@ describe GitlabMarkdownHelper do | ||
299 | let(:issues) { create_list(:issue, 2, project: project) } | 299 | let(:issues) { create_list(:issue, 2, project: project) } |
300 | 300 | ||
301 | it "should handle references nested in links with all the text" do | 301 | it "should handle references nested in links with all the text" do |
302 | - actual = link_to_gfm("This should finally fix ##{issues[0].id} and ##{issues[1].id} for real", commit_path) | 302 | + actual = link_to_gfm("This should finally fix ##{issues[0].iid} and ##{issues[1].iid} for real", commit_path) |
303 | 303 | ||
304 | # Break the result into groups of links with their content, without | 304 | # Break the result into groups of links with their content, without |
305 | # closing tags | 305 | # closing tags |
@@ -311,7 +311,7 @@ describe GitlabMarkdownHelper do | @@ -311,7 +311,7 @@ describe GitlabMarkdownHelper do | ||
311 | 311 | ||
312 | # First issue link | 312 | # First issue link |
313 | groups[1].should match(/href="#{project_issue_url(project, issues[0])}"/) | 313 | groups[1].should match(/href="#{project_issue_url(project, issues[0])}"/) |
314 | - groups[1].should match(/##{issues[0].id}$/) | 314 | + groups[1].should match(/##{issues[0].iid}$/) |
315 | 315 | ||
316 | # Internal commit link | 316 | # Internal commit link |
317 | groups[2].should match(/href="#{commit_path}"/) | 317 | groups[2].should match(/href="#{commit_path}"/) |
@@ -319,7 +319,7 @@ describe GitlabMarkdownHelper do | @@ -319,7 +319,7 @@ describe GitlabMarkdownHelper do | ||
319 | 319 | ||
320 | # Second issue link | 320 | # Second issue link |
321 | groups[3].should match(/href="#{project_issue_url(project, issues[1])}"/) | 321 | groups[3].should match(/href="#{project_issue_url(project, issues[1])}"/) |
322 | - groups[3].should match(/##{issues[1].id}$/) | 322 | + groups[3].should match(/##{issues[1].iid}$/) |
323 | 323 | ||
324 | # Trailing commit link | 324 | # Trailing commit link |
325 | groups[4].should match(/href="#{commit_path}"/) | 325 | groups[4].should match(/href="#{commit_path}"/) |
@@ -332,7 +332,7 @@ describe GitlabMarkdownHelper do | @@ -332,7 +332,7 @@ describe GitlabMarkdownHelper do | ||
332 | end | 332 | end |
333 | 333 | ||
334 | it "escapes HTML passed in as the body" do | 334 | it "escapes HTML passed in as the body" do |
335 | - actual = "This is a <h1>test</h1> - see ##{issues[0].id}" | 335 | + actual = "This is a <h1>test</h1> - see ##{issues[0].iid}" |
336 | link_to_gfm(actual, commit_path).should match('<h1>test</h1>') | 336 | link_to_gfm(actual, commit_path).should match('<h1>test</h1>') |
337 | end | 337 | end |
338 | end | 338 | end |
@@ -345,25 +345,25 @@ describe GitlabMarkdownHelper do | @@ -345,25 +345,25 @@ describe GitlabMarkdownHelper do | ||
345 | end | 345 | end |
346 | 346 | ||
347 | it "should handle references in headers" do | 347 | it "should handle references in headers" do |
348 | - actual = "\n# Working around ##{issue.id}\n## Apply !#{merge_request.id}" | 348 | + actual = "\n# Working around ##{issue.iid}\n## Apply !#{merge_request.iid}" |
349 | 349 | ||
350 | - markdown(actual).should match(%r{<h1[^<]*>Working around <a.+>##{issue.id}</a></h1>}) | ||
351 | - markdown(actual).should match(%r{<h2[^<]*>Apply <a.+>!#{merge_request.id}</a></h2>}) | 350 | + markdown(actual).should match(%r{<h1[^<]*>Working around <a.+>##{issue.iid}</a></h1>}) |
351 | + markdown(actual).should match(%r{<h2[^<]*>Apply <a.+>!#{merge_request.iid}</a></h2>}) | ||
352 | end | 352 | end |
353 | 353 | ||
354 | it "should handle references in lists" do | 354 | it "should handle references in lists" do |
355 | project.team << [user, :master] | 355 | project.team << [user, :master] |
356 | 356 | ||
357 | - actual = "\n* dark: ##{issue.id}\n* light by @#{member.user.username}" | 357 | + actual = "\n* dark: ##{issue.iid}\n* light by @#{member.user.username}" |
358 | 358 | ||
359 | - markdown(actual).should match(%r{<li>dark: <a.+>##{issue.id}</a></li>}) | 359 | + markdown(actual).should match(%r{<li>dark: <a.+>##{issue.iid}</a></li>}) |
360 | markdown(actual).should match(%r{<li>light by <a.+>@#{member.user.username}</a></li>}) | 360 | markdown(actual).should match(%r{<li>light by <a.+>@#{member.user.username}</a></li>}) |
361 | end | 361 | end |
362 | 362 | ||
363 | it "should handle references in <em>" do | 363 | it "should handle references in <em>" do |
364 | - actual = "Apply _!#{merge_request.id}_ ASAP" | 364 | + actual = "Apply _!#{merge_request.iid}_ ASAP" |
365 | 365 | ||
366 | - markdown(actual).should match(%r{Apply <em><a.+>!#{merge_request.id}</a></em>}) | 366 | + markdown(actual).should match(%r{Apply <em><a.+>!#{merge_request.iid}</a></em>}) |
367 | end | 367 | end |
368 | 368 | ||
369 | it "should leave code blocks untouched" do | 369 | it "should leave code blocks untouched" do |
@@ -379,19 +379,19 @@ describe GitlabMarkdownHelper do | @@ -379,19 +379,19 @@ describe GitlabMarkdownHelper do | ||
379 | end | 379 | end |
380 | 380 | ||
381 | it "should leave ref-like autolinks untouched" do | 381 | it "should leave ref-like autolinks untouched" do |
382 | - markdown("look at http://example.tld/#!#{merge_request.id}").should == "<p>look at <a href=\"http://example.tld/#!#{merge_request.id}\">http://example.tld/#!#{merge_request.id}</a></p>\n" | 382 | + markdown("look at http://example.tld/#!#{merge_request.iid}").should == "<p>look at <a href=\"http://example.tld/#!#{merge_request.iid}\">http://example.tld/#!#{merge_request.iid}</a></p>\n" |
383 | end | 383 | end |
384 | 384 | ||
385 | it "should leave ref-like href of 'manual' links untouched" do | 385 | it "should leave ref-like href of 'manual' links untouched" do |
386 | - markdown("why not [inspect !#{merge_request.id}](http://example.tld/#!#{merge_request.id})").should == "<p>why not <a href=\"http://example.tld/#!#{merge_request.id}\">inspect </a><a href=\"#{project_merge_request_url(project, merge_request)}\" class=\"gfm gfm-merge_request \" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.id}</a><a href=\"http://example.tld/#!#{merge_request.id}\"></a></p>\n" | 386 | + markdown("why not [inspect !#{merge_request.iid}](http://example.tld/#!#{merge_request.iid})").should == "<p>why not <a href=\"http://example.tld/#!#{merge_request.iid}\">inspect </a><a href=\"#{project_merge_request_url(project, merge_request)}\" class=\"gfm gfm-merge_request \" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.iid}</a><a href=\"http://example.tld/#!#{merge_request.iid}\"></a></p>\n" |
387 | end | 387 | end |
388 | 388 | ||
389 | it "should leave ref-like src of images untouched" do | 389 | it "should leave ref-like src of images untouched" do |
390 | - markdown("screen shot: ").should == "<p>screen shot: <img src=\"http://example.tld/#!#{merge_request.id}\" alt=\"some image\"></p>\n" | 390 | + markdown("screen shot: ").should == "<p>screen shot: <img src=\"http://example.tld/#!#{merge_request.iid}\" alt=\"some image\"></p>\n" |
391 | end | 391 | end |
392 | 392 | ||
393 | it "should generate absolute urls for refs" do | 393 | it "should generate absolute urls for refs" do |
394 | - markdown("##{issue.id}").should include(project_issue_url(project, issue)) | 394 | + markdown("##{issue.iid}").should include(project_issue_url(project, issue)) |
395 | end | 395 | end |
396 | 396 | ||
397 | it "should generate absolute urls for emoji" do | 397 | it "should generate absolute urls for emoji" do |
spec/helpers/issues_helper_spec.rb
@@ -8,7 +8,7 @@ describe IssuesHelper do | @@ -8,7 +8,7 @@ describe IssuesHelper do | ||
8 | describe :title_for_issue do | 8 | describe :title_for_issue do |
9 | it "should return issue title if used internal tracker" do | 9 | it "should return issue title if used internal tracker" do |
10 | @project = project | 10 | @project = project |
11 | - title_for_issue(issue.id).should eq issue.title | 11 | + title_for_issue(issue.iid).should eq issue.title |
12 | end | 12 | end |
13 | 13 | ||
14 | it "should always return empty string if used external tracker" do | 14 | it "should always return empty string if used external tracker" do |
@@ -61,7 +61,7 @@ describe IssuesHelper do | @@ -61,7 +61,7 @@ describe IssuesHelper do | ||
61 | 61 | ||
62 | it "should return internal path if used internal tracker" do | 62 | it "should return internal path if used internal tracker" do |
63 | @project = project | 63 | @project = project |
64 | - url_for_issue(issue.id).should match(int_expected) | 64 | + url_for_issue(issue.iid).should match(int_expected) |
65 | end | 65 | end |
66 | 66 | ||
67 | it "should return path to external tracker" do | 67 | it "should return path to external tracker" do |
@@ -73,7 +73,7 @@ describe IssuesHelper do | @@ -73,7 +73,7 @@ describe IssuesHelper do | ||
73 | it "should return empty string if project nil" do | 73 | it "should return empty string if project nil" do |
74 | @project = nil | 74 | @project = nil |
75 | 75 | ||
76 | - url_for_issue(issue.id).should eq "" | 76 | + url_for_issue(issue.iid).should eq "" |
77 | end | 77 | end |
78 | end | 78 | end |
79 | 79 |
spec/models/concerns/issuable_spec.rb
@@ -11,7 +11,9 @@ describe Issue, "Issuable" do | @@ -11,7 +11,9 @@ describe Issue, "Issuable" do | ||
11 | end | 11 | end |
12 | 12 | ||
13 | describe "Validation" do | 13 | describe "Validation" do |
14 | + before { subject.stub(set_iid: false) } | ||
14 | it { should validate_presence_of(:project) } | 15 | it { should validate_presence_of(:project) } |
16 | + it { should validate_presence_of(:iid) } | ||
15 | it { should validate_presence_of(:author) } | 17 | it { should validate_presence_of(:author) } |
16 | it { should validate_presence_of(:title) } | 18 | it { should validate_presence_of(:title) } |
17 | it { should ensure_length_of(:title).is_at_least(0).is_at_most(255) } | 19 | it { should ensure_length_of(:title).is_at_least(0).is_at_most(255) } |