Commit 536373ad05b55c69442e7d7d6cb549791031cac2
1 parent
86bf684f
Exists in
spb-stable
and in
2 other branches
Dont allow mr compare with empty branches
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
3 changed files
with
22 additions
and
17 deletions
Show diff stats
app/views/projects/merge_requests/_new_compare.html.haml
@@ -2,11 +2,12 @@ | @@ -2,11 +2,12 @@ | ||
2 | %hr | 2 | %hr |
3 | 3 | ||
4 | = form_for [@project, @merge_request], url: new_project_merge_request_path(@project), method: :get, html: { class: "merge-request-form form-inline" } do |f| | 4 | = form_for [@project, @merge_request], url: new_project_merge_request_path(@project), method: :get, html: { class: "merge-request-form form-inline" } do |f| |
5 | + .hide.alert.alert-danger.mr-compare-errors | ||
5 | .merge-request-branches.row | 6 | .merge-request-branches.row |
6 | .col-md-6 | 7 | .col-md-6 |
7 | .panel.panel-default | 8 | .panel.panel-default |
8 | .panel-heading | 9 | .panel-heading |
9 | - From | 10 | + %strong Source branch |
10 | .panel-body | 11 | .panel-body |
11 | = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2 span3', disabled: @merge_request.persisted? }) | 12 | = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2 span3', disabled: @merge_request.persisted? }) |
12 | | 13 | |
@@ -17,7 +18,7 @@ | @@ -17,7 +18,7 @@ | ||
17 | .col-md-6 | 18 | .col-md-6 |
18 | .panel.panel-default | 19 | .panel.panel-default |
19 | .panel-heading | 20 | .panel-heading |
20 | - To | 21 | + %strong Target branch |
21 | .panel-body | 22 | .panel-body |
22 | - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] | 23 | - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] |
23 | = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted? }) | 24 | = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted? }) |
@@ -45,8 +46,9 @@ | @@ -45,8 +46,9 @@ | ||
45 | %span.label-branch #{@merge_request.target_branch} | 46 | %span.label-branch #{@merge_request.target_branch} |
46 | are the same. | 47 | are the same. |
47 | 48 | ||
49 | + | ||
48 | %hr | 50 | %hr |
49 | - = f.submit 'Compare branches', class: "btn btn-primary" | 51 | + = f.submit 'Compare branches', class: "btn btn-primary mr-compare-btn" |
50 | 52 | ||
51 | :javascript | 53 | :javascript |
52 | var source_branch = $("#merge_request_source_branch") | 54 | var source_branch = $("#merge_request_source_branch") |
@@ -61,9 +63,22 @@ | @@ -61,9 +63,22 @@ | ||
61 | }); | 63 | }); |
62 | source_branch.on("change", function() { | 64 | source_branch.on("change", function() { |
63 | $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() }); | 65 | $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() }); |
66 | + $(".mr-compare-errors").fadeOut(); | ||
67 | + $(".mr-compare-btn").enable(); | ||
64 | }); | 68 | }); |
65 | target_branch.on("change", function() { | 69 | target_branch.on("change", function() { |
66 | $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() }); | 70 | $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() }); |
71 | + $(".mr-compare-errors").fadeOut(); | ||
72 | + $(".mr-compare-btn").enable(); | ||
67 | }); | 73 | }); |
68 | 74 | ||
69 | 75 | ||
76 | +:coffeescript | ||
77 | + | ||
78 | + $(".merge-request-form").on 'submit', -> | ||
79 | + if $("#merge_request_source_branch").val() is "" or $('#merge_request_target_branch').val() is "" | ||
80 | + $(".mr-compare-errors").html("You must select source and target branch to proceed") | ||
81 | + $(".mr-compare-errors").fadeIn() | ||
82 | + event.preventDefault() | ||
83 | + return | ||
84 | + |
features/project/forked_merge_requests.feature
@@ -30,11 +30,10 @@ Feature: Project Forked Merge Requests | @@ -30,11 +30,10 @@ Feature: Project Forked Merge Requests | ||
30 | Given I visit project "Forked Shop" merge requests page | 30 | Given I visit project "Forked Shop" merge requests page |
31 | And I click link "New Merge Request" | 31 | And I click link "New Merge Request" |
32 | And I fill out an invalid "Merge Request On Forked Project" merge request | 32 | And I fill out an invalid "Merge Request On Forked Project" merge request |
33 | - And I submit the merge request | ||
34 | Then I should see validation errors | 33 | Then I should see validation errors |
35 | 34 | ||
36 | @javascript | 35 | @javascript |
37 | Scenario: Merge request should target fork repository by default | 36 | Scenario: Merge request should target fork repository by default |
38 | Given I visit project "Forked Shop" merge requests page | 37 | Given I visit project "Forked Shop" merge requests page |
39 | And I click link "New Merge Request" | 38 | And I click link "New Merge Request" |
40 | - Then the target repository should be the original repository | ||
41 | \ No newline at end of file | 39 | \ No newline at end of file |
40 | + Then the target repository should be the original repository |
features/steps/project/forked_merge_requests.rb
@@ -53,6 +53,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps | @@ -53,6 +53,7 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps | ||
53 | 53 | ||
54 | find(:select, "merge_request_source_branch", {}).value.should == 'master' | 54 | find(:select, "merge_request_source_branch", {}).value.should == 'master' |
55 | find(:select, "merge_request_target_branch", {}).value.should == 'stable' | 55 | find(:select, "merge_request_target_branch", {}).value.should == 'stable' |
56 | + click_button "Compare branches" | ||
56 | 57 | ||
57 | fill_in "merge_request_title", with: "Merge Request On Forked Project" | 58 | fill_in "merge_request_title", with: "Merge Request On Forked Project" |
58 | end | 59 | end |
@@ -148,29 +149,19 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps | @@ -148,29 +149,19 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps | ||
148 | current_path.should == edit_project_merge_request_path(@project, @merge_request) | 149 | current_path.should == edit_project_merge_request_path(@project, @merge_request) |
149 | page.should have_content "Edit merge request ##{@merge_request.id}" | 150 | page.should have_content "Edit merge request ##{@merge_request.id}" |
150 | find("#merge_request_title").value.should == "Merge Request On Forked Project" | 151 | find("#merge_request_title").value.should == "Merge Request On Forked Project" |
151 | - find("#merge_request_source_project_id").value.should == @forked_project.id.to_s | ||
152 | - find("#merge_request_target_project_id").value.should == @project.id.to_s | ||
153 | - find("#merge_request_source_branch").value.should have_content "master" | ||
154 | - verify_commit_link(".mr_source_commit",@forked_project) | ||
155 | - find("#merge_request_target_branch").value.should have_content "stable" | ||
156 | - verify_commit_link(".mr_target_commit",@project) | ||
157 | end | 152 | end |
158 | 153 | ||
159 | step 'I fill out an invalid "Merge Request On Forked Project" merge request' do | 154 | step 'I fill out an invalid "Merge Request On Forked Project" merge request' do |
160 | - #If this isn't filled in the rest of the validations won't be triggered | ||
161 | - fill_in "merge_request_title", with: "Merge Request On Forked Project" | ||
162 | - | ||
163 | select "Select branch", from: "merge_request_target_branch" | 155 | select "Select branch", from: "merge_request_target_branch" |
164 | - | ||
165 | find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s | 156 | find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s |
166 | find(:select, "merge_request_target_project_id", {}).value.should == project.id.to_s | 157 | find(:select, "merge_request_target_project_id", {}).value.should == project.id.to_s |
167 | find(:select, "merge_request_source_branch", {}).value.should == "" | 158 | find(:select, "merge_request_source_branch", {}).value.should == "" |
168 | find(:select, "merge_request_target_branch", {}).value.should == "" | 159 | find(:select, "merge_request_target_branch", {}).value.should == "" |
160 | + click_button "Compare branches" | ||
169 | end | 161 | end |
170 | 162 | ||
171 | step 'I should see validation errors' do | 163 | step 'I should see validation errors' do |
172 | - page.should have_content "Source branch can't be blank" | ||
173 | - page.should have_content "Target branch can't be blank" | 164 | + page.should have_content "You must select source and target branch" |
174 | end | 165 | end |
175 | 166 | ||
176 | step 'the target repository should be the original repository' do | 167 | step 'the target repository should be the original repository' do |