Repository navigation
Add local CSV downloads to Insights charts - #104272
Conversation
|
🚧 luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Looks solid to me. Thanks for the quick work on this one! |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@neil-marcellini Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Only additional thought from me is: Should we should skip the save dialogue? I kind of think we should and just save it to the default download location of the browser. |
|
@JmillsExpensify this already uses the browser's default download flow. If "Ask where to save each file" is enabled in the browser settings, it will still show a save dialog. We can't override that from the app. |
|
Updated |
|
I will check it soon |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-10-20.at.10.14.17.movAndroid: mWeb ChromeScreen.Recording.2026-10-20.at.10.15.45.moviOS: HybridAppScreen.Recording.2026-10-20.at.10.19.08.moviOS: mWeb SafariScreen.Recording.2026-10-20.at.10.20.20.movMacOS: Chrome / SafariScreen.Recording.2026-10-20.at.10.04.07.mov |
| if (getDateRange && dateRange.start && dateRange.end) { | ||
| const rowsByDate = new Map(rows.map((row) => [row.label, row])); | ||
| rows = getPeriodStarts(groupBy, {start: parseISO(dateRange.start), end: parseISO(dateRange.end)}).map((date) => { | ||
| const label = format(date, CONST.DATE.FNS_FORMAT_STRING); | ||
| return rowsByDate.get(label) ?? {label, count: 0, total: 0, currency}; | ||
| }); | ||
| } |
There was a problem hiding this comment.
| if (getDateRange && dateRange.start && dateRange.end) { | |
| const rowsByDate = new Map(rows.map((row) => [row.label, row])); | |
| rows = getPeriodStarts(groupBy, {start: parseISO(dateRange.start), end: parseISO(dateRange.end)}).map((date) => { | |
| const label = format(date, CONST.DATE.FNS_FORMAT_STRING); | |
| return rowsByDate.get(label) ?? {label, count: 0, total: 0, currency}; | |
| }); | |
| } | |
| if (getDateRange && dateRange.start && dateRange.end) { | |
| const start = parseISO(dateRange.start); | |
| // Don't pad periods that haven't happened yet: a future period has no data yet, so a 0 row would read as "no spend". | |
| const parsedEnd = parseISO(dateRange.end); | |
| const end = parsedEnd > new Date() ? new Date() : parsedEnd; | |
| // Skip zero-fill for an invalid or inverted range (start > end, which also covers a range that begins in the future). | |
| if (isValid(start) && isValid(end) && start <= end) { | |
| const rowsByDate = new Map(rows.map((row) => [row.label, row])); | |
| rows = getPeriodStarts(groupBy, {start, end}).map((date) => { | |
| const label = format(date, CONST.DATE.FNS_FORMAT_STRING); | |
| return rowsByDate.get(label) ?? {label, count: 0, total: 0, currency}; | |
| }); | |
| } | |
| } |
Screen.Recording.2026-10-17.at.16.00.50.mov
@luacmartins I found an interesting case. Nov and Dec are periods that haven't happened yet, but they show up in the CSV as "periods with zero spend", when in reality there is no data for them yet.
Steps to reproduce:
- Set the Date filter to a custom range: Oct 1, 2026 to Dec 31, 2026.
- Download the CSV from the "Spend over time" chart and open it.
Actual result: the CSV contains rows for Nov and Dec with 0 expenses and 0 total:
Expected result: I think future periods shouldn't appear. They haven't happened yet, so a 0 row reads as "no spend" instead of "no data yet".
There was a problem hiding this comment.
Expected result: I think future periods shouldn't appear.
I'm not sure. It's possible for users to create expenses in future dates. @JmillsExpensify what's your take here?
There was a problem hiding this comment.
Future periods should appear, if they have future dated expenses. I'm not sure it's a bug for this PR really. It's more of a question of whether we should be showing an empty state in that case, versus plotting $0. Isn't that what this PR is doing though? #104337
There was a problem hiding this comment.
Yea, that makes sense. So NAB for this PR
@luacmartins For a date range with no spend (screenshot), Spend over time still shows "Download CSV" in its menu. The chart renders a flat $0 line, while Top spenders and Top merchants correctly show "Chart unavailable" with no menu. The exported CSV would only contain zero rows. Should Download CSV be hidden here too, or is this chart intentionally not treated as empty? |
| return; | ||
| } | ||
|
|
||
| const content = buildInsightsChartCSV({data, groupBy, dateRange: getDateFilterRange(queryJSON), currency: filters.groupCurrency, getCurrencyDecimals, translate}); |
There was a problem hiding this comment.
Screen.Recording.2026-10-17.at.16.59.51.mov
the same way View on Spend builds its query:
@luacmartins Issue says the ranking export should follow View on Spend (no limit), but we export data, which is capped at 5 — is top 5 intended, or should it include every group with spend?
There was a problem hiding this comment.
Yea, I think top 5 is intended here since that's what the chart shows
There was a problem hiding this comment.
Agreed, top 5 is intended.
| } | ||
|
|
||
| const content = buildInsightsChartCSV({data, groupBy, dateRange: getDateFilterRange(queryJSON), currency: filters.groupCurrency, getCurrencyDecimals, translate}); | ||
| localFileDownload(`Expensify-${chart.graphKey}.csv`, content, translate); |
There was a problem hiding this comment.
The download reuses our existing export flow; no new component. Small files download immediately, and larger files are delivered by Concierge, as with other exports.
@luacmartins The issue says the download should reuse the existing export flow (Concierge for large files), but this generates the file locally with localFileDownload — is the local approach intended, e.g. to support offline export?
There was a problem hiding this comment.
Yea, we changed the approach to local only since that allows for offline export of the data that's already available. cc @JmillsExpensify
There was a problem hiding this comment.
Correct. We changed it and I never updated the OP, sorry.
Makes sense, thanks. |
|
@luacmartins could you sync with main? |
I have a fix PR up that makes the Spend over time correctly recognized as empty in this case: #104337 |
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Looks good thanks
Done |
|
@huult @neil-marcellini this is ready for you again |
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Chat it with Jason, approving this one since Neil already approved, just so we can move it ahead.
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.5.15-1 🚀
|
|
No help site update is needed: the new Download CSV chart option lives only on the Insights dashboard, which is still gated behind the Details
|
8 participants
Explanation of Change
Add Download CSV below View on Spend in each Insights chart menu. Generate the file locally from the loaded groups instead of exporting individual expenses or queuing a server export.
Export current-period totals, expense counts, and currency. Rankings include the displayed top five groups. Time-series exports identify buckets by their start date and include zero-spend periods across the selected date range. Comparison columns are excluded.
Fixed Issues
$ #104209
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari