feat(api): add annotations to customer #3091
Conversation
📝 WalkthroughWalkthroughA new "annotations" field was introduced to the Customer entity across the API, database schema, and codebase. This field is a set of key-value pairs managed by the system, read-only for users, and supports storage, migration, and API exposure. Supporting logic, migration scripts, and OpenAPI specifications were updated accordingly. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (16)
⏰ 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)
✨ 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: 1
🧹 Nitpick comments (4)
openmeter/ent/db/runtime.go (1)
861-863: Index-based access is fragile—regenerate on every schema change
customerFields[3]is hard-wired to the current position of theannotationsfield; if the field order changes (e.g., another column is inserted before it), the application will panic at init time. This is fine for code-generated files, but make sure the Ent codegen step is rerun whenever you touchCustomer’s schema so the index stays in sync.api/openapi.yaml (1)
13503-13508: Explicitly markannotationsas nullable and enrich the exampleAdd
nullable: true(or set a default{}) so generators don’t assume the property is always present and populated. Including a small example under theexample:block will make the docs clearer.title: Annotations + nullable: true # field may legitimately be null / omitted readOnly: trueopenmeter/customer/httpdriver/apimapping.go (2)
82-95: Remove redundant length check in FromMetadata function.The function has an unnecessary duplicate check for empty metadata.
func FromMetadata(metadata models.Metadata) *api.Metadata { if len(metadata) == 0 { return nil } result := make(api.Metadata) - if len(metadata) > 0 { - for k, v := range metadata { - result[k] = v - } - } + for k, v := range metadata { + result[k] = v + } return &result }
97-110: Remove redundant length check in FromAnnotations function.The function has the same unnecessary duplicate check as FromMetadata.
func FromAnnotations(annotations models.Annotations) *api.Annotations { if len(annotations) == 0 { return nil } result := make(api.Annotations) - if len(annotations) > 0 { - for k, v := range annotations { - result[k] = v - } - } + for k, v := range annotations { + result[k] = v + } return &result }
📜 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 (21)
api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(1 hunks)api/spec/src/customer.tsp(1 hunks)openmeter/customer/adapter/customer.go(2 hunks)openmeter/customer/adapter/entitymapping.go(2 hunks)openmeter/customer/customer.go(2 hunks)openmeter/customer/httpdriver/apimapping.go(2 hunks)openmeter/ent/db/customer.go(4 hunks)openmeter/ent/db/customer/customer.go(5 hunks)openmeter/ent/db/customer/where.go(1 hunks)openmeter/ent/db/customer_create.go(9 hunks)openmeter/ent/db/customer_update.go(4 hunks)openmeter/ent/db/migrate/schema.go(1 hunks)openmeter/ent/db/mutation.go(10 hunks)openmeter/ent/db/predicate/predicate.go(1 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/db/setorclear.go(1 hunks)openmeter/ent/schema/customer.go(2 hunks)tools/migrate/migrations/20250711113121_customer_annotations.down.sql(1 hunks)tools/migrate/migrations/20250711113121_customer_annotations.up.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
openmeter/ent/db/predicate/predicate.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.
🧬 Code Graph Analysis (7)
openmeter/customer/customer.go (3)
api/client/javascript/src/client/schemas.ts (1)
Annotations(10350-10350)api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)
openmeter/customer/adapter/entitymapping.go (2)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)
openmeter/ent/db/setorclear.go (1)
openmeter/ent/db/customer_update.go (2)
CustomerUpdate(26-30)CustomerUpdateOne(875-880)
openmeter/ent/db/runtime.go (2)
openmeter/ent/db/customer/customer.go (1)
ValueScanner(152-154)openmeter/ent/db/subscriptionitem/subscriptionitem.go (1)
ValueScanner(158-164)
openmeter/ent/db/customer_update.go (1)
openmeter/ent/db/customer/customer.go (2)
ValueScanner(152-154)FieldAnnotations(53-53)
openmeter/ent/db/customer/customer.go (3)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)openmeter/ent/db/app/app.go (1)
OrderOption(140-140)
openmeter/ent/db/customer.go (3)
api/api.gen.go (1)
Annotations(1008-1008)api/client/go/client.gen.go (1)
Annotations(923-923)openmeter/ent/db/customer/customer.go (2)
FieldAnnotations(53-53)ValueScanner(152-154)
🔇 Additional comments (45)
tools/migrate/migrations/20250711113121_customer_annotations.up.sql (1)
1-2: LGTM! Safe migration implementation.The migration correctly adds a nullable JSONB column for storing annotations. Using JSONB is appropriate for key-value data and adding a nullable column is safe for existing records.
tools/migrate/migrations/20250711113121_customer_annotations.down.sql (1)
1-2: LGTM! Proper rollback migration.The down migration correctly reverses the up migration by dropping the annotations column.
openmeter/ent/db/customer/where.go (1)
1285-1293: LGTM! Generated predicate functions follow established patterns.The new
AnnotationsIsNilandAnnotationsNotNilfunctions are properly generated and consistent with other predicate functions in the file, enabling filtering customers based on annotations presence.openmeter/customer/customer.go (2)
24-24: LGTM! Field naming follows API contract.The
Annotationfield with JSON tag"annotations,omitempty"correctly aligns with the API specification. The singular field name with plural JSON tag is consistent with how this field is exposed in the API.
57-57: LGTM! Consistent annotation field in mutation struct.The
CustomerMutatestruct correctly includes the sameAnnotationfield with consistent typing and JSON serialization. No additional validation is needed since annotations are system-managed.api/client/javascript/src/client/schemas.ts (1)
3677-3681: LGTM! Well-implemented schema addition.The new
annotationsproperty is correctly implemented:
- Properly typed with
components['schemas']['Annotations']- Marked as
readonlyand optional for backward compatibility- Clear JSDoc documentation explaining its purpose and restrictions
- Consistent formatting with the existing codebase
api/spec/src/customer.tsp (1)
295-301: LGTM! Clean implementation of read-only annotations field.The annotations field is properly implemented with:
- Read-only visibility restriction preventing user modification
- Clear documentation explaining system management
- Consistent optional field pattern
- Proper TypeSpec annotations
openmeter/customer/adapter/entitymapping.go (2)
29-33: LGTM! Proper handling of optional annotations field.The logic correctly handles the optional annotations field by checking for non-empty data before creating a pointer.
51-51: Ignore field name consistency suggestion
TheCustomerdomain model in openmeter/customer/customer.go defines the field asAnnotation(singular) with a JSON tag of"annotations". The mapping in openmeter/customer/adapter/entitymapping.go correctly uses the Go struct field nameAnnotation. No change is needed.Likely an incorrect or invalid review comment.
openmeter/ent/schema/customer.go (2)
14-14: LGTM! Proper import addition for models package.
38-43: LGTM – AnnotationsValueScanner VerifiedVerified that
AnnotationsValueScanneris defined inopenmeter/ent/schema/notification.go, correctly implementing JSON marshalling and unmarshalling. No further changes needed—approving these schema updates.openmeter/ent/db/predicate/predicate.go (1)
155-164: LGTM! Consistent predicate error handling pattern.The
CustomerOrErrfunction follows the established pattern used by other predicate types in the file, providing consistent error handling for Customer predicates.api/openapi.yaml (1)
13505-13505: Component Exists:AnnotationsFound in components.schemasI’ve confirmed that the
Annotationsschema is defined undercomponents.schemasin api/openapi.yaml (around line 11505). The reference- $ref: '#/components/schemas/Annotations'is valid and the spec will compile as expected.
openmeter/ent/db/setorclear.go (1)
1920-1932: LGTM! Clean implementation following established patterns.The new
SetOrClearAnnotationsmethods for bothCustomerUpdateandCustomerUpdateOnefollow the exact same pattern as all other methods in this generated file. The implementation correctly handles the nil check and delegates to the appropriateClearAnnotations()orSetAnnotations()methods. The type signature*map[string]interface{}is consistent with other annotation methods in the codebase.api/openapi.cloud.yaml (1)
13345-13350: Markannotationsas nullable in the schemaThe OpenAPI spec describes
annotationsas an optional, system-managed field. If the backend ever omits this property or returnsnullwhen no annotations exist, strict clients and validators will reject the response unless the schema allowsnull. Please confirm your API’s behavior:
- If the response sometimes omits
annotationsor returnsnull, addnullable: true.- If the backend always returns an empty object (
{}), you can leave it non-nullable.Affected location:
api/openapi.cloud.yaml, around lines 13345–13350Suggested diff:
annotations: allOf: - $ref: '#/components/schemas/Annotations' + nullable: true description: Set of key-value pairs managed by the system. Cannot be modified by user. title: Annotations readOnly: trueopenmeter/customer/adapter/customer.go (2)
171-173: LGTM! Annotation handling follows established patterns.The implementation correctly follows the same pattern used for metadata handling, with proper nil checking and field mapping.
452-456: LGTM! Update annotation handling is properly implemented.The implementation correctly handles both setting and clearing annotations based on input presence, following the same pattern as metadata handling.
openmeter/ent/db/customer.go (4)
58-59: LGTM! Annotations field properly added to Customer struct.The field definition follows established patterns with correct type and JSON tags.
141-142: LGTM! Annotations field properly integrated into scanValues.The implementation correctly uses the ValueScanner for type conversion, following established patterns.
279-284: LGTM! Annotations field properly integrated into assignValues with error handling.The implementation correctly uses the ValueScanner for type conversion and includes proper error handling.
418-420: LGTM! Annotations field properly added to String method.The implementation correctly includes the annotations field in the string representation, following established patterns.
openmeter/ent/db/customer/customer.go (5)
10-10: LGTM! Import added for ValueScanner functionality.The field import is necessary for the ValueScanner struct and follows established patterns.
52-53: LGTM! FieldAnnotations constant properly defined.The constant follows the established naming pattern and is correctly defined.
123-123: LGTM! FieldAnnotations properly added to Columns slice.The field is correctly added to the columns list, maintaining consistency with other fields.
151-154: LGTM! ValueScanner struct properly defined for annotations.The ValueScanner is correctly defined with the appropriate type for JSON data handling.
245-248: LGTM! ByAnnotations ordering function properly implemented.The ordering function follows established patterns and is correctly implemented.
openmeter/ent/db/customer_update.go (4)
310-320: LGTM! SetAnnotations and ClearAnnotations methods properly implemented.The methods follow established patterns and correctly handle setting and clearing annotations.
1154-1164: LGTM! SetAnnotations and ClearAnnotations methods properly implemented for CustomerUpdateOne.The methods follow established patterns and are consistent with the CustomerUpdate implementation.
643-652: LGTM! Annotations handling properly implemented in sqlSave method.The implementation correctly handles both setting and clearing annotations with proper error handling and type conversion.
1517-1526: LGTM! Annotations handling properly implemented in CustomerUpdateOne sqlSave method.The implementation is consistent with the CustomerUpdate approach and correctly handles both setting and clearing annotations with proper error handling.
openmeter/customer/httpdriver/apimapping.go (1)
125-126: No changes needed:Customer.Annotationis correctly mapped toapi.Annotations
The domainCustomerstruct defines its annotations field asAnnotation(pointer tomodels.Annotations), and the call toFromAnnotations(lo.FromPtr(c.Annotation))correctly converts that internal field into the API’s pluralAnnotations. Although other domain types use a field namedAnnotations, this mapping is consistent with theCustomermodel as defined—no rename or code update is required here.openmeter/ent/db/customer_create.go (4)
247-251: Generated SetAnnotations method looks correct.The method properly sets annotations through the mutation object with the correct type
map[string]interface{}.
431-434: Improved error handling in createSpec method.The changes properly handle errors from the
createSpecmethod by updating the return signature and propagating errors in the call sites.Also applies to: 453-453
531-538: Annotations field handling with proper value scanning.The implementation correctly uses the value scanner for annotations serialization and handles potential conversion errors appropriately.
929-945: Complete upsert support for annotations.All upsert builders (CustomerUpsert, CustomerUpsertOne, CustomerUpsertBulk) include the full set of annotations methods: SetAnnotations, UpdateAnnotations, and ClearAnnotations.
Also applies to: 1302-1321, 1848-1867
openmeter/ent/db/mutation.go (10)
28766-28766: LGTM: Field declaration follows the established pattern.The
annotationsfield is properly declared as a nullable pointer tomap[string]interface{}, which is consistent with other optional fields in the struct.
29672-29719: LGTM: Comprehensive annotations field support implementation.The implementation provides complete mutation support for the
annotationsfield:
- Proper setter/getter methods with null safety
- Correct error handling in
OldAnnotations()for operation type validation- Appropriate clearing and reset functionality
- Consistent with existing field patterns
The type checking and pointer handling are implemented correctly.
30010-30010: LGTM: Field count updated correctly.The field count is properly incremented from 17 to 18 to accommodate the new
annotationsfield.
30062-30064: LGTM: Annotations field included in Fields() method.The
annotationsfield is correctly appended to the fields slice when present, following the same pattern as other optional fields.
30107-30108: LGTM: Annotations case added to Field() method.The
annotationsfield case is properly handled in theField()method, returning the result ofAnnotations()getter.
30152-30153: LGTM: Annotations case added to OldField() method.The
annotationsfield case is correctly handled in theOldField()method, callingOldAnnotations()which includes proper error handling.
30282-30288: LGTM: Type-safe field setting for annotations.The
SetField()method properly handles theannotationsfield with:
- Correct type assertion to
map[string]interface{}- Appropriate error message on type mismatch
- Consistent error handling pattern
30358-30360: LGTM: Annotations included in cleared fields check.The
ClearedFields()method correctly includes theannotationsfield when it has been cleared, maintaining consistency with other nullable fields.
30414-30416: LGTM: Annotations clearing support added.The
ClearField()method properly handles theannotationsfield case by callingClearAnnotations(), consistent with other nullable fields.
30476-30478: LGTM: Annotations reset support added.The
ResetField()method correctly handles theannotationsfield case by callingResetAnnotations(), following the established pattern.
chrisgacsal
force-pushed
the
feat/customer-annotations
branch
from
e5a611f to
2599931
Compare
2 participants
Overview
Add
annotationstoCustomer.Summary by CodeRabbit
New Features
Database
Bug Fixes
Documentation