refactor: rename custom invoicing settings #2709
Conversation
This approach better describes the user journey: initially most likely only payment status manipuliation is required. Going forward if they need state storage they can enable the hooks as needed.
📝 WalkthroughWalkthroughThis change systematically renames and inverts the logic of two boolean fields across the "CustomInvoicingApp" feature in both backend and frontend codebases. The fields previously named 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 🔧 ast-grep (0.31.1)openmeter/ent/db/mutation.go🔧 SQLFluff (3.3.0)tools/migrate/migrations/20250422174622_rename-custominvoicing-settings.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/20250422174622_rename-custominvoicing-settings.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/breaking-change
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
api/openapi.cloud.yaml (2)
12474-12485: Add defaults and enrich descriptions for clarity.Right now clients must always provide the flag and it’s not clear what happens if they omit it. Suggest adding a
default: falseto both flags and updating the descriptions to something like:@@ -12474,7 +12474,8 @@ - enableDraftSyncHook: - type: boolean + enableDraftSyncHook: + type: boolean + default: false description: |- - Enable draft.sync hook. + When true, enables the `draft.sync` hook. + Defaults to `false` (invoice progresses automatically).Repeat the same change for
enableIssuingSyncHook.
12528-12539: Clarify update‐model behavior when flags are omitted.In the
replaceUpdateschema, explain that omitting these fields leaves the existing setting unchanged. For example:@@ CustomInvoicingAppReplaceUpdate - enableDraftSyncHook: - type: boolean + enableDraftSyncHook: + type: boolean description: |- When true, enables the `draft.sync` hook. When false, disables it. + Omitting this field preserves the current configuration.Apply a similar description for
enableIssuingSyncHook.api/openapi.yaml (4)
12311-12312: Confirm whether the new boolean flags should be required
The fieldsenableDraftSyncHookandenableIssuingSyncHookhave been added to therequiredarray but have no default. If these hooks are optional, consider removing them fromrequiredand definingdefault: falseunder their schemas to avoid breaking clients that omit them.Suggested change:
components: schemas: CustomInvoicingApp: required: - status - default - type - - enableDraftSyncHook - - enableIssuingSyncHook + # Optional flags – remove from required if clients may omit them +# - enableDraftSyncHook +# - enableIssuingSyncHook properties: enableDraftSyncHook: type: boolean + default: false enableIssuingSyncHook: type: boolean + default: false
12380-12391: Refine hook descriptions for clarity and consistency
The descriptions can be tightened and the hook names formatted as inline code. For example, change “Enable draft.sync hook.” to “Enable thedraft.synchook.” and streamline the second sentence.Proposed diff:
enableDraftSyncHook: type: boolean description: |- - Enable draft.sync hook. - - If the hook is not enabled, the invoice will be progressed to the next state automatically. + Enable the `draft.sync` hook. + + If this hook is disabled, the invoice automatically progresses to the next state. enableIssuingSyncHook: type: boolean description: |- - Enable issuing.sync hook. - - If the hook is not enabled, the invoice will be progressed to the next state automatically. + Enable the `issuing.sync` hook. + + If this hook is disabled, the invoice automatically progresses to the next state.
12403-12404: Verify required status of flags in the update model
Similar to the main schema,enableDraftSyncHookandenableIssuingSyncHookare marked as required in the replace‐update model without defaults. Confirm whether they should be mandatory or be treated as optional withdefault: false.Suggested change:
components: schemas: CustomInvoicingAppReplaceUpdate: required: - name - default - type - - enableDraftSyncHook - - enableIssuingSyncHook + # Optional flags – remove from required if not always provided +# - enableDraftSyncHook +# - enableIssuingSyncHook properties: enableDraftSyncHook: type: boolean + default: false enableIssuingSyncHook: type: boolean + default: false
12434-12445: Mirror description refinements in the update model
Apply the same wording and formatting improvements here: wrap hook names in backticks, add “the”, and tighten the phrasing.Proposed diff:
enableDraftSyncHook: type: boolean description: |- - Enable draft.sync hook. - - If the hook is not enabled, the invoice will be progressed to the next state automatically. + Enable the `draft.sync` hook. + + If this hook is disabled, the invoice automatically progresses to the next state. enableIssuingSyncHook: type: boolean description: |- - Enable issuing.sync hook. - - If the hook is not enabled, the invoice will be progressed to the next state automatically. + Enable the `issuing.sync` hook. + + If this hook is disabled, the invoice automatically progresses to the next state.
📜 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 (19)
api/client/javascript/src/client/schemas.ts(2 hunks)api/openapi.cloud.yaml(4 hunks)api/openapi.yaml(4 hunks)api/spec/src/app/custominvoicing.tsp(1 hunks)openmeter/app/custominvoicing/adapter/appconfig.go(2 hunks)openmeter/app/custominvoicing/app.go(1 hunks)openmeter/app/httpdriver/app.go(1 hunks)openmeter/app/httpdriver/mapper.go(1 hunks)openmeter/ent/db/appcustominvoicing.go(4 hunks)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go(4 hunks)openmeter/ent/db/appcustominvoicing/where.go(2 hunks)openmeter/ent/db/appcustominvoicing_create.go(7 hunks)openmeter/ent/db/appcustominvoicing_update.go(4 hunks)openmeter/ent/db/migrate/schema.go(1 hunks)openmeter/ent/db/mutation.go(7 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/schema/appcustominvoicing.go(1 hunks)tools/migrate/migrations/20250422174622_rename-custominvoicing-settings.down.sql(1 hunks)tools/migrate/migrations/20250422174622_rename-custominvoicing-settings.up.sql(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
openmeter/app/custominvoicing/app.go (1)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)
openmeter/app/httpdriver/app.go (1)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)
openmeter/ent/db/appcustominvoicing.go (2)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (2)
FieldEnableDraftSyncHook(26-26)FieldEnableIssuingSyncHook(28-28)
openmeter/app/httpdriver/mapper.go (2)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/app/custominvoicing/app.go (1)
Configuration(18-21)
openmeter/ent/db/runtime.go (1)
openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (2)
DefaultEnableDraftSyncHook(82-82)DefaultEnableIssuingSyncHook(84-84)
openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (1)
openmeter/ent/db/appcustominvoicingcustomer/appcustominvoicingcustomer.go (1)
OrderOption(91-91)
openmeter/app/custominvoicing/adapter/appconfig.go (3)
openmeter/app/custominvoicing/app.go (1)
Configuration(18-21)openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (2)
FieldID(16-16)FieldNamespace(18-18)
openmeter/ent/db/appcustominvoicing_update.go (2)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (2)
FieldEnableDraftSyncHook(26-26)FieldEnableIssuingSyncHook(28-28)
openmeter/ent/db/mutation.go (5)
openmeter/ent/db/ent.go (1)
Op(63-63)openmeter/ent/db/appcustominvoicing.go (2)
AppCustomInvoicing(17-37)AppCustomInvoicing(71-86)openmeter/ent/schema/appcustominvoicing.go (5)
AppCustomInvoicing(15-17)AppCustomInvoicing(19-25)AppCustomInvoicing(27-32)AppCustomInvoicing(34-38)AppCustomInvoicing(40-51)openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (2)
FieldEnableDraftSyncHook(26-26)FieldEnableIssuingSyncHook(28-28)
openmeter/ent/db/appcustominvoicing_create.go (3)
openmeter/ent/db/appcustominvoicing/where.go (2)
EnableDraftSyncHook(89-91)EnableIssuingSyncHook(94-96)openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (4)
DefaultEnableDraftSyncHook(82-82)DefaultEnableIssuingSyncHook(84-84)FieldEnableDraftSyncHook(26-26)FieldEnableIssuingSyncHook(28-28)openmeter/ent/db/ent.go (1)
ValidationError(255-258)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test
- GitHub Check: E2E
- GitHub Check: Developer environment
- GitHub Check: CI
- GitHub Check: Analyze (go)
🔇 Additional comments (58)
tools/migrate/migrations/20250422174622_rename-custominvoicing-settings.up.sql (2)
1-4: Well-structured migration for renaming and inverting draft sync hook field.This SQL code correctly renames the column from negative "skip_draft_sync_hook" to positive "enable_draft_sync_hook" and properly inverts the boolean values to maintain semantic correctness. The "atlas:nolint BC102" comment appropriately suppresses backward compatibility warnings for this intentional breaking change.
5-8: Well-structured migration for renaming and inverting issuing sync hook field.This SQL code correctly renames the column from negative "skip_issuing_sync_hook" to positive "enable_issuing_sync_hook" and properly inverts the boolean values to maintain semantic correctness. The "atlas:nolint BC102" comment appropriately suppresses backward compatibility warnings for this intentional breaking change.
openmeter/app/httpdriver/app.go (1)
196-199: Field renaming aligns with database schema changes.The field name changes from
SkipDraftSyncHookandSkipIssuingSyncHooktoEnableDraftSyncHookandEnableIssuingSyncHookalign with the database schema changes and create a more intuitive configuration API by using positive terminology.tools/migrate/migrations/20250422174622_rename-custominvoicing-settings.down.sql (2)
1-4: Down migration correctly reverts issuing sync hook changes.This SQL migration correctly reverts the column rename from "enable_issuing_sync_hook" back to "skip_issuing_sync_hook" and inverts the boolean values to maintain semantic correctness, allowing for proper rollbacks if needed.
5-7: Down migration correctly reverts draft sync hook changes.This SQL migration correctly reverts the column rename from "enable_draft_sync_hook" back to "skip_draft_sync_hook" and inverts the boolean values to maintain semantic correctness, allowing for proper rollbacks if needed.
openmeter/app/custominvoicing/app.go (1)
18-21: Renamed fields with updated JSON tags.The Configuration struct fields have been renamed from negative terminology (Skip*) to positive terminology (Enable*) with corresponding JSON tag updates. This change improves clarity and aligns with the database schema changes.
openmeter/app/httpdriver/mapper.go (1)
105-106: Field renaming aligns with semantic inversion across the codebaseThe field names have been changed from negative flags (
SkipDraftSyncHook,SkipIssuingSyncHook) to positive flags (EnableDraftSyncHook,EnableIssuingSyncHook), which better communicates their purpose. This change is part of a larger refactoring to improve semantic clarity.openmeter/ent/db/migrate/schema.go (1)
212-213: Database column renaming follows semantic inversion patternThe column names in the database schema have been updated from negative flags to positive ones (
skip_→enable_), while keeping the same boolean type and default value offalse. This change maintains the same default behavior (hooks disabled by default) while making the intent clearer.openmeter/ent/schema/appcustominvoicing.go (1)
29-30: Schema definition updated with inverted boolean semanticsThe Ent schema field definitions have been renamed from negative flags to positive flags, which better reflects their purpose while maintaining the same default behavior (hooks disabled by default). This change aligns with the PR objective to improve clarity in the user journey.
openmeter/ent/db/appcustominvoicing.go (4)
29-32: Struct field names updated to follow positive flag naming conventionThe field definitions in the
AppCustomInvoicingstruct have been changed to use positive flag naming, which improves code readability and makes the configuration more intuitive. This change is consistent with the renaming pattern applied throughout the codebase.
75-75: SQL scanning logic updated for renamed fieldsThe SQL scanning logic has been updated to reference the new field constants, ensuring database results are properly mapped to the struct fields. This is an essential part of the field renaming process.
127-138: Value assignment logic updated for renamed fieldsThe value assignment logic in
assignValueshas been properly updated to reference the new field names in error messages and assign scanned values to the correctly renamed struct fields. All references to the old naming have been consistently updated.
199-203: String representation updated to reflect renamed fieldsThe
String()method has been updated to include the renamed fields in its output, maintaining consistency in debugging and logging output. This change ensures that all aspects of the struct properly reflect the new field naming.api/spec/src/app/custominvoicing.tsp (2)
18-22: Approve improved naming for draft sync hook with inverted logic.The renamed property
enableDraftSyncHookuses positive logic which is more intuitive than the previous negativeskipDraftSyncHook. The updated documentation clearly explains that when the hook is not enabled, invoices auto-progress to the next state.
25-30: Approve improved naming for issuing sync hook with inverted logic.Similarly, renaming to
enableIssuingSyncHookfollows the same pattern of converting from negative to positive logic, making the API more intuitive to use and understand.openmeter/app/custominvoicing/adapter/appconfig.go (3)
42-43: Approve consistent field renaming in the database setter methods.The
Set*methods have been properly updated to use the new positive-logic field names, maintaining consistency with the API model changes.
47-47: Improved conflict resolution with generic UpdateNewValues.The change from updating specific fields to using
UpdateNewValues()simplifies the code and makes it more maintainable when fields are added or renamed.
67-68: Approve consistent mapping in returned Configuration struct.The mapping function correctly returns the new field names, completing the renaming throughout the adapter layer.
openmeter/ent/db/runtime.go (1)
194-201: Approve consistent descriptor and default value renaming.The schema descriptors and default value variables have been consistently renamed throughout the generated runtime code, ensuring the database layer properly reflects the new positive-logic field names.
api/client/javascript/src/client/schemas.ts (2)
3191-3194: Improved property naming in CustomInvoicingApp interfaceThe renaming from
skipDraftSyncHooktoenableDraftSyncHookmakes the configuration more intuitive by directly reflecting what the property enables rather than what it skips. The updated JSDoc comments also clearly explain the behavior when hooks are not enabled.Also applies to: 3195-3198
3225-3228: Consistent naming applied to CustomInvoicingAppReplaceUpdate interfaceThe same naming improvement has been consistently applied to the update interface, making the API more intuitive and consistent throughout. The property names now use positive language (enable vs. skip) which aligns better with user expectations.
Also applies to: 3229-3232
openmeter/ent/db/appcustominvoicing/appcustominvoicing.go (4)
25-28: Field constants renamed to use positive framingThe renaming from
skip_*toenable_*improves clarity by using affirmative language, making the configuration more intuitive for users. This better aligns with the PR objective of improving the clarity of the user journey related to invoicing settings.
58-59: Column names updated to match field renamingThe Columns slice entries have been correctly updated to match the field renaming from
skip_*toenable_*.
81-84: Default boolean variables renamed consistentlyThe default value variables have been properly renamed to match the new field names while maintaining the same behavior.
117-124: Query ordering functions updated to reflect renamed fieldsThe
By*ordering functions have been renamed to match the field renaming while maintaining the same functionality. This ensures that query ordering capabilities remain consistent with the renamed fields.openmeter/ent/db/appcustominvoicing/where.go (2)
88-96: Equality predicates renamed to match new field semanticsThe basic predicate functions have been renamed from
Skip*toEnable*to match the new field naming convention while maintaining the same functionality. This improves code readability by using positive language in the predicate names.
293-311: Specialized predicate functions updated consistentlyAll equality (EQ) and inequality (NEQ) predicate functions have been renamed to match the new field names. This change ensures consistent naming throughout the codebase and maintains the functionality of query filtering capabilities.
openmeter/ent/db/mutation.go (8)
4538-4559: Clean field renaming in AppCustomInvoicingMutation structThe renaming from "skip" to "enable" for the draft and issuing sync hook fields improves clarity by making the settings more intuitive for users. This change aligns with the overall refactoring goal of better reflecting the typical user journey where these features are opt-in rather than opt-out.
4822-4856: Consistent method renaming for the EnableDraftSyncHook fieldAll getter, setter, and utility methods have been appropriately renamed to match the new field name. The error messages have also been updated to reflect the new terminology. This maintains code consistency while improving API clarity.
4858-4892: Consistent method renaming for the EnableIssuingSyncHook fieldAll getter, setter, and utility methods have been appropriately renamed to match the new field name. The error messages have also been updated to reflect the new terminology. This maintains code consistency while improving API clarity.
5031-5039: Updated field references in Fields() methodThe field references in the Fields() method have been properly updated to use the new constant names from the appcustominvoicing package. This ensures database operations will use the correct field names.
5053-5060: Updated field references in Field() methodThe case statements and method calls in the Field() method have been correctly updated to reflect the renamed fields and methods.
5074-5081: Updated field references in OldField() methodThe case statements and method calls in the OldField() method have been correctly updated to reflect the renamed fields and associated methods.
5115-5132: Updated field references in SetField() methodThe field references and method calls in the SetField() method have been correctly updated to use the new field names and setter methods.
5199-5207: Updated field references in ResetField() methodThe field references and method calls in the ResetField() method have been correctly updated to use the new field names and reset methods.
openmeter/ent/db/appcustominvoicing_update.go (9)
58-61: Consistent rename from negative to positive semanticsThe renaming from
SetSkipDraftSyncHooktoSetEnableDraftSyncHookchanges the boolean field from negative semantics ("skip") to positive semantics ("enable"), making the code more intuitive and less prone to double-negative confusion.
64-68: Clean transition of nillable setter methodThe nillable setter method has been correctly renamed while preserving the same functionality for handling nil values.
72-74: Field rename maintains consistent patternThis change follows the same pattern as the draft sync hook, renaming from negative to positive semantic for better clarity.
78-82: Consistent renaming pattern maintainedThe nillable setter for the issuing sync hook correctly follows the same renaming pattern as the other methods.
181-182: SQL specification update aligned with field renameThe SQL specification logic correctly uses the new field name in the mutation getter.
184-185: Consistent update pattern in SQL save operationThe SQL save operation correctly uses the renamed field for the EnableIssuingSyncHook field.
278-281: Single instance update follows same renaming patternThe
AppCustomInvoicingUpdateOnestruct's methods correctly implement the same renaming pattern as the bulk update methods.
431-432: SQL specification in single update properly renamedThe SQL field specification for single entity updates correctly uses the new field name.
434-435: Consistent field handling in single update operationThe field specification for the EnableIssuingSyncHook is correctly updated in the single entity update context.
openmeter/ent/db/appcustominvoicing_create.go (14)
76-79: Create method setter properly renamedThe create methods follow the same renaming pattern as the update methods, maintaining consistency across the codebase.
82-86: Nillable setter in create context properly updatedThe nillable setter method for EnableDraftSyncHook is correctly implemented in the create context.
90-93: Consistent create method renaming for issuing hookThe create method for the issuing sync hook follows the same pattern as the draft sync hook.
195-198: Default value assignment correctly updatedThe default value assignment in the
defaultsmethod properly uses the new field name and its associated default value.
199-202: Consistent default handling for both fieldsBoth EnableDraftSyncHook and EnableIssuingSyncHook fields have consistent default value handling.
225-226: Validation checks properly renamedThe validation logic in the
checkmethod correctly validates the renamed field.
228-229: Consistent validation for both fieldsBoth renamed fields have consistent validation logic in the
checkmethod.
283-286: Create spec field assignment correctly updatedThe field assignment in the
createSpecmethod correctly uses the new field name.
287-290: Node field assignment properly updatedThe node field assignment for both fields is correctly updated to use the new field names.
406-409: Upsert type setter correctly renamedThe
AppCustomInvoicingUpserttype's setters are properly renamed to match the new field names.
412-415: Update method for excluded fields renamed consistentlyThe update method for handling excluded fields in conflict resolution is correctly renamed.
519-523: UpsertOne type setters properly renamedThe
AppCustomInvoicingUpsertOnetype's setter methods correctly use the new field names.
803-807: Bulk upsert type setters consistently renamedThe
AppCustomInvoicingUpsertBulktype's setter methods follow the same renaming pattern.
810-814: Bulk update method for excluded fields properly renamedThe bulk update method for excluded fields in conflict resolution uses the correct new field name.
| - enableDraftSyncHook | ||
| - enableIssuingSyncHook |
There was a problem hiding this comment.
Remove flags from required in the update model.
Update schemas should allow partial updates—clients shouldn’t be forced to resupply every field. Remove enableDraftSyncHook and enableIssuingSyncHook from the required list of the update schema so they become optional.
| - enableDraftSyncHook | ||
| - enableIssuingSyncHook |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Ensure new boolean flags aren’t unintentionally breaking existing clients.
By adding enableDraftSyncHook and enableIssuingSyncHook to the required list, every client will now be forced to supply these values in all requests. If you intend these hooks to be opt‑in and maintain backward compatibility, consider removing them from required and instead provide a default (for example, false) so clients that don’t care about these fields continue to work without change.
Ensure optional booleans don’t break existing clients
By adding enableDraftSyncHook and enableIssuingSyncHook to the required list, every client must now supply those values. To maintain backward compatibility, please choose one of the following:
- Remove
enableDraftSyncHookandenableIssuingSyncHookfrom therequiredarray - OR define a
default: falsefor each flag so that clients who don’t specify them continue to work unchanged
File: api/openapi.cloud.yaml (lines 12405–12406)
2 participants
Overview
This approach better describes the user journey: initially most likely only payment status manipuliation is required.
Going forward if they need state storage they can enable the hooks as needed.
Summary by CodeRabbit