feat: Download receipt as PDF#875
feat: Download receipt as PDF#875C0mberry wants to merge 12 commits intosolana-foundation:masterfrom
Conversation
|
@C0mberry is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds a "Download PDF" button to the receipt page, generating a structured jsPDF document that includes on-chain payment details, editable supplier/items fields, a QR code for verification, and an optional USD value sourced from a new Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant DownloadReceiptItem
participant useDownloadReceipt
participant receipt-page (downloadPdf)
participant loadPdfDeps
participant generateReceiptPdf
participant useTokenPrice
participant /api/receipt/price
participant Jupiter API
User->>DownloadReceiptItem: click Download > PDF
DownloadReceiptItem->>useDownloadReceipt: trigger()
useDownloadReceipt->>useDownloadReceipt: setState('downloading')
useDownloadReceipt->>receipt-page (downloadPdf): download()
receipt-page (downloadPdf)->>loadPdfDeps: import('jspdf'), import('qrcode')
loadPdfDeps-->>receipt-page (downloadPdf): { JsPDF, qrToDataURL }
receipt-page (downloadPdf)->>generateReceiptPdf: generateReceiptPdf(deps, receipt, sig, url, txUrl, usdValue)
Note over receipt-page (downloadPdf),generateReceiptPdf: usdValue sourced from useTokenPrice (SWR cache)
generateReceiptPdf->>generateReceiptPdf: build jsPDF document (layout, editable fields, QR code, logo)
generateReceiptPdf->>User: doc.save('solana-receipt-<sig>.pdf')
generateReceiptPdf-->>receipt-page (downloadPdf): resolved
receipt-page (downloadPdf)-->>useDownloadReceipt: resolved
useDownloadReceipt->>useDownloadReceipt: setState('downloaded') → scheduleReset(2000ms) → setState('idle')
Note over useTokenPrice,Jupiter API: Price fetch (mainnet-beta only, SWR-cached 5 min)
useTokenPrice->>/api/receipt/price: GET /api/receipt/price/{mintAddress}
/api/receipt/price->>Jupiter API: GET api.jup.ag/price/v3?ids={mintAddress}
Jupiter API-->>/api/receipt/price: { usdPrice }
/api/receipt/price-->>useTokenPrice: { price: number | null }
Last reviewed commit: c8f207d |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { CACHE_HEADERS, NO_STORE_HEADERS } from './config'; | ||
|
|
||
| const JupiterPriceTokenSchema = type({ | ||
| usdPrice: number(), |
There was a problem hiding this comment.
suggestion: Maybe use refine(number(), 'positive', (value) => value > 0) to make sure it makes sense.
There was a problem hiding this comment.
Can you resolve threads here? I can't. Resolved
| return new Intl.NumberFormat('en-US', { maximumFractionDigits }).format(sol); | ||
| } | ||
|
|
||
| export function formatUsdValue(amount: number, price: number): string { |
There was a problem hiding this comment.
suggestion: It should be tested. It seems simple, but nothing currently filters out negatives or NaN values.
| qrToDataURL: typeof ToDataURL; | ||
| }; | ||
|
|
||
| const LOGO_SVG = `<svg xmlns="http://www.w3.org/2000/svg" width="229" height="28" fill="none" viewBox="0 0 229 28"><mask id="a" width="229" height="28" x="0" y="0" maskUnits="userSpaceOnUse" style="mask-type:luminance"><path fill="#fff" d="M228.83 0H0v27.538h228.83z"/></mask><g mask="url(#a)"><path fill="#1a1a1a" d="M67.472 11.015h-16.68V5.508H71.79V0H50.753a5.53 5.53 0 0 0-3.895 1.602 5.45 5.45 0 0 0-1.614 3.867v5.584c0 1.451.58 2.842 1.614 3.868a5.53 5.53 0 0 0 3.895 1.602h16.68v5.508H45.641v5.507h21.831a5.53 5.53 0 0 0 3.896-1.602 5.45 5.45 0 0 0 1.613-3.867v-5.584c0-1.45-.58-2.842-1.613-3.868a5.53 5.53 0 0 0-3.896-1.602M99.775 0H83.033a5.5 5.5 0 0 0-2.108.416 5.5 5.5 0 0 0-2.979 2.96 5.4 5.4 0 0 0-.418 2.093v16.6a5.44 5.44 0 0 0 1.611 3.867 5.5 5.5 0 0 0 1.786 1.186 5.5 5.5 0 0 0 2.108.416h16.742a5.53 5.53 0 0 0 3.896-1.602 5.45 5.45 0 0 0 1.613-3.867v-16.6a5.45 5.45 0 0 0-1.613-3.867A5.53 5.53 0 0 0 99.775 0m-.038 22.03H83.095V5.509h16.642zM158.369 0h-16.326a5.53 5.53 0 0 0-3.895 1.602 5.45 5.45 0 0 0-1.614 3.867v22.07h5.547v-9.05h16.25v9.05h5.547V5.468a5.44 5.44 0 0 0-1.613-3.868 5.5 5.5 0 0 0-1.787-1.186A5.5 5.5 0 0 0 158.369 0m-.038 12.981h-16.25V5.508h16.25zM223.32 0h-16.322a5.55 5.55 0 0 0-3.903 1.598 5.46 5.46 0 0 0-1.617 3.871v22.07h5.547v-9.05h16.257v9.05h5.547V5.468c0-1.45-.58-2.841-1.614-3.867A5.53 5.53 0 0 0 223.32 0m-.038 12.981h-16.246V5.508h16.246zm-32.278 9.049h-2.219l-7.951-19.735a3.66 3.66 0 0 0-1.35-1.667 3.7 3.7 0 0 0-2.06-.628h-4.938c-.974 0-1.908.384-2.596 1.068a3.63 3.63 0 0 0-1.076 2.577v23.893h5.547V5.508h2.22l7.947 19.735a3.65 3.65 0 0 0 1.348 1.668 3.7 3.7 0 0 0 2.057.627h4.939c.974 0 1.908-.384 2.596-1.067a3.63 3.63 0 0 0 1.075-2.578V0h-5.547zM115.747 0h-5.548v22.069c0 1.45.58 2.842 1.614 3.867a5.53 5.53 0 0 0 3.895 1.602h16.681v-5.507h-16.642z"/><g clip-path="url(#b)" transform="scale(.4375)"><path fill="url(#d)" d="M70.665 50.177 58.939 62.775a2.72 2.72 0 0 1-1.992.867H1.361a1.36 1.36 0 0 1-1.248-.82 1.37 1.37 0 0 1 .253-1.474L12.101 48.75a2.72 2.72 0 0 1 1.986-.867h55.582a1.36 1.36 0 0 1 1.249.82 1.37 1.37 0 0 1-.253 1.474M58.939 24.808a2.72 2.72 0 0 0-1.992-.867H1.361a1.36 1.36 0 0 0-1.248.82 1.37 1.37 0 0 0 .253 1.474l11.735 12.599a2.72 2.72 0 0 0 1.986.866h55.582a1.36 1.36 0 0 0 1.249-.82 1.37 1.37 0 0 0-.253-1.474zm-57.578-9.05h55.586a2.72 2.72 0 0 0 1.992-.866L70.665 2.294A1.364 1.364 0 0 0 69.669 0H14.087A2.72 2.72 0 0 0 12.1.867L.369 13.465a1.364 1.364 0 0 0 .992 2.294"/></g></g><defs><linearGradient id="d" x1="5.996" x2="64.404" y1="65.159" y2="-.566" gradientUnits="userSpaceOnUse"><stop offset=".08" stop-color="#9945ff"/><stop offset=".3" stop-color="#8752f3"/><stop offset=".5" stop-color="#5497d5"/><stop offset=".6" stop-color="#43b4ca"/><stop offset=".72" stop-color="#28e0b9"/><stop offset=".97" stop-color="#19fb9b"/></linearGradient><clipPath id="b"><path fill="#fff" d="M0 0h71.031v63.642H0z"/></clipPath></defs></svg>`; |
There was a problem hiding this comment.
nit: Maybe it would be better to extract it to a different file. It’s not something that one would actually read as code in this case.
| @@ -0,0 +1,39 @@ | |||
| import type { Meta, StoryObj } from '@storybook/react'; | |||
There was a problem hiding this comment.
issue: Isn't working for me: Cannot read properties of undefined (reading 'customEqualityTesters')
app/styles.css
Outdated
| mask: conic-gradient(from -45deg at bottom,#0000,#000 1deg 89deg,#0000 90deg) 50%/21px 100% | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
question: Is this code dead? I can't see how it's being used.
| @@ -0,0 +1,41 @@ | |||
| import { useCallback, useEffect, useRef, useState } from 'react'; | |||
There was a problem hiding this comment.
suggestion: Let's use the same naming conventions at least inside a feature, should be use-download-receipt.ts (+ tests file)
|
|
||
| useEffect(() => { | ||
| return () => { | ||
| clearTimeout(timeoutRef.current); |
There was a problem hiding this comment.
note: nothing checks the progress of the download, so setting state on an unmounted component is still possible. Not that important in this case, though
There was a problem hiding this comment.
Resolved (we could also use the AbortController, but might be an overkill)
| expect(download).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('should clean up timeout on unmount', async () => { |
There was a problem hiding this comment.
chore: The intent here is not clear, the is no expect at all. Expect something 😀
|
@greptile-apps issue was addressed |
| const trigger = useCallback(() => { | ||
| if (state === 'downloading') return; | ||
|
|
||
| clearTimeout(timeoutRef.current); | ||
| setState('downloading'); | ||
|
|
||
| download().then( | ||
| () => { | ||
| if (!mountedRef.current) return; | ||
| setState('downloaded'); | ||
| scheduleReset(); | ||
| }, | ||
| (error: unknown) => { | ||
| if (!mountedRef.current) return; | ||
| console.error('Download failed:', error); | ||
| setState('errored'); | ||
| scheduleReset(); | ||
| } | ||
| ); | ||
| }, [state, download, scheduleReset]); |
There was a problem hiding this comment.
Stale closure makes the in-flight guard unreliable
trigger captures state from the last render. Because state is in the useCallback dependency array, React replaces the trigger reference only after a re-render. If the user clicks twice in rapid succession before React has flushed the state update and re-rendered, both invocations see state === 'idle' and both call download().
A ref-based guard is immune to this race because it reads the current value synchronously, not the captured render-time value:
const stateRef = useRef<DownloadState>('idle');
const trigger = useCallback(() => {
if (stateRef.current === 'downloading') return;
clearTimeout(timeoutRef.current);
stateRef.current = 'downloading';
setState('downloading');
download().then(
() => {
stateRef.current = 'downloaded';
if (!mountedRef.current) return;
setState('downloaded');
scheduleReset();
},
(error: unknown) => {
stateRef.current = 'errored';
if (!mountedRef.current) return;
console.error('Download failed:', error);
setState('errored');
scheduleReset();
}
);
}, [download, scheduleReset]); // state removed from depsThis also removes state from the useCallback dependency array, meaning trigger keeps the same reference across state transitions and avoids unnecessary re-renders in child components.
| async function fetchPrice([, mintAddress]: PriceSwrKey): Promise<PriceResult> { | ||
| try { | ||
| const response = await fetch(`/api/receipt/price/${mintAddress}`); | ||
|
|
||
| if (!response.ok) { | ||
| if (response.status === 429) { | ||
| return { price: null, status: PriceStatus.RateLimited }; | ||
| } | ||
| return { price: null, status: PriceStatus.FetchFailed }; | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| if (!is(data, PriceResponseSchema)) { | ||
| return { price: null, status: PriceStatus.FetchFailed }; | ||
| } | ||
|
|
||
| return { price: data.price, status: PriceStatus.Success }; | ||
| } catch { | ||
| return { price: null, status: PriceStatus.FetchFailed }; | ||
| } |
There was a problem hiding this comment.
Transient API failures are permanently cached for dedupingInterval
fetchPrice never throws — it always returns a PriceResult even on error. SWR only retries a key when the fetcher throws (or rejects); when the fetcher returns data, that result is cached as "successful" data for the full dedupingInterval (5 minutes, per PRICE_SWR_CONFIG).
This means a transient failure (e.g. a temporary Jupiter outage at page load) results in { price: null, status: FetchFailed } being cached as valid data for the rest of the session window. The USD amount will be silently absent until the cache expires.
Consider throwing for error conditions you want SWR to retry (e.g., non-429 network errors), and only returning FetchFailed for cases that genuinely should not be retried (e.g., token not found / 404):
async function fetchPrice([, mintAddress]: PriceSwrKey): Promise<PriceResult> {
const response = await fetch(`/api/receipt/price/${mintAddress}`);
if (!response.ok) {
if (response.status === 429) {
return { price: null, status: PriceStatus.RateLimited };
}
// Throw so SWR retries transiently failing requests
throw new Error(`Price fetch failed: ${response.status}`);
}
// ...
}| return NextResponse.json( | ||
| { error: 'Failed to fetch price data' }, | ||
| { headers: NO_STORE_HEADERS, status: response.status } | ||
| ); |
There was a problem hiding this comment.
Upstream status codes are proxied verbatim to clients
When Jupiter returns an error (e.g., 503 Service Unavailable, 404 Not Found), the response status is forwarded directly to the caller via status: response.status. The client-side fetchPrice only special-cases 429; everything else is treated as a generic FetchFailed. While this is functionally OK today, proxying raw upstream statuses leaks implementation details and can cause unexpected behaviour if Jupiter introduces new codes.
Consider normalising non-429 errors to a consistent 502 (bad gateway) or 503 instead:
return NextResponse.json(
{ error: 'Failed to fetch price data' },
{ headers: NO_STORE_HEADERS, status: response.status === 429 ? 429 : 502 }
);Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| }, | ||
| (error: unknown) => { | ||
| if (!mountedRef.current) return; | ||
| console.error('Download failed:', error); |
There was a problem hiding this comment.
todo: Please user Logger.error instead
3 participants
Description
Type of change
Screenshots
Testing
Related Issues
HOO-326
Checklist
build:infoscript to update build information