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

Feature idea: deletion warnings #275

Closed
wenottingham opened this issue Sep 25, 2017 · 11 comments
Closed

Feature idea: deletion warnings #275

wenottingham opened this issue Sep 25, 2017 · 11 comments

Comments

@wenottingham
Copy link
Contributor

wenottingham commented Sep 25, 2017

ISSUE TYPE
  • Feature Idea
COMPONENT NAME
  • API
  • UI
SUMMARY

We have a variety of items that can be deleted out from other items that use them.

  • inventory from job template / workflow
  • credential from job template / project / inventory
  • job template from workflow
  • project from job template / workflow
  • notification template from
  • custom inventory scripts

The AWX UI should warn when deleting an item in use by another item, and show where it is used.

The API does not need to warn (but may need updated so the UI can properly determine this).

ADDITIONAL INFORMATION

This becomes "fun" in the context of #19

@wenottingham
Copy link
Contributor Author

wenottingham commented Sep 25, 2017

For most items, this would be a query of 'related' item counts, to yield: "Removing this would break X items. Are you sure?"

For organizations, this would be a hardcoded: "This makes everything in this organization unavailable. Are you sure?" rather than querying the API.

Can wordsmith as necessary.

@mabashian
Copy link
Member

@wenottingham

We had a meeting about this today and wanted to run some things by you. We came up with a few different options for tackling this and would like to get your input on how best to proceed.

If we want to display counts in the delete confirmation then we've kind of got two options:

  1. The UI can basically hard code all the necessary requests and fire them off when the user hits the delete button.
  2. The API can implement some sort of "get all the resources that this would impact" endpoint that the UI would hit when the user hits the delete icon.

We're concerned about the overhead associated with doing either of these things. The hairiest example is probably credentials. There are quite a few different places that a credential could be used and the number of requests needed to gather up all of that info could be significant (at least half a dozen, maybe more). Even if we had a defined endpoint so that only a single request is made by the UI, the API will still have to hit the database multiple times to gather this information.

If we didn't include a count and opted for an implementation similar to what you suggested for organizations everywhere then we could avoid the need for any api work on this feature and greatly reduce the logic that we'd have to add to the UI. @trahman73 was in the meeting and seemed to be OK with us extending the delete warning to say something like (using project as an example):

Are you sure you want to delete this project? Doing so will invalidate any Job Templates, Workflow Job Templates, and Inventory sources that use this project.

I'd be interested to see what you think about that.

@trahman73
Copy link

@mabashian @wenottingham

Option 1: Best UX but extremely expensive on the API side.
image

Option 2: Good UX but still a bit expensive on the API/UI side. (My personal choice.)
image

Option 3: Decent UX with no API work and little effort from the UI as we would just be expanding the delete message. (Possible light fix until we can properly implement option 2.)
image

@wenottingham
Copy link
Contributor Author

I like 2 or 3. I'd ask the API team if there is any work that can simplify this from the query side.

@mabashian mabashian self-assigned this Nov 1, 2017
@mabashian
Copy link
Member

mabashian commented Nov 1, 2017

OK I sat down and ran through what kind of information we'd have to gather to implement option 2:

When deleting a project

  • Check Job Templates count
  • Check SCM Inventory Source count
  • Check Workflow nodes count

When deleting a job template

  • Check Workflow nodes count

When deleting an inventory

  • Check Job Templates count

When deleting an inventory source

  • Check Workflow nodes count

When deleting a credential

  • Check Projects count
  • Check Job Templates count
  • Check Inventory count (if credential type is insights)
  • Check Inventory Source count

When deleting a credential type

  • Check Credentials count

When deleting an inventory script

  • Check Custom Inventory Source count

@mabashian
Copy link
Member

@trahman73 @wenottingham I've got some followup questions on this feature:

  1. If the user is about to delete an inventory that is used by a job template but the creator of the job template selected ask_inventory_on_launch do we want to count that as a job template that will be invalidated by the deletion of the inventory? The same goes for deleting a machine credential where the user has ask_credentials_on_launch set to true for the JT.

  2. I was originally planning on checking for inventories that use a particular insights credential before the user deletes said insights credential. Technically speaking deleting an insights credential wouldn't invalidate the inventory since insights credential isn't a required field but it will definitely mess up the process. How should I handle that?

@trahman73
Copy link

1.) I would say no because the user will have the option to select anything else that is available on launch.

2.) I would say we should warn the user because even though the Insights credential isn't required, the process will be broken.

@wenottingham
Copy link
Contributor Author

  1. I'm OK with either answer here - I see both Tafique's point, and the point that it will now change the job to require the user to set one.

  2. Agree with Tafique.

@mabashian
Copy link
Member

mabashian commented Nov 8, 2017

I realized during implementation that the api prevents a user from deleting a credential type if that credential type is in use (a credential has been created using that type). If we detect this, then I've added a static message to the delete modal:

screen shot 2017-11-08 at 3 50 01 pm

@trahman73 does that seem OK to you? Feel free to wordsmith as needed.

@trahman73
Copy link

@mabashian Looks good to me.

@jlaska
Copy link
Contributor

jlaska commented Nov 17, 2017

The wording for the modal seems funky, particularly the term invalidate. I'd suggest something more akin to ...

The {{ item }} is currently being used by other resources. Are you sure you want to delete this {{ item }}?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants