Refactor groups registration for easier modification
What does this MR do?
- Breaks refactor out of the refactor + behaviour change in !52371 (merged)
- Refactors the
groups_controller.rb
for enhanced readability (trying to keep branching to once per method). - Refactors and adds to the testing of
groups_controller_spec.rb
.
What this MR doesn't do?
- Adhere to DRY principles in
groups_controller.rb
and some other areas of testing.- Intentional since this area is VERY complex and laying it out this way allows for easier extension in !52371 (merged). Readability trumps DRYness in this case.
- Adhere to Avoidance of Test Duplication.
- As mentioned this area is very complex, so while there is overlap in some areas of testing feature/unit/etc. This is intentional.
- Solve the overall complexity reduction of this area by using more controllers or other methods.
- Surfaced the complexity of this area as a concern here.
- Any further modifications to this area will again take many cycles and enhance risk profile. Adds to some of the reasoning behind duplication of testing layers.
- Surfaced the complexity of this area as a concern here.
- Use
shared_examples
inside testing - especially insidegroups_controller_spec.rb
- Readability of this fragile area takes priority over DRYness.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #219544
Edited by Doug Stull