Evaluate consolidating GroupPushRuleFinder with PushRuleFinder
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Summary
During the implementation of group push rules migration in !206152 (comment 2786060316), we made the decision to create a separate GroupPushRuleFinder class rather than extending the existing PushRuleFinder. This was done to avoid mixing responsibilities during the transition period and maintain precise control over which table gets accessed.
Now that the migration is complete, we should revisit this architectural decision and evaluate whether these finders should be consolidated.
Current State
-
PushRuleFinder: Handles organization-level rules (legacy global rules andOrganizationPushRule) -
GroupPushRuleFinder: Handles group-level rules with feature flag logic forgroup_push_rulesvspush_rulestables
Proposal
Evaluate consolidating these finders into a unified approach that could:
- Single Responsibility: Create one finder that can handle all push rule contexts (organization, group, project)
- Consistent Interface: Provide a uniform API for accessing push rules regardless of scope
- Reduced Duplication: Eliminate similar logic across multiple finder classes
- Simplified Maintenance: Single place to update push rule retrieval logic
Considerations
- The original separation was valuable during migration to avoid mixed responsibilities
- Organization rules have specific logic that should be preserved
Related Issues
- Original migration: #498936 (closed)
- MR discussion: !206152 (comment 2786060316)
Edited by 🤖 GitLab Bot 🤖