-
-
Notifications
You must be signed in to change notification settings - Fork 13
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(console): Billing and Invoicing - Invoice and payments integration #2457
feat(console): Billing and Invoicing - Invoice and payments integration #2457
Conversation
8717089
to
0e0c57d
Compare
invoices: { | ||
amount: number | ||
timestamp: number | ||
status: string | null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to have a default value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it's currently string | null
to cast nicely from Stripe client's types:
/**
* The status of the invoice, one of `draft`, `open`, `paid`, `uncollectible`, or `void`. [Learn more](https://stripe.com/docs/billing/invoices/workflow#workflow-overview)
*/
status: Invoice.Status | null;
I guess there could be some default value, but we'd need to handle the casting from the stripe res, not sure if better:
invoices = stripeInvoices.invoices.data.map((i) => ({
amount: i.total / 100,
timestamp: i.created * 1000,
status: i.status, // Here
url: i.hosted_invoice_url ?? undefined,
}))
returnURL, | ||
}: CustomerPortalParams) => { | ||
const stripeClient = new Stripe(STRIPE_API_SECRET, { | ||
apiVersion: '2022-11-15', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to make this an environment variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export type LatestApiVersion = '2022-11-15';
export type HttpAgent = Agent;
export type HttpProtocol = 'http' | 'https';
export interface StripeConfig {
/**
* This library's types only reflect the latest API version.
*
* We recommend upgrading your account's API Version to the latest version
* if you wish to use TypeScript with this library.
*
* If you wish to remain on your account's default API version,
* you may pass `null` or another version instead of the latest version,
* and add a `@ts-ignore` comment here and anywhere the types differ between API versions.
*
* @docs https://stripe.com/docs/api/versioning
*/
apiVersion: LatestApiVersion;
If we put an env variable, we'd still need to cast it to the actual string value, not sure that's worth it or cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant it for updating the value actually.
|
||
import { Context } from '../../context' | ||
import { ServicePlanType } from '@proofzero/types/account' | ||
import { ReconciliationNotificationType } from '../../../../email/src/jsonrpc/methods/sendReconciliationNotification' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@proofzero/platform.client
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I extracted the enum into the types package as it spans both e-mail and address workers.
const planApps = apps.filter((app) => app.appPlan === plan) | ||
if (planApps.length > count) { | ||
const targetApps = planApps | ||
.sort((a, b) => +b.createdTimestamp! - +a.createdTimestamp!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to avoid !
? undefined
might not work well with subtraction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to undefined being a possibility, there's a condition (L53) when adding items to the apps
list:
if (appDetails.createdTimestamp != null) {
so we'd expect only items with a timestamp to be present in the planApps
. I think it's a case of Typescript not picking it up, so the !
is mostly to stop it from crying. 😁
e6d0039
to
2053546
Compare
Description
Related Issues
Testing
Checklist