Refactoring after moving fast-forward merge to CE

app/controllers/projects_controller.rb

@@ -347,6 +347,7 @@ class ProjectsController < Projects::ApplicationController
       :tag_list,
       :visibility_level,
       :template_name,
+      :merge_method,
 
       project_feature_attributes: %i[
         builds_access_level

app/models/license.rb

@@ -11,7 +11,6 @@ class License < ActiveRecord::Base
   DEPLOY_BOARD_FEATURE = 'GitLab_DeployBoard'.freeze
   ELASTIC_SEARCH_FEATURE = 'GitLab_ElasticSearch'.freeze
   EXPORT_ISSUES_FEATURE  = 'GitLab_ExportIssues'.freeze
-  FAST_FORWARD_MERGE_FEATURE = 'GitLab_FastForwardMerge'.freeze
   FILE_LOCKS_FEATURE = 'GitLab_FileLocks'.freeze
   GEO_FEATURE = 'GitLab_Geo'.freeze
   GROUP_WEBHOOKS_FEATURE = 'GitLab_GroupWebhooks'.freeze
@@ -57,7 +56,6 @@ class License < ActiveRecord::Base
     cross_project_pipelines: CROSS_PROJECT_PIPELINES_FEATURE,
     deploy_board: DEPLOY_BOARD_FEATURE,
     export_issues: EXPORT_ISSUES_FEATURE,
-    fast_forward_merge: FAST_FORWARD_MERGE_FEATURE,
     file_locks: FILE_LOCKS_FEATURE,
     group_webhooks: GROUP_WEBHOOKS_FEATURE,
     issuable_default_templates: ISSUABLE_DEFAULT_TEMPLATES_FEATURE,
@@ -88,7 +86,6 @@ class License < ActiveRecord::Base
     { CONTRIBUTION_ANALYTICS_FEATURE => 1 },
     { ELASTIC_SEARCH_FEATURE => 1 },
     { EXPORT_ISSUES_FEATURE => 1 },
-    { FAST_FORWARD_MERGE_FEATURE => 1 },
     { GROUP_WEBHOOKS_FEATURE => 1 },
     { ISSUABLE_DEFAULT_TEMPLATES_FEATURE => 1 },
     { ISSUE_BOARD_FOCUS_MODE_FEATURE => 1 },
@@ -146,7 +143,6 @@ class License < ActiveRecord::Base
     { DEPLOY_BOARD_FEATURE => 1 },
     { ELASTIC_SEARCH_FEATURE => 1 },
     { EXPORT_ISSUES_FEATURE => 1 },
-    { FAST_FORWARD_MERGE_FEATURE => 1 },
     { FILE_LOCKS_FEATURE => 1 },
     { GEO_FEATURE => 1 },
     { GROUP_WEBHOOKS_FEATURE => 1 },

ee/app/controllers/ee/projects_controller.rb

@@ -12,7 +12,6 @@ module EE
         approver_group_ids
         approver_ids
         issues_template
-        merge_method
         merge_requests_template
         disable_overriding_approvers_per_merge_request
         repository_size_limit

ee/app/views/projects/ee/_merge_request_settings.html.haml

 - form = local_assigns.fetch(:form)
 - project = local_assigns.fetch(:project)
 
-- if project.feature_available?(:merge_request_rebase) || project.feature_available?(:fast_forward_merge)
-  .form-group
-    = label_tag :merge_method_merge, class: 'label-light' do
-      Merge method
+.form-group
+  = label_tag :merge_method_merge, class: 'label-light' do
+    Merge method
+  .radio
+    = label_tag :project_merge_method_merge do
+      = form.radio_button :merge_method, :merge, class: "js-merge-method-radio"
+      %strong Merge commit
+      %br
+      %span.descr
+        A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
+
+  - if project.feature_available?(:merge_request_rebase)
     .radio
-      = label_tag :project_merge_method_merge do
-        = form.radio_button :merge_method, :merge, class: "js-merge-method-radio"
-        %strong Merge commit
+      = label_tag :project_merge_method_rebase_merge do
+        = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio"
+        %strong Merge commit with semi-linear history
         %br
         %span.descr
-          A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
-
-    - if project.feature_available?(:merge_request_rebase)
-      .radio
-        = label_tag :project_merge_method_rebase_merge do
-          = form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio"
-          %strong Merge commit with semi-linear history
-          %br
-          %span.descr
-            A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible.
-            This way you could make sure that if this merge request would build, after merging to target branch it would also build.
-          %br
-          %span.descr
-            When fast-forward merge is not possible, the user is given the option to rebase.
+          A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible.
+          This way you could make sure that if this merge request would build, after merging to target branch it would also build.
+        %br
+        %span.descr
+          When fast-forward merge is not possible, the user is given the option to rebase.
 
-    - if project.feature_available?(:fast_forward_merge)
-      .radio
-        = label_tag :project_merge_method_ff do
-          = form.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 could be fast-forwarded.
-          - if project.feature_available?(:merge_request_rebase)
-            %br
-            %span.descr
-              When fast-forward merge is not possible, the user is given the option to rebase.
+  .radio
+    = label_tag :project_merge_method_ff do
+      = form.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 could be fast-forwarded.
+      - if project.feature_available?(:merge_request_rebase)
+        %br
+        %span.descr
+          When fast-forward merge is not possible, the user is given the option to rebase.
 
 - if @project.feature_available?(:issuable_default_templates)
   .form-group

ee/app/views/shared/promotions/_promote_mr_features.html.haml

-- if show_promotions? && show_callout?('promote_mr_features_dismissed') && (!@project.feature_available?(:merge_request_approvers) || !@project.feature_available?(:fast_forward_merge))
+- if show_promotions? && show_callout?('promote_mr_features_dismissed') && !@project.feature_available?(:merge_request_approvers)
   .user-callout.promotion-callout.append-bottom-20.js-mr-approval-callout#promote_mr_features{ data: { uid: 'promote_mr_features_dismissed' } }
     .bordered-box.content-block
       %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss Merge Request promotion' }
@@ -15,10 +15,6 @@
               = link_to 'Merge Request Approvals', help_page_path('user/project/merge_requests/merge_request_approvals.html'), target: '_blank'
               %p
                 Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.
-          - unless @project.feature_available?(:fast_forward_merge)
-            %li
-              = link_to 'Fast-forward Merge', help_page_path('user/project/merge_requests/fast_forward_merge.html'), target: '_blank'
-              %p
-                If you prefer a linear Git history and a way to accept merge requests without creating merge commits, you can configure this on a per-project basis.
+            
 
         = render 'shared/promotions/promotion_link_project'

spec/controllers/projects_controller_spec.rb

@@ -312,6 +312,24 @@ describe ProjectsController do
       end
     end
 
+    it 'updates Fast Forward Merge attributes' do
+      controller.instance_variable_set(:@project, project)
+
+      params = {
+        merge_method: :ff
+      }
+
+      put :update,
+          namespace_id: project.namespace,
+          id: project.id,
+          project: params
+
+      expect(response).to have_http_status(302)
+      params.each do |param, value|
+        expect(project.public_send(param)).to eq(value)
+      end
+    end
+
     def update_project(**parameters)
       put :update,
           namespace_id: project.namespace.path,

spec/ee/spec/controllers/ee/projects_controller_spec.rb

@@ -109,38 +109,6 @@ describe ProjectsController do
       end
     end
 
-    it 'updates Fast Forward Merge attributes' do
-      params = {
-        merge_method: :ff
-      }
-
-      put :update,
-          namespace_id: project.namespace,
-          id: project.id,
-          project: params
-
-      expect(response).to have_http_status(302)
-      params.each do |param, value|
-        expect(project.public_send(param)).to eq(value)
-      end
-    end
-
-    it 'updates Fast Forward Merge attributes' do
-      params = {
-        merge_method: :ff
-      }
-
-      put :update,
-          namespace_id: project.namespace,
-          id: project.id,
-          project: params
-
-      expect(response).to have_http_status(302)
-      params.each do |param, value|
-        expect(project.public_send(param)).to eq(value)
-      end
-    end
-
     it 'updates Service Desk attributes' do
       allow(Gitlab::IncomingEmail).to receive(:enabled?) { true }
       allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }

spec/ee/spec/models/ee/project_spec.rb

@@ -722,23 +722,15 @@ describe Project do
   end
 
   describe '#merge_method' do
-    where(:ff, :rebase, :ff_licensed, :rebase_licensed, :method) do
-      true  | true  | true  | true  | :ff
-      true  | true  | true  | false | :ff
-      true  | true  | false | true  | :rebase_merge
-      true  | true  | false | false | :merge
-      true  | false | true  | true  | :ff
-      true  | false | true  | false | :ff
-      true  | false | false | true  | :merge
-      true  | false | false | false | :merge
-      false | true  | true  | true  | :rebase_merge
-      false | true  | true  | false | :merge
-      false | true  | false | true  | :rebase_merge
-      false | true  | false | false | :merge
-      false | false | true  | true  | :merge
-      false | false | true  | false | :merge
-      false | false | false | true  | :merge
-      false | false | false | false | :merge
+    where(:ff, :rebase, :rebase_licensed, :method) do
+      true  | true  | true  | :ff
+      true  | true  | false | :ff
+      true  | false | true  | :ff
+      true  | false | false | :ff
+      false | true  | true  | :rebase_merge
+      false | true  | false | :merge
+      false | false | true  | :merge
+      false | false | false | :merge
     end
 
     with_them do
@@ -747,7 +739,7 @@ describe Project do
       subject { project.merge_method }
 
       before do
-        stub_licensed_features(merge_request_rebase: rebase_licensed, fast_forward_merge: ff_licensed)
+        stub_licensed_features(merge_request_rebase: rebase_licensed)
       end
 
       it { is_expected.to eq(method) }

spec/serializers/merge_request_entity_spec.rb

@@ -48,10 +48,11 @@ describe MergeRequestEntity do
                                :create_issue_to_resolve_discussions_path,
                                :source_branch_path, :target_branch_commits_path,
                                :target_branch_tree_path, :commits_count, :merge_ongoing,
+                               :ff_only_enabled,
                                ## EE
                                :can_push_to_source_branch, :approvals_before_merge,
                                :squash, :rebase_commit_sha, :rebase_in_progress,
-                               :approvals_path, :ff_only_enabled)
+                               :approvals_path)
   end
 
   it 'has email_patches_path' do