Document what the merge request approvals endpoints return in EE

What does this MR do and why?

This is the follow-up to !259766 (merged) that @idurham asked for and that @Saahmed suggested after finding that the API reference shows only Community Edition fields for GET /projects/:id/merge_requests/:merge_request_iid/approvals.

That route and POST .../approve and POST .../unapprove declare ::API::Entities::MergeRequestApprovals in their desc blocks, and doc/api/openapi/openapi_v3.yaml is generated from those declarations with the Enterprise Edition code loaded. Enterprise Edition overrides the present_approval helper the three routes call (ee/lib/ee/api/merge_request_approvals.rb), so they present merge_request.approval_state with ::API::Entities::ApprovalState instead. The generated document therefore describes these routes with 4 attributes, while every Enterprise Edition instance, GitLab.com included and whatever its tier, returns 24. It also types approved_by as a single object rather than an array.

This MR:

  • Prepends EE::API::Entities::MergeRequestApprovals onto the CE entity. In Enterprise Edition the entity exposes the merge request's approval state, merged and rendered by ApprovalState, so it renders exactly what the routes returned before: the same attributes, the same values and the same key order.
  • Removes the present_approval override, which the entity makes redundant. The three routes now render, through the CE helper, the entity they declare. The deprecated POST .../approvals, which calls the same helper and declares ApprovalState, returns the same attributes as before.
  • Regenerates openapi_v3.yaml. APIEntitiesMergeRequestApprovals, which the three routes reference, goes from 4 properties to 24, the same 24 as APIEntitiesApprovalState in the same order. Nothing else in the document changes.
  • Runs the CE entity spec only in Community Edition, where these routes still return the four attributes, the way spec/lib/api/work_items/parity_spec.rb does, and adds an EE spec that holds the entity to what ApprovalState renders for the same merge request, key order included.

Community Edition is unchanged: no EE module is loaded there, and the routes keep presenting the four attributes.

On annotating the Enterprise Edition fields: gitlab-grape-openapi 0.8.0 can annotate an operation from a route_setting, which is how config/initializers/gitlab_grape_openapi.rb configures x-gitlab-tier and x-gitlab-lifecycle, but nothing comparable reaches a single property: an exposure's documentation reaches the document only as its type, format, description, example and allowed values. The document is generated with Enterprise Edition loaded, so it describes these routes the way it already describes approvals_before_merge under APIEntitiesMergeRequestBasic, which only Enterprise Edition returns too. A per-field edition marker would have to start in the generator.

References

  • !259766 (merged), the documentation page this follows up, with @idurham's request and @Saahmed's note on the API reference.
  • #408183 proposes the other direction, moving the CE code into EE. This MR does not do that and does not close it.
  • !260117 adds route_setting :tier to two of the routes ee/lib/ee/api/merge_request_approvals.rb defines, and !260551 rewords the parameter descriptions in the same file. Both regenerate openapi_v3.yaml, and both change different lines of the two files than this MR does.
  • I found this while maintaining gitlab-mcp-server, an MCP server for the GitLab API, which publishes what each edition returns from these routes. The project keeps a record of everything it finds and does on its dependencies and on GitLab in upstream-bugs.md.

Screenshots or screen recordings

APIEntitiesMergeRequestApprovals in doc/api/openapi/openapi_v3.yaml, the schema the three routes reference:

Before After
4 properties: user_has_approved, user_can_approve, approved, and approved_by typed as one APIEntitiesApprovals object 24 properties, equal to APIEntitiesApprovalState in content and order: id, iid, project_id, title, description, state, created_at, updated_at, merge_status, approved, approvals_required, approvals_left, require_password_to_approve, approved_by (an array of APIEntitiesApprovals), suggested_approvers, approvers, approver_groups, user_has_approved, user_can_approve, approval_rules_left, has_approval_rules, merge_request_approvers_available, multiple_approval_rules_available, invalid_approvers_rules
Why an entity, and why merged

A desc block is evaluated when the CE class body runs, before prepend_mod_with applies the EE module, so an EE module cannot change the entity a CE route declares. The way the codebase lets a CE annotation describe more in Enterprise Edition is an EE module prepended onto the CE entity, as EE::API::Entities::MergeRequestBasic does for approvals_before_merge, and the generator reads that entity with the EE module applied. I found no route in lib/api that chooses its documented entity per edition any other way.

Merging ApprovalState gives the document the 24 attributes without defining any exposure twice, and it keeps the response as it was. Adding the 20 Enterprise Edition exposures after the CE four would have changed the key order, and two of the CE four compute their values differently from ApprovalState: approved is approvals.present? where ApprovalState returns approval_state.approved?, and user_can_approve calls merge_request.eligible_for_approval_by?, which defers to the approval state only where the merge_request_approvers feature is available. The new EE spec shows the first one: for a merge request with one approval of the two its rule requires, the CE entity says approved: true and ApprovalState says approved: false. The entity reads merge_request.approval_state with no target branch, the same memoized object the override presented.

What this MR leaves as it is: openapi_v2.yaml

No CI job checks doc/api/openapi/openapi_v2.yaml (scripts/static-analysis runs only the two v3 tasks), and regenerating it on this branch changes 12,642 lines, almost none of them related to this MR. In that regeneration API_Entities_MergeRequestApprovals gets the same 24 properties as API_Entities_ApprovalState. I left the file alone rather than edit a generated document by hand.

How I checked it

On this branch, on top of master a20857c3:

  • bin/rake gitlab:openapi:v3:generate reproduces the committed document byte for byte, GITLAB_SIMULATE_SAAS=false bin/rake gitlab:openapi:v3:check_docs reports it up to date, and bin/rake gitlab:openapi:v3:validate_tag_docs reports the tag content valid. Against master the document changes in one schema and no path: 70 lines added and 3 removed, all in APIEntitiesMergeRequestApprovals.
  • With Enterprise Edition loaded, ee/spec/lib/ee/api/entities/merge_request_approvals_spec.rb, ee/spec/requests/api/merge_request_approvals_spec.rb, spec/lib/api/entities/merge_request_approvals_spec.rb and spec/requests/api/merge_request_approvals_spec.rb: 123 examples, 0 failures, 7 pending (the pending ones are the existing granular token examples, "namespace has no top-level group").
  • With FOSS_ONLY=1, the two CE spec files: 63 examples, 0 failures, 4 pending.
  • With the entity and helper code as they are on master and the new EE spec in place, the two EE spec files: the new spec fails, because the entity renders the four CE attributes, and the request spec passes. So the new spec catches the documentation defect.
  • With the prepend_mod_with line taken out of the CE entity, so with neither the new EE module nor the old override, the EE request spec: 13 of 61 examples fail, on attributes only Enterprise Edition returns and, once, on approved, which the CE entity computes differently, across GET .../approvals, POST .../approve, POST .../unapprove and the deprecated POST .../approvals. So the request specs cover the path the override used to serve.
  • RuboCop on the five Ruby files: no offenses. scripts/lint/commit_linter.rb and the Danger commit linter: no problems.

How to set up and validate locally

  1. On an Enterprise Edition GDK, call GET /api/v4/projects/:id/merge_requests/:merge_request_iid/approvals for any merge request. The response has the same 24 keys in the same order on master and on this branch.
  2. Run bin/rake gitlab:openapi:v3:generate. APIEntitiesMergeRequestApprovals in doc/api/openapi/openapi_v3.yaml lists the same 24 properties as APIEntitiesApprovalState.
  3. Run bundle exec rspec ee/spec/lib/ee/api/entities/merge_request_approvals_spec.rb ee/spec/requests/api/merge_request_approvals_spec.rb.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports

Loading
Loading