feat: add discount reason #2606
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new discount reason across the system by adding an Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 🔧 SQLFluff (3.3.0)tools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
turip
force-pushed
the
feat/add-discount-reason
branch
from
2772421 to
4e076ae
Compare
turip
force-pushed
the
feat/add-discount-reason
branch
2 times, most recently
from
cc437a7 to
fe90ce9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
openmeter/ent/schema/billing.go (1)
519-521: Consider adding a default value to the enum field.The new
reasonfield is properly defined as an enum field that uses thebilling.LineDiscountReasontype. Since this field appears to be required (not marked as optional or nullable), consider adding a default value to ensure that existing records are properly handled during migration.Example:
field.Enum("reason"). GoType(billing.LineDiscountReason("")). + Default(string(billing.LineDiscountReasonRatecardDiscount))openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (1)
90-98: LGTM: Validator added for reason field with minor improvement opportunityThe
ReasonValidatorfunction properly validates that the reason field contains only allowed values.Consider using the defined constants
LineDiscountReasonMaximumSpendandLineDiscountReasonRatecardDiscountinstead of string literals in the validation switch for better maintainability, unless this is a limitation of the code generation tool.api/spec/src/billing/invoices/discounts.tsp (1)
1-124: Consider future extensibility of discount reasons.The current enum has two values (
maximumSpendandratecardDiscount). If you anticipate additional discount types in the future, ensure your implementation and database migrations are designed to accommodate extensions easily.
📜 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(4 hunks)api/openapi.cloud.yaml(5 hunks)api/openapi.yaml(5 hunks)api/spec/src/billing/invoices/discounts.tsp(2 hunks)openmeter/billing/adapter/invoicelinemapper.go(1 hunks)openmeter/billing/adapter/invoicelines.go(1 hunks)openmeter/billing/httpdriver/invoiceline.go(1 hunks)openmeter/billing/invoiceline.go(2 hunks)openmeter/billing/service/lineservice/usagebasedline.go(2 hunks)openmeter/billing/service/lineservice/usagebasedline_test.go(10 hunks)openmeter/ent/db/billinginvoicelinediscount.go(5 hunks)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go(5 hunks)openmeter/ent/db/billinginvoicelinediscount/where.go(2 hunks)openmeter/ent/db/billinginvoicelinediscount_create.go(7 hunks)openmeter/ent/db/billinginvoicelinediscount_update.go(7 hunks)openmeter/ent/db/migrate/schema.go(3 hunks)openmeter/ent/db/mutation.go(8 hunks)openmeter/ent/schema/billing.go(1 hunks)test/billing/adapter_test.go(3 hunks)tools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sql(1 hunks)tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sql(1 hunks)
🧰 Additional context used
🧬 Code Definitions (10)
openmeter/billing/httpdriver/invoiceline.go (3)
api/api.gen.go (1)
InvoiceDiscountReason(3023-3023)api/client/go/client.gen.go (1)
InvoiceDiscountReason(2763-2763)api/client/javascript/src/client/schemas.ts (1)
InvoiceDiscountReason(9040-9041)
openmeter/billing/adapter/invoicelines.go (4)
api/api.gen.go (1)
Discount(1925-1927)api/client/go/client.gen.go (1)
Discount(1790-1792)api/client/javascript/src/client/schemas.ts (1)
Discount(8961-8961)openmeter/productcatalog/discount.go (1)
Discount(53-57)
openmeter/ent/schema/billing.go (1)
openmeter/billing/invoiceline.go (2)
LineDiscountReason(729-729)LineDiscountReason(736-741)
openmeter/billing/service/lineservice/usagebasedline.go (1)
openmeter/billing/invoiceline.go (1)
LineDiscountReasonMaximumSpend(732-732)
openmeter/billing/service/lineservice/usagebasedline_test.go (1)
openmeter/billing/invoiceline.go (1)
LineDiscountReasonMaximumSpend(732-732)
test/billing/adapter_test.go (1)
openmeter/billing/invoiceline.go (2)
LineDiscountReasonMaximumSpend(732-732)LineDiscountReasonRatecardDiscount(733-733)
openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (1)
openmeter/billing/invoiceline.go (2)
LineDiscountReason(729-729)LineDiscountReason(736-741)
openmeter/ent/db/migrate/schema.go (2)
openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
Columns(165-198)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (1)
Columns(53-65)
api/client/javascript/src/client/schemas.ts (2)
api/client/go/client.gen.go (1)
InvoiceDiscountReason(2763-2763)api/api.gen.go (1)
InvoiceDiscountReason(3023-3023)
openmeter/ent/db/billinginvoicelinediscount_create.go (2)
openmeter/billing/invoiceline.go (2)
LineDiscountReason(729-729)LineDiscountReason(736-741)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (2)
ReasonValidator(91-98)FieldReason(30-30)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Quickstart
- GitHub Check: E2E
- GitHub Check: CI
- GitHub Check: Lint
- GitHub Check: Developer environment
- GitHub Check: Commit hooks
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (78)
tools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sql (1)
1-2: LGTM: The down migration correctly drops the reason column.The SQL statement properly reverses the changes made in the up migration by removing the "reason" column from the "billing_invoice_line_discounts" table.
openmeter/billing/adapter/invoicelinemapper.go (1)
218-218: LGTM: Reason field correctly added to LineDiscount struct.The addition of the discount.Reason field to the LineDiscount struct correctly implements the mapping from the database entity to the business model. This addition aligns with the PR objective of introducing a discount reason field.
tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sql (1)
1-3: LGTM: Migration properly adds reason column with appropriate defaults.The migration follows best practices by:
- Adding the "reason" column with a default value of 'maximum_spend' to ensure existing records have a valid value
- Making the column NOT NULL to enforce data integrity
- Dropping the default value to require explicit reason values for future entries
This approach ensures a smooth transition for existing data while enforcing proper validation for new records.
openmeter/billing/service/lineservice/usagebasedline.go (2)
980-980: LGTM: Maximum spend discount reason correctly added.The addition of the Reason field set to billing.LineDiscountReasonMaximumSpend properly identifies the discount type for 100% discounts that occur when maximum spend has been reached.
990-990: LGTM: Partial maximum spend discount reason correctly added.The addition of the Reason field set to billing.LineDiscountReasonMaximumSpend properly identifies the discount type for partial discounts that occur when maximum spend is partially exceeded.
openmeter/billing/adapter/invoicelines.go (1)
192-192: Good addition of the new required field.The
SetReasonmethod call ensures that the discount reason is properly set during the creation of invoice line discounts. This aligns with the PR objective to add discount reasons to the database.openmeter/ent/db/billinginvoicelinediscount.go (5)
13-13: LGTM - Appropriate import added.The new import for the
billingpackage is correctly added to support theLineDiscountReasontype.
33-34: LGTM - New field properly added to struct.The
Reasonfield is correctly defined with the appropriate typebilling.LineDiscountReasonin theBillingInvoiceLineDiscountstruct.
76-77: LGTM - Field properly added to scanValues.The
FieldReasonis correctly included in the list of string fields for scanning values from SQL rows.
132-137: LGTM - Field properly handled in assignValues.The new field is correctly handled in the
assignValuesmethod with proper type checking and conversion from SQL value to the Go type.
223-225: LGTM - Field properly included in String representation.The
Reasonfield is correctly included in the string representation of theBillingInvoiceLineDiscountstruct.openmeter/billing/httpdriver/invoiceline.go (1)
451-451:Details
❓ Verification inconclusive
Verify this change doesn't break existing API consumers.
The
Codefield has been replaced withReasonin the API representation. While this aligns with the PR objective to unify the usage of metadata fields, ensure that all API consumers are updated to use the new field name, especially if they rely on the previousCodefield.Run this script to find any references to the old field name in the codebase:
🏁 Script executed:
#!/bin/bash # Search for references to the old Code field echo "Searching for references to the old 'Code' field in API consumers..." rg "InvoiceLineDiscountAmount.*Code" --type ts rg "InvoiceLineDiscount.*Code" --type ts echo "" echo "Searching for references to the old field in API tests..." rg "Code.*InvoiceLineDiscount" --type goLength of output: 423
Action Required: Verify API Consumer Updates for Field Name Change
The code now uses the
Reasonfield instead of the previousCodefield (see openmeter/billing/httpdriver/invoiceline.go, line 451). While our automated searches for the oldCodereferences in both TypeScript consumers and Go test files didn't return any results, the absence of output warrants a manual review to confirm that all API consumers have been fully updated.
- File Affected:
openmeter/billing/httpdriver/invoiceline.go(line 451)- Change: Replaced
CodewithReason(e.g.,Reason: api.InvoiceDiscountReason(discount.Reason))Next Steps:
Please manually verify that no consumer code or API tests still rely on the oldCodefield to ensure the change won’t break existing integrations.openmeter/billing/service/lineservice/usagebasedline_test.go (7)
408-408: LGTM: Reason field properly added for maximum spend discountThe addition of the
Reasonfield set tobilling.LineDiscountReasonMaximumSpendcorrectly identifies this discount as one applied due to maximum spend limit being reached.
593-593: LGTM: Reason field correctly specified for dynamic pricing maximum spend discountAppropriate use of
LineDiscountReasonMaximumSpendfor this discount that's applied when exceeding the maximum spend threshold in the dynamic pricing test.
835-835: LGTM: Reason field properly added for package price maximum spend discountThe
Reasonfield correctly usesLineDiscountReasonMaximumSpendfor the discount applied when package pricing exceeds the maximum spend limit.
1240-1240: LGTM: Reason field added to tiered volume calculation discountThe
Reasonfield is correctly set toLineDiscountReasonMaximumSpendfor this discount applied in tiered volume calculations when the maximum spend is reached.
1490-1490: LGTM: Reason field added for graduated tiered discountConsistent use of
LineDiscountReasonMaximumSpendfor this maximum spend discount in graduated tiered pricing test, maintaining the pattern established throughout the file.
1505-1505: LGTM: Reason field added for second tier graduated discountProper addition of the
Reasonfield withLineDiscountReasonMaximumSpendfor another tier of the graduated tiered pricing discount.
1562-1562: LGTM: Consistent reason field implementation in discount overage testsThe
Reasonfield is correctly added to the discount structure in the TestAddDiscountForOverage function, using the appropriate maximum spend value.test/billing/adapter_test.go (6)
647-647: LGTM: Reason field added to first test discount with maximum spend reasonThe
Reasonfield is correctly set tobilling.LineDiscountReasonMaximumSpendfor this discount that's related to maximum spend constraints, as indicated by its reference ID.
653-653: LGTM: Reason field added to second test discount with maximum spend reasonAppropriate assignment of
LineDiscountReasonMaximumSpendto this discount that's also related to maximum spend constraints with a different reference ID.
658-658: LGTM: Reason field added to manual discount with ratecard discount reasonThe
Reasonfield correctly usesbilling.LineDiscountReasonRatecardDiscountfor this manual discount, showing proper differentiation between discount types.
687-687: LGTM: Reason field preserved in discount updateThe
Reasonfield withLineDiscountReasonMaximumSpendis properly included in the discount update, ensuring the reason is maintained through the update process.
700-700: LGTM: Reason field maintained with proper value during discount updateThe second updated discount correctly maintains
LineDiscountReasonRatecardDiscountas its reason during the update operation.
705-705: LGTM: Reason field added to new test discountNew manual discount correctly includes the
Reasonfield withLineDiscountReasonRatecardDiscount, validating that new discounts can be created with the required reason field.openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (3)
29-30: LGTM: Field constant added for reason database fieldThe
FieldReasonconstant is properly defined to represent the "reason" field in the database schema.
60-60: LGTM: Reason field added to database columns listThe
FieldReasonis correctly added to theColumnsslice, ensuring it's included in database operations.
133-136: LGTM: Order function added for reason fieldThe
ByReasonfunction properly provides a way to order query results by the new "reason" field, following the pattern used for other fields.openmeter/billing/invoiceline.go (3)
729-734: LGTM: LineDiscountReason type and constants definedThe new
LineDiscountReasontype is properly defined as a string-based enum with appropriate constants for "maximum_spend" and "ratecard_discount" values, clearly identifying the possible reasons for discounts.
736-741: LGTM: Values method implemented for LineDiscountReasonThe
Values()method properly returns a slice containing all valid values for theLineDiscountReasontype, following the pattern used for other enum types in the codebase.
754-754: LGTM: Reason field added to LineDiscount structThe
Reasonfield of typeLineDiscountReasonis properly added to theLineDiscountstruct. The field is correctly marked as required (noomitemptytag), aligning with the PR objective of replacing an optional code field with a required reason field.api/spec/src/billing/invoices/discounts.tsp (3)
7-14: Good addition of the InvoiceDiscountReason enum.The new enum provides better type safety and clarity compared to using a string field. The naming follows conventions well with camelCase for enum members and snake_case for actual values.
41-52: Good refactoring of externalIds and rateCardDiscount.Moving these fields from child models to the base model improves code organization by reducing duplication and maintaining consistency. The fields are properly documented with appropriate visibility settings.
32-33:Details
✅ Verification successful
Verify migration strategy for breaking change.
Replacing an optional
codestring field with a requiredreasonenum field is a breaking change. Ensure there's a proper migration strategy to populate this field for existing records.
🏁 Script executed:
#!/bin/bash # Check for migration files related to this change echo "Checking for migration files..." fd -e sql -e go ".*discount.*reason.*" | grep -i migrationLength of output: 287
Migration strategy verified – Migration scripts detected for discount reason update.
The existence of both the up and down migration files:
tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sqltools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sqlconfirms that a migration strategy has been implemented for the breaking change. Please ensure that these scripts correctly update existing records by converting the optional
codestring field to the requiredreasonenum field.api/openapi.cloud.yaml (5)
13816-13821: New InvoiceDiscountReason Schema Added:
The new schema definition forInvoiceDiscountReasonis correctly defined as a string enum with the valuesmaximum_spendandratecard_discount. The description “Reason code.” is concise and helpful. Ensure that any documentation or client code referencing discount reasons is updated to reflect this change.
14282-14288: Including the New “reason” Field in Required List:
The addition of thereasonproperty to the required fields list (line 14285) in the relevant component helps enforce that every discount object includes this critical information. Verify that any API client validations or downstream processing logic are updated accordingly.
14313-14337: Enhancing Discount Object with New Properties:
This hunk adds thereasonfield (using anallOfreference toInvoiceDiscountReason), as well asexternalIdsandrateCardDiscountproperties, to the discount object. The use ofallOffor schema composition is appropriate, and marking these fields as read-only ensures they aren’t unintentionally modified. Ensure that the removal of the oldcodefield (if applicable) is consistently handled across all API documentation and client implementations.
14344-14357: Updating InvoiceLineDiscountUsage Schema:
The insertion of thereasonproperty into the required fields list forInvoiceLineDiscountUsageis consistent with the changes made for discount amounts. This improvement unifies the discount metadata and strengthens data consistency. It’s recommended to verify that all consumers of this API update their payloads to include the new required field.
14382-14406: Completing Discount Usage Object Updates:
Here, the discount usage object now includes the newreasonfield—again using anallOfreference toInvoiceDiscountReason—as well as theexternalIdsandrateCardDiscountproperties. The consistent pattern between the discount amount and discount usage schemas improves clarity and maintainability. Double-check any client-side integrations to ensure they properly handle these new, read-only fields.openmeter/ent/db/billinginvoicelinediscount/where.go (5)
11-11: Import correctly added for the new LineDiscountReason type.The billing package import has been properly added to use the LineDiscountReason enum type.
375-379: Well-implemented equality predicate.The ReasonEQ function follows the established pattern in this generated file, correctly applying the equality predicate on the new "reason" field.
381-385: Well-implemented not-equal predicate.The ReasonNEQ function maintains consistency with other predicate functions in this file, properly implementing the not-equal operation.
387-394: Correctly implemented "In" predicate with proper type conversion.The ReasonIn function properly converts the typed LineDiscountReason values to a slice of any type before passing to sql.FieldIn, which is the correct pattern for handling enum types in SQL queries.
396-403: Correctly implemented "NotIn" predicate with proper type conversion.The ReasonNotIn function follows the same pattern as ReasonIn, maintaining consistency with other similar functions in the file while properly handling the enum type conversion.
api/openapi.yaml (5)
13890-13895: NewInvoiceDiscountReasonSchema Definition Added.The schema is correctly defined as a string type with the enumerated values
maximum_spendandratecard_discountand includes a clear description ("Reason code."). This aligns with the PR objective of adding a standardized discount reason.
14356-14362: Updated Required Fields to Includereason.The required list now includes the
reasonfield, ensuring that every discount amount object must specify a discount reason. Please verify that all client-side and backend validations are updated to set this field accordingly.
14390-14408: Consistent Addition of Metadata in Discount Amount Schema.The discount amount object now integrates the
reasonproperty referencingInvoiceDiscountReason, along with the newexternalIdsandrateCardDiscountproperties. All added properties are marked as read-only and use the appropriate$refpointers. This consistent approach enhances clarity and reduces duplication across the API schemas.
14418-14431: Enforcedreasonas Required in Discount Usage.The discount usage object’s required fields now include
reason, reinforcing the need to always provide a discount reason. This update promotes consistency across the API. Please ensure that any data creation flows or client logic that instantiate this object are updated to provide a valid reason.
14456-14480: Consistent Schema Enhancements for Discount Usage.The discount usage object now features the
reasonproperty (referencingInvoiceDiscountReason), along withexternalIdsandrateCardDiscountfields defined viaallOf. This mirrors the changes made in the discount amount schema, ensuring uniformity in the API. Verify that the updated documentation and client integrations reflect these enhancements.openmeter/ent/db/migrate/schema.go (3)
818-818: Added new enum field for discount reason trackingThe new
reasonfield is correctly defined as an enum type with the values "maximum_spend" and "ratecard_discount", aligning with the PR objectives to distinguish between different discount types.
833-834: Updated foreign key column reference to maintain integrityThe foreign key reference has been properly updated to reflect the new column order after adding the
reasonfield.
852-857: Updated index definitions to include new reason columnThe index definitions have been properly updated to accommodate the new column position, maintaining database integrity.
openmeter/ent/db/billinginvoicelinediscount_update.go (7)
15-15: Added import for billing packageAppropriate import for the billing package that contains the LineDiscountReason type needed for the new field.
74-86: Added setter methods for the new reason fieldThese methods follow the established pattern for field setters in the codebase:
SetReasonsets the reason directlySetNillableReasonsets the reason only if the provided pointer is not nilGood implementation that maintains consistency with the existing codebase style.
222-226: Added validation for the reason fieldThe validation properly checks that the reason value adheres to the constraints defined by the
ReasonValidatorfunction, ensuring data integrity.
254-256: Included reason field in SQL update operationsThis ensures the reason field will be properly persisted to the database when an update operation is performed.
367-379: Added reason setters to UpdateOne builderConsistent implementation of the same setter methods for the UpdateOne variant, maintaining the pattern used throughout the codebase.
528-532: Added validation to UpdateOne builderValidation logic is correctly mirrored in the UpdateOne struct's check method, ensuring data integrity is maintained for both update paths.
577-579: Included reason field in SQL update operations for UpdateOneSimilar to the multi-update variant, this ensures the reason field will be properly persisted to the database during single-entity updates.
openmeter/ent/db/mutation.go (8)
18088-18088: LGTM: New field addition aligned with PR objectives.The addition of the
reasonfield of type*billing.LineDiscountReasonto theBillingInvoiceLineDiscountMutationstruct correctly implements the discount reason functionality described in the PR objectives.
18398-18432: LGTM: Well-implemented accessor methods for the new reason field.The implementation of
SetReason,Reason,OldReason, andResetReasonmethods follows the established pattern in the codebase. TheOldReasonmethod correctly includes validation to ensure it's only called during anUpdateOneoperation and requires a valid ID, with appropriate error messages for different failure cases.
18691-18691: LGTM: Proper capacity adjustment.The capacity of the fields slice has been increased from 9 to 10 to accommodate the new reason field, ensuring efficient memory allocation.
18707-18709: LGTM: Correctly included in Fields method.The new reason field is properly added to the list of fields when it's been modified, following the established pattern for other fields.
18740-18741: LGTM: Properly included in Field method.The case for the reason field is correctly added to the Field method, allowing access to the field value by name.
18769-18770: LGTM: Properly included in OldField method.The case for the reason field is correctly added to the OldField method, allowing access to the previous field value by name.
18823-18829: LGTM: Correctly implemented in SetField method.The case for setting the reason field is properly added to the SetField method, including appropriate type checking to ensure the value is of the correct type.
18949-18951: LGTM: Properly included in ResetField method.The case for the reason field is correctly added to the ResetField method, allowing for resetting changes to this field by name.
openmeter/ent/db/billinginvoicelinediscount_create.go (7)
16-16: Correctly added billing package import.The import for the billing package is necessary to access the LineDiscountReason type used in the new methods.
83-87: Well-implemented SetReason method.The SetReason method follows the same pattern as other setter methods in this file, maintaining consistency with the codebase. It correctly sets the reason field in the mutation and returns the builder for method chaining.
230-237: Proper validation for the new required reason field.The validation logic correctly:
- Checks that the reason field is present (making it required)
- Validates the value using the ReasonValidator to ensure it's either "maximum_spend" or "ratecard_discount"
This is consistent with the PR objectives of adding a discount reason field.
296-299: Correctly implemented reason field in createSpec.The implementation properly sets the reason field in both the database operation specification and the returned node object, following the pattern used for other fields.
427-437: Well-implemented SetReason and UpdateReason methods for upsert operations.These methods properly implement:
- Setting a specific value for the reason field
- Using the value provided in the create operation
This maintains consistency with how other fields are handled in upsert operations.
608-620: Consistent implementation of SetReason and UpdateReason for single upsert.The methods follow the established pattern for field operations in the BillingInvoiceLineDiscountUpsertOne struct, ensuring consistency throughout the codebase.
969-981: Properly implemented SetReason and UpdateReason for bulk upsert operations.The implementation for bulk operations correctly follows the pattern used for other fields, ensuring a consistent API across the codebase.
api/client/javascript/src/client/schemas.ts (4)
4755-4759: Well-structured enum definition for discount reasons.The new
InvoiceDiscountReasonenum clearly defines the two possible discount reason types ('maximum_spend' and 'ratecard_discount'), which aligns with the PR objective of indicating whether a discount is applied due to reaching the maximum amount or as a result of a rate card.
5165-5171: Consistent schema changes applied to both discount types.The same changes have been applied to both discount schemas (amount-based and usage-based), ensuring consistency across the API. This unified approach to metadata fields fulfills the stated PR objective.
9040-9041: Appropriate type export.Exporting the
InvoiceDiscountReasontype makes it available for use by consumers of this API client, which is necessary for proper type checking when working with the updated discount schemas.
5116-5122:Details
❓ Verification inconclusive
Good enhancement of the invoice line discount schema.
The changes appropriately replace the optional
codefield with a requiredreasonfield, ensuring that all discounts now have a standardized reason. The additional fields (externalIdsandrateCardDiscount) provide valuable context about the discount's origin and related systems.
🏁 Script executed:
#!/bin/bash # Verify that all consumers of this API are updated to handle the required reason field # instead of the optional code field that was previously used # Check for any references to discount code in the codebase echo "Checking for references to discount code that might need updating:" rg "discount.*code" --type ts --type goLength of output: 184
API Consumer Update Verification Required for Invoice Discount Schema Changes
The invoice discount schema update looks solid. The previously optional
codefield has been replaced with a requiredreasonfield, ensuring standardized discount reasons across the application. Additionally, the inclusion ofexternalIdsandrateCardDiscountprovides valuable context regarding the discount's origin and related systems.Since our automated search (
rg "discount.*code" --type ts --type go) did not yield any results, there is low confidence in its completeness. Please manually verify that all consumers of this API have been updated to handle the requiredreasonfield, ensuring that no legacy references to the oldcodefield remain.
- File:
api/client/javascript/src/client/schemas.ts(lines 5116-5122)- Change: Replace the optional
codefield with a requiredreasonfield and add the newexternalIdsandrateCardDiscountfields.
turip
force-pushed
the
feat/add-discount-reason
branch
from
fe90ce9 to
4cf7f1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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(4 hunks)api/openapi.cloud.yaml(5 hunks)api/openapi.yaml(5 hunks)api/spec/src/billing/invoices/discounts.tsp(2 hunks)openmeter/billing/adapter/invoicelinemapper.go(1 hunks)openmeter/billing/adapter/invoicelines.go(1 hunks)openmeter/billing/httpdriver/invoiceline.go(1 hunks)openmeter/billing/invoiceline.go(2 hunks)openmeter/billing/service/lineservice/usagebasedline.go(2 hunks)openmeter/billing/service/lineservice/usagebasedline_test.go(10 hunks)openmeter/ent/db/billinginvoicelinediscount.go(5 hunks)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go(5 hunks)openmeter/ent/db/billinginvoicelinediscount/where.go(2 hunks)openmeter/ent/db/billinginvoicelinediscount_create.go(7 hunks)openmeter/ent/db/billinginvoicelinediscount_update.go(7 hunks)openmeter/ent/db/migrate/schema.go(3 hunks)openmeter/ent/db/mutation.go(8 hunks)openmeter/ent/schema/billing.go(1 hunks)test/billing/adapter_test.go(3 hunks)tools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sql(1 hunks)tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- tools/migrate/migrations/20250406215430_billing-line-discount-reason.down.sql
- openmeter/billing/adapter/invoicelines.go
- openmeter/ent/db/billinginvoicelinediscount.go
- test/billing/adapter_test.go
- openmeter/billing/httpdriver/invoiceline.go
- tools/migrate/migrations/20250406215430_billing-line-discount-reason.up.sql
- openmeter/billing/adapter/invoicelinemapper.go
- openmeter/billing/service/lineservice/usagebasedline_test.go
- openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go
- api/client/javascript/src/client/schemas.ts
- openmeter/billing/service/lineservice/usagebasedline.go
- openmeter/billing/invoiceline.go
- openmeter/ent/db/migrate/schema.go
- api/openapi.yaml
🧰 Additional context used
🧬 Code Definitions (2)
openmeter/ent/schema/billing.go (1)
openmeter/billing/invoiceline.go (2)
LineDiscountReason(729-729)LineDiscountReason(736-741)
openmeter/ent/db/billinginvoicelinediscount/where.go (4)
openmeter/billing/invoiceline.go (2)
LineDiscountReason(729-729)LineDiscountReason(736-741)openmeter/ent/db/billinginvoicelinediscount.go (2)
BillingInvoiceLineDiscount(19-47)BillingInvoiceLineDiscount(70-85)openmeter/ent/schema/billing.go (5)
BillingInvoiceLineDiscount(500-502)BillingInvoiceLineDiscount(504-510)BillingInvoiceLineDiscount(512-541)BillingInvoiceLineDiscount(543-552)BillingInvoiceLineDiscount(554-562)openmeter/ent/db/billinginvoicelinediscount/billinginvoicelinediscount.go (1)
FieldReason(30-30)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test
- GitHub Check: Quickstart
- GitHub Check: Lint
- GitHub Check: E2E
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: Commit hooks
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (33)
api/openapi.cloud.yaml (5)
13816-13821: New InvoiceDiscountReason Schema Addition
The new schema definition forInvoiceDiscountReasonis correctly added with a string type and an enum limited tomaximum_spendandratecard_discount. The description "Reason code." is concise and clear.
14282-14288: Include 'reason' in Required Properties
The required properties list for the discount component now includesreason, ensuring that a discount reason is mandatory. This update aligns well with replacing the previouscodefield in the API schema.
14313-14334: Integration of New Discount-Related Properties in InvoiceLineDiscountAmount
The changes add thereason,externalIds, andrateCardDiscountproperties into the discount amount component. By usingallOfreferences to the existing schemas (InvoiceDiscountReason,InvoiceLineAppExternalIds, andDiscount), consistency is maintained. Please verify that downstream systems using this schema are updated accordingly.
14344-14357: Addition of 'reason' to InvoiceLineDiscountUsage Required Fields
Including thereasonfield in the required list forInvoiceLineDiscountUsagesolidifies the schema's consistency across discount components. This ensures that every discount usage record includes a discount reason, as intended by the new feature.
14382-14406: Updating Discount Properties in InvoiceLineDiscountUsage
The updates within this discount object now embed thereason,externalIds, andrateCardDiscountproperties—each referencing their appropriate schemas and marked as read-only. These modifications provide a clearer, unified structure for discount-related data.api/spec/src/billing/invoices/discounts.tsp (3)
7-14: Well-designed enum for discount reasons.The newly added
InvoiceDiscountReasonenum properly defines the two supported discount reason types with clear and specific values, enhancing type safety throughout the system.
32-33: Good enhancement to type safety.Changing from an optional string
codeto a required enumreasonimproves type safety by restricting the possible values and making the field required, which enforces consistent data throughout the application.
41-52: Effective consolidation of metadata fields.Moving
externalIdsandrateCardDiscountto the base model successfully unifies these fields, reducing duplication as mentioned in the PR objectives. This change improves maintainability and consistency.openmeter/ent/db/billinginvoicelinediscount/where.go (2)
11-11: Appropriate import added.The import of the
billingpackage is necessary for using theLineDiscountReasontype in the new predicate functions.
375-403: Well-implemented predicates for the new reason field.The four new predicate functions for the "reason" field (
ReasonEQ,ReasonNEQ,ReasonIn,ReasonNotIn) follow the established pattern in this generated file. The implementation correctly handles type conversion for the variadic functions and maintains consistency with other predicates.openmeter/ent/db/mutation.go (9)
18088-18088: New field addition aligns with PR objectives.Adding the
reasonfield of type*billing.LineDiscountReasonis an appropriate implementation for tracking discount reasons, which matches the PR objective to indicate whether a discount is applied due to maximum amount or rate card.
18398-18432: LGTM! Well-implemented accessor methods for the new reason field.All necessary accessor methods for the reason field are properly implemented following the codebase's pattern:
SetReasonfor setting the fieldReasonfor retrieving the current valueOldReasonfor fetching the previous value with proper validationResetReasonfor clearing changesThe error handling in
OldReasonis thorough, with clear error messages for invalid scenarios.
18691-18691: Capacity adjustment for the new field.Increasing the capacity of the fields slice from 9 to 10 properly accounts for the new reason field, improving memory allocation efficiency.
18707-18709: LGTM! Fields method properly updated.The Fields method is correctly updated to include the reason field when it has been set, maintaining consistency in the mutation tracking logic.
18740-18741: Field method properly handles the new reason field.The Field method is appropriately updated to handle requests for the reason field value.
18769-18770: OldField method correctly updated.The OldField method now properly handles the reason field by calling OldReason when needed.
18823-18829: SetField implementation is robust.The SetField method is correctly updated with proper type checking for the reason field, ensuring type safety and consistency.
18949-18951: ResetField method properly updated.The ResetField method is correctly enhanced to handle resetting the reason field.
18088-18829:Details
❓ Verification inconclusive
Verify database schema updates and enum validation.
The mutation code appears well-implemented, but I recommend verifying:
- That corresponding database schema migrations exist to add this field to the database
- That
LineDiscountReasonenum validation is handled appropriately elsewhereThese validations would ensure complete implementation of the feature.
🏁 Script executed:
#!/bin/bash # Check for corresponding schema updates in migration files echo "Checking for database migration files that add the reason field:" fd -e go -e sql "migration|schema" | xargs grep -l "reason.*LineDiscountReason" | head -5 # Check for LineDiscountReason enum definition echo -e "\nChecking for LineDiscountReason enum definition:" fd -g "*billing*" -g "*.go" | xargs grep -l "type LineDiscountReason" | head -3 # Check for enum validation if it exists echo -e "\nChecking for validation of LineDiscountReason values:" fd -g "*billing*" -g "*.go" | xargs grep -A 10 -B 3 "LineDiscountReason.*valid" | head -15Length of output: 825
Action Required: Verify Database Migrations and Enum Validations
- Confirm that the schema migration files include the necessary update to add the new field (linked to
LineDiscountReason).- Ensure that the definition and validation logic for the
LineDiscountReasonenum are correctly implemented in the billing-related code.- Note: Our automated search encountered errors (e.g., invalid search paths), so please verify these points manually.
openmeter/ent/db/billinginvoicelinediscount_update.go (7)
15-15: Properly added the required import for LineDiscountReason.The import for the billing package is correctly added to support the new reason field.
74-86: Good implementation of the reason field setters for BillingInvoiceLineDiscountUpdate.The implementation follows the same pattern as other fields in the struct, with both direct setter and nil-safe setter methods.
222-226: Validation logic correctly implemented for the reason field.The check method properly validates the reason field when provided, using the ReasonValidator function from the billinginvoicelinediscount package.
254-256: SQL save operation correctly handles the reason field.The reason field is properly included in the SQL update specification when present in the mutation.
367-379: Setter methods for BillingInvoiceLineDiscountUpdateOne properly implemented.Both SetReason and SetNillableReason methods are correctly added to the UpdateOne struct, following the same pattern as other fields.
528-532: Validation logic properly implemented in the UpdateOne check method.The check method for BillingInvoiceLineDiscountUpdateOne correctly validates the reason field when provided.
577-579: SQL save for UpdateOne correctly handles the reason field.The reason field is properly included in the SQL update specification for the UpdateOne operation.
openmeter/ent/db/billinginvoicelinediscount_create.go (7)
16-16: Properly added the required import for LineDiscountReason.The import for the billing package is correctly added to support the new reason field.
83-87: SetReason method correctly implemented for BillingInvoiceLineDiscountCreate.The method follows the same pattern as other field setters in the struct.
230-237: Validation properly makes reason a required field with validation.The check method ensures that:
- The reason field is present (required)
- The value passes the ReasonValidator
This aligns perfectly with the PR objective of adding a meaningful discount reason to the database.
296-299: CreateSpec correctly includes the reason field.The reason field is properly added to the SQL creation specification, ensuring it will be stored in the database.
427-437: Upsert operations properly handle the reason field.Both SetReason and UpdateReason methods are correctly implemented for the BillingInvoiceLineDiscountUpsert struct.
608-620: UpsertOne operations properly implement reason field methods.The BillingInvoiceLineDiscountUpsertOne struct correctly implements SetReason and UpdateReason methods.
969-981: UpsertBulk operations properly implement reason field methods.The BillingInvoiceLineDiscountUpsertBulk struct correctly implements SetReason and UpdateReason methods, maintaining consistency across all operation types.
2 participants
Overview
This patch adds the discount reason to the database that can signify if a discount is due to the max amount or a ratecard.
Besides the patch unitifies certain metadata field usage inside the InvoiceLineDiscount API type to reduce duplicates.
Summary by CodeRabbit
New Features
Chores