Make SeatTypeCalculator handle root_ancestor internally instead of requiring callers to pass it
Summary
The SeatTypeCalculator class should handle calling root_ancestor internally rather than requiring all callers to pass the root namespace explicitly. This would simplify the API and reduce the cognitive load on developers using the calculator.
Problem
In the discussion !198594 (merged) we realized that we want to ensure that we always pass the root ancestor of a namespace to the seat type calculator or ensure that the seat type calculator is always considering the root ancestor of the namespace. This is needed because subscriptions are always applied to the root ancestor and not to subgroups. The code that is currently not considering the root ancestor of a namespace in the SeatTypeCalculator is the following:
def subscription_tier(namespace)
return FREE_TIER if namespace.free_plan?
namespace.exclude_guests? ? ULTIMATE_TIER : PREMIUM_TIER
end
Proposal
Update the SeatTypeCalculator to call namespace.root_ancestor internally, so callers can simply pass any namespace in the hierarchy:
# Before
seat_types = GitlabSubscriptions::SeatTypeCalculator.bulk_execute(users, source.root_ancestor)
# After
seat_types = GitlabSubscriptions::SeatTypeCalculator.bulk_execute(users, source)
Benefits
-
Simpler API: Callers don't need to remember to call
root_ancestor -
Less error-prone: Eliminates the possibility of forgetting to call
root_ancestor - Clearer intent: The calculator handles the hierarchy traversal internally
- Better encapsulation: Implementation details are hidden from callers
Implementation Notes
- Update the
SeatTypeCalculatormethods to callnamespace.root_ancestorat the beginning. - Update all existing call sites to remove the explicit
root_ancestorcalls. - Ensure test coverage validates that the calculator works correctly when passed the sub-namespace.
Related
- Merge request: !198594 (merged)
- Epic: &16982 (Seat Assignment Management)