Return each bypassable security policy once per merge request

What does this MR do and why?

The Bypass Policy Violation modal lists the same policy once for each of its rules that applies to the merge request. The duplicates come from the backend rather than the Vue component:

project.approval_policies.with_bypass_settings
  .joins(:approval_policy_rules)
  .where(approval_policy_rules: { id: <the MR's approval policy rule IDs> })

The joins returns one row per matching rule. policyBypassStatuses, which feeds the exception-mode dropdown, therefore contains repeated entries with the same policy id. The warn-mode path already deduplicates on the frontend (uniqBy(policies, 'securityPolicyId')), but policyBypassStatuses does not.

This MR filters the policies with a subquery on the matching approval policy rules instead of a join, so each policy is returned once. The fix applies to every consumer of the GraphQL field, not just the modal. The other caller, the mergeability check, only uses .any?, so its behaviour is unchanged.

A spec now covers a policy with two rules that both apply to the merge request.

Database

This changes the query behind MergeRequest#security_policies_with_bypass_settings, which backs the policyBypassStatuses GraphQL field. approval_merge_request_rules is the merge request's approval rules subquery; it is unchanged.

Before (one row per matching rule):

SELECT "security_policies".* FROM "security_policies"
INNER JOIN "security_policy_project_links" ON "security_policies"."id" = "security_policy_project_links"."security_policy_id"
INNER JOIN "approval_policy_rules" ON "approval_policy_rules"."security_policy_id" = "security_policies"."id"
WHERE "security_policy_project_links"."project_id" = 1
  AND "security_policies"."type" = 0
  AND (security_policies.content->'bypass_settings' IS NOT NULL)
  AND (security_policies.content->'bypass_settings' <> '{}')
  AND "approval_policy_rules"."id" IN (
    SELECT "approval_merge_request_rules"."approval_policy_rule_id" FROM "approval_merge_request_rules"
    WHERE "approval_merge_request_rules"."merge_request_id" = 1
      AND "approval_merge_request_rules"."approval_policy_rule_id" IS NOT NULL
  );

After (one row per policy):

SELECT "security_policies".* FROM "security_policies"
INNER JOIN "security_policy_project_links" ON "security_policies"."id" = "security_policy_project_links"."security_policy_id"
WHERE "security_policy_project_links"."project_id" = 1
  AND "security_policies"."type" = 0
  AND (security_policies.content->'bypass_settings' IS NOT NULL)
  AND (security_policies.content->'bypass_settings' <> '{}')
  AND "security_policies"."id" IN (
    SELECT "approval_policy_rules"."security_policy_id" FROM "approval_policy_rules"
    WHERE "approval_policy_rules"."id" IN (
      SELECT "approval_merge_request_rules"."approval_policy_rule_id" FROM "approval_merge_request_rules"
      WHERE "approval_merge_request_rules"."merge_request_id" = 1
        AND "approval_merge_request_rules"."approval_policy_rule_id" IS NOT NULL
    )
  );

The inner subquery is limited to one merge request's approval rules, and the outer query to one project's policies, so both sets are small. The subquery can use approval_policy_rules_pkey for the rule lookup, and the project filter uses idx_security_policy_project_links_on_project_id_and_id. The preload of approval_policy_merge_request_bypass_events is unchanged.

I don't have access to postgres.ai, so I couldn't attach execution plans. Could a database reviewer run them, or point me to someone who can? Thanks!

References

Screenshots or screen recordings

The issue has the reproduction steps. After this change, each policy appears once in the Select policies to bypass dropdown.

How to set up and validate locally

Follow the steps in the issue: create two approval policies with several rules matching the target branch and bypass_settings naming a user, then open Bypass as that user on an MR into main.

Or query the field directly:

query {
  project(fullPath: "<project>") {
    mergeRequest(iid: "<iid>") {
      policyBypassStatuses { id name }
    }
  }
}

Each policy should appear once.

Specs: bin/rspec ee/spec/models/merge_request_spec.rb -e 'security_policies_with_bypass_settings'

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.

Edited by Arjun Singh

Merge request reports

Loading
Loading