Skip to content

FE ManageBoxes: remove state-specific preloads (Solution B option 2)#2702

Open
Copilot wants to merge 2 commits into
masterfrom
copilot/fe-manage-boxes-performance-update
Open

FE ManageBoxes: remove state-specific preloads (Solution B option 2)#2702
Copilot wants to merge 2 commits into
masterfrom
copilot/fe-manage-boxes-performance-update

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 4, 2026

Summary

Applies a partial performance improvement to BoxesView by removing the three state-specific preload requests while keeping the full background fetch.

Previously, on initial mount the BoxesView component fired 4–5 parallel GraphQL requests simultaneously:

  1. One via useBackgroundQuery (50 boxes, current filter)
  2. Three state-specific preloads in useEffect (InStock, Donated, Scrap — each 50 boxes)
  3. One unfiltered full fetch (all boxes, no pagination limit)

Each request triggered ~15 SQL queries on the backend, resulting in ~75 SQL queries per page load. This change removes the three state-specific preloads (#2), leaving the useBackgroundQuery fetch and the full background fetch — reducing the backend load from ~75 to ~30 SQL queries on initial page load.

Changes

  • BoxesView.tsx: removed the state-specific preload loop from the useEffect (the InStock/Donated/Scrap parallel fetches); the full unfiltered apolloClient.query background fetch, isBackgroundFetchOfBoxesLoading state, and hasExecutedInitialFetchOfBoxes prop are retained
  • BoxesTable.tsx: no interface changes; isBackgroundFetchOfBoxesLoading and hasExecutedInitialFetchOfBoxes remain in the component props and all related UI conditionals (total-summary skeleton, "Data unavailable" fallback, autoResetSelectedRows, pagination skeleton) are preserved
  • App.tsx: hasExecutedInitialFetchOfBoxes ref and prop remain unchanged
  • BoxesView.test.tsx: updated mock arrays to include the initial useBackgroundQuery mock + full background fetch mock + actionsQuery (removed the three state-specific preload mocks); updated the boxesQuery factory to return boxes for state="ALL" so the invalid-state-id test still finds data
  • BoxesViewActions.test.tsx: removed the three state-specific preload mocks from every test; added the boxesQuery({ paginationInput: 100000 }) full background fetch mock; for MarkedForShipment and Donated tests, added the corresponding full background fetch mock with the appropriate state filter

Verification

  • pnpm check-types passes
  • pnpm lint:all passes
  • pnpm format:check:all passes
  • pnpm -C front build passes
  • ✅ All 257 frontend tests pass

@sentry
Copy link
Copy Markdown

sentry Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.80%. Comparing base (2b8fa90) to head (2997ba9).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2702      +/-   ##
==========================================
- Coverage   76.83%   76.80%   -0.04%     
==========================================
  Files         300      300              
  Lines       22233    22203      -30     
  Branches     2245     2241       -4     
==========================================
- Hits        17082    17052      -30     
  Misses       5104     5104              
  Partials       47       47              
Flag Coverage Δ
backend 99.65% <ø> (ø)
frontend 69.05% <ø> (-0.06%) ⬇️
sharedComponents 68.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread front/src/views/Boxes/BoxesView.tsx
Copilot AI changed the title FE ManageBoxes: eliminate preloading (Solution B option 1) FE ManageBoxes: remove state-specific preloads (Solution B option 2) May 4, 2026
Copilot finished work on behalf of pylipp May 4, 2026 16:27
Copilot AI requested a review from pylipp May 4, 2026 16:27
@pylipp pylipp requested a review from Copilot May 4, 2026 16:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reduces initial backend load when mounting Manage Boxes by removing the three state-specific preload GraphQL requests (InStock/Donated/Scrap) while keeping the initial paginated query plus the full background fetch used to populate totals/cache.

Changes:

  • Removed the state-specific preload loop from BoxesView’s initial mount effect, leaving only the full background fetch.
  • Updated BoxesView and actions test suites to reflect the new request pattern (initial PAGE_SIZE background query + full background fetch + actions/options query).
  • Adjusted test query factories/mocks (notably handling of state="ALL") so “invalid state id” scenarios still yield data.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Show a summary per file
File Description
front/src/views/Boxes/BoxesView.tsx Removes state-specific preload queries; keeps initial background query + full background fetch logic.
front/src/views/Boxes/BoxesView.test.tsx Updates mocks to match the new query sequence; ensures invalid state_ids test still returns data via state="ALL".
front/src/views/Boxes/BoxesViewActions.test.tsx Removes preload mocks across actions tests and adds/adjusts full-background-fetch mocks per state where needed.
front/src/views/Boxes/components/BoxesTable.tsx No functional changes noted in review; continues to use isBackgroundFetchOfBoxesLoading / hasExecutedInitialFetchOfBoxes for UI states.
front/src/App.tsx Continues to pass the persistent hasExecutedInitialFetchOfBoxes ref into BoxesView to avoid repeating the expensive full fetch on navigation.

@pylipp pylipp marked this pull request as ready for review May 5, 2026 05:46
@pylipp
Copy link
Copy Markdown
Contributor

pylipp commented May 5, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants