Mobile: add trade history CSV export - #30839
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThe changes add 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e96ab734-15c9-47b5-bedd-aca2f5191f7b
⛔ Files ignored due to path filters (7)
packages/icons/src/generated/icons/CoinSlash.tsxis excluded by!**/generated/**suite-common/icons/cryptoAssets/cryptoIcons/hype.svgis excluded by!**/*.svgsuite-common/icons/cryptoAssets/cryptoIcons/rhc.svgis excluded by!**/*.svgsuite-common/icons/cryptoAssets/networkIcons/hype.svgis excluded by!**/*.svgsuite-common/icons/cryptoAssets/networkIcons/rhc.svgis excluded by!**/*.svgsuite-common/icons/iconFontsMobile/TrezorSuiteIcons.ttfis excluded by!**/*.ttfyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
suite-common/icons/generateIconFont.tssuite-common/icons/iconFontsMobile/TrezorSuiteIcons.jsonsuite-common/trading/src/index.tssuite-common/trading/src/utils/tradeHistoryExportUtils.test.tssuite-common/trading/src/utils/tradeHistoryExportUtils.tssuite-native/intl/src/messages.tssuite-native/module-trading/src/screens/TradingHistoryScreen.tsxsuite-native/trading-history/package.jsonsuite-native/trading-history/src/components/TradingHistoryExportButton.test.tsxsuite-native/trading-history/src/components/TradingHistoryExportButton.tsxsuite-native/trading-history/src/exportTradingHistoryCsv.tssuite-native/trading-history/src/hooks/useTradingHistoryCsvColumnLabels.tssuite-native/trading-history/src/index.ts
| jest.mock('@suite-native/alerts', () => ({ | ||
| useAlert: () => ({ showAlert: mockShowAlert, hideAlert: mockHideAlert }), | ||
| })); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Spread requireActual in the @suite-native/alerts mock.
This mock replaces the whole module and exposes only useAlert. The @suite-native/toasts mock on lines 20-23 spreads jest.requireActual. If renderWithTradingHistoryProvider or any component in the rendered tree later imports another member of @suite-native/alerts, that import resolves to undefined and the render fails with an error that does not point at this mock. Match the toasts pattern.
♻️ Proposed change
jest.mock('`@suite-native/alerts`', () => ({
+ ...jest.requireActual('`@suite-native/alerts`'),
useAlert: () => ({ showAlert: mockShowAlert, hideAlert: mockHideAlert }),
}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jest.mock('@suite-native/alerts', () => ({ | |
| useAlert: () => ({ showAlert: mockShowAlert, hideAlert: mockHideAlert }), | |
| })); | |
| jest.mock('`@suite-native/alerts`', () => ({ | |
| ...jest.requireActual('`@suite-native/alerts`'), | |
| useAlert: () => ({ showAlert: mockShowAlert, hideAlert: mockHideAlert }), | |
| })); |
| newFile.write(content); | ||
| } else if (Platform.OS === 'ios') { | ||
| await Sharing.shareAsync(cachedFile.uri, { | ||
| mimeType: CSV_MIME_TYPE, |
|
|
||
| const result = await exportTradingHistoryCsv(csvContent); | ||
|
|
||
| setIsExporting(false); |
There was a problem hiding this comment.
🧠 Resetting the loading state in finally block, just to be sure as rabbit suggested
| const newFile = dir.createFile(fileName, CSV_MIME_TYPE); | ||
| newFile.write(content); | ||
| } else if (Platform.OS === 'ios') { | ||
| await Sharing.shareAsync(cachedFile.uri, { |
There was a problem hiding this comment.
🥲 Expo sharing is incomplete and they are missing flag when user cancel the sharing. Currently when I press export and then cancel the sharing dialog the app shows toast Trade History Exported
| setIsExporting(true); | ||
|
|
||
| // wait for alert animation to finish before starting the export, 500 ms ought to be enough for anybody. | ||
| await new Promise(resolve => setTimeout(resolve, 500)); |
There was a problem hiding this comment.
🧠 This one is interesting and i have no idea why its acting weird without it
There was a problem hiding this comment.
I even tried to slow animations on emulator to see whats happening. It was ... interesting experience.
|
✅ Previously successful run of [Test] suite-native Android E2E workflow has been found. |
| if (cachedFile.exists) { | ||
| cachedFile.delete(); | ||
| } | ||
|
|
There was a problem hiding this comment.
This creates a timestamped cache file containing exact amounts and transaction IDs for every export and never deletes it. Android does not use this cached file at all, so successful and cancelled attempts both leave unnecessary confidential copies behind; iOS also retains its temporary share file after completion. Branch on the platform before creating the cache file, and delete the iOS temporary file in finally after shareAsync settles.
| const trades = selectDeviceTradingTradesOrderedByDate(state); | ||
| const csvContent = prepareTradingHistoryCsv(columnLabels)(state, trades); | ||
|
|
||
| result = await exportTradingHistoryCsv(csvContent); |
There was a problem hiding this comment.
The finally block resets loading, but it does not convert a thrown selector/CSV-preparation error into the initialized exportFailed result. Any exception in this try skips the result handling below and rejects the alert callback, so no failure toast appears. Add a catch that assigns { success: false, reason: 'exportFailed' }, then retain this finally. Please add a rejection test as well.
3 participants
Description
Adds a CSV export action to the mobile Trade history screen (a share icon in the header, hidden when there are no trades). Pressing it builds a CSV of all trades for the active device and opens the native share sheet (iOS) / directory picker (Android). The CSV-preparation logic lives in
@suite-common/trading(prepareTradingHistoryCsv+ a platform-agnosticbuildTradingHistoryCsvcore with injected provider/ticker resolvers) so the desktop app can reuse it (#29956). Columns follow the Figma spec and normalize buy/sell/exchange into a common spend/receive shape:Typemapsexchange→swap, fiat legs leave ticker=currency-code and network empty, provider is shown in human-readable form, and values are formula-injection-safe.Notes for QA
Open Trading → Trade history on mobile and tap the share icon in the header. Verify the exported
.csvopens with the 14 spec columns and correct values across buy, sell and swap trades; confirm the button is hidden when there is no trade history. Note: for swap (exchange) trades the "Spend transaction ID" column is intentionally empty (the exchange trade record has no dedicated user-send tx hash).Related Issue
Resolve #29954
Screenshots:
Simulator.Screen.Recording.-.iPhone.17.-.2026-08-04.at.23.21.38.mov
🌐 Preview deployments
🌐 Suite Web preview: https://dev.suite.sldev.cz/suite-web/sofia-local/web/
🔍 Currents Test Results
🔍 Suite web test results: View in Currents
🔍 Suite desktop test results: View in Currents
🔍 Suite native android test results: View in Currents
🔒 Quarantined E2E Tests
Trezor Suite (desktop) — 2 test(s)
Updated: 2026-08-05T08:37:51.378Z • 2 test(s) total
Trezor Suite (web) — 3 test(s)
Updated: 2026-08-05T08:37:56.936Z • 3 test(s) total