## Context | | | |---|---| | **Phase** | 1 of 6 | | **Parallel with** | — | | **Blocked by** | — | | **Unblocks** | 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+ | ## Summary Add `member_role_id` column to the three protected branch access level tables. This is the foundational database change that all other issues in this epic depend on. ## Background Protected branch access is controlled via three join tables, each storing an "access level row" that is currently one of: a fixed role (`access_level` integer), a user (`user_id`), a group (`group_id`), or a deploy key (`deploy_key_id` — push only). To support custom roles, a new mutually exclusive type — `member_role_id` — must be added to all three tables. ## Tables to migrate - `protected_branch_merge_access_levels` - `protected_branch_push_access_levels` - `protected_branch_unprotect_access_levels` ## Changes required ### Migration Write a standard GitLab migration (in `db/migrate/`) adding to each of the three tables: ```ruby add_column :protected_branch_merge_access_levels, :member_role_id, :bigint add_column :protected_branch_push_access_levels, :member_role_id, :bigint add_column :protected_branch_unprotect_access_levels, :member_role_id, :bigint ``` ### Foreign key constraints Add FK constraints to `member_roles.id` on each table: ```ruby add_concurrent_foreign_key :protected_branch_merge_access_levels, :member_roles, column: :member_role_id, on_delete: :restrict add_concurrent_foreign_key :protected_branch_push_access_levels, :member_roles, column: :member_role_id, on_delete: :restrict add_concurrent_foreign_key :protected_branch_unprotect_access_levels, :member_roles, column: :member_role_id, on_delete: :restrict ``` > **Note on `ON DELETE` behaviour:** The FK is proposed as `ON DELETE RESTRICT` (prevent deletion of a `MemberRole` if it is referenced by any protected branch access level row). The alternative is `ON DELETE SET NULL` (nullify `member_role_id`, leaving the row in place but reverting it to an ambiguous state). Please confirm with the team which behaviour is correct before merging. Both options and their tradeoffs should be discussed in this issue. > > - `RESTRICT`: Protects data integrity — admins must explicitly remove the protected branch rule before deleting the custom role. Safer but more friction. > - `SET NULL`: Silently degrades — the access level row becomes a role-type entry with `access_level` whatever was set at creation time, potentially granting broader access than intended. ### Indexes ```ruby add_concurrent_index :protected_branch_merge_access_levels, :member_role_id add_concurrent_index :protected_branch_push_access_levels, :member_role_id add_concurrent_index :protected_branch_unprotect_access_levels, :member_role_id ``` ### No backfill needed This is a new feature column. All existing rows will have `member_role_id = NULL`. ### Post-migration Update `db/structure.sql` (handled automatically by Rails migration runner). ## Testing - Migration spec verifying columns, indexes, and FK constraints exist on all three tables after running the migration - Verify `db:migrate` and `db:rollback` both succeed cleanly ## Dependencies None — this is the first issue in the epic and unblocks all backend work. ## Labels