-
-
Notifications
You must be signed in to change notification settings - Fork 18
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): Service Plan - Application entitlement assignment #2451
feat(console): Service Plan - Application entitlement assignment #2451
Conversation
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'd be nice to avoid the long switch
statement in apps/console/app/routes/apps/$clientId/gnillib.tsx
.
published?: boolean | ||
createdTimestamp?: number |
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.
Why these are optional?
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, I couldn't say; just extracted a type from the initial LoaderData that can be used somewhere else.
apps/console/app/root.tsx
Outdated
} | ||
} | ||
|
||
export const loader: LoaderFunction = getRollupReqFunctionErrorWrapper( | ||
async ({ request, context }) => { | ||
const jwt = await requireJWT(request) | ||
const traceHeader = generateTraceContextHeaders(context.traceSpan) | ||
const parsedJwt = parseJwt(jwt) | ||
const parsedJwt = parseJwt(jwt!) |
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.
await requireJWT(request)
should be throwing or returning a jwt. The non-null assertion shouldn't be needed.
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.
Added a check
@@ -27,6 +27,12 @@ export const loader: LoaderFunction = getRollupReqFunctionErrorWrapper( | |||
accountURN, | |||
}) | |||
|
|||
return updatePaymentMethod({ customerID }) | |||
const headers = request.headers | |||
const returnURL = headers.get('Referer') as string |
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 check if there is a referer in case of an abnormal request.
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.
Checked
I extracted the two methods to make it a bit more readable I guess. |
Description
handled
metadata when creating / updating subscriptions. This can be set to whatever value and if it's present in the webhook event, it A - clears it from the subscription and B - returns without updating entitlements. This allows for sychronous payments.pays/app
)Related Issues
Testing
nodes/account.ts
file allows one to reset their clientID as well as paymentInformation and existing service plans:After that, it's a matter of going through the flows.
Checklist