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
- Enable the feature flag in console
Feature.enable(:group_merge_request_approval_settings_feature_flag)
- 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
-
📋 Does this MR need a changelog?-
I have included a changelog entry for the database change -
I have not included a changelog entry because _____________
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers - [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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)