feat(billing): ignore sync annotation #3073
Conversation
GAlexIHU
added
the
release-note/ignore
📝 WalkthroughWalkthroughThis change introduces support for an Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
test/billing/invoice_test.go (1)
256-259: Same literal as above – will be covered if you apply the previous refactor
🧹 Nitpick comments (3)
openmeter/ent/schema/billing.go (1)
321-327: Double-check migration and index footprint
The mixin brings in a GIN index on the new JSONB column. For largebilling_invoice_linestables this may:
- Extend migration time markedly.
- Increase write-path overhead (GIN indexes on JSONB are non-trivial).
Please verify that (a) the generated migration was committed and reviewed for lock-time, and (b) rollout plans account for the extra index build on production data (e.g., create concurrently, scheduled maintenance window).
test/billing/invoice_test.go (1)
143-146: Extract the annotation map to a reusable variable to avoid duplicationThe identical
models.Annotationsliteral is used twice in this test file (here and in theexpectedUSDLineassertion below). Defining it once and re-using keeps the test DRY and makes future edits less error-prone.- Annotations: models.Annotations{ - "string_key": "value", - "float_key": 1.0, - }, + Annotations: testAnnotations,Add near the top of the test (just below
nowor alongside the other helper vars):testAnnotations := models.Annotations{ "string_key": "value", "float_key": 1.0, }Then reuse the same variable at lines 256-259.
openmeter/ent/db/billinginvoiceline_create.go (1)
39-43: Consider type safety implications ofmap[string]interface{}.While the
map[string]interface{}type provides flexibility for annotations, consider establishing clear conventions or validation for annotation keys and value types to prevent runtime errors and ensure consistent usage across the billing system.Also applies to: 668-671, 1020-1036
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (24)
openmeter/billing/adapter/invoicelinemapper.go(1 hunks)openmeter/billing/adapter/invoicelines.go(1 hunks)openmeter/billing/annotations.go(1 hunks)openmeter/billing/invoiceline.go(7 hunks)openmeter/billing/worker/subscription/patch.go(1 hunks)openmeter/billing/worker/subscription/phaseiterator.go(1 hunks)openmeter/billing/worker/subscription/sync.go(1 hunks)openmeter/billing/worker/subscription/sync_test.go(1 hunks)openmeter/ent/db/billinginvoiceline.go(4 hunks)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go(2 hunks)openmeter/ent/db/billinginvoiceline/where.go(1 hunks)openmeter/ent/db/billinginvoiceline_create.go(8 hunks)openmeter/ent/db/billinginvoiceline_query.go(2 hunks)openmeter/ent/db/billinginvoiceline_update.go(4 hunks)openmeter/ent/db/migrate/schema.go(3 hunks)openmeter/ent/db/mutation.go(9 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/db/setorclear.go(1 hunks)openmeter/ent/schema/billing.go(1 hunks)openmeter/subscription/subscriptionspec.go(1 hunks)pkg/timeutil/openperiod_test.go(1 hunks)test/billing/invoice_test.go(2 hunks)tools/migrate/migrations/20250707075725_billingline-annotations.down.sql(1 hunks)tools/migrate/migrations/20250707075725_billingline-annotations.up.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
openmeter/billing/annotations.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/billing/adapter/invoicelinemapper.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/ent/db/billinginvoiceline.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
pkg/timeutil/openperiod_test.go (3)
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2945
File: pkg/framework/lockr/locker.go:89-126
Timestamp: 2025-06-05T11:34:14.035Z
Learning: The lockr package's transaction detection using `transaction_timestamp() != statement_timestamp()` correctly handles nested transactions and subtransactions, as proven by comprehensive test coverage in pkg/framework/lockr/locker_test.go.
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2945
File: pkg/framework/lockr/locker.go:89-126
Timestamp: 2025-06-05T11:34:14.035Z
Learning: The lockr package's transaction detection using `transaction_timestamp() != statement_timestamp()` correctly handles nested transactions and subtransactions, as proven by the test "Should be able to acquire same lock in sub-transaction" in pkg/framework/lockr/locker_test.go.
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
openmeter/billing/worker/subscription/phaseiterator.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/ent/db/billinginvoiceline/where.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2692
File: openmeter/productcatalog/plan/adapter/mapping.go:64-74
Timestamp: 2025-04-20T11:15:07.499Z
Learning: In the OpenMeter codebase, Ent's edge methods ending in "OrErr" (like AddonsOrErr()) only return NotLoadedError when the edge wasn't loaded, and cannot return DB errors. Simple err != nil checks are sufficient for these methods.
openmeter/ent/db/billinginvoiceline_query.go (2)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
openmeter/subscription/subscriptionspec.go (2)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2626
File: openmeter/subscription/addon/service/change_test.go:23-92
Timestamp: 2025-04-08T11:02:14.464Z
Learning: The ChangeQuantity method in the subscription addon service allows setting ActiveFrom to a time in the past. No validation is needed to check if ActiveFrom is in the past.
openmeter/billing/invoiceline.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/ent/db/migrate/schema.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
openmeter/ent/db/billinginvoiceline_update.go (1)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
test/billing/invoice_test.go (1)
undefined
<retrieved_learning>
Learnt from: chrisgacsal
PR: #2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In productcatalog.UsageBasedRateCard, the BillingCadence field is a non-pointer isodate.Period, while in productcatalog.FlatFeeRateCard, BillingCadence is a pointer type (*isodate.Period). This means MonthPeriod should be used directly for UsageBasedRateCard (not &MonthPeriod).
</retrieved_learning>
openmeter/ent/db/runtime.go (2)
Learnt from: chrisgacsal
PR: openmeterio/openmeter#2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
openmeter/billing/worker/subscription/sync_test.go (2)
undefined
<retrieved_learning>
Learnt from: GAlexIHU
PR: #2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like openmeter/entitlement/metered/lateevents_test.go may use variables like meterSlug and namespace without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
</retrieved_learning>
<retrieved_learning>
Learnt from: chrisgacsal
PR: #2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In productcatalog.UsageBasedRateCard, the BillingCadence field is a non-pointer isodate.Period, while in productcatalog.FlatFeeRateCard, BillingCadence is a pointer type (*isodate.Period). This means MonthPeriod should be used directly for UsageBasedRateCard (not &MonthPeriod).
</retrieved_learning>
openmeter/ent/db/mutation.go (1)
undefined
<retrieved_learning>
Learnt from: chrisgacsal
PR: #2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In productcatalog.UsageBasedRateCard, the BillingCadence field is a non-pointer isodate.Period, while in productcatalog.FlatFeeRateCard, BillingCadence is a pointer type (*isodate.Period). This means MonthPeriod should be used directly for UsageBasedRateCard (not &MonthPeriod).
</retrieved_learning>
🧬 Code Graph Analysis (14)
openmeter/ent/schema/billing.go (1)
pkg/framework/entutils/mixins.go (3)
AnnotationsMixin(157-159)AnnotationsMixin(162-170)AnnotationsMixin(172-181)
openmeter/billing/adapter/invoicelinemapper.go (3)
api/api.gen.go (2)
Metadata(4412-4412)Annotations(1008-1008)openmeter/billing/profile.go (1)
Metadata(22-22)openmeter/billing/invoice.go (1)
InvoiceID(214-214)
openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
openmeter/ent/db/subscriptionitem/subscriptionitem.go (1)
FieldAnnotations(30-30)
openmeter/billing/worker/subscription/sync.go (2)
api/api.gen.go (1)
Annotations(1008-1008)openmeter/billing/annotations.go (1)
AnnotationSubscriptionSyncIgnore(4-4)
openmeter/ent/db/setorclear.go (1)
openmeter/ent/db/billinginvoiceline_update.go (2)
BillingInvoiceLineUpdate(31-35)BillingInvoiceLineUpdateOne(1313-1318)
pkg/timeutil/openperiod_test.go (1)
pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
openmeter/billing/adapter/invoicelines.go (2)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)
openmeter/ent/db/billinginvoiceline/where.go (3)
openmeter/ent/db/subscriptionitem/where.go (2)
AnnotationsIsNil(358-360)AnnotationsNotNil(363-365)openmeter/ent/db/predicate/predicate.go (1)
BillingInvoiceLine(71-71)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldAnnotations(21-21)
openmeter/billing/worker/subscription/patch.go (1)
openmeter/billing/annotations.go (1)
AnnotationSubscriptionSyncIgnore(4-4)
openmeter/subscription/subscriptionspec.go (2)
openmeter/ent/db/subscriptionitem/where.go (2)
ActiveFrom(90-92)ActiveTo(95-97)pkg/timeutil/closedperiod.go (1)
ClosedPeriod(7-10)
openmeter/billing/invoiceline.go (5)
api/api.gen.go (4)
Annotations(1008-1008)Metadata(4412-4412)Period(5248-5254)TaxConfig(6732-6744)api/client/go/client.gen.go (4)
Annotations(923-923)Metadata(4000-4000)Period(4793-4799)TaxConfig(6170-6182)openmeter/billing/profile.go (1)
Metadata(22-22)pkg/models/metadata.go (1)
Metadata(9-9)openmeter/productcatalog/tax.go (1)
TaxConfig(36-39)
openmeter/ent/db/billinginvoiceline_update.go (1)
openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldAnnotations(21-21)
test/billing/invoice_test.go (2)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)
openmeter/ent/db/mutation.go (3)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldAnnotations(21-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Artifacts / Container image
- GitHub Check: Quickstart
- GitHub Check: Developer environment
- GitHub Check: E2E
- GitHub Check: Commit hooks
- GitHub Check: CI
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Migration Checks
- GitHub Check: Analyze (go)
🔇 Additional comments (53)
pkg/timeutil/openperiod_test.go (1)
351-362: LGTM! Excellent edge case coverage for zero-length periods.The new test cases correctly verify that zero-length periods (where
From == To) don't intersect with other periods, which is the expected behavior since they represent no actual time span. This aligns well with the billing period intersection logic mentioned in the AI summary.tools/migrate/migrations/20250707075725_billingline-annotations.down.sql (1)
1-4: LGTM! Migration rollback follows proper order and syntax.The down migration correctly reverses the up migration by dropping the index before the column, using proper PostgreSQL syntax.
openmeter/billing/adapter/invoicelines.go (1)
90-90: LGTM! Annotations field properly integrated into the create builder.The
SetAnnotations(line.Annotations)call is correctly placed alongside other field setters and follows the established pattern for metadata fields.openmeter/billing/annotations.go (1)
3-5: LGTM! Well-designed annotation constant with clear naming.The
AnnotationSubscriptionSyncIgnoreconstant follows Go conventions and uses a clear hierarchical naming scheme that makes its purpose immediately apparent.tools/migrate/migrations/20250707075725_billingline-annotations.up.sql (1)
1-4: LGTM! Proper PostgreSQL schema design for JSON annotations.The migration correctly uses JSONB (binary JSON) with a GIN index for efficient querying, and makes the column nullable for backward compatibility. This is the standard approach for flexible metadata storage in PostgreSQL.
openmeter/ent/schema/billing.go (1)
323-323: AnnotationsMixin addition LGTM
Integratingentutils.AnnotationsMixin{}cleanly exposes theannotationsJSONB column and aligns this schema with the rest of the PR. No functional or naming clashes detected with existing mixins on the entity.openmeter/ent/db/billinginvoiceline/where.go (1)
207-216: LGTM! Auto-generated predicates follow established patterns.The new
AnnotationsIsNilandAnnotationsNotNilpredicates are consistent with existing nullable field predicates likeMetadataIsNil/MetadataNotNiland align with similar predicates in other entities (e.g.,SubscriptionItem). These enable proper querying of invoice lines based on annotation presence.openmeter/billing/adapter/invoicelinemapper.go (1)
141-141: LGTM! Annotations mapping correctly implemented.The addition of
Annotations: dbLine.Annotations,follows the established pattern of the adjacentMetadatamapping and correctly transfers annotation data from the database entity to the domain model. This completes the data flow for the new annotations feature.openmeter/subscription/subscriptionspec.go (1)
788-790: LGTM! Improved boundary condition handling for active period checks.This change correctly allows the exact start time of the item cadence to be considered active, addressing edge cases where
IsActiveAtmight not include the boundary moment. The enhanced error message with the actual period range[%s, %s]will also improve debugging capabilities.openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
20-21: LGTM! Consistent addition of annotations field support.The generated code correctly adds the
FieldAnnotationsconstant and includes it in theColumnsslice, following the established patterns in the codebase. This aligns with the broader changes to support annotations on billing invoice lines throughout the system.Also applies to: 188-188
openmeter/ent/db/billinginvoiceline_query.go (1)
663-663: Generated code updates look correct.The comment examples have been updated to reflect the new
Annotationsfield, which is consistent with the schema changes introducing annotations support.Also applies to: 668-668, 686-686, 690-690
openmeter/ent/db/setorclear.go (1)
982-994: SetOrClearAnnotations implementation is correct and consistentLogic matches the existing SetOrClear* helpers: a nil pointer clears, otherwise sets. Signature and behaviour align with other entities in this file, so no concerns here.
openmeter/ent/db/billinginvoiceline.go (4)
32-33: LGTM: Annotations field correctly added to entity.The
Annotationsfield is properly typed asmap[string]interface{}which aligns with the JSONB storage requirements for flexible annotation data.
261-261: LGTM: Proper field handling in scanValues method.The
FieldAnnotationsis correctly grouped with other JSON byte fields for database scanning.
296-303: LGTM: Robust JSON unmarshaling for annotations.The implementation correctly handles:
- Type checking for byte array
- Null/empty value validation
- JSON unmarshaling with proper error wrapping
622-624: LGTM: Annotations included in string representation.The String method correctly includes the new annotations field in the entity's string representation.
openmeter/billing/worker/subscription/phaseiterator.go (1)
352-360: LGTM: Proper handling of zero-length period intersections.The refined logic correctly addresses the edge case where zero-length periods (instantaneous events) are treated as non-intersecting by the standard intersection method. The special handling ensures that instantaneous periods are properly processed in the billing system.
This change aligns with the improved period boundary handling mentioned in the broader annotation feature implementation.
openmeter/ent/db/runtime.go (4)
508-513: Check the newly-introducedmixinFields2slice is really neededThe 3rd mixin (
billinginvoicelineMixin[2]) is now captured inbillinginvoicelineMixinFields2.
If the only purpose is thecurrencyfield (line 529) this is fine, but double-check the schema:
– If the currency lives in the entity fields (not the mixin) or in a different mixin index, this slice will silently return the wrong descriptor at runtime.
– Conversely, if additional fields exist in that mixin, consider adding explicit assignments later in the file to avoid future foot-guns.
515-527: Verify the field offsets after insertingAnnotationsMixinNamespace/CreatedAt/UpdatedAt are now fetched from
mixinFields1at indices 1, 3, 4.
Across the codebase the pattern is usually:index 0 -> ID index 1 -> Namespace index 3 -> CreatedAt index 4 -> UpdatedAt…but some entities (e.g.
BalanceSnapshot) keepNamespaceat index 0.
A bad offset will not panic but will wire the wrong validator/default function, leading to silent data corruption.Please compile the project (or run
go test ./...) after regenerating withent generateto confirm the offsets are correct.
529-532: Currency descriptor now pulled from a different mixin
billinginvoicelineDescCurrency := billinginvoicelineMixinFields2[0]Make sure the currency really moved to the third mixin; otherwise you’ll end up validating the wrong field (or nil-pointer panics if the slice is empty).
If this move was only to accommodate the new
AnnotationsMixin, consider renaming the mixins (or adding comments in the schema) to keep the ordering obvious.
536-538: ID descriptor offset changed – confirm against schemaID is now at
mixinFields1[0], which matches the usual pattern, but note that Namespace was taken frommixinFields1[1]above.
This asymmetry is easy to miss when the schema evolves again; a quick compile/run will catch any mismatch early.openmeter/ent/db/billinginvoiceline_update.go (4)
43-53: Generated code correctly implements annotations field for BillingInvoiceLineUpdate.The
SetAnnotationsandClearAnnotationsmethods follow the established pattern used by other JSON fields like metadata, with appropriate typemap[string]interface{}for the JSONB field.
827-832: SQL save logic correctly handles annotations field updates.The implementation properly checks for both setting and clearing the annotations field, using the appropriate
field.TypeJSONtype, consistent with the metadata field handling pattern.
1320-1330: Generated code correctly implements annotations field for BillingInvoiceLineUpdateOne.The methods mirror the implementation in
BillingInvoiceLineUpdate, maintaining consistency across both update builders.
2134-2139: SQL save logic correctly handles annotations field in UpdateOne.The implementation is consistent with the bulk update builder and follows the established pattern for JSON field handling.
openmeter/ent/db/migrate/schema.go (4)
720-720: LGTM: Annotations column added correctly.The new
annotationsJSONB column is properly defined as nullable and positioned as the second column in the table, which aligns with the PR objective to add annotation support for billing invoice lines.
765-807: LGTM: Foreign key column references updated systematically.All foreign key column references have been correctly incremented by 1 to account for the new annotations column. The updates are consistent across all foreign key definitions.
813-822: LGTM: GIN index for annotations is appropriate.The new
billinginvoiceline_annotationsGIN index is correctly configured for JSONB queries, which will provide efficient querying of the annotations field. This is the standard approach for indexing JSONB columns in PostgreSQL.
831-859: LGTM: Index column references updated consistently.All existing index column references have been systematically updated to reflect the new column positioning. The updates are consistent and maintain the correct column mappings.
openmeter/billing/invoiceline.go (7)
135-135: LGTM! Annotations field addition is well-structured.The addition of the
Annotationsfield to theLineBasestruct follows the same pattern as theMetadatafield and is correctly typed asmodels.Annotations. This aligns with the PR objective to support annotation-based functionality for billing sync control.
217-222: LGTM! Clone logic for Annotations is correctly implemented.The cloning logic for the
Annotationsfield properly creates a new map and copies all key-value pairs, ensuring deep cloning of the map structure. This follows the same pattern used for theMetadatafield and prevents sharing of mutable state between cloned instances.
602-602: LGTM! Annotations field added to input struct.The addition of the
Annotationsfield toNewFlatFeeLineInputmaintains consistency with theLineBasestruct definition and supports the new annotation functionality.
629-629: LGTM! Annotations properly assigned in constructor.The
NewFlatFeeLinefunction correctly assigns the input annotations to theLineBasestruct, maintaining consistency with how other fields are handled.
686-686: LGTM! Annotations properly assigned in usage-based constructor.The
NewUsageBasedFlatFeeLinefunction correctly assigns the input annotations to theLineBasestruct, maintaining consistency with the flat fee constructor.
1116-1121: LGTM! Update input structure properly supports optional Annotations.The
UpdateInvoiceLineBaseInputstruct correctly usesmo.Option[models.Annotations]to make theAnnotationsfield optional during updates, following the same pattern as other updateable fields likeMetadata.
1169-1171: LGTM! Annotations update logic is correctly implemented.The update application logic properly checks if the
Annotationsfield is present in the update input and applies it to the line. This follows the same pattern as other optional fields and ensures annotations can be updated when needed.openmeter/billing/worker/subscription/sync_test.go (2)
3380-3544: Well-structured test for sync ignore functionalityThe test comprehensively validates that lines marked with
billing.AnnotationSubscriptionSyncIgnoreare preserved during subscription synchronization, covering both draft and gathering invoices. The test properly verifies that new versions of lines are created while marked lines remain untouched.
3546-3645: Good edge case coverage for period changesThe test effectively validates that the sync ignore annotation preserves line periods even during subscription cancellation. The comparison between marked and unmarked lines clearly demonstrates the expected behavior.
openmeter/ent/db/billinginvoiceline_create.go (6)
39-43: Well-implemented annotations setter method.The
SetAnnotationsmethod follows the established pattern and correctly handles themap[string]interface{}type for flexible annotation storage.
668-671: Correct database field specification for annotations.The createSpec implementation properly sets the annotations field as
field.TypeJSON, which is appropriate for storing themap[string]interface{}data structure in the database.
975-975: Updated documentation examples include annotations usage.The example comments have been appropriately updated to demonstrate annotations usage in OnConflict scenarios, improving developer documentation.
Also applies to: 984-984
1020-1036: Complete upsert operations for annotations field.All three essential upsert operations are properly implemented:
SetAnnotations: Sets new valuesUpdateAnnotations: Uses values from create operationClearAnnotations: Nullifies the fieldThe implementation follows the established pattern used by other optional JSON fields.
1530-1549: Consistent UpsertOne annotations methods.The
BillingInvoiceLineUpsertOnestruct correctly implements all annotations operations following the same delegation pattern used by other fields.
2285-2304: Proper bulk upsert support for annotations.The bulk upsert operations for annotations are correctly implemented, maintaining consistency with the single-entity operations while supporting batch scenarios.
openmeter/ent/db/mutation.go (9)
16215-16215: LGTM: Annotations field addition follows established patterns.The new
annotations *map[string]interface{}field is properly typed as a pointer to support nil values for optional fields, consistent with other optional fields in the mutation struct.
16378-16425: LGTM: Complete lifecycle methods implementation for annotations field.The implementation includes all necessary methods for field lifecycle management:
SetAnnotations,Annotations,OldAnnotationsfor value operationsClearAnnotations,AnnotationsClearedfor clearing operationsResetAnnotationsfor reset operationsThe error handling in
OldAnnotationsis consistent with other similar methods, and the implementation follows the established patterns exactly.
18246-18249: LGTM: Fields count and annotation field inclusion are correct.The field count is correctly updated to 34 and the annotations field is properly included in the fields list when it's set.
18357-18358: LGTM: Field method correctly handles annotations field.The Field method properly delegates to the Annotations() method for field retrieval.
18434-18435: LGTM: OldField method correctly handles annotations field.The OldField method properly delegates to the OldAnnotations() method for retrieving old field values.
18511-18517: LGTM: SetField method includes proper type checking.The SetField method correctly validates the type as
map[string]interface{}and provides appropriate error messages for type mismatches.
18779-18781: LGTM: ClearedFields method correctly includes annotations field.The annotations field is properly included in the cleared fields list when it has been cleared.
18838-18840: LGTM: ClearField method correctly handles annotations field.The ClearField method properly delegates to the ClearAnnotations() method for clearing the field.
18891-18893: LGTM: ResetField method correctly handles annotations field.The ResetField method properly delegates to the ResetAnnotations() method for resetting field changes.
GAlexIHU
force-pushed
the
galexi/billing-ignore-line-sync
branch
from
b449140 to
8389d38
Compare
2 participants
Overview
Adds ignore annotation to billing (will be used for period fix migration)
Fixes OM-1239
Notes for reviewer
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Database