test: customer's billing profile id filter to cover the unpinned billing profiles#4506
test: customer's billing profile id filter to cover the unpinned billing profiles#4506borosr wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a new test ( ChangesBilling Profile Filter Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
openmeter/customer/adapter/customer_test.go (1)
392-419: ⚡ Quick winAdd one soft-deleted override case to lock in “live override only” behavior.
Nice coverage overall. One edge case still missing: a customer with a soft-deleted override row should behave like unpinned (fallback to namespace default). Adding that subtest would protect the
deleted_at IS NULLcontract from regressions.As per coding guidelines,
**/*_test.gochanges should be comprehensive and cover the changed behavior surface.🤖 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/customer/adapter/customer_test.go` around lines 392 - 419, Add a subtest that creates a customer with a billing profile override row that is soft-deleted (deleted_at set) and assert that when calling env.listCustomerIDsByBillingProfileID(t, ns, defaultProfileID) that customer is treated as unpinned (i.e., included with nullOverride/noOverride results) and when calling with otherProfileID it is excluded; reference the existing test helpers/vars (env.listCustomerIDsByBillingProfileID, ns, defaultProfileID, otherProfileID, and the customer ID vars like pinnedToDefault/pinnedToOther/nullOverride/noOverride) so the new case mirrors the "soft-deleted override behaves like unpinned" contract.Source: Coding guidelines
🤖 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/customer/adapter/customer_test.go`:
- Around line 392-419: Add a subtest that creates a customer with a billing
profile override row that is soft-deleted (deleted_at set) and assert that when
calling env.listCustomerIDsByBillingProfileID(t, ns, defaultProfileID) that
customer is treated as unpinned (i.e., included with nullOverride/noOverride
results) and when calling with otherProfileID it is excluded; reference the
existing test helpers/vars (env.listCustomerIDsByBillingProfileID, ns,
defaultProfileID, otherProfileID, and the customer ID vars like
pinnedToDefault/pinnedToOther/nullOverride/noOverride) so the new case mirrors
the "soft-deleted override behaves like unpinned" contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb7d25fd-b1eb-442f-8328-8723e84cc7e9
📒 Files selected for processing (1)
openmeter/customer/adapter/customer_test.go
3870828 to
db0b09e
Compare
Summary by CodeRabbit