-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix redundant API calls when batch updating #17213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… many queries after incremental updates and skip optimistic effects during the update.
|
Hey @charlesBochet :) I don't know if it has something to do with the cache. |
Greptile SummaryThis PR successfully optimizes incremental batch record updates by deferring all Apollo cache refetches and browser event dispatches until after all batches complete, eliminating the redundant FindMany API calls that were triggered for each batch. The implementation correctly disables optimistic effects during individual batch mutations and consolidates all updates into a single refetch operation at the end, directly addressing issue #17117. Key Changes:
Test Coverage:The test suite now verifies that the optimization works correctly including edge cases like process failures, where the finally block still executes to maintain consistency. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as UI Component
participant Hook as useIncrementalUpdateManyRecords
participant Fetch as useIncrementalFetchAndMutate
participant Update as useUpdateManyRecords
participant Apollo as Apollo Cache
participant Event as Browser Event
UI->>Hook: incrementalUpdateManyRecords(fields)
Hook->>Fetch: incrementalFetchAndMutate(callback)
Note over Fetch,Update: Batch 1 (Pages 1-3)
Fetch->>Fetch: Fetch page 1
Fetch->>Update: updateManyRecords({<br/>skipOptimisticEffect: true,<br/>skipRefetchAggregateQueries: true,<br/>skipRegisterObjectOperation: true})
Update->>Apollo: Mutate (no cache update)
Update-->>Fetch: Success
Fetch->>Fetch: Fetch page 2
Fetch->>Update: updateManyRecords({...skip flags})
Update->>Apollo: Mutate (no cache update)
Update-->>Fetch: Success
Fetch->>Fetch: Fetch page 3
Fetch->>Update: updateManyRecords({...skip flags})
Update->>Apollo: Mutate (no cache update)
Update-->>Fetch: Success
Note over Fetch: All batches complete
Fetch-->>Hook: Complete
Note over Hook,Event: Finally block executes
Hook->>Apollo: refetchFindManyRecords()
Apollo-->>Hook: Queries reloaded
Hook->>Apollo: refetchAggregateQueries()
Apollo-->>Hook: Aggregates reloaded
Hook->>Event: dispatchObjectRecordOperationBrowserEvent<br/>(all record IDs)
Event-->>UI: Listeners notified of bulk operation
Hook-->>UI: totalUpdatedCount
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 1 comment
...nty-front/src/modules/object-record/hooks/__tests__/useIncrementalUpdateManyRecords.test.tsx
Show resolved
Hide resolved
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:35138 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
Fix: Optimize incremental update many records
Problem
The batch update process was using
optimisticEffecton everyupdateManyRecordscall within the incremental loop. This caused redundant API calls and potential performance issues during the update process, as optimistic updates and side effects were triggered for each chunk of records rather than once for the entire operation.Solution
This PR optimizes the useIncrementalUpdateManyRecords hook by:
skipOptimisticEffect: true,skipRefetchAggregateQueries: true,skipRegisterObjectOperation: true).finallyblock after all batches are processed.dispatchObjectRecordOperationBrowserEventto run once at the end with all updated record IDs.Changes
skipOptimisticEffect: truetoupdateManyRecords.refetchFindManyRecords,refetchAggregateQueries, anddispatchObjectRecordOperationBrowserEventto thefinallyblock of theincrementalFetchAndMutateprocess.skipOptimisticEffectis true.After changes:
Closes #17117