Follow-up from "Update DismissPolicyViolations mutation with dismissal_types and comment"
The following discussion from !204903 (merged) should be addressed:
Thread: Refactor group_violations_by_policy
@imam_h started a discussion: (+2 comments)
Hi @Andyschoenen , could you please do an initial review of this MR? Thank you.
@sashi_kumar : Thanks! The changes in this MR LGTM👍
Comment by @sashi_kumar
But I've a couple of concerns withgroup_violations_by_policy:
group_by_security_policy_idis currently a scope, but it is not composable. Should that be a class method?group_by_security_policy_idpreloadsapproval_policy_ruleseven thoughfor_security_policiesdoes a join. Maybe we can combine to avoid the extra query to loadapproval_policy_rules.Maybe, we can create a single class method like:
def self.group_by_security_policy_id(security_policies) joins(:approval_policy_rule) .where(approval_policy_rule: { security_policy: security_policies }) .includes(:approval_policy_rule) .group_by { |v| v.approval_policy_rule&.security_policy_id } endThis will generate this query
SQL
SELECT
"scan_result_policy_violations"."id" AS t0_r0,
"scan_result_policy_violations"."scan_result_policy_id" AS t0_r1,
"scan_result_policy_violations"."merge_request_id" AS t0_r2,
"scan_result_policy_violations"."project_id" AS t0_r3,
"scan_result_policy_violations"."created_at" AS t0_r4,
"scan_result_policy_violations"."updated_at" AS t0_r5,
"scan_result_policy_violations"."violation_data" AS t0_r6,
"scan_result_policy_violations"."approval_policy_rule_id" AS t0_r7,
"scan_result_policy_violations"."status" AS t0_r8,
"approval_policy_rule"."id" AS t1_r0,
"approval_policy_rule"."security_policy_id" AS t1_r1,
"approval_policy_rule"."created_at" AS t1_r2,
"approval_policy_rule"."updated_at" AS t1_r3,
"approval_policy_rule"."rule_index" AS t1_r4,
"approval_policy_rule"."type" AS t1_r5,
"approval_policy_rule"."content" AS t1_r6,
"approval_policy_rule"."security_policy_management_project_id" AS t1_r7
FROM
"scan_result_policy_violations"
INNER JOIN
"approval_policy_rules" "approval_policy_rule"
ON "approval_policy_rule"."id" = "scan_result_policy_violations"."approval_policy_rule_id"
WHERE
"scan_result_policy_violations"."merge_request_id" = 292
AND "approval_policy_rule"."security_policy_id" IN (
SELECT
"security_policies"."id"
FROM
"security_policies"
WHERE
"security_policies"."id" IN (
97, 98, 99, 100, 101, 102, 103, 104, 105, 111
)
AND (
content->>'enforcement_type' = 'warn'
)
)
Let me know your thoughts
🏓
Edited by 🤖 GitLab Bot 🤖