Skip to content

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.
  • Use shared_examples inside testing - especially inside groups_controller_spec.rb
    • Readability of this fragile area takes priority over DRYness.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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

Merge request reports