Fix view picker small bugs #16987
Fix view picker small bugs #16987
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/views/hooks/internal/useUpdateView.ts">
<violation number="1" location="packages/twenty-front/src/modules/views/hooks/internal/useUpdateView.ts:60">
P1: Missing rollback on mutation failure. The optimistic update is applied before the mutation (lines 31-38), but if the mutation fails, the state is never reverted to its original value. This causes UI/backend desync where the user sees the optimistic change but the backend has the original data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| status: 'successful', | ||
| response: result, | ||
| }; | ||
| } catch (error) { |
There was a problem hiding this comment.
P1: Missing rollback on mutation failure. The optimistic update is applied before the mutation (lines 31-38), but if the mutation fails, the state is never reverted to its original value. This causes UI/backend desync where the user sees the optimistic change but the backend has the original data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/views/hooks/internal/useUpdateView.ts, line 60:
<comment>Missing rollback on mutation failure. The optimistic update is applied before the mutation (lines 31-38), but if the mutation fails, the state is never reverted to its original value. This causes UI/backend desync where the user sees the optimistic change but the backend has the original data.</comment>
<file context>
@@ -0,0 +1,84 @@
+ status: 'successful',
+ response: result,
+ };
+ } catch (error) {
+ if (error instanceof ApolloError) {
+ handleMetadataError(error, {
</file context>
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| primaryMetadataName: 'view', | ||
| }); | ||
| } else { | ||
| enqueueErrorSnackBar({ message: t`An error occurred.` }); | ||
| } | ||
|
|
||
| return { | ||
| status: 'failed', | ||
| error, | ||
| }; | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:3090 This environment will automatically shut down when the PR is closed or after 5 hours. |
| @@ -91,39 +88,6 @@ export const usePersistView = () => { | |||
| ], | |||
| ); | |||
|
|
|||
| const updateView = useCallback( | |||
There was a problem hiding this comment.
I would leave it in usePersistView
Current naming convention:
- persistViewFilter = create, update, delete in backend
- saveViewFilter = transform the recordFilters to viewFilters and call persistViewFilter
- remove / upsertRecordFilter = update record filters
There was a problem hiding this comment.
New naming proposal:
- performViewFilterAPICreate / performViewFilterAPIUpdate / performViewFilterAPIUpdate viewFilter => wrapping of api calls
- saveRecordFiltersToViewFilters
- upsert / remove recordFilter => update recordFilters state
There was a problem hiding this comment.
so this hook should performViewAPIUpdate
There was a problem hiding this comment.
1 issue found across 28 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/views/hooks/internal/usePerformViewAPIUpdate.ts">
<violation number="1" location="packages/twenty-front/src/modules/views/hooks/internal/usePerformViewAPIUpdate.ts:61">
P1: Missing rollback of optimistic update on error. If `updateCoreViewMutation` fails, the UI will still show the optimistically applied changes that weren't persisted to the server. Store the previous state before applying the optimistic update and restore it in the catch block.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| status: 'successful', | ||
| response: result, | ||
| }; | ||
| } catch (error) { |
There was a problem hiding this comment.
P1: Missing rollback of optimistic update on error. If updateCoreViewMutation fails, the UI will still show the optimistically applied changes that weren't persisted to the server. Store the previous state before applying the optimistic update and restore it in the catch block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/views/hooks/internal/usePerformViewAPIUpdate.ts, line 61:
<comment>Missing rollback of optimistic update on error. If `updateCoreViewMutation` fails, the UI will still show the optimistically applied changes that weren't persisted to the server. Store the previous state before applying the optimistic update and restore it in the catch block.</comment>
<file context>
@@ -0,0 +1,86 @@
+ status: 'successful',
+ response: result,
+ };
+ } catch (error) {
+ if (error instanceof ApolloError) {
+ handleMetadataError(error, {
</file context>
|
Hey @lucasbordeau! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
This PR solves small bugs around the view picker.
Since core views are not being handled by object metadata items anymore, and that all view logic is plugged on coreViewsState, this PR implemented optimistic effect by upserting into this state.
Fixes #15422
Fixes #16986
Before
Enregistrement.de.l.ecran.2026-01-07.a.15.23.20.mov
After
Enregistrement.de.l.ecran.2026-01-07.a.15.29.09.mov