Skip to content

Add more settings to group MR approval settings

What does this MR do?

Add the remaining settings to the group MR approval settings table and expose them via the existing API.

I have also taken the liberty to tweak the setting names so that they enable the false setting by default. For example, when allow_overrides_to_approver_list_per_merge_request is not set, we presume the default setting, which is to not allow overrides. The names of these settings on the project-level and instance-level are slightly different and I am looking to converge them to this naming scheme in future MRs (more context).

This feature is behind a feature flag: group_merge_request_approval_settings_feature_flag

🔍 Setup & Test

  1. Enable the feature flag in console
Feature.enable(:group_merge_request_approval_settings_feature_flag)
  1. Use your favourite REST client or cURL to hit the API
GET /groups/:id/merge_request_approval_setting
curl -i -H Authorization\:\ Bearer\ YDYtSYhMQrz3AzpmdDqw -XGET http\://gdk.test\:3000/api/v4/groups/22/merge_request_approval_setting

{
  "allow_author_approval": true,
  "allow_committer_approval": false,
  "allow_overrides_to_approver_list_per_merge_request": false,
  "retain_approvals_on_push": false,
  "require_password_to_approve": false
}
PUT /groups/:id/merge_request_approval_setting
curl -i -H Content-Type\:\ application/json -H Authorization\:\ Bearer\ YDYtSYhMQrz3AzpmdDqw -XPUT http\://gdk.test\:3000/api/v4/groups/22/merge_request_approval_setting -d \{'
'\ \ \"allow_author_approval\"\:\ true\,'
'\ \ \"allow_overrides_to_approver_list_per_merge_request\"\:\ true'
''
'\}'
'

{
  "allow_author_approval": true,
  "allow_committer_approval": false,
  "allow_overrides_to_approver_list_per_merge_request": true,
  "retain_approvals_on_push": false,
  "require_password_to_approve": false
}

🐘 Database

Migrate

== 20210310111009 AddSettingsToGroupMergeRequestApprovalSettings: migrating ===
-- change_table(:group_merge_request_approval_settings, {:bulk=>true})
   -> 0.0121s                                                                                             
== 20210310111009 AddSettingsToGroupMergeRequestApprovalSettings: migrated (0.0122s) 

Revert

== 20210310111009 AddSettingsToGroupMergeRequestApprovalSettings: reverting ===     
-- remove_column(:group_merge_request_approval_settings, :require_password_to_approve, :boolean, {:null=>false, :default=>false})
   -> 0.0073s                   
-- remove_column(:group_merge_request_approval_settings, :retain_approvals_on_push, :boolean, {:null=>false, :default=>false})
   -> 0.0024s                         
-- remove_column(:group_merge_request_approval_settings, :allow_overrides_to_approver_list_per_merge_request, :boolean, {:null=>false, :default=>false})
   -> 0.0026s                        
-- remove_column(:group_merge_request_approval_settings, :allow_committer_approval, :boolean, {:null=>false, :default=>false})                                                                                       
   -> 0.0024s          
== 20210310111009 AddSettingsToGroupMergeRequestApprovalSettings: reverted (0.0242s) 

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Related to #323127 (closed)

Edited by Tan Le

Merge request reports