## Context | | | |---|---| | **Phase** | 3 of 6 | | **Parallel with** | https://gitlab.com/gitlab-org/gitlab/-/work_items/594881+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594882+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594883+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594884+ | | **Blocked by** | https://gitlab.com/gitlab-org/gitlab/-/work_items/594877+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594878+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594879+ | | **Unblocks** | https://gitlab.com/gitlab-org/gitlab/-/work_items/594885+ <br> https://gitlab.com/gitlab-org/gitlab/-/work_items/594886+ | ## Summary Update the EE protected branches API helpers and `AccessLevelParams` service to accept and process `member_role_id` as a valid parameter in the `allowed_to_push`, `allowed_to_merge`, and `allowed_to_unprotect` arrays for the project-level protected branches API. ## Background The project protected branches REST API accepts `allowed_to_push`/`allowed_to_merge`/`allowed_to_unprotect` as arrays of objects. Currently valid keys per object are: `access_level`, `user_id`, `group_id`, `deploy_key_id` (push only), `id` (for update/destroy), `_destroy`. In EE, `ProtectedRefs::AccessLevelParams` (prepended by `EE::ProtectedRefs::AccessLevelParams`) processes these arrays. Currently `granular_access_levels` passes through entries that do **not** have `deploy_key_id`. Adding `member_role_id` requires ensuring these entries flow through correctly. ## Relevant files - `ee/lib/ee/api/helpers/protected_branches_helpers.rb` — Grape param declarations - `ee/app/services/ee/protected_refs/access_level_params.rb` — service that processes params - `app/services/protected_refs/access_level_params.rb` — CE base ## Changes required ### `ee/lib/ee/api/helpers/protected_branches_helpers.rb` Add `member_role_id` to the `shared_params` block and/or directly to each `allowed_to_*` array param block: ```ruby params :shared_params do optional :user_id, type: Integer, desc: 'ID of a user' optional :group_id, type: Integer, desc: 'ID of a group' optional :member_role_id, type: Integer, desc: 'ID of a custom member role' optional :id, type: Integer, desc: 'ID of a project' optional :_destroy, type: Grape::API::Boolean, desc: 'Delete the object when true' end ``` This will automatically propagate `member_role_id` into all three `allowed_to_*` array blocks that `use :shared_params`. Add API documentation for the new parameter. ### `ee/app/services/ee/protected_refs/access_level_params.rb` The current `granular_access_levels` method rejects entries with `deploy_key_id`. It passes through everything else (including `user_id`, `group_id`). Since `member_role_id` is not `deploy_key_id`, it will already pass through the filter — however verify explicitly: ```ruby def granular_access_levels entries = params[:"allowed_to_#{type}"] || [] entries.reject { |entry| entry[:deploy_key_id].present? } end ``` No change needed unless `member_role_id` entries also need to be excluded from `use_default_access_level?` — verify that when `member_role_id` entries are the only entries, `use_default_access_level?` returns `false` so that the Maintainer default is not prepended. Current logic: ```ruby def use_default_access_level?(params) return false unless super entries = params[:"allowed_to_#{type}"] || [] entries.reject { |entry| entry[:deploy_key_id].present? }.blank? end ``` Since `member_role_id` entries are not deploy key entries, `reject` will keep them, and `blank?` will return `false` — meaning `use_default_access_level?` correctly returns `false`. **No change needed**, but add a test to confirm this behaviour. ### Validation In the API endpoint (or via model validation from Issue 4), validate that the provided `member_role_id` belongs to the project's root namespace. If not, return a `422 Unprocessable Entity` with a clear error message. ## API examples **Create a protected branch with a custom role allowed to merge:** ``` POST /projects/:id/protected_branches { "name": "main", "allowed_to_merge": [{ "member_role_id": 42 }], "allowed_to_push": [{ "access_level": 40 }] } ``` **Update — add a custom role, remove an existing one:** ``` PATCH /projects/:id/protected_branches/main { "allowed_to_merge": [ { "id": 5, "_destroy": true }, { "member_role_id": 42 } ] } ``` ## Testing - API request spec: create protected branch with `member_role_id` in `allowed_to_merge` — verify access level row created with correct `member_role_id` - API request spec: create with `member_role_id` in `allowed_to_push` and `allowed_to_unprotect` - API request spec: update with `member_role_id` - API request spec: `member_role_id` from wrong namespace returns 422 - API request spec: passing only `member_role_id` entries does not add a default Maintainer role entry - Feature flag off: `member_role_id` param is ignored or returns 422 ## Dependencies - Issue 1 (DB migration) - Issues 3 & 4 (model concern) — model must accept and validate `member_role_id` ## Labels