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.* to report_type by adding report_type into approval_project_rules (similar to approval_merge_request_rules). This approach would allow new rules with various names to also rely on the existing report_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.

Edited Sep 28, 2021 by Zamir Martins
Assignee Loading
Time tracking Loading