Conversation
onearmy-community-platform
 |
||||||||||||||||||||||||||||
| Project |
onearmy-community-platform
|
| Branch Review |
feat/stripe
|
| Run status |
|
| Run duration | 09m 12s |
| Commit |
|
| Committer | Jacob Roberson |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
91
|
| View all changes introduced in this branch ↗︎ | |
…o avoid orphaned customers
| import { ThankYouAccountForm } from './ThankYouAccountForm'; | ||
| import { ThankYouLoginForm } from './ThankYouLoginForm'; | ||
|
|
||
| export const CURRENCY_SYMBOLS: Record<string, string> = { |
There was a problem hiding this comment.
We can avoid this, by using browser native currency api.
new Intl.NumberFormat(navigator.language, { style: 'currency', currency: 'eur', currencyDisplay: 'narrowSymbol' }).format(99)
returns: '€99.00'
note that navigator is not available server-side, if we need it server-side, we need to obtain it via the 'accept-language' header or have a fallback. (we should try to use it because some currencies differ based on the user language).
| gbp: '£', | ||
| }; | ||
|
|
||
| export const formatAmount = (cents: number) => (cents / 100).toFixed(0); |
There was a problem hiding this comment.
should use the browser native function
new Intl.NumberFormat(locale, { style: 'currency', currency }).format(cents / 100);
| if (previewMode) return; | ||
|
|
||
| if (isAuthenticated) { | ||
| window.location.assign('/settings?subscription=success'); |
There was a problem hiding this comment.
instead of all of these window.location usage, have you tried useLocation() from react-router?
| fontWeight: 'bold', | ||
| fontSize: 1, | ||
| fontFamily: 'body', | ||
| mb: 3, |
There was a problem hiding this comment.
I see a few usages of mb and mt. I usually try to avoid them in favor of flex and gap. Sometime it's fair to use margins, but please double check :)
|
|
||
| export const action = async ({ request }: ActionFunctionArgs) => { | ||
| if (request.method !== 'POST') { | ||
| return new Response('Method not allowed', { status: 405 }); |
There was a problem hiding this comment.
check new way to handle errors
| return data?.stripe_customer_id || null; | ||
| } | ||
|
|
||
| async getAuthIdByStripeCustomerId(stripeCustomerId: string): Promise<string | null> { |
There was a problem hiding this comment.
careful that this is using the admin client. shouldn't it filter by tenant_id?
There was a problem hiding this comment.
worst case it will find more than one profile and throw an error
| "tenant_id" text not null, | ||
| "created_at" timestamp with time zone not null default (now() AT TIME ZONE 'utc'::text), | ||
| constraint "stripe_customers_pkey" primary key ("id"), | ||
| constraint "stripe_customers_auth_id_fkey" foreign key ("auth_id") references "auth"."users"("id") on update cascade on delete cascade, |
PR Checklist
PR Type
What kind of change does this PR introduce?
What is the new behavior?
Describe the new behaviour
If useful, provide screenshot or capture to highlight main changes
Does this PR introduce a DB Schema Change or Migration?
Git Issues
Closes #
What happens next?
Thank you for the contribution! We will review it ASAP.
If you need more immediate feedback you can reach out to us on Discord in the Community Platform
developmentchannel.