[NoQA] [Insights] Show already-loaded charts on Insights and Home/Spend when fetched by different actions by mhawryluk · Pull Request #102311 · Expensify/App

mhawryluk · 2026-09-25T14:44:07Z

Explanation of Change

The Insights page and the Home insights widget can load the same chart with different commands (GetInsights and Search), and both write it to the same snapshot key. Each could only partly use what the other loaded: Insights only once its own GetInsights response had named the snapshot, and Home not at all, since GetInsights snapshots had no state stamp and reported the wrong search metadata.

getInsights now stamps each graph's snapshot with state: loaded in successData, the same way search does in finallyData. This lets Search readers like the Home widget treat those snapshots as loaded. Insights charts read their snapshot under their own query's hash instead of going through the dashboard record. A chart counts as loaded once the snapshot is loaded, not matter what command sent it.

The page now decides its state from every visible chart instead of only the headline. It shows the full-page offline state only when no chart has anything to show, and otherwise renders the charts, each with its own offline, loading or empty state. It shows the full-page empty state only when every chart is loaded without rows. Nothing is requested or read until the saved filters resolve, and each chart shows its loading state until then.

Fixed Issues

$ #102515
PROPOSAL: N/A

Tests

Precondition: the insightsPage beta is enabled and the account is an admin of a workspace with expenses this year.

Open Insights and wait for all charts. Open Home and verify the insights widget shows its chart right away, without the loading state. (on web there still might be a short loading spinner shown when switching between charts of different types, like between bar chart and a line chart, because of how Skia files are lazy loaded on web)
Clear cache and restart. Open Home and wait for Spend over time to load, then open Insights. Verify Spend over time shows right away while the other charts load.
On Insights, change the Spend over time group-by. Verify the other charts keep showing their data and no chart flashes the loading state when the new data arrives.
Change the group-by to Quarter for example, then reload the page. Verify each chart shows its loading state and then quarterly data (on headline), with no monthly data shown in between.
Set the date filter to a range with no expenses. Verify the full-page empty state is shown.

Offline tests

Clear cache and restart. Open Home and wait for the insight chart to load.
Go offline and open Insights.
Verify that particular chart shows and the other charts show the offline state.
Clear cache and restart, go offline and open Insights. Verify the full-page offline state is shown.
Open Insights online and wait for all charts, then go offline. Verify every chart stays on screen.

QA Steps

No tests.

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Nagranie.z.ekranu.2026-09-29.o.11.11.33.mov

…fferent commands

codecov · 2026-09-25T14:48:03Z

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.

Files with missing lines	Coverage Δ
src/libs/SearchUIUtils.ts	`72.25% <ø> (ø)`
src/libs/resolveInsightsChartData.ts	`100.00% <100.00%> (ø)`
src/pages/Insights/dashboardSpecs.ts	`25.00% <ø> (ø)`
src/pages/Insights/insightsQueries.ts	`92.85% <100.00%> (+0.54%)`	⬆️
src/pages/Insights/resolveDashboardState.ts	`100.00% <100.00%> (ø)`
src/components/Search/SearchChartView.tsx	`0.00% <0.00%> (ø)`
src/pages/home/InsightsSection/useInsightData.ts	`0.00% <0.00%> (-36.37%)`	⬇️
src/libs/actions/Insights.ts	`11.76% <0.00%> (-0.74%)`	⬇️
src/pages/Insights/charts/InsightsChartWidget.tsx	`0.00% <0.00%> (ø)`
...es/home/InsightsSection/InsightsSectionContent.tsx	`0.00% <0.00%> (ø)`
... and 2 more
... and 11 files with indirect coverage changes

…ts snapshots, until backend fixes the response

mhawryluk · 2026-09-28T10:59:32Z

@MelvinBot review

MelvinBot · 2026-09-28T11:03:30Z

Review

I found one blocking issue. The rest of the change looks sound.

🔴 A failed GetInsights response now shows the "no results" state instead of the error state, on both Insights and Home.

finallyData runs for failed responses too. The new stamp marks every chart snapshot state: loaded at src/libs/actions/Insights.ts:43-49. But failureData only writes errors to the dashboard record, not to the snapshots. So after a non-200 response, each snapshot has no data, no errors, and state: loaded. isSearchDataLoaded then treats it as settled with no results.

What the user sees with a fresh cache (your test step 7):

Insights: every chart counts as loaded, so hasChartData is true and the full-page error is skipped at resolveDashboardState.ts:47. No chart has rows, so the page shows EMPTY ("filters matched nothing").
Home widget: getInsightState finds no snapshot errors, so it shows EMPTY instead of ERROR.

search() avoids this because its failureData writes errors to the snapshot (Search.ts:844-859).

Suggested fix: Move the stamp from finallyData to successData, so only a successful response marks snapshots loaded. If you would rather match search() and add snapshot errors to failureData, also exclude errored snapshots from hasChartData. Otherwise step 7 shows per-chart errors instead of the full-page error. Either way, please add a getDashboardState test for a failed request that went through this path.

Other things I checked

groupBy = chart.groupBy ?? filters.groupBy matches what applyInsightsFilters puts into the query, so removing queryJSON?.groupBy is safe.
SearchChartView only reads queryJSON in handleItemPress, which the new guard covers.
Group-by change (step 3): supporting charts keep their query hash, and the snapshots now carry the stamped hash and state. isSearchDataLoaded accepts them while the new dashboard record is still empty, so they don't flash the loading state.
Keying useMultipleSnapshots output by String(hash) and reading it with a numeric hash works, because JS coerces object keys to strings.

view run

OlGierd03

Looks great overall! 🚀

melvin-bot · 2026-09-29T09:19:59Z

@truph01 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]

JmillsExpensify

Looks great!

mhawryluk · 2026-09-30T10:21:20Z

@truph01 I've addressed the comments

cristipaval · 2026-09-30T12:01:02Z

        return INSIGHTS_DASHBOARD_STATE.NO_EXPENSES;
    }
-    if (headlineSnapshot?.data && !Object.keys(headlineSnapshot.data).some(isGroupEntry)) {
+    if (chartStates.every(({isLoaded}) => isLoaded)) {


Could we exclude snapshots with errors here, so a failed Home Search doesn’t count as an empty result when opening Insights offline?

good call. I changed isLoaded to isSearchDataLoaded(snapshot, queryJSON) && isEmptyObject(snapshot?.errors)

…exists and all snapshots failed

luacmartins

LGTM. @cristipaval @aldo-expensify all yours

luacmartins · 2026-09-30T14:59:07Z

@mhawryluk conflicts

truph01 · 2026-09-30T16:01:25Z

@mhawryluk We have conflicts again

github-actions · 2026-09-30T19:05:10Z

🚧 luacmartins has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-09-30T19:09:52Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #102311.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-c7f37e8-0ca1693-84df9b6e0c3fbf6eac62e247473dd73e57297dbe/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-c7f37e8-0ca1693-2a474f54e0d6f87b68f34465adfceee352769222/index.html

Web 🕸️
https://102311.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-10-01T00:02:46Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.5.0-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot · 2026-10-01T00:07:14Z

No help site update needed. This PR changes how Insights charts load and show offline or empty states, and the Insights page and Home insights widget are still gated behind the insightsPage beta (CONST.BETAS.INSIGHTS_PAGE), which this PR doesn't remove.

view run

OSBotify · 2026-10-01T22:48:25Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.5.0-5 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	failure ❌

Bundle Size Analysis (Sentry):

Android

[Insights] Show loaded charts on Insights and Home when fetched by di…

b836b92

…fferent commands

melvin-bot Bot assigned mhawryluk Sep 25, 2026

mhawryluk added 4 commits September 28, 2026 11:33

Merge branch 'main' into insights/improve-chart-loading-state

43f32df

Decide Insights page state from every chart, not just the headline

a4f1929

Bring back special treatment for checking loading state for GetInsigh…

bafeee3

…ts snapshots, until backend fixes the response

Show an error state only if there is no data fetched already

27430a1

mhawryluk added 3 commits September 28, 2026 13:09

Small fixes

4343fd2

Switch finallyData to successData

01c40fc

Revert changes to the error state

73db7c5

OlGierd03 approved these changes Sep 28, 2026

View reviewed changes

Comment thread src/libs/actions/Insights.ts Resolved

Update comments

8684aab

mhawryluk mentioned this pull request Sep 29, 2026

[Due for payment 2026-10-01] [Insights][R1][App] Render the Spend dashboard #100652

Closed

MelvinBot mentioned this pull request Sep 29, 2026

[Due for payment 2026-10-08] [Insights][R1][App] Share loaded chart snapshots between the Insights page and Home widget #102515

Closed

mhawryluk added 2 commits September 29, 2026 10:57

Remove special handling for GetInsights snapshots after backend fix

2b71853

Remove unused export

cb0125d

mhawryluk marked this pull request as ready for review September 29, 2026 09:19

mhawryluk requested review from a team as code owners September 29, 2026 09:19

melvin-bot Bot requested review from garrettmknight and truph01 September 29, 2026 09:19

melvin-bot Bot removed request for a team September 29, 2026 09:19

cristipaval requested review from JmillsExpensify and removed request for garrettmknight September 29, 2026 09:21

JmillsExpensify previously approved these changes Sep 29, 2026

View reviewed changes

luacmartins requested a review from cristipaval September 29, 2026 16:05

truph01 approved these changes Sep 30, 2026

View reviewed changes

melvin-bot Bot requested review from aldo-expensify and luacmartins September 30, 2026 11:27

cristipaval reviewed Sep 30, 2026

View reviewed changes

mhawryluk added 3 commits September 30, 2026 15:24

Don't treat failed snapshots as loaded in getDashboardState

5d3da45

FIx per-chart error states instead of full-page when dashboard entry …

67869e4

…exists and all snapshots failed

Fix offline instead of no expenses when all snapshots are evicted

eef5a05

luacmartins previously approved these changes Sep 30, 2026

View reviewed changes

cristipaval previously approved these changes Sep 30, 2026

View reviewed changes

Merge branch 'main' into insights/improve-chart-loading-state

fa00934

mhawryluk dismissed stale reviews from cristipaval and luacmartins via fa00934 September 30, 2026 15:02

cristipaval previously approved these changes Sep 30, 2026

View reviewed changes

luacmartins previously approved these changes Sep 30, 2026

View reviewed changes

Merge branch 'main' into insights/improve-chart-loading-state

8976c7e

mhawryluk dismissed stale reviews from luacmartins and cristipaval via 8976c7e September 30, 2026 17:11

cristipaval approved these changes Sep 30, 2026

View reviewed changes

aldo-expensify approved these changes Sep 30, 2026

View reviewed changes

luacmartins approved these changes Sep 30, 2026

View reviewed changes

luacmartins merged commit c7f37e8 into Expensify:main Sep 30, 2026
49 of 71 checks passed

OSBotify mentioned this pull request Sep 30, 2026

Deploy Checklist: New Expensify 2026-09-30 #102701

Closed

70 tasks done

Conversation

mhawryluk commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Sep 25, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mhawryluk commented Sep 28, 2026

Uh oh!

MelvinBot commented Sep 28, 2026

Review

Uh oh!

OlGierd03 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

melvin-bot Bot commented Sep 29, 2026

Uh oh!

JmillsExpensify left a comment

Choose a reason for hiding this comment

Uh oh!

mhawryluk commented Sep 30, 2026

Uh oh!

cristipaval Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

mhawryluk Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

luacmartins left a comment

Choose a reason for hiding this comment

Uh oh!

luacmartins commented Sep 30, 2026

Uh oh!

truph01 commented Sep 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 30, 2026

Uh oh!

github-actions Bot commented Sep 30, 2026

Uh oh!

OSBotify commented Oct 1, 2026

Uh oh!

MelvinBot commented Oct 1, 2026

Uh oh!

OSBotify commented Oct 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

mhawryluk commented Sep 25, 2026 •

edited

Loading

codecov Bot commented Sep 25, 2026 •

edited by github-actions Bot

Loading