Move settings data model refreshMetadata to sync calls #9046
Conversation
charlesBochet
force-pushed
the
fix-snackbar-error
branch
from
1b6e4cc to
b1e648a
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR modifies how metadata refresh operations are handled across the application, moving from Apollo's built-in refetchQueries to explicit synchronous refresh calls.
- Removed Apollo's
refetchQueriesin favor of explicitrefreshObjectMetadataItems()calls across multiple metadata hooks - Added
network-onlyfetch policy touseRefreshObjectMetadataItemsfor stronger cache control - Commented out navigation-triggered metadata refresh in
PageChangeEffect.tsx, potentially affecting state management - Left debugging
console.logstatements inObjectMetadataItemsGaterandObjectMetadataItemsProviderthat should be removed - Modified
useLoadMockedObjectMetadataItemsto handleisAppWaitingForFreshObjectMetadataStatesynchronization
13 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -11,6 +11,8 @@ export const ObjectMetadataItemsGater = ({ | |||
| isAppWaitingForFreshObjectMetadataState, | |||
| ); | |||
|
|
|||
| console.log(isAppWaitingForFreshObjectMetadata); | |||
There was a problem hiding this comment.
style: Remove debugging console.log before merging to production
| @@ -11,6 +11,7 @@ export const ObjectMetadataItemsProvider = ({ | |||
| children, | |||
| }: React.PropsWithChildren) => { | |||
| const objectMetadataItems = useRecoilValue(objectMetadataItemsState); | |||
| console.log('objectMetadataItems', objectMetadataItems); | |||
There was a problem hiding this comment.
style: Remove console.log statement before merging to production
| const result = await mutate({ | ||
| variables: { input: { relation: formatRelationMetadataInput(input) } }, | ||
| awaitRefetchQueries: true, | ||
| refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], | ||
| }); |
There was a problem hiding this comment.
style: No error handling around the mutation. Consider wrapping in try/catch to handle potential network or validation errors.
| }); | ||
|
|
||
| await refreshObjectMetadataItems(); |
There was a problem hiding this comment.
logic: refreshObjectMetadataItems could fail silently if the network request fails, potentially leaving the UI in an inconsistent state
| const result = await mutate({ | ||
| variables: { | ||
| input: { | ||
| field: input, | ||
| }, | ||
| }, | ||
| awaitRefetchQueries: true, | ||
| refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], | ||
| }); |
There was a problem hiding this comment.
style: Consider handling mutation errors before proceeding with refresh
| const result = await mutate({ | ||
| variables: { | ||
| idToDelete, | ||
| }, | ||
| awaitRefetchQueries: true, | ||
| refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], | ||
| }); | ||
|
|
||
| await refreshObjectMetadataItems(); | ||
|
|
There was a problem hiding this comment.
logic: No error handling around the mutation or refresh call. If either fails, the error is silently propagated.
| const result = await mutate({ | ||
| variables: { | ||
| idToDelete, | ||
| }, | ||
| awaitRefetchQueries: true, | ||
| refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], | ||
| }); | ||
|
|
||
| await refreshObjectMetadataItems(); | ||
|
|
There was a problem hiding this comment.
style: Consider handling mutation errors before calling refreshObjectMetadataItems to avoid unnecessary refreshes on failure
| set(objectMetadataItemsState, toSetObjectMetadataItems); | ||
| set(isAppWaitingForFreshObjectMetadataState, false); | ||
| } |
There was a problem hiding this comment.
logic: setting isAppWaitingForFreshObjectMetadataState to false here but never setting it to true anywhere in this flow
| }); | ||
|
|
||
| await refreshObjectMetadataItems(); |
There was a problem hiding this comment.
style: Consider wrapping refreshObjectMetadataItems in try/catch to handle refresh failures gracefully
| }); | ||
|
|
||
| await refreshObjectMetadataItems(); |
There was a problem hiding this comment.
style: Consider error handling around refreshObjectMetadataItems call - if it fails, the workspace and views may be out of sync
In this PR, I'm