Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: billing add subscription metadata to lines #1959

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

turip
Copy link
Contributor

@turip turip commented Dec 13, 2024

Overview

Add subscription metadata for lines.

We only support setting it via internal APIs, externally the user cannot update/create lines with subscription references.

@@ -0,0 +1,7 @@
-- modify "billing_invoice_lines" table
ALTER TABLE "billing_invoice_lines" ADD COLUMN "subscription_id" character(26) NULL, ADD COLUMN "subscription_item_id" character(26) NULL, ADD COLUMN "subscription_phase_id" character(26) NULL, ADD
CONSTRAINT "billing_invoice_lines_subscription_items_billing_lines" FOREIGN KEY ("subscription_item_id") REFERENCES "subscription_items" ("id") ON UPDATE NO ACTION ON DELETE SET NULL, ADD
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hekike I have intentionally (not) set the cascade deletion as we should not delete items even if a subscription gets deleted. (We cannot falsify the past)

@turip turip added area/api release-note/feature Release note: Exciting New Features area/billing labels Dec 13, 2024
@turip turip marked this pull request as ready for review December 13, 2024 16:48
@turip turip requested review from hekike and tothandras December 13, 2024 16:48
@turip turip enabled auto-merge (squash) December 13, 2024 16:49
* Subscription are the references to the subscritpions that this line is related to.
*/
@visibility("read")
subscriptions?: InvoiceLineSubscriptionReference;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be singular? Looks like reference is for one.

// NOTE: this can be used to have a type, that we can later replace with the expanded type if needed without
// breaking api compatibility
@friendlyName("IDResource")
model IDResource {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value of doing IDResource? Is the resource type changing for the fields where it's used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This btw reminds me of GraphQL's global IDs where every resource's ID is globally unique. Usually it's a concatenation of the type and ID .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an API compatibility issue. This allows the response to be like this:

{
   "subscription": {
      "subscription": {
         "id": ...
      },
      "phase": {
          "id": ...
      }
...
}

This way if we want to extend the API with extending the objects later, we can do that without needing to break the API.

@turip turip merged commit 478560f into main Dec 13, 2024
26 of 27 checks passed
@turip turip deleted the feat/billing-subs-meta branch December 13, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/billing release-note/feature Release note: Exciting New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants