Skip to content

Sync feature/payment-conf-emails to main#14

Open
nicholas-fierro wants to merge 5 commits intomainfrom
feature/payment-conf-emails
Open

Sync feature/payment-conf-emails to main#14
nicholas-fierro wants to merge 5 commits intomainfrom
feature/payment-conf-emails

Conversation

@nicholas-fierro
Copy link
Copy Markdown
Collaborator

No description provided.

@vercel
Copy link
Copy Markdown

vercel bot commented Nov 18, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
dttd Ready Ready Preview Comment Nov 18, 2025 0:08am

@nicholas-fierro nicholas-fierro changed the title Feature/payment conf emails Sync feature/payment-conf-emails to main Nov 18, 2025
}
>
> {
const paymentIntentId = session.payment_intent as string
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is this assertion necessary? Since this area is important to ensure the system records payments correctly, I want to make sure potential errors are handled gracefully - i've also seen AI hallucinate and add unnecessary type assertions, which might be the case here

.single()

if (paymentRecordError) {
if (paymentRecordError.code === '23505') {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Any idea what this error code means? wondering if we can abstract to a constant for easier understanding

return ok(true)
}

function toDollarAmount(amount: number | null | undefined): number | null {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Might be good to add a block comment to this function explaining what we're expecting the input to be to this function, maybe extracting this to a lib file too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants