Destroy core view

[BOHEUS]

Solution for #16526

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[BOHEUS]

Not sure if destroyView function in usePersistView.ts should be removed if it's not used anywhere

[cubic-dev-ai]

No issues found across 6 files

[BOHEUS]

Also, should be a upgrade command removing all views with deletedAt != null?

[Copilot]

Pull request overview

This PR addresses issue #16526 where viewField records were not being soft-deleted when their parent view was deleted, leaving orphan records in the database. The solution changes the view deletion behavior from soft-delete to hard-delete by switching from deleteCoreView to destroyCoreView mutations. This leverages the database's CASCADE behavior to automatically remove all related child records (viewField, viewFilter, viewSort, viewGroup, viewFilterGroup) when a view is permanently deleted.

Key changes:

• Renamed useDeleteViewFromCurrentState hook to useDestroyViewFromCurrentState to reflect the new deletion behavior
• Added destroyView function to usePersistView hook that calls the destroyCoreView GraphQL mutation
• Updated all view deletion call sites to use the new destroyView function instead of deleteView

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/twenty-front/src/modules/views/view-picker/hooks/useDestroyViewFromCurrentState.ts	Renamed hook from useDeleteViewFromCurrentState and updated to use destroyView instead of deleteView
packages/twenty-front/src/modules/views/view-picker/components/ViewPickerOptionDropdown.tsx	Updated import to use renamed useDestroyViewFromCurrentState hook; added unused imports for destroy mutations
packages/twenty-front/src/modules/views/view-picker/components/ViewPickerEditButton.tsx	Updated import to use renamed useDestroyViewFromCurrentState hook
packages/twenty-front/src/modules/views/view-picker/components/ViewPickerCreateButton.tsx	Updated import to use renamed useDestroyViewFromCurrentState hook
packages/twenty-front/src/modules/views/hooks/internal/usePersistView.ts	Added destroyView function and useDestroyCoreViewMutation hook to support hard deletion of views
packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownCustomView.tsx	Updated import to use renamed useDestroyViewFromCurrentState hook

Comments suppressed due to low confidence (1)

packages/twenty-front/src/modules/views/view-picker/hooks/useDestroyViewFromCurrentState.ts:50

• The internal variable is still named deleteViewFromCurrentState but the hook has been renamed to useDestroyViewFromCurrentState. For consistency with the new naming convention, this variable should be renamed to destroyViewFromCurrentState to match the hook name and avoid confusion.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:15812

This environment will automatically shut down when the PR is closed or after 5 hours.

[prastoin]

Hey there thanks for your contribution !
About to push a fix for the naming

[prastoin]

Even though we might end up renaming the destroy ( hard delete ) to delete ( currently soft delete ) in the future, for the moment it's still two distinct operations so lets keep naming explicit for the moment

[prastoin]

Also, should be a upgrade command removing all views with deletedAt != null?

For the moment I think it's over-engineered, lets consider implementing such a command when we decide to totally deprecate soft deletion from now core view-xx entities

[prastoin]

[Weiko]

LGTM

[sentry]

Bug: The application crashes with a TypeError when deleting the last view for an object because the code attempts to access the .id property of undefined.
_{Severity: CRITICAL | Confidence: High}

🔍 Detailed Analysis

In useDestroyViewFromCurrentState.ts, when a user destroys the last remaining view for an object, the application will crash. The logic attempts to switch to a new view by filtering the list of views to find a remaining one. When no views are left, this filter returns an empty array. The code then attempts to access the id property of the first element of this empty array ([0].id), which evaluates to undefined.id. This triggers an unhandled TypeError, causing a crash.

💡 Suggested Fix

Before calling changeView, verify that the filtered list of remaining views is not empty. If there are remaining views, proceed to change the view. If not, handle this edge case, for example by navigating to a default state or preventing the deletion of the last view entirely.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-front/src/modules/views/view-picker/hooks/useDestroyViewFromCurrentState.ts#L79-L85

Potential issue: In `useDestroyViewFromCurrentState.ts`, when a user destroys the last
remaining view for an object, the application will crash. The logic attempts to switch
to a new view by filtering the list of views to find a remaining one. When no views are
left, this filter returns an empty array. The code then attempts to access the `id`
property of the first element of this empty array (`[0].id`), which evaluates to
`undefined.id`. This triggers an unhandled `TypeError`, causing a crash.

_{Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8124595}

[twenty-eng-sync]

Hey @Weiko! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

[github-actions]

Thanks @BOHEUS for your contribution!
This marks your 47th PR on the repo. You're top 1% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter