feat: add balance_id to create/update balance endpoints + delete balance endpoint#862
Open
joejohnson123[bot] wants to merge 1 commit intodevfrom
Open
feat: add balance_id to create/update balance endpoints + delete balance endpoint#862joejohnson123[bot] wants to merge 1 commit intodevfrom
joejohnson123[bot] wants to merge 1 commit intodevfrom
Conversation
…nce endpoint - Add external_id column to cusEntTable - Accept balance_id param in create balance (stored as external_id) - Accept balance_id param in update balance (filters by external_id) - Add externalId to CustomerEntitlementFilters - New POST /balances/delete endpoint (marks as expired rather than deleting) - API balance breakdown.id returns external_id when set Closes ENG-1092 Co-authored-by: Joe Johnson (JJ) <joejohnson123[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
cubic analysis
No issues found across 11 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Linked issue analysis
Linked issue: ENG-1092: Add balance_id to create/update balance endpoints + delete balance endpoint
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add `external_id` column to `cusEntTable` | Added external_id column in customerEntTable |
| ✅ | Add `external_id` to CustomerEntitlement schema/model | CustomerEntitlementSchema now includes external_id |
| ✅ | Add `externalId` to CustomerEntitlementFilters | Filters schema extended with externalId |
| ✅ | In getApiBalance, use breakdown.id: cusEnt.external_id ?? cusEnt.id | API balance id now falls back to internal id |
| ✅ | Create balance: accept `balance_id` param and store as `external_id` | Create params include balance_id and insertion sets external_id |
| ✅ | Update balance: accept `balance_id` param (add to customerEntitlementFilters) to target specific balance | Update params include balance_id; filters use externalId |
| ✅ | Delete balance endpoint: accept `customer_id`, `feature_id`, optional `balance_id` and `entity_id` | New delete handler validates and accepts required/optional params |
| ✅ | Delete behavior: mark matching customer entitlements as expired (set expires_at = now) rather than hard deleting | Handler updates expires_at for each matched entitlement |
| ✅ | Expose delete endpoint as RPC style `/balances.delete` as well | RPC router includes balances.delete route |
| ✅ | fullCustomerToCustomerEntitlements should filter by externalId when provided | Full-customer conversion now filters by externalId |
| feature_id: text("feature_id"), | ||
|
|
||
| // External ID for API consumers to reference this balance | ||
| external_id: text("external_id"), |
Contributor
There was a problem hiding this comment.
Consider adding a database index on external_id since it's used for filtering in update/delete operations
Suggested change
| external_id: text("external_id"), | |
| // External ID for API consumers to reference this balance | |
| external_id: text("external_id").index("idx_customer_entitlements_external_id"), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: shared/models/cusProductModels/cusEntModels/cusEntTable.ts
Line: 52
Comment:
Consider adding a database index on `external_id` since it's used for filtering in update/delete operations
```suggestion
// External ID for API consumers to reference this balance
external_id: text("external_id").index("idx_customer_entitlements_external_id"),
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+51
to
+60
| // Mark as expired rather than deleting | ||
| const now = Date.now(); | ||
| for (const cusEnt of cusEnts) { | ||
| await CusEntService.update({ | ||
| ctx, | ||
| id: cusEnt.id, | ||
| updates: { | ||
| expires_at: now, | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
If multiple customer entitlements share the same external_id, all will be expired - verify this is the intended behavior
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/balances/handlers/handleDeleteBalance.ts
Line: 51-60
Comment:
If multiple customer entitlements share the same `external_id`, all will be expired - verify this is the intended behavior
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ENG-1092
Changes
Schema:
external_idcolumn tocustomerEntitlementstableexternal_idtoCustomerEntitlementSchemaexternalIdtoCustomerEntitlementFiltersCreate balance (
POST /balances/create):balance_idparam, stored asexternal_idon the customer entitlementUpdate balance (
POST /balances/update):balance_idparam to target a specific balance by its external IDbuildCustomerEntitlementFilters→fullCustomerToCustomerEntitlementsDelete balance (
POST /balances/delete):customer_id,feature_id, optionalbalance_idandentity_idexpires_at = now) rather than hard deletingPOST /balances.delete(RPC style)API response:
breakdown[].idnow returnsexternal_idwhen set, falling back to internalidNote
external_idcolumn needs to be pushed viadb:pushbefore deployingSummary by cubic
Implements ENG-1092 by adding support for external balance IDs and a soft-delete endpoint. Create/update can target balances by balance_id; API returns external IDs in balance breakdowns.
New Features
Migration
Written for commit 7c6580c. Summary will update on new commits.
Greptile Summary
Adds support for external balance IDs (
balance_id) to enable API consumers to reference specific balances across create, update, and delete operations. Thebalance_idis stored asexternal_idin the database and returned in API responses.Key changes:
balance_idparameter to create/update endpoints, and new delete endpoint (POST /balances/delete)external_id(when set) instead of internal id inbreakdown[].idNote: The
external_idcolumn does not enforce uniqueness. If multiple balances share the sameexternal_id, update/delete operations will affect all of them.Confidence Score: 4/5
external_idhas no uniqueness constraint, which means operations can affect multiple balances. This appears intentional but should be verified. Consider adding a database index onexternal_idfor query performance.external_idin shared/models/cusProductModels/cusEntModels/cusEntTable.ts for performanceImportant Files Changed
external_idtext column for API consumers to reference balancesexternalIdfrombalance_idparameterexternalIdexternal_idas breakdown item id when set, falling back to internal idSequence Diagram
sequenceDiagram participant Client participant handleDeleteBalance participant CusService participant fullCustomerToCustomerEntitlements participant CusEntService participant Cache Client->>handleDeleteBalance: POST /balances/delete<br/>{customer_id, feature_id, balance_id?} handleDeleteBalance->>CusService: getFull(customer_id, entity_id) CusService-->>handleDeleteBalance: fullCustomer handleDeleteBalance->>fullCustomerToCustomerEntitlements: Filter by feature_id<br/>and external_id (balance_id) fullCustomerToCustomerEntitlements-->>handleDeleteBalance: matching cusEnts[] alt No matches found handleDeleteBalance-->>Client: 404 Not Found else Matches found loop For each cusEnt handleDeleteBalance->>CusEntService: update(id, expires_at=now) CusEntService-->>handleDeleteBalance: updated end handleDeleteBalance->>Cache: deleteCachedApiCustomer Cache-->>handleDeleteBalance: cleared handleDeleteBalance-->>Client: {success: true, deleted_count} endLast reviewed commit: 7c6580c