Skip to content
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

Support for deleting a database view #454

Closed
kgodey opened this issue Jul 20, 2021 · 4 comments
Closed

Support for deleting a database view #454

kgodey opened this issue Jul 20, 2021 · 4 comments
Labels
needs: unblocking Blocked by other work type: enhancement New feature or request

Comments

@kgodey
Copy link
Contributor

kgodey commented Jul 20, 2021

The Mathesar UI should allow users to delete existing views.

Scenarios

  1. The user wants to delete a view with no dependencies. We ask them to confirm the action and go ahead and delete it once they do.
  2. The user wants to delete a view with some dependencies. We show them all the existing dependencies that will be deleted if they delete the view and ask them to confirm the action. We go ahead and delete it and all its dependencies once they confirm.
  3. The user wants to delete a view (with or without dependencies). We ask them to confirm the action and they change their mind. We don't delete anything.
  4. The user wants to delete a view (with or without dependencies). We ask them to confirm the action, they agree. The backend call to delete fails and we don't delete anything.

Additional Context

@kgodey kgodey added this to the 08. Working with Views milestone Jul 20, 2021
@kgodey kgodey added restricted: design team ready Ready for implementation type: enhancement New feature or request labels Jul 20, 2021
@kgodey kgodey added pr-status: review A PR awaiting review status: draft and removed ready Ready for implementation pr-status: review A PR awaiting review labels Oct 12, 2021
@kgodey kgodey assigned kgodey and unassigned ghislaineguerin Oct 15, 2021
@kgodey kgodey changed the title Design for creating, editing, and deleting views Support for deleting a database view Oct 30, 2021
@kgodey kgodey added pr-status: review A PR awaiting review and removed status: draft labels Oct 30, 2021
@kgodey
Copy link
Contributor Author

kgodey commented Oct 30, 2021

@ghislaineguerin @pavish @mathemancer @seancolsen This issue has been updated and is ready for review. Please look through it and unassign yourself after you've added any feedback that you might have.

@kgodey kgodey self-assigned this Oct 30, 2021
@ghislaineguerin ghislaineguerin removed their assignment Nov 1, 2021
@mathemancer
Copy link
Contributor

mathemancer commented Nov 2, 2021

There's a subtle detail we'll need to handle on the back end w.r.t. scenarios (2) and (4). For (4), we'll want to be using CASCADE for safety, i.e., so we can roll back if the deletion of the object or its dependencies fails. However, scenario (2) would be safest if we deleted every dependency ourselves (without CASCADE) in a recursive fashion. That way, we guarantee we don't ever delete an object that we haven't first shown the user we'll delete, since we'll be using the dependency graph we just showed to the user to do the deletion. Otherwise, it may be there are some edge cases where we don't manage to detect a dependency that will be deleted if we use CASCADE.

I suggest we still go with CASCADE, but we'll need to be very clear with the user that such a deletion (i.e., deleting an object with dependencies using CASCADE) can have unintended consequences, and if they want to be super safe, they should themselves go through and delete the dependencies before deleting the view in question.

@mathemancer mathemancer removed their assignment Nov 2, 2021
@pavish pavish removed their assignment Nov 2, 2021
@seancolsen seancolsen removed their assignment Nov 4, 2021
@kgodey
Copy link
Contributor Author

kgodey commented Nov 9, 2021

I'm marking this as ready and moving to the backlog for @ghislaineguerin to take up when she has the time.

@kgodey kgodey added ready Ready for implementation and removed pr-status: review A PR awaiting review labels Nov 9, 2021
@kgodey kgodey removed their assignment Nov 9, 2021
@kgodey kgodey added status: draft needs: unblocking Blocked by other work and removed ready Ready for implementation status: draft labels Nov 26, 2021
@kgodey
Copy link
Contributor Author

kgodey commented Mar 7, 2022

Closing this issue in favor of #466

@kgodey kgodey closed this as completed Mar 7, 2022
@kgodey kgodey removed this from the [09] Working with Views milestone Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: unblocking Blocked by other work type: enhancement New feature or request
Projects
No open projects
Development

No branches or pull requests

5 participants