revert: remove invoice discounts #2586
Conversation
📝 WalkthroughWalkthroughThis pull request removes discount-related functionality from the billing system. It eliminates discount checks, related methods, fields, types, and even complete files that managed invoice discounts across adapter, entity, query, and mutation layers. Changes include updating discount types from enums to strings, removing discount edges and predicates, and introducing a new field for line identifiers. Additionally, corresponding tests and SQL migration scripts have been updated to reflect these schema and business logic modifications. 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/20250403195308_deprecate-invoice-discounts.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/20250403195308_deprecate-invoice-discounts.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
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:
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
added
release-note/ignore
turip
force-pushed
the
feat/remove-invoice-discounts
branch
from
3adc094 to
3c7fa86
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: 1
🔭 Outside diff range comments (5)
test/billing/invoice_test.go (5)
2150-2170: 🛠️ Refactor suggestionRemove Discount Assertions from UBP Progressive Invoicing Tests
In this section the test currently expects discount details on detailed line items (e.g. a discount keyed by
billing.LineMaximumSpendReferenceIDwith an amount of 1000). Since the PR intends to remove invoice‐level discount handling, all assertions related to discounts should be removed.
• Remove the discount expectation(s) from thelineExpectationsmap.
• Update any related expected totals so that discount amounts are no longer factored in (likely defaulting to 0).
3090-3098: 🛠️ Refactor suggestionAdjust Expected Totals in Non‑Progressive Invoicing Tests for Discount Removal
The expected totals for the invoice are still asserting a non‑zero value for
DiscountsTotal(e.g. 1000). With the removal of invoice discounts, these expectations need to be updated (likely settingDiscountsTotalto 0) to reflect the new business logic.
3193-3226: 🛠️ Refactor suggestionUpdate Detailed Line Assertions in Helper Function
The helper function
requireDetailedLinesis verifying discount amounts by grouping child lines using discount reference IDs. Since invoice discounts are now removed, this helper should be updated to either skip discount validations or assert that the discounts map is empty.
3228-3246: 🛠️ Refactor suggestionReassess the Expected Totals Structure
The
expectedTotalsstruct still contains a field forDiscountsTotal, and several tests are asserting non‑zero discount totals. In light of the discount removal, please confirm whether these fields should be removed altogether or simply expected to be 0. The test assertions (in multiple scenarios) must be updated accordingly.
1-3705: 💡 Verification agent❓ Verification inconclusive
Overall Consistency with PR Objectives
This pull request’s objective is to remove all invoice‑level discount handling. There remain several references within the tests (both in detailed line expectations and in overall invoice totals) that verify discount values. Please ensure that all tests are updated to remove discount logic so that they reflect the simplified invoice structure. In particular, any constant or key identifiers such as
billing.LineMaximumSpendReferenceIDused solely for discount purposes should also be removed (or at least set to a default “no discount” value) in the test expectations.
Action Required: Remove Discount Handling from Test Assertions
Please update the tests in test/billing/invoice_test.go to align with the PR’s objective of eliminating invoice‑level discount handling. In particular:
- Remove all assertions and expectations that verify discount amounts or rely on discount keys (e.g. references to
billing.LineMaximumSpendReferenceID).- Adjust detailed line expectations and overall invoice totals so that they no longer consider any discount values or calculations.
- Ensure that any default behavior for discount-related fields (if still present) is set to reflect “no discount” (e.g. a zero or nil value) in the test assertions.
Once these updates are made, the tests will consistently reflect the simplified invoice structure without discount handling.
🧹 Nitpick comments (6)
tools/migrate/migrations/20250403195308_deprecate-invoice-discounts.up.sql (2)
1-2: Enhance Idempotence for Dropping the Foreign Key Constraint on "billing_invoice_discounts"The migration correctly removes the foreign key constraint from the "billing_invoice_discounts" table as intended. To further improve robustness—particularly if this migration might be run more than once or if the constraint is unexpectedly absent—you might consider using the
IF EXISTSclause. For example:-ALTER TABLE "billing_invoice_discounts" DROP CONSTRAINT "billing_invoice_discounts_billing_invoices_invoice_discounts"; +ALTER TABLE "billing_invoice_discounts" DROP CONSTRAINT IF EXISTS "billing_invoice_discounts_billing_invoices_invoice_discounts";
3-4: Enhance Idempotence for Dropping the Foreign Key Constraint on "billing_invoice_lines"Similarly, the script drops the foreign key constraint from the "billing_invoice_lines" table. Adding the
IF EXISTSclause would ensure this command does not error out if the constraint is already absent:-ALTER TABLE "billing_invoice_lines" DROP CONSTRAINT "billing_invoice_lines_billing_invoice_discounts_lines"; +ALTER TABLE "billing_invoice_lines" DROP CONSTRAINT IF EXISTS "billing_invoice_lines_billing_invoice_discounts_lines";test/billing/invoice_test.go (1)
1-10: Organize and Modularize Large Test SuitesThis test file is very long and covers many different invoicing scenarios. For improved maintainability and readability it would be beneficial to split these tests into logically grouped files or packages (for example, grouping UBP progressive, UBP non-progressive, and gathering invoice tests separately).
openmeter/ent/db/mutation.go (1)
14301-14301: Slice capacity set to 31.
Please ensure this capacity matches the maximum possible fields.openmeter/ent/db/billinginvoiceline_update.go (2)
455-459: Consider clarifying or documenting how multiple IDs are stored.
If “line_ids” must hold multiple IDs, you may want to either store them in an array-type field (e.g.,[]string) or clarify via documentation that they are comma-separated (or similarly delimited). This helps avoid ambiguity and usage errors.
1551-1555: Same consideration for single-entity updates.
If multiple IDs are indeed stored in one field, consider documenting or converting to a more explicit structure. Otherwise, this setter is correct.
📜 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 (31)
openmeter/billing/adapter/invoice.go(0 hunks)openmeter/billing/adapter/invoicediscounts.go(0 hunks)openmeter/billing/invoice.go(1 hunks)openmeter/billing/invoicediscount.go(0 hunks)openmeter/ent/db/billinginvoice.go(1 hunks)openmeter/ent/db/billinginvoice/billinginvoice.go(0 hunks)openmeter/ent/db/billinginvoice/where.go(0 hunks)openmeter/ent/db/billinginvoice_create.go(0 hunks)openmeter/ent/db/billinginvoice_query.go(1 hunks)openmeter/ent/db/billinginvoice_update.go(0 hunks)openmeter/ent/db/billinginvoicediscount.go(3 hunks)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go(0 hunks)openmeter/ent/db/billinginvoicediscount/where.go(2 hunks)openmeter/ent/db/billinginvoicediscount_create.go(5 hunks)openmeter/ent/db/billinginvoicediscount_query.go(3 hunks)openmeter/ent/db/billinginvoicediscount_update.go(4 hunks)openmeter/ent/db/billinginvoiceline.go(5 hunks)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go(3 hunks)openmeter/ent/db/billinginvoiceline/where.go(2 hunks)openmeter/ent/db/billinginvoiceline_create.go(5 hunks)openmeter/ent/db/billinginvoiceline_query.go(2 hunks)openmeter/ent/db/billinginvoiceline_update.go(4 hunks)openmeter/ent/db/client.go(0 hunks)openmeter/ent/db/migrate/schema.go(6 hunks)openmeter/ent/db/mutation.go(22 hunks)openmeter/ent/db/setorclear.go(1 hunks)openmeter/ent/schema/billing.go(3 hunks)test/billing/invoice_test.go(1 hunks)test/billing/invoicediscount_test.go(0 hunks)tools/migrate/migrations/20250403195308_deprecate-invoice-discounts.down.sql(1 hunks)tools/migrate/migrations/20250403195308_deprecate-invoice-discounts.up.sql(1 hunks)
💤 Files with no reviewable changes (10)
- openmeter/ent/db/billinginvoice/billinginvoice.go
- openmeter/billing/adapter/invoice.go
- openmeter/ent/db/billinginvoice_create.go
- openmeter/billing/invoicediscount.go
- test/billing/invoicediscount_test.go
- openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go
- openmeter/ent/db/billinginvoice/where.go
- openmeter/ent/db/billinginvoice_update.go
- openmeter/ent/db/client.go
- openmeter/billing/adapter/invoicediscounts.go
🧰 Additional context used
🧬 Code Definitions (8)
openmeter/billing/invoice.go (3)
api/api.gen.go (1)
InvoiceExpand(3016-3016)api/client/go/client.gen.go (1)
InvoiceExpand(2756-2756)api/client/javascript/src/client/schemas.ts (1)
InvoiceExpand(9023-9023)
openmeter/ent/db/setorclear.go (1)
openmeter/ent/db/billinginvoiceline_update.go (2)
BillingInvoiceLineUpdate(29-33)BillingInvoiceLineUpdateOne(1130-1135)
openmeter/ent/db/billinginvoiceline_update.go (3)
openmeter/ent/db/billinginvoiceline/where.go (1)
LineIds(198-200)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldLineIds(80-80)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (1)
FieldLineIds(37-37)
openmeter/ent/db/billinginvoicediscount_update.go (5)
openmeter/ent/db/billinginvoicediscount/where.go (1)
InvoiceID(99-101)openmeter/ent/db/billinginvoiceline/where.go (1)
InvoiceID(137-139)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (2)
FieldInvoiceID(31-31)FieldType(33-33)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (2)
FieldInvoiceID(48-48)FieldType(60-60)openmeter/ent/db/billinginvoice/billinginvoice.go (1)
FieldType(82-82)
openmeter/ent/db/billinginvoicediscount/where.go (7)
openmeter/ent/db/billinginvoicediscount.go (2)
BillingInvoiceDiscount(18-45)BillingInvoiceDiscount(48-65)openmeter/ent/db/predicate/predicate.go (1)
BillingInvoiceDiscount(34-34)openmeter/ent/db/billinginvoice/billinginvoice.go (1)
FieldType(82-82)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (1)
FieldType(33-33)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldType(60-60)openmeter/ent/db/billinginvoice/where.go (4)
TypeEQ(2159-2162)TypeNEQ(2165-2168)TypeIn(2171-2177)TypeNotIn(2180-2186)openmeter/ent/db/billinginvoiceline/where.go (4)
TypeEQ(1118-1121)TypeNEQ(1124-1127)TypeIn(1130-1136)TypeNotIn(1139-1145)
openmeter/ent/db/billinginvoiceline_create.go (3)
openmeter/ent/db/billinginvoiceline/where.go (1)
LineIds(198-200)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldLineIds(80-80)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (1)
FieldLineIds(37-37)
openmeter/ent/db/billinginvoiceline/where.go (5)
openmeter/ent/db/billinginvoiceline.go (2)
BillingInvoiceLine(27-101)BillingInvoiceLine(224-247)openmeter/ent/db/predicate/predicate.go (1)
BillingInvoiceLine(40-40)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldLineIds(80-80)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (1)
FieldLineIds(37-37)openmeter/ent/db/billinginvoicediscount/where.go (2)
LineIdsIsNil(629-631)LineIdsNotNil(634-636)
openmeter/ent/db/mutation.go (3)
openmeter/ent/db/billinginvoiceline/where.go (1)
LineIds(198-200)openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (1)
FieldLineIds(80-80)openmeter/ent/db/billinginvoicediscount/billinginvoicediscount.go (1)
FieldLineIds(37-37)
🔇 Additional comments (74)
openmeter/ent/db/billinginvoiceline/billinginvoiceline.go (3)
79-80: LGTM - New field added for line IDsThe addition of
FieldLineIdsconstant properly defines the string representation for the line_ids field in the database, which aligns with the PR objective of transitioning away from invoice-level discounts toward line item level handling.
214-218: LGTM - Proper validation for new fieldThe update to the
ValidColumnfunction correctly ensures that the newline_idsfield is recognized as a valid column when validating column names.
415-418: LGTM - Ordering function added for new fieldThe addition of the
ByLineIdsfunction allows for proper ordering of query results based on theline_idsfield, which is consistent with other ordering functions in the file.openmeter/ent/db/billinginvoicediscount_query.go (3)
19-30: Confirmed removal of relationship fields from BillingInvoiceDiscountQuery struct.The changes to the struct definition align with the PR objective of removing invoice-level discount handling. The
withInvoiceandwithLinesfields have been removed, which eliminates the ability to load related invoice and line entities.
245-259: Clone method properly updated to reflect removed fields.The
Clonemethod has been correctly modified to exclude the previously availablewithInvoiceandwithLinesfields, maintaining consistency with the struct definition changes.
335-361: Related entity loading logic properly removed from sqlAll method.The
sqlAllmethod has been simplified to remove all logic related to loading invoice and line entities, which is consistent with the removal of the relationship fields from the struct and the PR's objective to eliminate invoice discount functionality.openmeter/ent/db/billinginvoicediscount/where.go (3)
103-106: Addition of Type convenience function is consistent with codebase styleThe new
Typefunction follows the established pattern in the codebase for equality predicates on fields, providing a convenient shorthand for theTypeEQfunction.
524-526: Parameter type change from enum to string aligns with PR objectivesThe change from
billing.InvoiceDiscountTypetostringin these core type functions (TypeEQ, TypeNEQ, TypeIn, TypeNotIn) is consistent with the PR objective of removing invoice discount enums. The implementation is also simplified by removing the unnecessary temporary variable (vc).Also applies to: 529-531, 534-536, 539-541
543-586: Addition of string comparison predicates enhances query capabilitiesThe new functions provide a comprehensive set of string operations for the
typefield:
- Comparison operators (GT, GTE, LT, LTE)
- String pattern matching (Contains, HasPrefix, HasSuffix)
- Case-insensitive operations (EqualFold, ContainsFold)
These additions align with the transition from enum to string type and follow consistent implementation patterns seen elsewhere in the codebase.
openmeter/ent/db/billinginvoicediscount_create.go (6)
107-111: Type field changed from enum to stringThe parameter type in
SetTypehas been changed from a specific enum type to a string, which aligns with the PR objective of removing invoice discount handling. This simplification makes the code more flexible while maintaining functionality until the related tables are deleted in a future PR.
280-283: Direct field setting instead of edge creationThe implementation now directly sets the
InvoiceIDfield from the mutation instead of creating edges for invoice relationships. This is consistent with the simplification effort to remove discount-related logic from the system.
284-286: Type field now stored as stringThe discount type is now stored as a string value rather than an enum type, aligning with the parameter type change in the
SetTypemethod. This change provides greater flexibility while the system transitions away from invoice-level discounts.
438-442: Upsert method signature updated for consistencyThe
SetTypemethod signature in theBillingInvoiceDiscountUpsertstruct has been updated to use a string parameter, maintaining consistency with the earlier changes to the creation methods.
639-644: UpsertOne method signature updated for consistencyThe
SetTypemethod signature in theBillingInvoiceDiscountUpsertOnestruct has been updated to use a string parameter, ensuring consistent handling of discount types across all related operations.
1014-1019: UpsertBulk method signature updated for consistencyThe
SetTypemethod signature in theBillingInvoiceDiscountUpsertBulkstruct has been updated to use a string parameter, completing the consistent approach to discount type handling across all create and update operations.openmeter/ent/db/billinginvoiceline/where.go (2)
197-200: Addition of LineIds functions aligns with discount handling transition.The new
LineIdsfunction correctly implements an equality predicate for the "line_ids" field as part of the broader transition from invoice-level discounts to line-item level discounts.
1696-1769:Details
❓ Verification inconclusive
Complete predicate set for line_ids follows consistent patterns.
The implementation adds a comprehensive set of predicate functions for the "line_ids" field, following the same patterns as other fields in the codebase. These functions provide filtering capabilities essential for queries involving line identifiers.
Note that based on the relevant code snippets, the
line_idsfield is marked as deprecated with a comment suggesting to useline_discountsinstead. Verify if these new predicates are being added as part of a transition strategy.
🏁 Script executed:
#!/bin/bash # Check for usages of the LineIds predicates and line_ids field rg -A 3 "LineIds[A-Z]" --type goLength of output: 9912
Based on the new context, the predicate functions for the
line_idsfield are implemented in multiple locations (for both BillingInvoiceLine and BillingInvoiceDiscount) and follow the established pattern. However, the review comment noted that theline_idsfield is marked as deprecated (with a recommendation to useline_discountsinstead) and requested a check on whether these new predicates are added as part of a transition strategy.From the search results, we see that the predicates are being defined and used throughout the codebase exactly as for non-deprecated fields. There is no additional handling (or commentary) in these predicate functions to indicate they belong to a transition phase. That is, while the functions are consistent and comprehensive, they mirror the already existing pattern without any special treatment for the deprecation note.
Action Items:
- Confirm that the deprecation of the
line_idsfield is formally documented elsewhere (or in comments) and that the new predicates are not introducing any breaking changes.- If the deprecation strategy requires alternate predicates for
line_discounts, consider adding clarifying comments or transitional logic to guide developers.Rewritten Review Comment:
Attention: Verify deprecation transition for
line_idspredicatesThe new predicate functions for the
line_idsfield in both BillingInvoiceLine and BillingInvoiceDiscount follow the expected consistent pattern. However, please verify that these implementations are part of the intended deprecation transition—especially since internal documentation indicates thatline_idsis deprecated in favor ofline_discounts. It might be worth adding a note or additional handling if these predicates should behave differently during the transition.
- Confirm if alternate predicates (or transition logic) for
line_discountsare required.- Ensure that code comments/documents clearly indicate the deprecation strategy for
line_ids.tools/migrate/migrations/20250403195308_deprecate-invoice-discounts.down.sql (2)
4-6: Foreign Key Constraint on "billing_invoice_discounts" Table
The statement here properly adds the foreign key constraint linking the "invoice_id" column in "billing_invoice_discounts" to the "id" column in "billing_invoices". The clause "ON UPDATE NO ACTION" combined with "ON DELETE CASCADE" ensures that while updates to invoices don’t propagate, deletions will appropriately remove associated discount records.
Please confirm that the cascade behavior aligns with your intended data integrity policies for invoice deletions.
1-3:Details
❓ Verification inconclusive
Foreign Key Constraint on "billing_invoice_lines" Table
The SQL statement correctly adds the foreign key constraint linking the "line_ids" column in "billing_invoice_lines" to the "id" column in "billing_invoice_discounts" with "ON UPDATE NO ACTION" and "ON DELETE NO ACTION".
However, the column name "line_ids" suggests that it might hold multiple values. Please verify that this column is intended to reference a single discount record. If it is meant to handle multiple identifiers, consider revisiting the schema design (e.g., using a join table) to properly normalize this relationship.
Action Required: Confirm the Intended Usage of "line_ids" in the Constraint
The SQL statement in
tools/migrate/migrations/20250403195308_deprecate-invoice-discounts.down.sql
correctly re-creates the foreign key constraint linking the"line_ids"column in"billing_invoice_lines"to the"id"column in"billing_invoice_discounts"withON UPDATE NO ACTIONandON DELETE NO ACTION. However, the column name"line_ids"implies that it might be designed to hold multiple values. Please verify that this column is indeed meant to reference a single discount record. If it is intended to associate with multiple identifiers, consider refactoring the schema (for example, implementing a join table) to properly normalize the relationship.openmeter/ent/db/setorclear.go (1)
898-910: Implementation follows established patternsThe new SetOrClearLineIds methods are well-implemented and follow the same pattern as other SetOrClear methods in the file. This addition supports the migration from invoice-level to line-item level discounts as stated in the PR objectives.
openmeter/ent/db/billinginvoiceline.go (4)
91-94: Appropriate deprecation notice includedThe addition of the LineIds field with a clear deprecation notice is a good approach. This field appears to be a transitional element to help with the migration from invoice-level to line-item level discounts, as mentioned in the PR objectives.
125-125: Consistent with removal of invoice discount functionalityThe reduction of loadedTypes array size from 10 to 9 aligns with the removal of invoice discount functionality mentioned in the PR objectives. This suggests that an edge (likely InvoiceDiscounts) has been removed.
456-462: Field assignment logic correctly implementedThe assignment logic for the new LineIds field is correctly implemented, consistent with the pattern used for other string pointer fields.
665-669: String method updated to include new fieldThe String method has been properly updated to include the new LineIds field, maintaining consistency in logging and debugging output.
openmeter/ent/db/billinginvoicediscount.go (4)
39-39: Type field switched from enum to string.The
Typefield has been changed frombilling.InvoiceDiscountTypetostring, which aligns with the removal of invoice discount functionality. This change simplifies the type handling but removes the type safety previously provided by the enum.
43-43: Added LineIds field to support line item level discounts.A new
LineIdsfield of type[]stringhas been added, which seems to be part of the transition from invoice-level discounts to line item level discounts as mentioned in the PR objectives.
137-137: Updated Type field assignment in assignValues method.The assignment of the
Typefield has been updated to directly assign a string value instead of converting frombilling.InvoiceDiscountType, consistent with the type change on line 39.
218-218: Updated string representation for the Type field.The string representation in the
String()method has been updated to handle theTypefield as a string rather than an enum.openmeter/ent/db/migrate/schema.go (3)
481-482: Reordered and changed type of fields in BillingInvoiceDiscountsColumns.The
invoice_idfield has been moved up and thetypefield has been changed from an enum to a string type. This aligns with the changes in theBillingInvoiceDiscountstruct and supports the removal of invoice-level discount functionality.
510-510: Updated column reference in index definition.The column reference in the index definition has been updated to reflect the repositioning of the
invoice_idfield in theBillingInvoiceDiscountsColumnsstructure.
1995-2000: Updated ForeignKeys references for BillingInvoiceLinesTable.Foreign key references in
BillingInvoiceLinesTablehave been updated, which is part of the restructuring related to the removal of invoice discount functionality. The changes maintain the integrity of relationships between tables in the database schema.openmeter/ent/db/billinginvoicediscount_update.go (8)
120-121: Use of string for the "type" field looks fine.
The change from an enum to a string is consistent with a simpler, more flexible approach to handling this field.
126-128: Nillable type setter is well-implemented.
This method guards against overwriting the existing field unless a non-nil value is provided.
239-241: Confirm necessity of "invoice_id".
Given the PR objective to remove invoice discount logic, verify if retaining “invoice_id” remains valid or should be stripped out.
243-243: String-based 'type' field assignment is consistent.
Assigning the field as type string aligns with the broader removal of the enum constraint.
366-367: Switched to string for 'type' method signature.
This matches the new data model direction and avoids dependencies on the removed enum.
372-374: Nillable type method is compatible with the recent refactor.
Maintains existing field values unless a non-nil pointer is set.
515-517: Reassess invoice_id usage if invoice discount logic is being removed.
Confirm continued usage or remove if no longer needed.
519-519: String-based 'type' field assignment in the update spec looks good.
No issues found in storing a plain string for the discount type.openmeter/ent/db/mutation.go (20)
10821-10821: Initialize slice capacity.
This seems consistent with the code removing discount edges. No issues found here.
10895-10895: Consistent slice allocation.
This capacity looks fine for the current needs.
10927-10927: Edge slice capacity.
Initializing with capacity 8 is acceptable. No concerns.
11050-11051: Transition to string fields for invoice references.
Switching from typed references (e.g., an enum) to strings may reduce type safety. If this aligns with the new discount-removal approach, it’s all good.
11458-11463: InvoiceID setter logic.
This setter matches common ent patterns and appears correct.
11489-11498: Reset for invoice_id and setter for _type.
The changes for resetting the invoice_id field and setting the type field are in line with ent mutation style.
11509-11509: Retrieving old discount type.
This is an expected ent helper for the old field lookup.
11685-11685: Conditional check for invoice_id field.
No issues found.
11824-11824: Type assertion to string.
This aligns with the removal of enum-based discount logic.
11959-11959: Empty edges handling.
Returning an empty slice for added/removed/cleared edges is appropriate now that discount edges are dropped.Also applies to: 11971-11971, 11983-11983
13884-13931: SetLineIds, OldLineIds, ClearLineIds implementation.
This is standard ent mutation logic. Looks correct.
14392-14394: Adding LineIds field conditionally.
Appending the line IDs field when set is standard practice.
14463-14463: Retrieving the LineIds field.
Logic appears correct.
14535-14535: Retrieving old LineIds.
This matches ent’s pattern for the old field value.
14755-14761: Setting line_ids via type assertion.
Returning an error if the value isn't a string is appropriate.
14825-14827: Checking if line_ids was cleared.
Straightforward logic, no issues.
14875-14877: Clearing line_ids.
Implementation follows ent’s typical clear pattern.
14976-14985: Resetting line_ids and adjusting edge capacity.
Implementation looks consistent with the prior removal of discount edges.
15066-15066: Edges slice with capacity 9.
No concerns with the slice initialization.
15098-15098: Creating edges slice.
No issues found here.openmeter/billing/invoice.go (2)
338-340: Discount field removed from Invoice structThe
Discountsfield has been removed from theInvoicestruct as part of the broader effort to remove invoice-level discount handling. This aligns with the PR objective to transition toward implementing discounts at the rate card or line item level.
342-354: Invoice validation simplified by removing discount validationThe
Validatemethod has been appropriately simplified by removing discount-related validation logic. The method now only validates theInvoiceBaseandLinesfields, which maintains the core validation requirements while removing the now-obsolete discount validation.openmeter/ent/schema/billing.go (3)
371-380: Added deprecated field for backward compatibilityA new
line_idsfield has been added to theBillingInvoiceLineschema with a clear deprecation notice. This field acts as a transitional element while migrating away from invoice-level discounts to line-level discounts.
552-553: TODO comment added for future cleanupA TODO comment has been added to indicate that the
BillingInvoiceDiscountschema should be removed in a future update. This is good practice to mark technical debt that should be addressed after this change has been established.
570-570: Simplified discount type fieldThe discount type field has been changed from an enum to a simple string type, which simplifies the schema while maintaining backward compatibility during this transitional phase of removing invoice discounts.
openmeter/ent/db/billinginvoice_query.go (1)
626-635: Updated loadedTypes array sizeThe
loadedTypesarray size has been reduced from 9 to 8 elements, reflecting the removal of the invoice discounts related edge from the query structure. This change is consistent with the removal of invoice discount functionality across the codebase.openmeter/ent/db/billinginvoiceline_query.go (1)
664-664: Array size reduction reflects removal of invoice discount functionality.The change in the
loadedTypesarray size from what was previously likely 10 to the current 9 elements is consistent with the PR objective of removing invoice-level discount handling. This adjustment aligns with the removal of discount-related query functionality throughout the codebase.openmeter/ent/db/billinginvoice.go (1)
161-161: Array size reduction reflects removal of InvoiceDiscounts field.The change in the
loadedTypesarray size from 9 to 8 elements is consistent with the removal of theInvoiceDiscountsfield from theBillingInvoiceEdgesstruct. This change aligns with the PR objective of removing invoice-level discount functionality, as the system transitions to implementing discounts at the rate card or line item level.openmeter/ent/db/billinginvoiceline_update.go (6)
461-467: No issues with the nillable setter.
This pattern follows usual ent conventions and looks good.
469-473: Clearer usage is correct.
Clearing theline_idsfield follows ent’s pattern and is straightforward.
818-823: Field assignment logic looks consistent.
Ensuring the newline_idsfield is updated and cleared appropriately duringsqlSaveis correct and aligned with ent’s conventions.
1557-1563: Nillable logic remains standard.
No concerns; matches ent’s typical approach.
1565-1569: Clear method is appropriate.
Clearing this field aligns well with ent’s typical patterns.
1944-1949: SQL mutation handling is properly integrated.
The addition ofline_idsfield checks here is correct and appears to be in full alignment with ent’s generated approach.openmeter/ent/db/billinginvoiceline_create.go (1)
313-325: New "line_ids" field handling methods added successfullyThe implementation adds comprehensive support for a new "line_ids" field across all relevant struct types (Create, Upsert, UpsertOne, UpsertBulk). These changes align with the PR's objective of removing invoice-level discount handling in favor of rate card or line item level discounts.
The new field is properly integrated in:
- Setter methods
- Nullable setters
- Create spec field mapping
- Update operations
- Bulk operations
The implementation follows the established patterns in the codebase for field management.
Also applies to: 717-720, 1302-1318, 1821-1840, 2510-2529
2 participants
Given we are shifting towards rateCard/Line level discounts remove the internal logic (that was never exposed) for invoice level discount handling.
Table deletion will happen later.
Summary by CodeRabbit