diff --git a/README.md b/README.md index bca5b24..ea23698 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![CI](https://github.com/meseer/monarch-uploader/actions/workflows/ci.yml/badge.svg)](https://github.com/meseer/monarch-uploader/actions/workflows/ci.yml) [![codecov](https://codecov.io/gh/meseer/monarch-uploader/branch/main/graph/badge.svg)](https://codecov.io/gh/meseer/monarch-uploader) -[![Version](https://img.shields.io/badge/version-6.10.1-blue)](https://github.com/meseer/monarch-uploader) +[![Version](https://img.shields.io/badge/version-6.11.0-blue)](https://github.com/meseer/monarch-uploader) [![License: CC BY-NC-SA 4.0](https://img.shields.io/badge/License-CC%20BY--NC--SA%204.0-lightgrey.svg)](https://creativecommons.org/licenses/by-nc-sa/4.0/) A userscript that automatically syncs balance history, transactions, holdings, and more from Canadian financial institutions to [Monarch Money](https://www.monarchmoney.com/). diff --git a/package.json b/package.json index a64a35f..2b652a3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "monarch-uploader", - "version": "6.10.1", + "version": "6.11.0", "description": "Violentmonkey userscript for uploading Questrade, Wealthsimple, Canada Life, and Rogers Bank data to Monarch Money", "main": "dist/monarch-uploader.user.js", "scripts": { diff --git a/src/api/questrade.ts b/src/api/questrade.ts index 3aa572b..8b31fe1 100644 Binary files a/src/api/questrade.ts and b/src/api/questrade.ts differ diff --git a/src/scriptInfo.json b/src/scriptInfo.json index 253cac0..a446f36 100644 --- a/src/scriptInfo.json +++ b/src/scriptInfo.json @@ -1,4 +1,4 @@ { - "version": "6.10.1", + "version": "6.11.0", "gistUrl": "https://gist.github.com/meseer/f00fb552c96efeb3eb4e4e1fd520d4e7/raw/monarch-uploader.user.js" } diff --git a/src/services/questrade/sync.ts b/src/services/questrade/sync.ts index 4cd7ae0..3df3416 100644 --- a/src/services/questrade/sync.ts +++ b/src/services/questrade/sync.ts @@ -147,7 +147,8 @@ async function syncAccountToMonarch(accountId, accountName, fromDate, toDate, pr // Get Monarch account mapping for transaction uploads const monarchAccountForTx = accountService.getMonarchAccountMapping(INTEGRATIONS.QUESTRADE, accountId); - // Step 3: Sync orders (trades) - gracefully handle failures + // Step 3: Sync orders (trades) - also produces signatures for cross-source dedup + let orderSignatures = null; if (progressDialog) { progressDialog.updateStepStatus(accountId, 'orders', 'processing', 'Syncing orders...'); } @@ -167,6 +168,9 @@ async function syncAccountToMonarch(accountId, accountName, fromDate, toDate, pr null, // Don't pass progressDialog to avoid double-updates ); + // Extract order signatures for activity trade deduplication + orderSignatures = ordersResult?.orderSignatures || null; + if (ordersResult.success) { const ordersCount = ordersResult.ordersProcessed || 0; let ordersMessage; @@ -197,7 +201,7 @@ async function syncAccountToMonarch(accountId, accountName, fromDate, toDate, pr } } - // Step 4: Sync activity (contributions, dividends, fees, etc.) - gracefully handle failures + // Step 4: Sync activity (all types, with cross-source trade dedup against orders) if (progressDialog) { progressDialog.updateStepStatus(accountId, 'activity', 'processing', 'Syncing activity...'); } @@ -215,6 +219,7 @@ async function syncAccountToMonarch(accountId, accountName, fromDate, toDate, pr fromDate, monarchAccountForTx.id, null, // Don't pass progressDialog to avoid double-updates + orderSignatures, // Pass order signatures for cross-source trade dedup ); if (activityResult.success) { diff --git a/src/services/questrade/transactionRules.ts b/src/services/questrade/transactionRules.ts index bb40087..43156a8 100644 --- a/src/services/questrade/transactionRules.ts +++ b/src/services/questrade/transactionRules.ts @@ -284,6 +284,69 @@ function formatQuantityNotes(normalized) { return lines.join('\n'); } +/** + * Format notes for trade transactions (buy/sell from Activity API) + * Produces notes similar to Orders API format: + * Line 1: Description (e.g., "VANGUARD GROWTH ETF PORTFOLIO ETF UNIT WE ACTED AS AGENT") + * Line 2: Filled quantity @ price per share, commission + * Line 3: Total amount with currency + * Line 4: Settlement date (only if different from transaction date) + * + * Data sources: + * - quantity: from details or activity list (with &fields=Quantity) + * - price: from details or activity list (with &fields=Price) — { currencyCode, amount } + * - commission: from details only (details.commission) + * - net: from details or activity list — { currencyCode, amount } + * + * @param {Object} normalized - Normalized transaction data + * @returns {string} Formatted trade notes + */ +export function formatTradeNotes(normalized) { + const lines = []; + + // Line 1: Description + if (normalized.description) { + lines.push(normalized.description); + } + + // Line 2: Filled quantity @ price, commission + const qty = normalized.quantity !== undefined && normalized.quantity !== null + ? formatNumber(normalized.quantity) : ''; + const priceAmount = normalized.price?.amount !== undefined && normalized.price?.amount !== null + ? formatNumber(normalized.price.amount) : ''; + const priceCurrency = cleanString(normalized.price?.currencyCode) || cleanString(normalized.net?.currencyCode) || 'CAD'; + + // Commission comes from details endpoint only (details.commission) + // eslint-disable-next-line no-underscore-dangle + const commission = normalized._details?.commission; + const commissionStr = commission !== undefined && commission !== null + ? formatNumber(commission) : ''; + + if (qty && priceAmount) { + let filledLine = `Filled ${qty} @ ${priceAmount}`; + if (commissionStr) { + filledLine += `, fees: ${commissionStr} ${priceCurrency}`; + } + lines.push(filledLine); + } + + // Line 3: Total amount with currency + const netAmount = normalized.net?.amount; + if (netAmount !== undefined && netAmount !== null) { + const totalStr = formatNumber(Math.abs(parseFloat(netAmount) || 0)); + if (totalStr) { + lines.push(`Total: ${totalStr} ${priceCurrency}`); + } + } + + // Line 4: Settlement Date - only if different from transaction date + if (normalized.settlementDate && normalized.settlementDate !== normalized.transactionDate) { + lines.push(`Settlement Date: ${normalized.settlementDate}`); + } + + return lines.join('\n'); +} + /** * Format FX conversion notes with exchange rate details * Uses correct API structure: @@ -696,6 +759,46 @@ export const QUESTRADE_TRANSACTION_RULES = [ }), }, + // ============================================ + // TRADES (from Activity API) + // Trades are also available from the Orders API (v1), but that only returns + // recent orders. Activity API trades are kept and deduplicated against orders + // to ensure historic trades are not lost. + // ============================================ + { + id: 'trades-buy', + description: 'Trades - Buy order', + match: (n) => n.transactionType === 'Trades' && n.action === 'Buy', + process: (n) => ({ + category: 'Buy', + merchant: n.symbol || 'Unknown Security', + originalStatement: formatOriginalStatement(n.transactionType, n.action, n.symbol), + notes: formatTradeNotes(n), + }), + }, + { + id: 'trades-sell', + description: 'Trades - Sell order', + match: (n) => n.transactionType === 'Trades' && n.action === 'Sell', + process: (n) => ({ + category: 'Sell', + merchant: n.symbol || 'Unknown Security', + originalStatement: formatOriginalStatement(n.transactionType, n.action, n.symbol), + notes: formatTradeNotes(n), + }), + }, + { + id: 'trades-fallback', + description: 'Trades - Other trade actions (catch-all)', + match: (n) => n.transactionType === 'Trades', + process: (n) => ({ + category: 'Investment', + merchant: n.symbol || 'Unknown Security', + originalStatement: formatOriginalStatement(n.transactionType, n.action, n.symbol), + notes: formatTradeNotes(n), + }), + }, + // ============================================ // FALLBACK - Unknown Type/Action // ============================================ @@ -754,17 +857,15 @@ export function applyTransactionRule(transaction, details = null) { /** * Check if a transaction should be filtered out - * Trades are handled by the orders API, not the activity API + * Previously filtered trades (handled by orders API), but now trades are kept + * and deduplicated against orders instead. * @param {Object} transaction - Transaction object * @returns {boolean} True if transaction should be excluded */ export function shouldFilterTransaction(transaction) { - // Filter out Trades - they're handled by the orders API - if (transaction.transactionType === 'Trades') { - return true; - } - - return false; + // Trades are no longer filtered — they are deduplicated against orders + // in the transaction processing pipeline instead + return !transaction && false; // always returns false; uses parameter to avoid unused warning } /** diff --git a/src/services/questrade/transactions.ts b/src/services/questrade/transactions.ts index b18c8d5..183b3ad 100644 --- a/src/services/questrade/transactions.ts +++ b/src/services/questrade/transactions.ts @@ -3,8 +3,12 @@ * Handles fetching orders and activity transactions from Questrade and uploading them to Monarch * * This service uses TWO data sources: - * 1. Orders API (v1) - For trade orders (buy/sell securities) - * 2. Activity API (v3) - For non-trade transactions (dividends, fees, deposits, etc.) + * 1. Orders API (v1) - For trade orders (buy/sell securities) - only returns recent orders + * 2. Activity API (v3) - For ALL transactions including trades, dividends, fees, deposits, etc. + * + * Trades from Activity API are deduplicated against Orders API data. + * Orders API data is preferred when available (richer data), but Activity API + * trades are kept for historic orders that the Orders API no longer returns. */ import { debugLog, getTodayLocal, saveLastUploadDate } from '../../core/utils'; @@ -23,8 +27,8 @@ import { } from '../../utils/transactionStorage'; import { applyTransactionRule, - shouldFilterTransaction, getTransactionId, + cleanString, } from './transactionRules'; import { showProgressDialog } from '../../ui/components/progressDialog'; @@ -121,39 +125,115 @@ function filterDuplicateTransactions(transactions, accountId) { } /** - * Filter orders to only include executed ones - * @param {Array} orders - Array of orders - * @returns {Array} Filtered orders with status="Executed" + * Build composite signature keys from orders for cross-source deduplication. + * These signatures allow matching Activity API trades against Orders API data. + * Format: "symbol:YYYY-MM-DD:action" (all lowercased for case-insensitive matching) + * + * @param {Array} orders - Array of executed orders from Orders API + * @returns {Set} Set of order signature strings */ -function filterExecutedOrders(orders) { +function buildOrderSignatures(orders) { + const signatures = new Set(); + if (!orders || !Array.isArray(orders)) { - return []; + return signatures; } - const executedOrders = orders.filter((order) => order.status === 'Executed'); - debugLog(`Filtered ${executedOrders.length} executed orders from ${orders.length} total`); - return executedOrders; + for (const order of orders) { + // Extract symbol from security object (Orders API uses security.symbol) + const symbol = cleanString(order.security?.symbol || '').toLowerCase(); + const action = cleanString(order.action || '').toLowerCase(); + + // Extract date from updatedDateTime (ISO format) + let date = ''; + if (order.updatedDateTime) { + const dateStr = order.updatedDateTime; + date = dateStr.includes('T') ? dateStr.split('T')[0] : dateStr; + } + + if (symbol && date && action) { + const signature = `${symbol}:${date}:${action}`; + signatures.add(signature); + } + } + + debugLog(`Built ${signatures.size} order signatures from ${orders.length} orders`); + return signatures; } /** - * Filter transactions to exclude trades (handled by orders API) - * @param {Array} transactions - Array of transactions from activity API - * @returns {Array} Filtered transactions without trades + * Filter Activity API trades that match Orders API data (cross-source deduplication). + * Removes activity trades that have a matching order, since Orders API data is preferred. + * + * Only filters transactions with transactionType === 'Trades'. + * Non-trade transactions pass through unchanged. + * + * @param {Array} processedTransactions - Array of {transaction, details, ruleResult} from activity processing + * @param {Set} orderSignatures - Set of order signatures from buildOrderSignatures() + * @returns {Object} Filtered transactions and statistics */ -function filterNonTradeTransactions(transactions) { - if (!transactions || !Array.isArray(transactions)) { - return []; +function filterActivityTradesMatchingOrders(processedTransactions, orderSignatures) { + if (!orderSignatures || orderSignatures.size === 0 || !processedTransactions || processedTransactions.length === 0) { + return { + transactions: processedTransactions || [], + matchedTradeCount: 0, + }; } - const nonTradeTransactions = transactions.filter((tx) => !shouldFilterTransaction(tx)); - const filteredCount = transactions.length - nonTradeTransactions.length; + const originalCount = processedTransactions.length; + let matchedTradeCount = 0; + + const filtered = processedTransactions.filter((pt) => { + const tx = pt.transaction; + + // Only attempt to match trades — let all other transaction types pass through + if (tx.transactionType !== 'Trades') { + return true; + } + + // Build the same composite key from the activity transaction + const symbol = cleanString(tx.symbol || pt.details?.symbol || '').toLowerCase(); + const action = cleanString(tx.action || '').toLowerCase(); + let date = cleanString(tx.transactionDate || ''); + if (date.includes('T')) { + date = date.split('T')[0]; + } - if (filteredCount > 0) { - debugLog(`Filtered out ${filteredCount} trade transactions (handled by orders API)`); + if (symbol && date && action) { + const signature = `${symbol}:${date}:${action}`; + if (orderSignatures.has(signature)) { + matchedTradeCount += 1; + return false; // Remove — this trade is covered by an order + } + } + + // No matching order found — keep this activity trade + return true; + }); + + if (matchedTradeCount > 0) { + debugLog(`Cross-source dedup: removed ${matchedTradeCount} activity trades matching orders (${originalCount} → ${filtered.length})`); } - debugLog(`Kept ${nonTradeTransactions.length} non-trade transactions`); - return nonTradeTransactions; + return { + transactions: filtered, + matchedTradeCount, + }; +} + +/** + * Filter orders to only include executed ones + * @param {Array} orders - Array of orders + * @returns {Array} Filtered orders with status="Executed" + */ +function filterExecutedOrders(orders) { + if (!orders || !Array.isArray(orders)) { + return []; + } + + const executedOrders = orders.filter((order) => order.status === 'Executed'); + debugLog(`Filtered ${executedOrders.length} executed orders from ${orders.length} total`); + return executedOrders; } /** @@ -355,6 +435,7 @@ async function fetchQuestradeOrders(accountId, fromDate) { /** * Fetch activity transactions and their details for an account + * Includes ALL transaction types (trades, dividends, fees, etc.) * @param {string} accountId - Questrade account ID * @param {string} fromDate - Start date in YYYY-MM-DD format * @param {Object} progressDialog - Optional progress dialog @@ -368,7 +449,7 @@ async function fetchAndProcessActivityTransactions(accountId, fromDate, progress progressDialog.updateProgress(accountId, 'processing', 'Loading transactions from Questrade...'); } - // Fetch all transactions since the date + // Fetch all transactions since the date (including trades) const transactions = await questradeApi.fetchTransactionsSinceDate(accountId, fromDate); if (!transactions || transactions.length === 0) { @@ -376,34 +457,26 @@ async function fetchAndProcessActivityTransactions(accountId, fromDate, progress return []; } - debugLog(`Fetched ${transactions.length} activity transactions`); - - // Filter out trades (handled by orders API) - const nonTradeTransactions = filterNonTradeTransactions(transactions); - - if (nonTradeTransactions.length === 0) { - debugLog('No non-trade transactions to process'); - return []; - } + debugLog(`Fetched ${transactions.length} activity transactions (including trades)`); if (progressDialog) { progressDialog.updateProgress( accountId, 'processing', - `Loading transaction details (0/${nonTradeTransactions.length})...`, + `Loading transaction details (0/${transactions.length})...`, ); } // Fetch details for each transaction const processedTransactions = []; - for (let i = 0; i < nonTradeTransactions.length; i += 1) { - const tx = nonTradeTransactions[i]; + for (let i = 0; i < transactions.length; i += 1) { + const tx = transactions[i]; if (progressDialog && i % 5 === 0) { progressDialog.updateProgress( accountId, 'processing', - `Loading transaction details (${i + 1}/${nonTradeTransactions.length})...`, + `Loading transaction details (${i + 1}/${transactions.length})...`, ); } @@ -411,7 +484,7 @@ async function fetchAndProcessActivityTransactions(accountId, fromDate, progress let details = null; if (tx.transactionUrl) { try { - details = await questradeApi.fetchTransactionDetails(tx.transactionUrl); + details = await questradeApi.fetchTransactionDetails(tx.transactionUrl as string); } catch (detailError) { debugLog(`Failed to fetch details for transaction ${getTransactionId(tx)}:`, detailError); // Continue without details - rules can still process with basic info @@ -438,14 +511,19 @@ async function fetchAndProcessActivityTransactions(accountId, fromDate, progress /** * Process and upload activity transactions for a Questrade account + * Includes all transaction types. Trades are deduplicated against Orders API data + * using order signatures — Activity trades matching an order are removed since + * the Orders API provides richer data for recent trades. + * * @param {string} accountId - Questrade account ID * @param {string} accountName - Account name for display * @param {string} fromDate - Start date for transactions * @param {string} monarchAccountId - Monarch account ID to upload to * @param {Object} progressDialog - Optional progress dialog + * @param {Set} orderSignatures - Optional set of order signatures for cross-source trade dedup * @returns {Promise} Upload result */ -async function processAndUploadActivityTransactions(accountId, accountName, fromDate, monarchAccountId, progressDialog = null) { +async function processAndUploadActivityTransactions(accountId, accountName, fromDate, monarchAccountId, progressDialog = null, orderSignatures = null) { try { debugLog(`Processing activity transactions for account ${accountName} (${accountId})`); @@ -461,19 +539,29 @@ async function processAndUploadActivityTransactions(accountId, accountName, from }; } - // Filter out duplicates - const transactionsForDedup = processedTransactions.map((pt) => pt.transaction); + // Step 1: Cross-source dedup — remove activity trades that match an order + let afterCrossDedup = processedTransactions; + let crossDedupCount = 0; + if (orderSignatures && orderSignatures.size > 0) { + const crossResult = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + afterCrossDedup = crossResult.transactions; + crossDedupCount = crossResult.matchedTradeCount; + } + + // Step 2: Standard dedup — remove already-uploaded transactions + const transactionsForDedup = afterCrossDedup.map((pt) => pt.transaction); const filterResult = filterDuplicateTransactions(transactionsForDedup, accountId); // Create a set of IDs to keep const idsToKeep = new Set(filterResult.transactions.map((tx) => getTransactionId(tx))); // Filter the processed transactions - const newProcessedTransactions = processedTransactions.filter((pt) => idsToKeep.has(getTransactionId(pt.transaction))); + const newProcessedTransactions = afterCrossDedup.filter((pt) => idsToKeep.has(getTransactionId(pt.transaction))); if (newProcessedTransactions.length === 0) { - const message = filterResult.duplicateCount > 0 - ? `All ${filterResult.duplicateCount} activity transactions have already been uploaded` + const totalSkipped = filterResult.duplicateCount + crossDedupCount; + const message = totalSkipped > 0 + ? `All activity transactions already uploaded or matched by orders (${totalSkipped} skipped)` : 'No new activity transactions to upload'; debugLog(message); return { @@ -481,13 +569,15 @@ async function processAndUploadActivityTransactions(accountId, accountName, from message, transactionsProcessed: 0, skippedDuplicates: filterResult.duplicateCount, + matchedByOrders: crossDedupCount, }; } if (progressDialog) { - const dupMsg = filterResult.duplicateCount > 0 - ? ` (${filterResult.duplicateCount} duplicates skipped)` - : ''; + const dupParts = []; + if (filterResult.duplicateCount > 0) dupParts.push(`${filterResult.duplicateCount} duplicates`); + if (crossDedupCount > 0) dupParts.push(`${crossDedupCount} matched by orders`); + const dupMsg = dupParts.length > 0 ? ` (${dupParts.join(', ')} skipped)` : ''; progressDialog.updateProgress( accountId, 'processing', @@ -532,9 +622,11 @@ async function processAndUploadActivityTransactions(accountId, accountName, from // Save to consolidated storage (shared with orders) saveUploadedTransactionsToConsolidated(accountId, transactionsWithDates); - const successMessage = filterResult.duplicateCount > 0 - ? `Successfully uploaded ${newProcessedTransactions.length} activity transactions (${filterResult.duplicateCount} duplicates skipped)` - : `Successfully uploaded ${newProcessedTransactions.length} activity transactions`; + const skipParts = []; + if (filterResult.duplicateCount > 0) skipParts.push(`${filterResult.duplicateCount} duplicates`); + if (crossDedupCount > 0) skipParts.push(`${crossDedupCount} matched by orders`); + const skipMsg = skipParts.length > 0 ? ` (${skipParts.join(', ')} skipped)` : ''; + const successMessage = `Successfully uploaded ${newProcessedTransactions.length} activity transactions${skipMsg}`; debugLog(successMessage); return { @@ -542,6 +634,7 @@ async function processAndUploadActivityTransactions(accountId, accountName, from message: successMessage, transactionsProcessed: newProcessedTransactions.length, skippedDuplicates: filterResult.duplicateCount, + matchedByOrders: crossDedupCount, }; } @@ -664,6 +757,9 @@ async function processAndUploadOrders(accountId, accountName, fromDate, monarchA saveUploadedTransactionsToConsolidated(accountId, transactionsWithDates); saveLastUploadDate(accountId, toDate, 'questrade'); + // Build order signatures for cross-source deduplication with activity trades + const signatures = buildOrderSignatures(executedOrders); + const successMessage = filterResult.duplicateCount > 0 ? `Successfully uploaded ${ordersToUpload.length} orders (${filterResult.duplicateCount} duplicates skipped)` : `Successfully uploaded ${ordersToUpload.length} orders`; @@ -674,6 +770,7 @@ async function processAndUploadOrders(accountId, accountName, fromDate, monarchA message: successMessage, ordersProcessed: ordersToUpload.length, skippedDuplicates: filterResult.duplicateCount, + orderSignatures: signatures, }; } @@ -714,9 +811,12 @@ async function processAndUploadTransactions(accountId, accountName, fromDate, pr activity: null, }; - // Process orders (trades) + // Process orders (trades) — also produces signatures for cross-source dedup + let orderSignatures = null; try { results.orders = await processAndUploadOrders(accountId, accountName, fromDate, monarchAccount.id, progressDialog); + // Extract order signatures for activity trade deduplication + orderSignatures = results.orders?.orderSignatures || null; } catch (orderError) { debugLog('Error processing orders:', orderError); results.orders = { @@ -726,9 +826,9 @@ async function processAndUploadTransactions(accountId, accountName, fromDate, pr }; } - // Process activity transactions (non-trades) + // Process activity transactions (all types, with cross-source trade dedup) try { - results.activity = await processAndUploadActivityTransactions(accountId, accountName, fromDate, monarchAccount.id, progressDialog); + results.activity = await processAndUploadActivityTransactions(accountId, accountName, fromDate, monarchAccount.id, progressDialog, orderSignatures); } catch (activityError) { debugLog('Error processing activity transactions:', activityError); results.activity = { @@ -740,7 +840,7 @@ async function processAndUploadTransactions(accountId, accountName, fromDate, pr // Combine results const totalProcessed = (results.orders?.ordersProcessed || 0) + (results.activity?.transactionsProcessed || 0); - const totalDuplicates = (results.orders?.skippedDuplicates || 0) + (results.activity?.skippedDuplicates || 0); + const totalDuplicates = (results.orders?.skippedDuplicates || 0) + (results.activity?.skippedDuplicates || 0) + (results.activity?.matchedByOrders || 0); const overallSuccess = (results.orders?.success ?? true) && (results.activity?.success ?? true); @@ -794,6 +894,7 @@ async function processAndUploadTransactions(accountId, accountName, fromDate, pr /** * Fetch and process ALL activity transactions for an account (no date filter) + * Includes all transaction types (trades, dividends, fees, etc.) * @param {string} accountId - Questrade account ID * @param {Object} progressDialog - Optional progress dialog * @returns {Promise} Array of processed transactions with details @@ -806,7 +907,7 @@ async function fetchAndProcessAllActivityTransactions(accountId, progressDialog progressDialog.updateProgress(accountId, 'processing', 'Loading all transactions from Questrade...'); } - // Fetch ALL transactions (no date filter) + // Fetch ALL transactions (no date filter, including trades) const transactions = await questradeApi.fetchAllTransactions(accountId); if (!transactions || transactions.length === 0) { @@ -814,34 +915,26 @@ async function fetchAndProcessAllActivityTransactions(accountId, progressDialog return []; } - debugLog(`Fetched ${transactions.length} total activity transactions`); - - // Filter out trades (handled by orders API) - const nonTradeTransactions = filterNonTradeTransactions(transactions); - - if (nonTradeTransactions.length === 0) { - debugLog('No non-trade transactions to process'); - return []; - } + debugLog(`Fetched ${transactions.length} total activity transactions (including trades)`); if (progressDialog) { progressDialog.updateProgress( accountId, 'processing', - `Loading transaction details (0/${nonTradeTransactions.length})...`, + `Loading transaction details (0/${transactions.length})...`, ); } // Fetch details for each transaction const processedTransactions = []; - for (let i = 0; i < nonTradeTransactions.length; i += 1) { - const tx = nonTradeTransactions[i]; + for (let i = 0; i < transactions.length; i += 1) { + const tx = transactions[i]; if (progressDialog && i % 10 === 0) { progressDialog.updateProgress( accountId, 'processing', - `Loading transaction details (${i + 1}/${nonTradeTransactions.length})...`, + `Loading transaction details (${i + 1}/${transactions.length})...`, ); } @@ -849,7 +942,7 @@ async function fetchAndProcessAllActivityTransactions(accountId, progressDialog let details = null; if (tx.transactionUrl) { try { - details = await questradeApi.fetchTransactionDetails(tx.transactionUrl); + details = await questradeApi.fetchTransactionDetails(tx.transactionUrl as string); } catch (detailError) { debugLog(`Failed to fetch details for transaction ${getTransactionId(tx)}:`, detailError); // Continue without details - rules can still process with basic info @@ -1186,5 +1279,6 @@ export default { filterExecutedOrders, filterDuplicateOrders, filterDuplicateTransactions, - filterNonTradeTransactions, + buildOrderSignatures, + filterActivityTradesMatchingOrders, }; diff --git a/test/services/questrade/tradeDedup.test.js b/test/services/questrade/tradeDedup.test.js new file mode 100644 index 0000000..14190e0 --- /dev/null +++ b/test/services/questrade/tradeDedup.test.js @@ -0,0 +1,289 @@ +/** + * Tests for cross-source trade deduplication between Orders API and Activity API + */ + +import transactionsService from '../../../src/services/questrade/transactions'; +import { cleanString } from '../../../src/services/questrade/transactionRules'; + +// Mock dependencies (same as transactions.test.js) +jest.mock('../../../src/api/questrade'); +jest.mock('../../../src/api/monarch'); +jest.mock('../../../src/utils/csv'); +jest.mock('../../../src/ui/toast'); +jest.mock('../../../src/mappers/category'); +jest.mock('../../../src/ui/components/categorySelector'); +jest.mock('../../../src/utils/transactionStorage', () => ({ + getTransactionIdsFromArray: jest.fn(() => new Set()), + getRetentionSettingsFromAccount: jest.fn(() => ({ days: 91, count: 1000 })), + mergeAndRetainTransactions: jest.fn((existing, newTx) => [...(existing || []), ...newTx]), +})); +jest.mock('../../../src/services/common/accountService', () => ({ + __esModule: true, + default: { + getMonarchAccountMapping: jest.fn(), + getAccountData: jest.fn(), + upsertAccount: jest.fn(), + updateAccountInList: jest.fn(), + }, +})); + +describe('Cross-Source Trade Deduplication', () => { + describe('buildOrderSignatures', () => { + const { buildOrderSignatures } = transactionsService; + + test('builds signatures from executed orders', () => { + const orders = [ + { + security: { symbol: 'VGRO.TO', displayName: 'Vanguard Growth ETF' }, + action: 'Buy', + updatedDateTime: '2024-10-09T14:30:00Z', + }, + { + security: { symbol: 'AMZN', displayName: 'Amazon.com' }, + action: 'Sell', + updatedDateTime: '2025-04-23T10:00:00Z', + }, + ]; + + const signatures = buildOrderSignatures(orders); + expect(signatures.size).toBe(2); + expect(signatures.has('vgro.to:2024-10-09:buy')).toBe(true); + expect(signatures.has('amzn:2025-04-23:sell')).toBe(true); + }); + + test('returns empty set for null/undefined input', () => { + expect(buildOrderSignatures(null).size).toBe(0); + expect(buildOrderSignatures(undefined).size).toBe(0); + expect(buildOrderSignatures([]).size).toBe(0); + }); + + test('skips orders with missing fields', () => { + const orders = [ + { security: { symbol: 'AAPL' }, action: 'Buy' }, // missing date + { security: {}, action: 'Sell', updatedDateTime: '2025-01-01T10:00:00Z' }, // missing symbol + { security: { symbol: 'MSFT' }, updatedDateTime: '2025-01-01T10:00:00Z' }, // missing action + { + security: { symbol: 'GOOG' }, + action: 'Buy', + updatedDateTime: '2025-01-01T10:00:00Z', + }, // valid + ]; + + const signatures = buildOrderSignatures(orders); + expect(signatures.size).toBe(1); + expect(signatures.has('goog:2025-01-01:buy')).toBe(true); + }); + + test('handles date without time component', () => { + const orders = [ + { + security: { symbol: 'AAPL' }, + action: 'Buy', + updatedDateTime: '2025-03-15', + }, + ]; + + const signatures = buildOrderSignatures(orders); + expect(signatures.has('aapl:2025-03-15:buy')).toBe(true); + }); + + test('deduplicates same-day same-symbol same-action orders', () => { + const orders = [ + { + security: { symbol: 'AAPL' }, + action: 'Buy', + updatedDateTime: '2025-01-15T10:00:00Z', + }, + { + security: { symbol: 'AAPL' }, + action: 'Buy', + updatedDateTime: '2025-01-15T14:00:00Z', + }, + ]; + + const signatures = buildOrderSignatures(orders); + // Same composite key — Set deduplicates automatically + expect(signatures.size).toBe(1); + }); + + test('differentiates buy and sell of same symbol on same day', () => { + const orders = [ + { + security: { symbol: 'AAPL' }, + action: 'Buy', + updatedDateTime: '2025-01-15T10:00:00Z', + }, + { + security: { symbol: 'AAPL' }, + action: 'Sell', + updatedDateTime: '2025-01-15T14:00:00Z', + }, + ]; + + const signatures = buildOrderSignatures(orders); + expect(signatures.size).toBe(2); + expect(signatures.has('aapl:2025-01-15:buy')).toBe(true); + expect(signatures.has('aapl:2025-01-15:sell')).toBe(true); + }); + }); + + describe('filterActivityTradesMatchingOrders', () => { + const { filterActivityTradesMatchingOrders } = transactionsService; + + test('removes activity trades that match order signatures', () => { + const orderSignatures = new Set(['vgro.to:2024-10-09:buy', 'amzn:2025-04-23:sell']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', symbol: 'VGRO.TO', transactionDate: '2024-10-09' }, + details: {}, + ruleResult: { category: 'Buy' }, + }, + { + transaction: { transactionType: 'Dividends', action: 'DIV', symbol: 'VGRO.TO', transactionDate: '2024-10-09' }, + details: {}, + ruleResult: { category: 'Dividends & Capital Gains' }, + }, + { + transaction: { transactionType: 'Trades', action: 'Sell', symbol: 'AMZN', transactionDate: '2025-04-23' }, + details: {}, + ruleResult: { category: 'Sell' }, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.transactions.length).toBe(1); + expect(result.matchedTradeCount).toBe(2); + // Only the dividend should remain + expect(result.transactions[0].transaction.transactionType).toBe('Dividends'); + }); + + test('keeps activity trades that do NOT match any order', () => { + const orderSignatures = new Set(['vgro.to:2024-10-09:buy']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', symbol: 'AOA', transactionDate: '2024-12-30' }, + details: {}, + ruleResult: { category: 'Buy' }, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.transactions.length).toBe(1); + expect(result.matchedTradeCount).toBe(0); + }); + + test('passes through all transactions when orderSignatures is null or empty', () => { + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', symbol: 'AAPL', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + { + transaction: { transactionType: 'Dividends', action: 'DIV', symbol: 'AAPL', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + ]; + + const resultNull = filterActivityTradesMatchingOrders(processedTransactions, null); + expect(resultNull.transactions.length).toBe(2); + expect(resultNull.matchedTradeCount).toBe(0); + + const resultEmpty = filterActivityTradesMatchingOrders(processedTransactions, new Set()); + expect(resultEmpty.transactions.length).toBe(2); + expect(resultEmpty.matchedTradeCount).toBe(0); + }); + + test('handles empty processedTransactions', () => { + const orderSignatures = new Set(['aapl:2025-01-15:buy']); + + const result = filterActivityTradesMatchingOrders([], orderSignatures); + expect(result.transactions.length).toBe(0); + expect(result.matchedTradeCount).toBe(0); + }); + + test('non-trade transactions always pass through', () => { + const orderSignatures = new Set(['aapl:2025-01-15:buy']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Dividends', action: 'DIV', symbol: 'AAPL', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + { + transaction: { transactionType: 'Deposits', action: 'DEP', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + { + transaction: { transactionType: 'FX conversion', action: 'FXT', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.transactions.length).toBe(3); + expect(result.matchedTradeCount).toBe(0); + }); + + test('uses symbol from details as fallback when not on transaction', () => { + const orderSignatures = new Set(['aapl:2025-01-15:buy']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', transactionDate: '2025-01-15' }, + details: { symbol: 'AAPL' }, + ruleResult: {}, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.transactions.length).toBe(0); + expect(result.matchedTradeCount).toBe(1); + }); + + test('case-insensitive matching', () => { + const orderSignatures = new Set(['aapl:2025-01-15:buy']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', symbol: 'AAPL', transactionDate: '2025-01-15' }, + details: {}, + ruleResult: {}, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.matchedTradeCount).toBe(1); + }); + + test('handles date with time component', () => { + const orderSignatures = new Set(['aapl:2025-01-15:buy']); + + const processedTransactions = [ + { + transaction: { transactionType: 'Trades', action: 'Buy', symbol: 'AAPL', transactionDate: '2025-01-15T14:30:00Z' }, + details: {}, + ruleResult: {}, + }, + ]; + + const result = filterActivityTradesMatchingOrders(processedTransactions, orderSignatures); + expect(result.matchedTradeCount).toBe(1); + }); + }); + + describe('formatTradeNotes (via cleanString import)', () => { + // cleanString is imported to verify it's available + test('cleanString helper works for trade data normalization', () => { + expect(cleanString(null)).toBe(''); + expect(cleanString(' AAPL ')).toBe('AAPL'); + expect(cleanString(undefined)).toBe(''); + }); + }); +}); \ No newline at end of file diff --git a/test/services/questrade/transactionRules.test.js b/test/services/questrade/transactionRules.test.js index b976691..5f4b0af 100644 --- a/test/services/questrade/transactionRules.test.js +++ b/test/services/questrade/transactionRules.test.js @@ -9,6 +9,7 @@ import { getTransactionId, formatOriginalStatement, formatTransactionNotes, + formatTradeNotes, formatFxNotes, applyTransactionRule, shouldFilterTransaction, @@ -234,9 +235,9 @@ describe('Questrade Transaction Rules', () => { }); describe('shouldFilterTransaction', () => { - test('filters out trades', () => { + test('does not filter trades (they are deduplicated instead)', () => { const tx = { transactionType: 'Trades' }; - expect(shouldFilterTransaction(tx)).toBe(true); + expect(shouldFilterTransaction(tx)).toBe(false); }); test('does not filter non-trade transactions', () => { @@ -608,6 +609,83 @@ describe('Questrade Transaction Rules', () => { }); }); + describe('Trades', () => { + test('Buy - Buy order', () => { + const tx = { transactionType: 'Trades', action: 'Buy', symbol: 'VGRO.TO' }; + const details = { + transactionType: 'Trades', + action: 'Buy', + symbol: 'VGRO.TO', + quantity: 4.0, + price: { currencyCode: 'CAD', amount: 36.3 }, + net: { currencyCode: 'CAD', amount: -145.21 }, + commission: 0.0, + description: 'VANGUARD GROWTH ETF PORTFOLIO ETF UNIT WE ACTED AS AGENT', + transactionDate: '2024-10-09', + settlementDate: '2024-10-11', + }; + const result = applyTransactionRule(tx, details); + expect(result.category).toBe('Buy'); + expect(result.merchant).toBe('VGRO.TO'); + expect(result.ruleId).toBe('trades-buy'); + expect(result.originalStatement).toBe('Trades:Buy:VGRO.TO'); + expect(result.notes).toContain('VANGUARD GROWTH ETF'); + expect(result.notes).toContain('Filled 4 @ 36.3'); + expect(result.notes).toContain('Total: 145.21 CAD'); + expect(result.notes).toContain('Settlement Date: 2024-10-11'); + }); + + test('Sell - Sell order', () => { + const tx = { transactionType: 'Trades', action: 'Sell', symbol: 'AMZN' }; + const details = { + transactionType: 'Trades', + action: 'Sell', + symbol: 'AMZN', + quantity: 50.0, + price: { currencyCode: 'USD', amount: 271.6984 }, + net: { currencyCode: 'USD', amount: 13584.92 }, + commission: 0.0, + description: 'AMAZON.COM INC WE ACTED AS AGENT', + transactionDate: '2025-04-23', + settlementDate: '2025-04-25', + }; + const result = applyTransactionRule(tx, details); + expect(result.category).toBe('Sell'); + expect(result.merchant).toBe('AMZN'); + expect(result.ruleId).toBe('trades-sell'); + }); + + test('Buy with no details (list data only)', () => { + const tx = { + transactionType: 'Trades', + action: 'Buy', + symbol: 'AOA', + quantity: 1.0, + price: { currencyCode: 'USD', amount: 76.9799 }, + net: { currencyCode: 'USD', amount: -76.98 }, + description: 'ISHARES CORE AGGRESSIVE ALLOCATION FUND ETF WE ACTED AS AGENT', + transactionDate: '2024-12-30', + }; + const result = applyTransactionRule(tx, null); + expect(result.category).toBe('Buy'); + expect(result.merchant).toBe('AOA'); + expect(result.ruleId).toBe('trades-buy'); + }); + + test('Unknown trade action uses trades-fallback', () => { + const tx = { transactionType: 'Trades', action: 'Short' }; + const result = applyTransactionRule(tx, null); + expect(result.category).toBe('Investment'); + expect(result.ruleId).toBe('trades-fallback'); + }); + + test('Trade with no symbol uses Unknown Security', () => { + const tx = { transactionType: 'Trades', action: 'Buy' }; + const result = applyTransactionRule(tx, null); + expect(result.merchant).toBe('Unknown Security'); + }); + }); + describe('Fallback', () => { test('Unknown transaction type uses fallback', () => { const tx = { transactionType: 'Unknown Type', action: 'XYZ' }; @@ -629,8 +707,8 @@ describe('Questrade Transaction Rules', () => { describe('Rules Array', () => { test('has expected number of rules', () => { - // 22 rules + 1 fallback - expect(QUESTRADE_TRANSACTION_RULES.length).toBe(23); + // 22 original rules + 3 trade rules + 1 fallback = 26 + expect(QUESTRADE_TRANSACTION_RULES.length).toBe(26); }); test('all rules have required properties', () => {