Refactor report_approver project approval rules name mapping in favor of rule_type param
Summary
With the work in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13109#note_190292339 we introduced a quick mapping to auto-set approval_project_rules.rule_type
according to the rule name. This is ugly and would be better handled by the frontend passing the appropriate rule_type
. Naively this could be handled by moving the conditional param merge into the frontend but ideally it would involve introducing a new UI to allow a user to select their rule type; via checkbox etc.
See related parent ticket for further discussion https://gitlab.com/gitlab-org/gitlab-ee/issues/9928#decisions
Improvements
Reduce magic auto-setting of project rule rule_type
Risks
report_approver
rule types aren't created as expected. This should be covered by existing tests but should be manually tested to confirm.
Involved components
ee/app/services/approval_rules/create_service.rb
ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue
Implementation plan
-
backend Replace the existing direct deduction (REPORT_TYPES_BY_DEFAULT_NAME) when converting DEFAULT_NAME.*
toreport_type
by addingreport_type
intoapproval_project_rules
(similar toapproval_merge_request_rules
). This approach would allow new rules with various names to also rely on the existingreport_type
values (e.g.vulnerability
). -
backend frontend Replace the existing logic when rule_type
is part of REPORT_TYPES_BY_DEFAULT_NAME by making it part of the payload through API and UI. This would allow new rules with various names to also use the rule type:report_approver
without the need of changing this logic.
Note: The goal is to make the underlying code more flexible for new rules but not changing constraints in regards to (1) License-Check
, (2) Vulnerability-Check
, (3) Coverage-Check
.