feat(hsbc-uk): Add credit card support#17
feat(hsbc-uk): Add credit card support#17prisonblues wants to merge 3 commits intoshuckster:masterfrom
Conversation
- Detect CC accounts via normalisedProductCategoryCode - Add fetchCreditCardTransactions with BILLED/UN_BILLED support - Fix pagination using json.pagination.nextPageIndex - Combine UN_BILLED and BILLED into single synthetic statement - Handle CC account display (masked card number)
Add ability to download all credit card PDF statements going back to 2020. - New "CC PDFs" button appears when CC accounts are detected - Downloads PDFs individually with 5-second rate limiting - Handles both raw PDF and base64-encoded responses - Saves each PDF immediately as fetched (no batching) - Filename format: CC_Statement_XXXX_YYYY-MM-DD.pdf Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds HSBC UK credit card support to OBIS by detecting CC accounts, fetching CC transactions via the CARD transactions endpoint, and enabling bulk download of CC statement PDFs from the statements service.
Changes:
- Identify CC accounts via
normalisedProductCategoryCode === 'CC'and surface them through the store/UI. - Fetch CC transactions (both
UN_BILLEDandBILLED) using a new CC transactions URL builder and fetcher. - Add a “CC PDFs” UI action that downloads statement PDFs (handling raw PDF vs base64 data URL responses).
Reviewed changes
Copilot reviewed 10 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/index.js | Wires new DOWNLOAD_CC_PDFS UI action to PDF download handler. |
| src/ui/components/obis-overlay-widget/YearsAndActionButtons.jsx | Adds conditional “CC PDFs” button when CC accounts exist. |
| src/ui/components/app.jsx | Passes new CC PDF download handler down to the overlay widget. |
| src/common/obis/actions.js | Introduces DOWNLOAD_CC_PDFS / DOWNLOADED_CC_PDFS action constants. |
| src/common/obis/zip.js | Adds makePdfZip() flow to save CC PDFs individually using a plugin-registered fetcher. |
| src/plugins/hsbc-uk/urls.js | Adds URL builders for CC transactions and statement PDF download. |
| src/plugins/hsbc-uk/api/accounts.js | Extends accounts parsing to include availableBalance and normalisedProductCategoryCode. |
| src/plugins/hsbc-uk/api/transactions.js | Adds CC transaction fetcher with pagination and CC-specific parsing. |
| src/plugins/hsbc-uk/api/statements.js | Adds statement PDF download + a loop to fetch all CC PDFs with rate limiting. |
| src/plugins/hsbc-uk/plugin.js | Integrates CC account detection, synthetic CC “statement”, CC transaction fetching, and registers CC PDF fetcher on obis. |
| dist/extension/ui.css | Rebuilt asset output. |
| dist/extension/statement.css | Rebuilt asset output. |
| dist/extension/obis-hsbc-uk.js | Rebuilt bundled extension output reflecting source changes. |
| dist/bookmarklet/ui.js | Rebuilt bookmarklet UI bundle reflecting source changes. |
| dist/bookmarklet/main.js | Rebuilt bookmarklet main bundle reflecting source changes. |
| dist/bookmarklet/ui.css | Rebuilt asset output. |
| dist/bookmarklet/statement.css | Rebuilt asset output. |
| dist/bookmarklet/plugins/hsbc-uk.js | Rebuilt bundled plugin output reflecting source changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| /* globals obis, store */ | ||
|
|
||
| import { makePromise } from '@/cjs/promises' | ||
| import { getGenerators } from '@/obis/generators' | ||
| import { dateTimeString, zeroPad } from '@/obis/utils/dates' | ||
| import { compatMakeStatements } from '@/obis/statements' |
There was a problem hiding this comment.
This module uses store() as a global (/* globals ... store */) rather than importing it (e.g. import { store } from '@/obis/store' as done in src/common/obis/statements.js). This makes the module implicitly depend on window.store and can break if makePdfZip is invoked in a different runtime context.
| /* globals obis, store */ | |
| import { makePromise } from '@/cjs/promises' | |
| import { getGenerators } from '@/obis/generators' | |
| import { dateTimeString, zeroPad } from '@/obis/utils/dates' | |
| import { compatMakeStatements } from '@/obis/statements' | |
| /* globals obis */ | |
| import { makePromise } from '@/cjs/promises' | |
| import { getGenerators } from '@/obis/generators' | |
| import { dateTimeString, zeroPad } from '@/obis/utils/dates' | |
| import { compatMakeStatements } from '@/obis/statements' | |
| import { store } from '@/obis/store' |
| const DOWNLOAD_DELAY_MS = 500 // Delay between saveAs calls to avoid browser blocking | ||
|
|
||
| async function makePdfZip() { |
There was a problem hiding this comment.
DOWNLOAD_DELAY_MS is declared but never used. Either remove it, or implement the delay that the comment implies so the code and intent stay in sync.
| iscacheable: 'false' | ||
| } | ||
| }) | ||
|
|
There was a problem hiding this comment.
fetchCreditCardTransactions calls response.json() without checking response.ok. If the API returns a non-2xx status (often HTML or an error payload), this will throw and the error context is lost; add an ok check (and ideally include status/text) before attempting to parse.
| if (!response.ok) { | |
| let errorBody = '' | |
| try { | |
| errorBody = await response.text() | |
| } catch (e) { | |
| // Ignore body read errors; we'll still throw with status info. | |
| } | |
| const truncatedBody = errorBody.length > 500 ? errorBody.slice(0, 500) + '...' : errorBody | |
| throw new Error( | |
| `[OBIS] Failed to fetch credit card transactions (page ${pageNum}) for card ${ | |
| cardIdentifier?.slice(-30) || 'unknown' | |
| }: ${response.status} ${response.statusText} - ${truncatedBody}` | |
| ) | |
| } |
|
|
||
| if (isCreditCard) { | ||
| // Credit cards show masked number like "•••• •••• •••• 6137" | ||
| // Extract last 4 digits as identifier |
There was a problem hiding this comment.
This comment says the last 4 digits are extracted “as identifier”, but in this flow the API identifier used later is accountResponse.id / accountId (passed as cardIdentifier). Consider rewording to avoid implying the last-4 is used as the API identifier.
| // Extract last 4 digits as identifier | |
| // Extract last 4 digits for the displayable accountNumber (API identifier remains accountResponse.id) |
| // Check if there are any CC accounts | ||
| const hasCcAccounts = store().accounts.some(a => a.isCreditCard) | ||
|
|
There was a problem hiding this comment.
hasCcAccounts reads store() via a presumed global, but other UI components import store from @/obis/store. Relying on a global here makes the component harder to reuse/test and depends on window.store being set before render; consider importing store instead of using a /* globals store */ annotation.
| // | ||
| // Fetch all CC statement PDFs for an account | ||
| // Returns: Array of { bytes: Uint8Array, filename: string } | ||
| // | ||
|
|
||
| const RATE_LIMIT_MS = 5000 // 5 seconds between API requests | ||
| const DEBUG_LIMIT = 0 // Set to 0 for no limit, or a number to limit PDFs fetched | ||
|
|
||
| const delay = ms => new Promise(resolve => setTimeout(resolve, ms)) | ||
|
|
||
| export const fetchAllCcStatementPdfs = async ({ host = '', accountId, cardLastFour, onProgress, onPdfReady } = {}) => { | ||
| // First get the list of statements |
There was a problem hiding this comment.
The comment says this returns an array of PDFs, but the function actually returns successCount (a number). Please update the comment to match the return value, or change the implementation to return the described array.
| allTransactions.map(transaction => { | ||
| const { date, debit, credit, type, payee, note } = transaction | ||
| const { accountNumber, sortCode } = store().accounts.find( | ||
| const account = store().accounts.find( | ||
| acct => acct.id === transaction.accountId | ||
| ) |
There was a problem hiding this comment.
This map(...) call is used for side effects (mutating each transaction) and the returned array is ignored. Using forEach here (or assigning the result of map) would avoid confusion and prevents accidental bugs if this gets refactored later.
Summary
normalisedProductCategoryCode === 'CC')Changes
CC Transaction Support
identifierType=CARDendpointtransactionCreditDebitCode)CC PDF Downloads
CC_Statement_XXXX_YYYY-MM-DD.pdfTest plan
🤖 Generated with Claude Code