From 5d88de092f37497d9c08878954b099c425376bda Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 12 Apr 2016 11:43:15 +0530 Subject: Refactor `Issue#related_branches` - Previously, the controller held the logic to calculate related branches, which was: ` - ` - This logic belongs in the `related_branches` method, not in the controller. This commit makes this change. - This means that `Issue#related_branches` now needs to take a `User`. When we find the branches that have a merge request referenced in the current issue, this is limited to merge requests that the current user has access to. - This is not directly related to #14566, but is a related refactoring. --- spec/models/issue_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 15052aaca28..f33b705810e 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -192,10 +192,11 @@ describe Issue, models: true do describe '#related_branches' do it "selects the right branches" do + user = build(:user) allow(subject.project.repository).to receive(:branch_names). and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) - expect(subject.related_branches).to eq([subject.to_branch_name]) + expect(subject.related_branches(user)).to eq([subject.to_branch_name]) end end -- cgit v1.2.1 From 91034af3c998ce4a4f83281525304e8c50add384 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 12 Apr 2016 15:50:19 +0530 Subject: Augment the tests for `Issue#related_branches` - Test the case where we have a referenced merge request that's being - excluded as a "related branch" - This took a while to figure out, especially the `create_cross_references!` line. --- spec/models/issue_spec.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index f33b705810e..8cacce6a7bf 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -191,11 +191,27 @@ describe Issue, models: true do end describe '#related_branches' do - it "selects the right branches" do - user = build(:user) + let(:user) { build(:user, :admin) } + let(:merge_request) { create(:merge_request, description: "Closes ##{subject.iid}", + source_project: subject.project, source_branch: "branch-#{subject.iid}") } + + before(:each) do allow(subject.project.repository).to receive(:branch_names). - and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) + and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name, "branch-#{subject.iid}"]) + + # Without this stub, the `create(:merge_request)` above fails because it can't find + # the source branch. This seems like a reasonable compromise, in comparison with + # setting up a full repo here. + allow_any_instance_of(MergeRequest).to receive(:create_merge_request_diff) + end + + it "selects the right branches when there are no referenced merge requests" do + expect(subject.related_branches(user)).to eq([subject.to_branch_name, "branch-#{subject.iid}"]) + end + it "selects the right branches when there is a referenced merge request" do + merge_request.create_cross_references!(user) + expect(subject.referenced_merge_requests).to_not be_empty expect(subject.related_branches(user)).to eq([subject.to_branch_name]) end end -- cgit v1.2.1 From 66ca80181bcb5aef97296fde567b899603186be6 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 12 Apr 2016 16:01:44 +0530 Subject: Test the `Issue#to_branch_name` method. --- spec/models/issue_spec.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 8cacce6a7bf..22dabaa0404 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -228,10 +228,19 @@ describe Issue, models: true do end describe "#to_branch_name" do - let(:issue) { create(:issue, title: 'a' * 30) } + let(:issue) { create(:issue, title: 'testing-issue') } - it "starts with the issue iid" do + it "ends with the issue iid" do expect(issue.to_branch_name).to match /-#{issue.iid}\z/ end + + it "contains the issue title if not confidential" do + expect(issue.to_branch_name).to match /\Atesting-issue/ + end + + it "does not contain the issue title if confidential" do + issue = create(:issue, title: 'testing-issue', confidential: true) + expect(issue.to_branch_name).to match /\Aissue/ + end end end -- cgit v1.2.1 From 8dd1a6fc917231e83fb64dbfa65e1edbe9ab3fee Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 13 Apr 2016 09:04:08 +0530 Subject: Fix the rubocop check. --- spec/models/issue_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 22dabaa0404..202ce846dca 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -192,8 +192,6 @@ describe Issue, models: true do describe '#related_branches' do let(:user) { build(:user, :admin) } - let(:merge_request) { create(:merge_request, description: "Closes ##{subject.iid}", - source_project: subject.project, source_branch: "branch-#{subject.iid}") } before(:each) do allow(subject.project.repository).to receive(:branch_names). @@ -210,6 +208,9 @@ describe Issue, models: true do end it "selects the right branches when there is a referenced merge request" do + merge_request = create(:merge_request, { description: "Closes ##{subject.iid}", + source_project: subject.project, + source_branch: "branch-#{subject.iid}" }) merge_request.create_cross_references!(user) expect(subject.referenced_merge_requests).to_not be_empty expect(subject.related_branches(user)).to eq([subject.to_branch_name]) -- cgit v1.2.1 From f5ce601c2b052b18398d2207c64ce8828b628521 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 15 Apr 2016 09:31:26 +0530 Subject: Make a few style changes based on MR feedback. --- spec/models/issue_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 202ce846dca..ed982d8a9d4 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -191,9 +191,9 @@ describe Issue, models: true do end describe '#related_branches' do - let(:user) { build(:user, :admin) } + let(:user) { build(:admin) } - before(:each) do + before do allow(subject.project.repository).to receive(:branch_names). and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name, "branch-#{subject.iid}"]) -- cgit v1.2.1 From f801e2243dcce6d3bdce01acf62fbf5a49a301da Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 19 Apr 2016 09:22:55 +0530 Subject: A new branch created for a confidential issue is named `-confidential-issue`. --- spec/models/issue_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index da1c673653a..060e6599104 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -248,7 +248,7 @@ describe Issue, models: true do it "does not contain the issue title if confidential" do issue = create(:issue, title: 'testing-issue', confidential: true) - expect(issue.to_branch_name).to match /\Aissue/ + expect(issue.to_branch_name).to match /confidential-issue\z/ end end end -- cgit v1.2.1