Skip to content

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 and OrganizationPushRule)
  • GroupPushRuleFinder: Handles group-level rules with feature flag logic for group_push_rules vs push_rules tables

Proposal

Evaluate consolidating these finders into a unified approach that could:

  1. Single Responsibility: Create one finder that can handle all push rule contexts (organization, group, project)
  2. Consistent Interface: Provide a uniform API for accessing push rules regardless of scope
  3. Reduced Duplication: Eliminate similar logic across multiple finder classes
  4. 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

Edited by 🤖 GitLab Bot 🤖