fix: active entitlement validation blocking meter delete - #4835
Conversation
📝 WalkthroughWalkthrough
ChangesEntitlement filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openmeter/meter/adapter/adapter_test.go`:
- Around line 149-155: The entitlement deletion test fixture around
HasEntitlementForMeter currently exercises Customer.DeletedAt and an expired
ActiveTo, not Entitlement.DeletedAt. Add a separate otherwise-active entitlement
case with Entitlement.DeletedAt set to now and expect no entitlement, while
keeping the existing expired-entitlement scenario independent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e2e74c6f-a839-4263-bd14-226c525828e9
📒 Files selected for processing (2)
openmeter/meter/adapter/adapter_test.goopenmeter/meter/adapter/manage.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openmeter/meter/adapter/manage.go (1)
152-165: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd an isolated customer-deletion test.
The supplied test case at
openmeter/meter/adapter/adapter_test.go:127-214combines an ended entitlement with a deleted customer. It can pass because of the expiration filter even ifHasCustomerWithis removed. Add an active, non-ended entitlement for a deleted customer and assertfalse. Keep a separate ended-entitlement case to verify both predicates independently.Suggested test adjustment
- name: "ended entitlement for deleted customer", + name: "ended entitlement", activeFrom: now.Add(-2 * time.Hour), activeTo: lo.ToPtr(now.Add(-time.Hour)), - customerDeleted: true, want: false, }, + { + name: "active entitlement for deleted customer", + activeFrom: now.Add(-time.Hour), + customerDeleted: true, + want: false, + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openmeter/meter/adapter/manage.go` around lines 152 - 165, Add an isolated case in the entitlement query tests around the existing adapter test setup: create an active, non-ended entitlement linked to a deleted customer and assert the lookup returns false, ensuring this result depends on HasCustomerWith. Retain a separate ended-entitlement case so expiration filtering and customer deletion are verified independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openmeter/meter/adapter/manage.go`:
- Around line 152-165: Add an isolated case in the entitlement query tests
around the existing adapter test setup: create an active, non-ended entitlement
linked to a deleted customer and assert the lookup returns false, ensuring this
result depends on HasCustomerWith. Retain a separate ended-entitlement case so
expiration filtering and customer deletion are verified independently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cc61fed-eeca-4cf2-973a-aba6193d0bde
📒 Files selected for processing (2)
openmeter/meter/adapter/adapter_test.goopenmeter/meter/adapter/manage.go
🚧 Files skipped from review as they are similar to previous changes (1)
- openmeter/meter/adapter/adapter_test.go
Overview
Fix validation of active entitlements for meter delete where wrong boundary check prevented meter delete even if the entitlements referencing the meter via it feature were not considered "active" anymore.
Summary by CodeRabbit
Greptile Summary
The PR corrects meter-deletion validation so only currently active entitlements belonging to non-deleted customers block deletion.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported deleted-customer case is excluded by the new customer-edge predicate, which matches the canonical entitlement lifecycle query.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: include customer lookup" | Re-trigger Greptile