Skip to content

fix(spreadsheet): prevent duplicate equipment fetch#3830

Open
TheMaskedTurtle wants to merge 3 commits intomainfrom
jorism/fix/spreadsheet-multi-fetches
Open

fix(spreadsheet): prevent duplicate equipment fetch#3830
TheMaskedTurtle wants to merge 3 commits intomainfrom
jorism/fix/spreadsheet-multi-fetches

Conversation

@TheMaskedTurtle
Copy link
Contributor

PR Summary

As fetching elements for spreadsheet is heavy on resource we want to limit them as much as possible. We noticed some cases of fetch duplication that were not necessary :

  • when several spreadsheet are of the same type we want to fetch this type only once in automatic re-fetch
  • when tab is active we don't want to fetch if automatic re-fetch is already doing it

So this PR fixes these two cases.

- add isFetching check
- add a set on equipment types

Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>
Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 302a94b4-af8d-444a-8366-918ecd51fd35

📥 Commits

Reviewing files that changed from the base of the PR and between a40f0f4 and 807d7a9.

📒 Files selected for processing (2)
  • src/components/spreadsheet-view/hooks/use-spreadsheet-equipments.ts
  • src/components/spreadsheet-view/spreadsheet/spreadsheet-content/spreadsheet-content.tsx

📝 Walkthrough

Walkthrough

Two changes prevent duplicate equipment data fetching: the hook now deduplicates spreadsheet equipment types using a Set, and the component adds an isFetching condition to prevent redundant fetches when the hook is already re-fetching data.

Changes

Cohort / File(s) Summary
Type Deduplication in Hook
src/components/spreadsheet-view/hooks/use-spreadsheet-equipments.ts
Modified applyToAllTypes to iterate over a Set of tablesDefinitions[*].type values instead of a potentially duplicated array, ensuring each equipment type is processed at most once per invocation.
Fetch Guard in Component
src/components/spreadsheet-view/spreadsheet/spreadsheet-content/spreadsheet-content.tsx
Added !equipments.isFetching condition to tab-open fetch logic to prevent concurrent fetch calls when the hook is already performing an automatic re-fetch; updated useEffect dependencies to include equipments.isFetching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing duplicate equipment fetches in the spreadsheet component.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for preventing duplicate fetches and detailing the two specific cases addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

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.

1 participant