Merge branch 'support-edit-target-branch-in-mr' into 'master'

CHANGELOG

@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 7.12.0 (unreleased)
   - Shorten merge request WIP text.
   - Add option to disallow users from registering any application to use GitLab as an OAuth provider
+  - Support editing target branch of merge request (Stan Hu)
   - Refactor permission checks with issues and merge requests project settings (Stan Hu)
   - Fix Markdown preview not working in Edit Milestone page (Stan Hu)
   - Fix Zen Mode not closing with ESC key (Stan Hu)

app/models/merge_request.rb

@@ -197,7 +197,6 @@ class MergeRequest < ActiveRecord::Base
   def update_merge_request_diff
     if source_branch_changed? || target_branch_changed?
       reload_code
-      mark_as_unchecked
     end
   end
 

app/services/issuable_base_service.rb

@@ -20,4 +20,10 @@ class IssuableBaseService < BaseService
     SystemNoteService.change_title(
       issuable, issuable.project, current_user, old_title)
   end
+
+  def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
+    SystemNoteService.change_branch(
+      issuable, issuable.project, current_user, branch_type,
+      old_branch, new_branch)
+  end
 end

app/services/merge_requests/update_service.rb

@@ -5,7 +5,7 @@ require_relative 'close_service'
 module MergeRequests
   class UpdateService < MergeRequests::BaseService
     def execute(merge_request)
-      # We dont allow change of source/target projects
+      # We don't allow change of source/target projects
       # after merge request was created
       params.except!(:source_project_id)
       params.except!(:target_project_id)
@@ -41,6 +41,12 @@ module MergeRequests
           )
         end
 
+        if merge_request.previous_changes.include?('target_branch')
+          create_branch_change_note(merge_request, 'target',
+                                    merge_request.previous_changes['target_branch'].first,
+                                    merge_request.target_branch)
+        end
+
         if merge_request.previous_changes.include?('milestone_id')
           create_milestone_note(merge_request)
         end
@@ -54,6 +60,11 @@ module MergeRequests
           create_title_change_note(merge_request, merge_request.previous_changes['title'].first)
         end
 
+        if merge_request.previous_changes.include?('target_branch') ||
+            merge_request.previous_changes.include?('source_branch')
+          merge_request.mark_as_unchecked
+        end
+
         merge_request.notice_added_references(merge_request.project, current_user)
         execute_hooks(merge_request, 'update')
       end

app/services/system_note_service.rb

@@ -149,6 +149,25 @@ class SystemNoteService
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
 
+  # Called when a branch in Noteable is changed
+  #
+  # noteable    - Noteable object
+  # project     - Project owning noteable
+  # author      - User performing the change
+  # branch_type - 'source' or 'target'
+  # old_branch  - old branch name
+  # new_branch  - new branch nmae
+  #
+  # Example Note text:
+  #
+  #   "Target branch changed from `Old` to `New`"
+  #
+  # Returns the created Note object
+  def self.change_branch(noteable, project, author, branch_type, old_branch, new_branch)
+    body = "#{branch_type} branch changed from `#{old_branch}` to `#{new_branch}`".capitalize
+    create_note(noteable: noteable, project: project, author: author, note: body)
+  end
+
   # Called when a Mentionable references a Noteable
   #
   # noteable  - Noteable object being referenced

app/views/projects/_issuable_form.html.haml

@@ -79,6 +79,24 @@
     - if can? current_user, :admin_label, issuable.project
       = link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank
 
+- if issuable.is_a?(MergeRequest)
+  %hr
+    - unless @merge_request.persisted?
+      .form-group
+        = f.label :source_branch, class: 'control-label' do
+          %i.fa.fa-code-fork
+          Source Branch
+        .col-sm-10
+          = f.select(:source_branch, [@merge_request.source_branch], { }, { class: 'source_branch select2 span2', disabled: true })
+          %p.help-block
+          = link_to 'Change source branch', mr_change_branches_path(@merge_request)
+  .form-group
+    = f.label :target_branch, class: 'control-label' do
+      %i.fa.fa-code-fork
+      Target Branch
+    .col-sm-10
+      = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, { class: 'target_branch select2 span2' })
+
 .form-actions
   - if !issuable.project.empty_repo? && (guide_url = contribution_guide_url(issuable.project)) && !issuable.persisted?
     %p

features/project/merge_requests.feature

@@ -207,3 +207,11 @@ Feature: Project Merge Requests
     Then I should see that I am subscribed
     When I click button "Unsubscribe"
     Then I should see that I am unsubscribed
+
+  @javascript
+  Scenario: I can change the target branch
+    Given I visit merge request page "Bug NS-04"
+    And I click link "Edit" for the merge request
+    When I click the "Target branch" dropdown
+    And I select a new target branch
+    Then I should see new target branch changes

features/steps/dashboard/dashboard.rb

@@ -23,8 +23,8 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps
   step 'I see prefilled new Merge Request page' do
     current_path.should == new_namespace_project_merge_request_path(@project.namespace, @project)
     find("#merge_request_target_project_id").value.should == @project.id.to_s
-    find("#merge_request_source_branch").value.should == "fix"
-    find("#merge_request_target_branch").value.should == "master"
+    find("input#merge_request_source_branch").value.should == "fix"
+    find("input#merge_request_target_branch").value.should == "master"
   end
 
   step 'user with name "John Doe" joined project "Shop"' do

features/steps/project/merge_requests.rb

@@ -305,6 +305,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
     fill_in 'issue_search', with: "Fe"
   end
 
+  step 'I click the "Target branch" dropdown' do
+    first('.target_branch').click
+  end
+
+  step 'I select a new target branch' do
+    select "feature", from: "merge_request_target_branch"
+    click_button 'Save'
+  end
+
+  step 'I should see new target branch changes' do
+    page.should have_content 'From fix into feature'
+    page.should have_content 'Target branch changed from master to feature'
+  end
+
   def merge_request
     @merge_request ||= MergeRequest.find_by!(title: "Bug NS-05")
   end

spec/services/merge_requests/update_service_spec.rb

@@ -20,7 +20,8 @@ describe MergeRequests::UpdateService do
           description: 'Also please fix',
           assignee_id: user2.id,
           state_event: 'close',
-          label_ids: [label.id]
+          label_ids: [label.id],
+          target_branch: 'target'
         }
       end
 
@@ -39,6 +40,7 @@ describe MergeRequests::UpdateService do
       it { expect(@merge_request).to be_closed }
       it { expect(@merge_request.labels.count).to eq(1) }
       it { expect(@merge_request.labels.first.title).to eq('Bug') }
+      it { expect(@merge_request.target_branch).to eq('target') }
 
       it 'should execute hooks with update action' do
         expect(service).to have_received(:execute_hooks).
@@ -77,6 +79,13 @@ describe MergeRequests::UpdateService do
         expect(note).not_to be_nil
         expect(note.note).to eq 'Title changed from **Old title** to **New title**'
       end
+
+      it 'creates system note about branch change' do
+        note = find_note('Target')
+
+        expect(note).not_to be_nil
+        expect(note.note).to eq 'Target branch changed from `master` to `target`'
+      end
     end
   end
 end

spec/services/system_note_service_spec.rb

@@ -228,6 +228,20 @@ describe SystemNoteService do
     end
   end
 
+  describe '.change_branch' do
+    subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) }
+    let(:old_branch) { 'old_branch'}
+    let(:new_branch) { 'new_branch'}
+
+    it_behaves_like 'a system note'
+
+    context 'when target branch name changed' do
+      it 'sets the note text' do
+        expect(subject.note).to eq "Target branch changed from `#{old_branch}` to `#{new_branch}`"
+      end
+    end
+  end
+
   describe '.cross_reference' do
     subject { described_class.cross_reference(noteable, mentioner, author) }