Skip to content

Remove unified logic for required_approval_count

Hunter Stewart requested to merge hustewart-count-rules into master

What does this MR do and why?

See Remove unified code path in required count appr... (#462896 - closed)

Since unified approval rules were migrated, we no longer need conditional logic checking the required_approval_count.

The inclusion of this logic makes environments fall back to old, unwanted behavior if their approval rules are removed. So we should remove it.

This change also makes the method more consistent by having it always return a number.

This came about while I was working on a larger bug fix, but this seems isolated enough for its own MR.

Finally, this MR improves our test coverage by updating old unified approval rule [deprecated] tests to use the multi access levels style approval rules for testing.

About the spec changes

For some reason the initial pipelines passed. After approval they started failing. Investigating the failure revealed they were related to the changes.

Commit !153749 (4fd8a4ff) updates the specs.

Commit message:

Unified approval rules have been deprecated and migrated to multi access
level approval rules in milesetone 17.0.

After making a small change in `ee/app/models/ee/environment.rb` to stop
relying on unified approval rules for determining
`required_approval_count` for an environment, a number of specs broke.

After analyzing the failures, it looks like the specs were broken due to
testing the unified approval system. Additionally, many of these specs
did not have coverage around the multi_access_level system.

This commit does the following:
- removes tests that were unique to testing the behavior of the unified
  system around the change in `ee/app/models/ee/environment.rb`
- updates the spec test data everywhere else to get test coverage around
  the multi_access_level system
- adds some missing feature categories in specs

MR acceptance checklist

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

How to set up and validate locally

on master

  • protect an environment
  • give the environment a required_approval_count > 0
  • give the environment no rules
  • run a pipeline
  • see that the environment falls back to the old behavior of needing approval based on the required_approval_count

on hustewart-count-rules

  • go to the same project
  • run the pipeline
  • it should run right away as it is not considered pending approval

Numbered steps to set up and validate the change are strongly suggested.

Edited by Hunter Stewart

Merge request reports