feat: Download receipt as csv#880
feat: Download receipt as csv#880C0mberry wants to merge 22 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. |
|
Let's consider adding tests for CSV into the receipt.e2e |
4daccce to
07cdfb5
Compare
Greptile SummaryThis PR adds CSV export for Solana receipts, complementing the existing PDF download. A new Key observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ReceiptView
participant ReceiptContent (receipt-page.tsx)
participant generateReceiptCsv
participant fast-csv
participant Browser
User->>ReceiptView: clicks "Download" popover
ReceiptView-->>User: shows CSV + PDF buttons
User->>ReceiptView: clicks "CSV"
ReceiptView->>ReceiptContent (receipt-page.tsx): downloadCsv()
ReceiptContent (receipt-page.tsx)->>generateReceiptCsv: generateReceiptCsv(receipt, signature, usdValue)
generateReceiptCsv->>generateReceiptCsv: buildReceiptCsvRow() → sanitizeCsvField(memo)
generateReceiptCsv->>fast-csv: writeToString([row], { headers })
fast-csv-->>generateReceiptCsv: CSV string
generateReceiptCsv->>Browser: new Blob → createObjectURL → <a>.click()
Browser-->>User: file download (solana-receipt-{sig}.csv)
generateReceiptCsv->>Browser: revokeObjectURL(url)
Reviews (2): Last reviewed commit: "added sanitizeCsvField" | Re-trigger Greptile |
|
@greptile-apps check again |
package.json
Outdated
| "codama": "1.2.11", | ||
| "cross-fetch": "3.2.0", | ||
| "cross-spawn": "7.0.6", | ||
| "fast-csv": "^5.0.5", |
There was a problem hiding this comment.
suggestion: Please use exact version of the package to align with the current project practices.
| test('shows CSV and PDF options in download menu', async ({ page }) => { | ||
| await waitForPage(page, VALID_TX, 'receipt'); | ||
|
|
||
| await page |
There was a problem hiding this comment.
suggestion: The duplicated part is pretty big and complex, some helper like navigateToReceipt would improve readability
app/styles.css
Outdated
| } | ||
| } | ||
|
|
||
| @media print { |
There was a problem hiding this comment.
question: What are these styles for? I tried printing http://localhost:3000/tx/2toZ2zRM4DfTyi6G4bSv9M9YfNMs4NQvnhyL4vmTMK45kf77FCPRR354Safnzr99HfoDae1PPYigKpCX1kwqejkS?view=receipt, but it doesn't seem to be printable anyway?
| } | ||
|
|
||
| export function buildReceiptCsvRow(receipt: FormattedReceipt, signature: string, usdValue?: string): string[] { | ||
| const mint = 'mint' in receipt ? receipt.mint : undefined; |
There was a problem hiding this comment.
suggestion: In all other cases this is like "const mint = 'mint' in receipt ? receipt.mint : undefined;", not a problem but consistency.
| usdValue?: string, | ||
| ): Promise<void> { | ||
| const row = buildReceiptCsvRow(receipt, signature, usdValue); | ||
| const csv = await writeToString([row], { headers: [...CSV_HEADERS] }); |
There was a problem hiding this comment.
note: I'm not saying it's wrong, but just a heads up: this function does a lot under the hood to handle large CSVs, while we only need to write one line. So the code below is all we require (just an example; no need to use it).
function toCsv(headers: readonly string[], row: string[]): string {
const escape = (v: string) =>
v.includes(',') || v.includes('"') || v.includes('\n')
? `"${v.replace(/"/g, '""')}"`
: v;
return [headers, row].map(r => r.map(escape).join(',')).join('\n');
} |
Could you check, please, that by sending the event to GA we can distinct PDF receipt from the CSV one |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| margin: 0.5in; | ||
| size: A4; | ||
| } | ||
| } |
There was a problem hiding this comment.
This media block could be deleted
Description
Type of change
Screenshots
solana-receipt.csv
Testing
Related Issues
HOO-327/
Checklist
build:infoscript to update build information