Merge branch 'revert-allow-editing-commit-changes' into 'master'

Revert "Prevent editing commits" feature

See merge request gitlab-org/gitlab!66754

app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue

@@ -3,7 +3,6 @@ import { GlIcon, GlSprintf, GlLink, GlFormCheckbox, GlToggle } from '@gitlab/ui'
 
 import settingsMixin from 'ee_else_ce/pages/projects/shared/permissions/mixins/settings_pannel_mixin';
 import { s__ } from '~/locale';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import {
   visibilityOptions,
   visibilityLevelDescriptions,
@@ -48,7 +47,7 @@ export default {
     GlFormCheckbox,
     GlToggle,
   },
-  mixins: [settingsMixin, glFeatureFlagsMixin()],
+  mixins: [settingsMixin],
 
   props: {
     requestCveAvailable: {
@@ -737,22 +736,5 @@ export default {
         }}</template>
       </gl-form-checkbox>
     </project-setting-row>
-    <project-setting-row
-      v-if="glFeatures.allowEditingCommitMessages"
-      ref="allow-editing-commit-messages"
-      class="gl-mb-4"
-    >
-      <input
-        :value="allowEditingCommitMessages"
-        type="hidden"
-        name="project[project_setting_attributes][allow_editing_commit_messages]"
-      />
-      <gl-form-checkbox v-model="allowEditingCommitMessages">
-        {{ s__('ProjectSettings|Allow editing commit messages') }}
-        <template #help>{{
-          s__('ProjectSettings|Commit authors can edit commit messages on unprotected branches.')
-        }}</template>
-      </gl-form-checkbox>
-    </project-setting-row>
   </div>
 </template>

app/controllers/projects_controller.rb

@@ -31,10 +31,6 @@ class ProjectsController < Projects::ApplicationController
   # Project Export Rate Limit
   before_action :export_rate_limit, only: [:export, :download_export, :generate_new_export]
 
-  before_action only: [:edit] do
-    push_frontend_feature_flag(:allow_editing_commit_messages, @project)
-  end
-
   before_action do
     push_frontend_feature_flag(:refactor_blob_viewer, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:increase_page_size_exponentially, @project, default_enabled: :yaml)
@@ -399,7 +395,6 @@ class ProjectsController < Projects::ApplicationController
     %i[
       show_default_award_emojis
       squash_option
-      allow_editing_commit_messages
       mr_default_target_self
     ]
   end

app/helpers/projects_helper.rb

@@ -491,7 +491,6 @@ module ProjectsHelper
 
   def project_permissions_settings(project)
     feature = project.project_feature
-
     {
       packagesEnabled: !!project.packages_enabled,
       visibilityLevel: project.visibility_level,
@@ -511,7 +510,6 @@ module ProjectsHelper
       metricsDashboardAccessLevel: feature.metrics_dashboard_access_level,
       operationsAccessLevel: feature.operations_access_level,
       showDefaultAwardEmojis: project.show_default_award_emojis?,
-      allowEditingCommitMessages: project.allow_editing_commit_messages?,
       securityAndComplianceAccessLevel: project.security_and_compliance_access_level
     }
   end

app/models/project.rb

@@ -435,7 +435,7 @@ class Project < ApplicationRecord
   delegate :restrict_user_defined_variables, :restrict_user_defined_variables=, to: :ci_cd_settings, allow_nil: true
   delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
   delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?,
-    :allow_merge_on_skipped_pipeline=, :has_confluence?, :allow_editing_commit_messages?,
+    :allow_merge_on_skipped_pipeline=, :has_confluence?,
     to: :project_setting
   delegate :active?, to: :prometheus_integration, allow_nil: true, prefix: true
 

app/models/project_setting.rb

 # frozen_string_literal: true
 
 class ProjectSetting < ApplicationRecord
+  include IgnorableColumns
+
+  ignore_column :allow_editing_commit_messages, remove_with: '14.4', remove_after: '2021-09-10'
+
   belongs_to :project, inverse_of: :project_setting
 
   enum squash_option: {

config/feature_flags/development/allow_editing_commit_messages.yml

----
-name: allow_editing_commit_messages
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49152/
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/290779
-milestone: '13.7'
-type: development
-group:
-default_enabled: false

db/migrate/20201203144655_add_allow_to_edit_commit_to_project_settings.rb

@@ -6,14 +6,10 @@ class AddAllowToEditCommitToProjectSettings < ActiveRecord::Migration[6.0]
   DOWNTIME = false
 
   def up
-    with_lock_retries do
-      add_column :project_settings, :allow_editing_commit_messages, :boolean, default: false, null: false
-    end
+    # no-op
   end
 
   def down
-    with_lock_retries do
-      remove_column :project_settings, :allow_editing_commit_messages
-    end
+    # no-op
   end
 end

ee/app/models/ee/project_setting.rb

@@ -8,23 +8,6 @@ module EE
       belongs_to :push_rule
 
       scope :has_vulnerabilities, -> { where('has_vulnerabilities IS TRUE') }
-
-      validate :allow_editing_commits
-
-      private
-
-      def allow_editing_commits
-        return unless signed_commits_required?
-
-        error_message = _("can't be enabled because signed commits are required for this project")
-        errors.add(:allow_editing_commit_messages, error_message)
-      end
-
-      def signed_commits_required?
-        return false unless push_rule
-
-        push_rule.reject_unsigned_commits?
-      end
     end
   end
 end

ee/spec/models/ee/project_setting_spec.rb

@@ -13,35 +13,4 @@ RSpec.describe ProjectSetting do
 
     it { is_expected.to contain_exactly(setting_1) }
   end
-
-  describe '#allow_editing_commits' do
-    subject(:setting) { build(:project_setting) }
-
-    context 'with a push rule' do
-      context 'when reject unsigned commits is enabled' do
-        it 'prevents editing commits' do
-          setting.build_push_rule
-          setting.push_rule.reject_unsigned_commits = true
-
-          expect(setting).not_to be_valid
-          expect(setting.errors[:allow_editing_commit_messages]).to be_present
-        end
-      end
-
-      context 'when reject unsigned commits is disabled' do
-        it 'allows editing commits' do
-          setting.build_push_rule
-          setting.push_rule.reject_unsigned_commits = false
-
-          expect(setting).to be_valid
-        end
-      end
-    end
-
-    context 'without a push rule' do
-      it 'allows editing commits' do
-        expect(setting).to be_valid
-      end
-    end
-  end
 end

locale/gitlab.pot

@@ -25701,9 +25701,6 @@ msgstr ""
 msgid "ProjectSettings|Allow"
 msgstr ""
 
-msgid "ProjectSettings|Allow editing commit messages"
-msgstr ""
-
 msgid "ProjectSettings|Always show thumbs-up and thumbs-down award emoji buttons on issues, merge requests, and snippets."
 msgstr ""
 
@@ -25731,9 +25728,6 @@ msgstr ""
 msgid "ProjectSettings|Choose your merge method, merge options, merge checks, merge suggestions, and set up a default description template for merge requests."
 msgstr ""
 
-msgid "ProjectSettings|Commit authors can edit commit messages on unprotected branches."
-msgstr ""
-
 msgid "ProjectSettings|Configure your project resources and monitor their health."
 msgstr ""
 
@@ -38304,9 +38298,6 @@ msgstr ""
 msgid "can only have one escalation policy"
 msgstr ""
 
-msgid "can't be enabled because signed commits are required for this project"
-msgstr ""
-
 msgid "can't be the same as the source project"
 msgstr ""
 

spec/controllers/projects_controller_spec.rb

@@ -767,8 +767,7 @@ RSpec.describe ProjectsController do
             id: project.path,
             project: {
               project_setting_attributes: {
-                show_default_award_emojis: boolean_value,
-                allow_editing_commit_messages: boolean_value
+                show_default_award_emojis: boolean_value
               }
             }
           }
@@ -776,7 +775,6 @@ RSpec.describe ProjectsController do
           project.reload
 
           expect(project.show_default_award_emojis?).to eq(result)
-          expect(project.allow_editing_commit_messages?).to eq(result)
         end
       end
     end

spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js

@@ -27,7 +27,6 @@ const defaultProps = {
     emailsDisabled: false,
     packagesEnabled: true,
     showDefaultAwardEmojis: true,
-    allowEditingCommitMessages: false,
   },
   isGitlabCom: true,
   canDisableEmails: true,
@@ -53,7 +52,7 @@ describe('Settings Panel', () => {
   let wrapper;
 
   const mountComponent = (
-    { currentSettings = {}, glFeatures = {}, ...customProps } = {},
+    { currentSettings = {}, ...customProps } = {},
     mountFn = shallowMount,
   ) => {
     const propsData = {
@@ -64,9 +63,6 @@ describe('Settings Panel', () => {
 
     return mountFn(settingsPanel, {
       propsData,
-      provide: {
-        glFeatures,
-      },
     });
   };
 
@@ -100,8 +96,6 @@ describe('Settings Panel', () => {
   const findShowDefaultAwardEmojis = () =>
     wrapper.find('input[name="project[project_setting_attributes][show_default_award_emojis]"]');
   const findMetricsVisibilitySettings = () => wrapper.find({ ref: 'metrics-visibility-settings' });
-  const findAllowEditingCommitMessages = () =>
-    wrapper.find({ ref: 'allow-editing-commit-messages' }).exists();
   const findOperationsSettings = () => wrapper.find({ ref: 'operations-settings' });
 
   afterEach(() => {
@@ -582,18 +576,6 @@ describe('Settings Panel', () => {
     );
   });
 
-  describe('Settings panel with feature flags', () => {
-    describe('Allow edit of commit message', () => {
-      it('should show the allow editing of commit messages checkbox', () => {
-        wrapper = mountComponent({
-          glFeatures: { allowEditingCommitMessages: true },
-        });
-
-        expect(findAllowEditingCommitMessages()).toBe(true);
-      });
-    });
-  });
-
   describe('Analytics', () => {
     it('should show the analytics toggle', () => {
       wrapper = mountComponent();

spec/helpers/projects_helper_spec.rb

@@ -917,34 +917,4 @@ RSpec.describe ProjectsHelper do
       subject
     end
   end
-
-  describe '#project_permissions_settings' do
-    context 'with no project_setting associated' do
-      it 'includes a value for edit commit messages' do
-        settings = project_permissions_settings(project)
-
-        expect(settings[:allowEditingCommitMessages]).to be_falsy
-      end
-    end
-
-    context 'when commits are allowed to be edited' do
-      it 'includes the edit commit message value' do
-        project.create_project_setting(allow_editing_commit_messages: true)
-
-        settings = project_permissions_settings(project)
-
-        expect(settings[:allowEditingCommitMessages]).to be_truthy
-      end
-    end
-
-    context 'when commits are not allowed to be edited' do
-      it 'returns false to the edit commit message value' do
-        project.create_project_setting(allow_editing_commit_messages: false)
-
-        settings = project_permissions_settings(project)
-
-        expect(settings[:allowEditingCommitMessages]).to be_falsy
-      end
-    end
-  end
 end

spec/models/project_spec.rb

@@ -655,7 +655,6 @@ RSpec.describe Project, factory_default: :keep do
     it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) }
     it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) }
     it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) }
-    it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) }
     it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) }
     it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) }
 

spec/requests/api/project_attributes.yml

@@ -134,7 +134,6 @@ project_feature:
 
 project_setting:
   unexposed_attributes:
-    - allow_editing_commit_messages
     - created_at
     - has_confluence
     - has_vulnerabilities