Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Delete Datastore [Frontend] #683

Merged
merged 5 commits into from
Jun 22, 2022
Merged

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Jun 20, 2022

❗ Dependent on #674, merge that first.

Purpose

Demonstrate that a user can delete a connected datastore from the Datastore Connection Management UI.
-- The datastore should be removed from the UI
-- When attempting to delete the connection, the user must be presented with a modal saying "Deleting a datastore connection may impact any subject request that is currently in progress. Do you wish to proceed?" (similar flow to deleting a user)

Changes

  • Add a delete connection modal file which has the menu item "delete" and the modal which asks for confirmation before sending a request to DELETE ${CONNECTION_ROUTE}/{connection_key}

Delete intentionally slowed down to take 5 seconds here:
https://user-images.githubusercontent.com/9755598/174892119-5f50f174-c94b-4ce0-a5e9-0eb347a1b152.mov

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #604

@pattisdr pattisdr marked this pull request as ready for review June 21, 2022 02:13
@TheAndrewJackson
Copy link
Contributor

@pattisdr Looks good! I just finished my first pass and I have a few updates.

The first is vertically centering the modal along with horizontally centering it. This will make it match the DenyPrivacyRequestModal. I believe this can be done with the isCentered property on the Modal component.
Screen Shot 2022-06-21 at 14 15 41

The second is to update the "Delete Connection" button to have a loading spinner while the delete is happening and then close the modal once the delete request is finished. The DenyPrivacyRequestModal does this right now so there is a working example. The code is split between DenyPrivacyRequestModal and RequestRow. The mutation function you're using returns a second parameter that exposes an isLoading variable example. Passing that into the isLoading prop on Chakra UIs button will make it show up. I got the modal to close after the request is finished by using a .then() on the mutation function like this.

I also added some code that prevented the modal from closing while the request is happening example

@pattisdr
Copy link
Contributor Author

thanks for the review @TheAndrewJackson and pointing me towards some code examples 🏆 , I've responded to your comments.

@pattisdr
Copy link
Contributor Author

pattisdr commented Jun 21, 2022

argh I forgot the "prevent modal from closing while request is happening" one moment

EDIT: added below

Base automatically changed from 601_datastore_management_landing_page to main June 22, 2022 14:56
@pattisdr
Copy link
Contributor Author

@TheAndrewJackson requested changes made, but I'm going to rebase this off of main now that your datastore landing page has been merged.

@pattisdr pattisdr force-pushed the fidesops_604_delete_datastore branch from 8ff5e3f to 51f4842 Compare June 22, 2022 17:19
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🚀

@TheAndrewJackson TheAndrewJackson merged commit e90413b into main Jun 22, 2022
@TheAndrewJackson TheAndrewJackson deleted the fidesops_604_delete_datastore branch June 22, 2022 18:09
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
* Link Delete Connection menu item with confirmation modal, patterned off of user delete modal.

* Update changelog.

* Vertically center modals.

* Add a spinner while collection is being deleted. Only close modal after successful deletion.

* Prevent closing modal while we're still making the delete request.
dependabot bot added a commit that referenced this pull request Sep 23, 2022
* Link Delete Connection menu item with confirmation modal, patterned off of user delete modal.

* Update changelog.

* Vertically center modals.

* Add a spinner while collection is being deleted. Only close modal after successful deletion.

* Prevent closing modal while we're still making the delete request.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Datastore Management] Delete Datastore FRONTEND
2 participants