Support semi-linear history with rebasing and merge commits.

app/controllers/projects/merge_requests_controller.rb

@@ -179,7 +179,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       return
     end
 
-    if params[:ff].present? && !@merge_request.ff_merge_possible?
+    if @project.ff_merge_must_be_possible? && !@merge_request.ff_merge_possible?
       @status = :failed
       return
     end
@@ -355,7 +355,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def merge_params
-    params.permit(:should_remove_source_branch, :commit_message, :ff)
+    params.permit(:should_remove_source_branch, :commit_message)
   end
 
   # Make sure merge requests created before 8.0

app/controllers/projects_controller.rb

@@ -247,8 +247,7 @@ class ProjectsController < ApplicationController
       :approvals_before_merge,
       :approver_ids,
       :issues_template,
-      :merge_requests_ff_only_enabled,
-      :merge_requests_rebase_enabled,
+      :merge_method,
       :merge_requests_template,
       :mirror,
       :mirror_user_id,

app/models/project.rb

@@ -1037,4 +1037,32 @@ class Project < ActiveRecord::Base
       PagesWorker.perform_in(5.minutes, :remove, namespace.path, temp_path)
     end
   end
+
+  def merge_method
+    if self.merge_requests_ff_only_enabled
+      :ff
+    elsif self.merge_requests_rebase_enabled
+      :rebase_merge
+    else
+      :merge
+    end
+  end
+
+  def merge_method=(method)
+    case method.to_s
+    when "ff"
+      self.merge_requests_ff_only_enabled = true
+      self.merge_requests_rebase_enabled = true
+    when "rebase_merge"
+      self.merge_requests_ff_only_enabled = false
+      self.merge_requests_rebase_enabled = true
+    when "merge"
+      self.merge_requests_ff_only_enabled = false
+      self.merge_requests_rebase_enabled = false
+    end
+  end
+
+  def ff_merge_must_be_possible?
+    self.merge_requests_ff_only_enabled || self.merge_requests_rebase_enabled
+  end
 end

app/services/merge_requests/merge_service.rb

@@ -9,9 +9,7 @@ module MergeRequests
     attr_reader :merge_request
 
     def execute(merge_request)
-      # Delete the ff param so we don't get into an infinite loop if it's present,
-      # since FfMergeService inherits from MergeService.
-      if params.delete(:ff).present?
+      if @project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService)
         FfMergeService.new(project, current_user, params).execute(merge_request)
         return
       end

app/views/projects/_merge_request_settings.html.haml

@@ -3,22 +3,39 @@
     Merge requests:
 
   .form-group
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :merge_requests_ff_only_enabled do
-          = f.check_box :merge_requests_ff_only_enabled
-          %strong Only fast-forward merging
+    = label_tag :merge_method_merge, class: 'control-label' do
+      Merge method
+    .col-sm-10
+      .radio
+        = label_tag :project_merge_method_merge do
+          = f.radio_button :merge_method, :merge, class: "js-merge-method-radio"
+          %strong Merge commit
           %br
-          %span.descr The accept merge request button will only show when a merge without a merge commit is possible.
+          %span.descr
+            A merge commit will be created for every merge, and merging is allowed as long as there are no conflicts.
 
-  .form-group.rebase-feature
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :merge_requests_rebase_enabled do
-          = f.check_box :merge_requests_rebase_enabled
-          %strong Rebase button
+      .radio
+        = label_tag :project_merge_method_rebase_merge do
+          = f.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio"
+          %strong Merge commit with semi-linear history
+          %br
+          %span.descr
+            A merge commit will be created for every merge, but merging is only allowed if the branch has been rebased.
+            This way you get a history that reads linearly (as with fast-forward merges), with the addition of merge commits.
           %br
-          %span.descr Allows rebasing of merge requests before fast-forward merge.
+          %span.descr
+            When the branch has not been rebased, the user is given the option to do so.
+
+      .radio
+        = label_tag :project_merge_method_ff do
+          = f.radio_button :merge_method, :ff, class: "js-merge-method-radio"
+          %strong Fast-forward merge
+          %br
+          %span.descr
+            No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch has been rebased.
+          %br
+          %span.descr
+            When the branch has not been rebased, the user is given the option to do so.
 
   .form-group
     = f.label :merge_requests_template, class: 'control-label' do
@@ -69,12 +86,3 @@
 
 :coffeescript
   new UsersSelect()
-
-  mergeRequestsRebaseVisibilityCheck = ->
-    is_rebase_enabled = $("input#project_merge_requests_ff_only_enabled").prop("checked")
-    $(".rebase-feature").toggle(is_rebase_enabled)
-
-  mergeRequestsRebaseVisibilityCheck()
-
-  $("input#project_merge_requests_ff_only_enabled").change ->
-    mergeRequestsRebaseVisibilityCheck()

app/views/projects/merge_requests/widget/_open.html.haml

@@ -19,12 +19,9 @@
       = render 'projects/merge_requests/widget/open/merge_when_build_succeeds'
     - elsif !@merge_request.can_be_merged_by?(current_user)
       = render 'projects/merge_requests/widget/open/not_allowed'
-    - elsif @project.merge_requests_ff_only_enabled
-      - if @merge_request.ff_merge_possible?
-        = render 'projects/merge_requests/widget/open/ff_accept'
-      - else
-        = render 'projects/merge_requests/widget/open/rebase'
-    - elsif @merge_request.can_be_merged?
+    - elsif @project.ff_merge_must_be_possible? && !@merge_request.ff_merge_possible?
+      = render 'projects/merge_requests/widget/open/rebase'
+    - else
       = render 'projects/merge_requests/widget/open/accept'
 
   - if @closes_issues.present?

app/views/projects/merge_requests/widget/open/_accept.html.haml

@@ -30,14 +30,20 @@
           = label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
             = check_box_tag :should_remove_source_branch
             Remove source branch
+
       .accept-control.right
-        = link_to "#", class: "modify-merge-commit-link js-toggle-button" do
-          = icon('edit')
-          Modify commit message
-    .js-toggle-content.hide.prepend-top-default
-      = render 'shared/commit_message_container', params: params,
-          text: @merge_request.merge_commit_message,
-          rows: 14, hint: true
+        - if @project.merge_requests_ff_only_enabled
+          Fast-forward merge without a merge commit
+        - else
+          = link_to "#", class: "modify-merge-commit-link js-toggle-button" do
+            = icon('edit')
+            Modify commit message
+
+    - unless @project.merge_requests_ff_only_enabled
+      .js-toggle-content.hide.prepend-top-default
+        = render 'shared/commit_message_container', params: params,
+            text: @merge_request.merge_commit_message,
+            rows: 14, hint: true
 
     = hidden_field_tag :merge_when_build_succeeds, "", autocomplete: "off"
 

app/views/projects/merge_requests/widget/open/_conflicts.html.haml

@@ -3,7 +3,7 @@
   This merge request contains merge conflicts
 
 %p
-  Please resolve these conflicts or 
+  Please resolve these conflicts or
   - if @merge_request.can_be_merged_by?(current_user)
     #{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
   - else

app/views/projects/merge_requests/widget/open/_ff_accept.html.haml

-- status_class = @ci_commit ? " ci-#{@ci_commit.status}" : nil
-
-= form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f|
-  = hidden_field_tag :authenticity_token, form_authenticity_token
-  .accept-merge-holder.clearfix.js-toggle-container
-    .accept-action
-      - if @ci_commit && @ci_commit.active?
-        %span.btn-group
-          = button_tag class: "btn btn-create js-merge-button merge_when_build_succeeds" do
-            Merge When Build Succeeds
-          = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do
-            %span.caret
-            %span.sr-only
-              Select Merge Moment
-          %ul.js-merge-dropdown.dropdown-menu.dropdown-menu-right{ role: 'menu' }
-            %li
-              = link_to "#", class: "merge_when_build_succeeds" do
-                = icon('check fw')
-                Merge When Build Succeeds
-            %li
-              = link_to "#", class: "accept_merge_request" do
-                = icon('warning fw')
-                Merge Immediately
-      - else
-        = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do
-          Accept Merge Request
-    - if @merge_request.can_remove_source_branch?(current_user)
-      .accept-control.checkbox
-        = label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
-          = check_box_tag :should_remove_source_branch
-          Remove source branch
-
-    .accept-control.right
-      Fast-forward merge without creating merge commit
-
-    = hidden_field_tag :merge_when_build_succeeds, "", autocomplete: "off"
-    = hidden_field_tag :ff, "1"
-
-:javascript
-  $('.accept-mr-form').on('ajax:send', function() {
-    $(".accept-mr-form :input").disable();
-  });
-
-  $('.accept_merge_request').on('click', function() {
-    $('.js-merge-button').html("<i class='fa fa-spinner fa-spin'></i> Merge in progress");
-  });
-
-  $('.merge_when_build_succeeds').on('click', function() {
-    $("#merge_when_build_succeeds").val("1");
-  });
-
-  $('.js-merge-dropdown a').on('click', function(e) {
-    e.preventDefault();
-    $(this).closest("form").submit();
-  });

app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml

@@ -5,7 +5,7 @@
   - should_remove_source_branch = @merge_request.merge_params["should_remove_source_branch"].present?
   %p
     = succeed '.' do
-      - if @merge_request.merge_params["ff"].present?
+      - if @project.merge_requests_ff_only_enabled
         The changes will be fast-forward merged into
       - else
         The changes will be merged into

app/views/projects/merge_requests/widget/open/_rebase.html.haml

@@ -3,13 +3,13 @@
     = icon("spinner spin")
     Rebase in progress…
   %p
-    This merge request is in the process of being rebased, during which time it is locked and cannot be closed.
+    This merge request is in the process of being rebased.
 
   :javascript
     $(function() {
       merge_request_widget.rebaseInProgress()
     });
-- elsif !@merge_request.target_project.merge_requests_rebase_enabled || !can_push_branch?(@merge_request.source_project, @merge_request.source_branch)
+- elsif !can_push_branch?(@merge_request.source_project, @merge_request.source_branch)
   %h4
     = icon("exclamation-triangle")
     Fast-forward merge is not possible
@@ -25,7 +25,7 @@
         = f.button class: "btn btn-reopen js-rebase-button" do
           Rebase onto #{@merge_request.target_branch}
       .accept-control
-        Fast-forward merge is not possible. Branch must be rebased first
+        Fast-forward merge is not possible. Rebase the source branch onto the target branch to allow this merge request to be merged.
 
   :javascript
     $('.rebase-mr-form').on('ajax:send', function() {