Commit 4917dc64693ad7131f6943497e4caee13f1c55ef
Exists in
spb-stable
and in
2 other branches
Merge branch '2-step-mr' into 'master'
2 step merge request process
Showing
15 changed files
with
229 additions
and
76 deletions
Show diff stats
app/assets/javascripts/project_users_select.js.coffee
1 | 1 | @projectUsersSelect = |
2 | 2 | init: -> |
3 | 3 | $('.ajax-project-users-select').each (i, select) -> |
4 | - project_id = $('body').data('project-id') | |
4 | + project_id = $(select).data('project-id') || $('body').data('project-id') | |
5 | 5 | |
6 | 6 | $(select).select2 |
7 | 7 | placeholder: $(select).data('placeholder') || "Search for a user" | ... | ... |
app/assets/stylesheets/sections/merge_requests.scss
app/controllers/projects/merge_requests_controller.rb
... | ... | @@ -62,11 +62,27 @@ class Projects::MergeRequestsController < Projects::ApplicationController |
62 | 62 | @merge_request.source_project = @project unless @merge_request.source_project |
63 | 63 | @merge_request.target_project ||= (@project.forked_from_project || @project) |
64 | 64 | @target_branches = @merge_request.target_project.nil? ? [] : @merge_request.target_project.repository.branch_names |
65 | - | |
66 | 65 | @merge_request.target_branch ||= @merge_request.target_project.default_branch |
67 | - | |
68 | 66 | @source_project = @merge_request.source_project |
69 | - @merge_request | |
67 | + | |
68 | + if @merge_request.target_branch && @merge_request.source_branch | |
69 | + compare_action = Gitlab::Satellite::CompareAction.new( | |
70 | + current_user, | |
71 | + @merge_request.target_project, | |
72 | + @merge_request.target_branch, | |
73 | + @merge_request.source_project, | |
74 | + @merge_request.source_branch | |
75 | + ) | |
76 | + | |
77 | + @commits = compare_action.commits | |
78 | + @commits.map! { |commit| Commit.new(commit) } | |
79 | + @commit = @commits.first | |
80 | + | |
81 | + @diffs = compare_action.diffs | |
82 | + @merge_request.title = @merge_request.source_branch.titleize.humanize | |
83 | + @target_project = @merge_request.target_project | |
84 | + @target_repo = @target_project.repository | |
85 | + end | |
70 | 86 | end |
71 | 87 | |
72 | 88 | def edit |
... | ... | @@ -80,7 +96,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController |
80 | 96 | @merge_request = MergeRequests::CreateService.new(project, current_user, params[:merge_request]).execute |
81 | 97 | |
82 | 98 | if @merge_request.valid? |
83 | - redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' | |
99 | + redirect_to project_merge_request_path(@merge_request.target_project, @merge_request), notice: 'Merge request was successfully created.' | |
84 | 100 | else |
85 | 101 | @source_project = @merge_request.source_project |
86 | 102 | @target_project = @merge_request.target_project | ... | ... |
app/helpers/issues_helper.rb
... | ... | @@ -82,7 +82,7 @@ module IssuesHelper |
82 | 82 | end |
83 | 83 | |
84 | 84 | def milestone_options object |
85 | - options_from_collection_for_select(@project.milestones.active, 'id', 'title', object.milestone_id) | |
85 | + options_from_collection_for_select(object.project.milestones.active, 'id', 'title', object.milestone_id) | |
86 | 86 | end |
87 | 87 | |
88 | 88 | def issue_box_class(item) | ... | ... |
app/helpers/selects_helper.rb
... | ... | @@ -14,7 +14,7 @@ module SelectsHelper |
14 | 14 | css_class << (opts[:class] || '') |
15 | 15 | value = opts[:selected] || '' |
16 | 16 | placeholder = opts[:placeholder] || 'Select user' |
17 | - | |
18 | - hidden_field_tag(id, value, class: css_class, 'data-placeholder' => placeholder) | |
17 | + project_id = opts[:project_id] || @project.id | |
18 | + hidden_field_tag(id, value, class: css_class, 'data-placeholder' => placeholder, 'data-project-id' => project_id) | |
19 | 19 | end |
20 | 20 | end | ... | ... |
app/models/merge_request.rb
... | ... | @@ -253,6 +253,14 @@ class MergeRequest < ActiveRecord::Base |
253 | 253 | end |
254 | 254 | end |
255 | 255 | |
256 | + def target_project_namespace | |
257 | + if target_project && target_project.namespace | |
258 | + target_project.namespace.path | |
259 | + else | |
260 | + "(removed)" | |
261 | + end | |
262 | + end | |
263 | + | |
256 | 264 | def source_branch_exists? |
257 | 265 | return false unless self.source_project |
258 | 266 | ... | ... |
app/views/projects/merge_requests/_form.html.haml
... | ... | @@ -14,33 +14,6 @@ |
14 | 14 | - @merge_request.errors.full_messages.each do |msg| |
15 | 15 | %div= msg |
16 | 16 | |
17 | - .merge-request-branches | |
18 | - .form-group | |
19 | - = label_tag nil, class: 'control-label' do | |
20 | - From | |
21 | - .col-sm-10 | |
22 | - .clearfix | |
23 | - .pull-left | |
24 | - = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2 span3', disabled: @merge_request.persisted? }) | |
25 | - .pull-left | |
26 | - | |
27 | - = f.select(:source_branch, @merge_request.source_branches, { include_blank: "Select branch" }, {class: 'source_branch select2 span2'}) | |
28 | - .mr_source_commit | |
29 | - %br | |
30 | - .form-group | |
31 | - = label_tag nil, class: 'control-label' do | |
32 | - To | |
33 | - .col-sm-10 | |
34 | - .clearfix | |
35 | - .pull-left | |
36 | - - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] | |
37 | - = 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? }) | |
38 | - .pull-left | |
39 | - | |
40 | - = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, {class: 'target_branch select2 span2'}) | |
41 | - .mr_target_commit | |
42 | - | |
43 | - %hr | |
44 | 17 | .merge-request-form-info |
45 | 18 | .form-group |
46 | 19 | = f.label :title, class: 'control-label' do |
... | ... | @@ -51,6 +24,23 @@ |
51 | 24 | .col-sm-10 |
52 | 25 | = f.text_area :description, class: "form-control js-gfm-input", rows: 14 |
53 | 26 | %p.hint Description is parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. |
27 | + %hr | |
28 | + .form-group | |
29 | + .issue-assignee | |
30 | + = f.label :assignee_id, class: 'control-label' do | |
31 | + %i.icon-user | |
32 | + Assign to | |
33 | + .col-sm-10 | |
34 | + = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id) | |
35 | + | |
36 | + = link_to 'Assign to me', '#', class: 'btn btn-small assign-to-me-link' | |
37 | + .form-group | |
38 | + .issue-milestone | |
39 | + = f.label :milestone_id, class: 'control-label' do | |
40 | + %i.icon-time | |
41 | + Milestone | |
42 | + .col-sm-10= f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2'}) | |
43 | + | |
54 | 44 | |
55 | 45 | .form-actions |
56 | 46 | - if @merge_request.new_record? |
... | ... | @@ -66,20 +56,7 @@ |
66 | 56 | |
67 | 57 | :javascript |
68 | 58 | disableButtonIfEmptyField("#merge_request_title", ".btn-save"); |
69 | - | |
70 | - var source_branch = $("#merge_request_source_branch") | |
71 | - , target_branch = $("#merge_request_target_branch") | |
72 | - , target_project = $("#merge_request_target_project_id"); | |
73 | - | |
74 | - $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: source_branch.val() }); | |
75 | - $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: target_branch.val() }); | |
76 | - | |
77 | - target_project.on("change", function() { | |
78 | - $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id: $(this).val() }); | |
79 | - }); | |
80 | - source_branch.on("change", function() { | |
81 | - $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() }); | |
82 | - }); | |
83 | - target_branch.on("change", function() { | |
84 | - $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() }); | |
59 | + $('.assign-to-me-link').on('click', function(e){ | |
60 | + $('#merge_request_assignee_id').val("#{current_user.id}").trigger("change"); | |
61 | + e.preventDefault(); | |
85 | 62 | }); | ... | ... |
app/views/projects/merge_requests/_new_compare.html.haml
0 → 100644
... | ... | @@ -0,0 +1,84 @@ |
1 | +%h3.page-title Compare branches for new Merge Request | |
2 | +%hr | |
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| | |
5 | + .hide.alert.alert-danger.mr-compare-errors | |
6 | + .merge-request-branches.row | |
7 | + .col-md-6 | |
8 | + .panel.panel-default | |
9 | + .panel-heading | |
10 | + %strong Source branch | |
11 | + .panel-body | |
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? }) | |
13 | + | |
14 | + = f.select(:source_branch, @merge_request.source_branches, { include_blank: "Select branch" }, {class: 'source_branch select2 span2'}) | |
15 | + .panel-footer | |
16 | + .mr_source_commit | |
17 | + | |
18 | + .col-md-6 | |
19 | + .panel.panel-default | |
20 | + .panel-heading | |
21 | + %strong Target branch | |
22 | + .panel-body | |
23 | + - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] | |
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? }) | |
25 | + | |
26 | + = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, {class: 'target_branch select2 span2'}) | |
27 | + .panel-footer | |
28 | + .mr_target_commit | |
29 | + | |
30 | + -if @merge_request.errors.any? | |
31 | + .alert.alert-danger | |
32 | + - @merge_request.errors.full_messages.each do |msg| | |
33 | + %div= msg | |
34 | + | |
35 | + - if @merge_request.source_branch.present? && @merge_request.target_branch.present? | |
36 | + .light-well | |
37 | + %center | |
38 | + %h4 | |
39 | + There isn't anything to merge. | |
40 | + %p.slead | |
41 | + - if @merge_request.source_branch == @merge_request.target_branch | |
42 | + You'll need to use different branch names to get a valid comparison. | |
43 | + - else | |
44 | + %span.label-branch #{@merge_request.source_branch} | |
45 | + and | |
46 | + %span.label-branch #{@merge_request.target_branch} | |
47 | + are the same. | |
48 | + | |
49 | + | |
50 | + %hr | |
51 | + = f.submit 'Compare branches', class: "btn btn-primary mr-compare-btn" | |
52 | + | |
53 | +:javascript | |
54 | + var source_branch = $("#merge_request_source_branch") | |
55 | + , target_branch = $("#merge_request_target_branch") | |
56 | + , target_project = $("#merge_request_target_project_id"); | |
57 | + | |
58 | + $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: source_branch.val() }); | |
59 | + $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: target_branch.val() }); | |
60 | + | |
61 | + target_project.on("change", function() { | |
62 | + $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id: $(this).val() }); | |
63 | + }); | |
64 | + source_branch.on("change", function() { | |
65 | + $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() }); | |
66 | + $(".mr-compare-errors").fadeOut(); | |
67 | + $(".mr-compare-btn").enable(); | |
68 | + }); | |
69 | + target_branch.on("change", function() { | |
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(); | |
73 | + }); | |
74 | + | |
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 | + | ... | ... |
... | ... | @@ -0,0 +1,82 @@ |
1 | +%h3.page-title | |
2 | + New merge request | |
3 | +%p.slead | |
4 | + From | |
5 | + %strong.monospace | |
6 | + #{@merge_request.source_project_namespace}:#{@merge_request.source_branch} | |
7 | + into | |
8 | + %strong.monospace | |
9 | + #{@merge_request.target_project_namespace}:#{@merge_request.target_branch} | |
10 | + | |
11 | + %span.pull-right | |
12 | + = link_to 'Change branches', new_project_merge_request_path(@project) | |
13 | + | |
14 | += form_for [@project, @merge_request], html: { class: "merge-request-form" } do |f| | |
15 | + .panel.panel-default | |
16 | + | |
17 | + .panel-body | |
18 | + .form-group | |
19 | + .light | |
20 | + = f.label :title do | |
21 | + = "Title *" | |
22 | + = f.text_field :title, class: "form-control input-lg js-gfm-input", maxlength: 255, rows: 5, required: true | |
23 | + .form-group | |
24 | + .light | |
25 | + = f.label :description, "Description" | |
26 | + = f.text_area :description, class: "form-control js-gfm-input", rows: 10 | |
27 | + %p.hint Description is parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. | |
28 | + .form-group | |
29 | + .issue-assignee | |
30 | + = f.label :assignee_id do | |
31 | + %i.icon-user | |
32 | + Assign to | |
33 | + %div | |
34 | + = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select a user', class: 'custom-form-control', selected: @merge_request.assignee_id, project_id: @merge_request.target_project_id) | |
35 | + | |
36 | + = link_to 'Assign to me', '#', class: 'btn btn-small assign-to-me-link' | |
37 | + .form-group | |
38 | + .issue-milestone | |
39 | + = f.label :milestone_id do | |
40 | + %i.icon-time | |
41 | + Milestone | |
42 | + %div= f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2'}) | |
43 | + .panel-footer | |
44 | + - if @target_repo.contribution_guide | |
45 | + - contribution_guide_url = project_blob_path(@target_project, tree_join(@target_repo.root_ref, @target_repo.contribution_guide.name)) | |
46 | + %p | |
47 | + Please review the | |
48 | + %strong #{link_to "guidelines for contribution", contribution_guide_url} | |
49 | + to this repository. | |
50 | + = f.hidden_field :source_project_id | |
51 | + = f.hidden_field :target_project_id | |
52 | + = f.hidden_field :target_branch | |
53 | + = f.hidden_field :source_branch | |
54 | + = f.submit 'Submit merge request', class: "btn btn-create" | |
55 | + | |
56 | +.mr-compare | |
57 | + %div.ui-box | |
58 | + .title | |
59 | + Commits (#{@commits.count}) | |
60 | + - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE | |
61 | + %ul.well-list | |
62 | + - Commit.decorate(@commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE)).each do |commit| | |
63 | + = render "projects/commits/inline_commit", commit: commit, project: @project | |
64 | + %li.warning-row.unstyled | |
65 | + other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden to prevent performance issues. | |
66 | + - else | |
67 | + %ul.well-list= render Commit.decorate(@commits), project: @project | |
68 | + | |
69 | + %h4 Changes | |
70 | + - if @diffs.present? | |
71 | + = render "projects/commits/diffs", diffs: @diffs, project: @project | |
72 | + - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE | |
73 | + .bs-callout.bs-callout-danger | |
74 | + %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. | |
75 | + %p To preserve performance the line changes are not shown. | |
76 | + | |
77 | + | |
78 | +:javascript | |
79 | + $('.assign-to-me-link').on('click', function(e){ | |
80 | + $('#merge_request_assignee_id').val("#{current_user.id}").trigger("change"); | |
81 | + e.preventDefault(); | |
82 | + }); | ... | ... |
app/views/projects/merge_requests/branch_from.js.haml
app/views/projects/merge_requests/new.html.haml
app/views/projects/merge_requests/show/_mr_title.html.haml
features/project/forked_merge_requests.feature
... | ... | @@ -30,11 +30,10 @@ Feature: Project Forked Merge Requests |
30 | 30 | Given I visit project "Forked Shop" merge requests page |
31 | 31 | And I click link "New Merge Request" |
32 | 32 | And I fill out an invalid "Merge Request On Forked Project" merge request |
33 | - And I submit the merge request | |
34 | 33 | Then I should see validation errors |
35 | 34 | |
36 | 35 | @javascript |
37 | 36 | Scenario: Merge request should target fork repository by default |
38 | 37 | Given I visit project "Forked Shop" merge requests page |
39 | 38 | And I click link "New Merge Request" |
40 | - Then the target repository should be the original repository | |
41 | 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 | 53 | |
54 | 54 | find(:select, "merge_request_source_branch", {}).value.should == 'master' |
55 | 55 | find(:select, "merge_request_target_branch", {}).value.should == 'stable' |
56 | + click_button "Compare branches" | |
56 | 57 | |
57 | 58 | fill_in "merge_request_title", with: "Merge Request On Forked Project" |
58 | 59 | end |
... | ... | @@ -148,29 +149,19 @@ class ProjectForkedMergeRequests < Spinach::FeatureSteps |
148 | 149 | current_path.should == edit_project_merge_request_path(@project, @merge_request) |
149 | 150 | page.should have_content "Edit merge request ##{@merge_request.id}" |
150 | 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 | 152 | end |
158 | 153 | |
159 | 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 | 155 | select "Select branch", from: "merge_request_target_branch" |
164 | - | |
165 | 156 | find(:select, "merge_request_source_project_id", {}).value.should == @forked_project.id.to_s |
166 | 157 | find(:select, "merge_request_target_project_id", {}).value.should == project.id.to_s |
167 | 158 | find(:select, "merge_request_source_branch", {}).value.should == "" |
168 | 159 | find(:select, "merge_request_target_branch", {}).value.should == "" |
160 | + click_button "Compare branches" | |
169 | 161 | end |
170 | 162 | |
171 | 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 | 165 | end |
175 | 166 | |
176 | 167 | step 'the target repository should be the original repository' do | ... | ... |
features/steps/project/merge_requests.rb
... | ... | @@ -61,9 +61,10 @@ class ProjectMergeRequests < Spinach::FeatureSteps |
61 | 61 | end |
62 | 62 | |
63 | 63 | step 'I submit new merge request "Wiki Feature"' do |
64 | - fill_in "merge_request_title", with: "Wiki Feature" | |
65 | 64 | select "master", from: "merge_request_source_branch" |
66 | 65 | select "notes_refactoring", from: "merge_request_target_branch" |
66 | + click_button "Compare branches" | |
67 | + fill_in "merge_request_title", with: "Wiki Feature" | |
67 | 68 | click_button "Submit merge request" |
68 | 69 | end |
69 | 70 | ... | ... |