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

Add modal prompt to delete integration and associated nodes #4348

Merged
merged 10 commits into from
Sep 30, 2020

Conversation

SEAjamieD
Copy link
Contributor

@SEAjamieD SEAjamieD commented Sep 15, 2020

🔩 Description: What code changed, and why?

This branch adds two new elements to deleting integrations.

  1. Upon choosing to delete an integration from the integrations list, the user is now prompted to check that they are sure.
  2. Nodes associated with the deleted integration will also be deleted.

⛓️ Related Resources

Initial Writeup: #4176
Fixes: #4335

👍 Definition of Done

Upon choosing to delete a node integration, the user is first prompted with a modal. Once finally deleted, all associated nodes are also deleted.

👟 How to Build and Test the Change

in root: build components/automate-ui-devproxy && start_all_services
in components/automate-ui: make serve

To Test:

  1. add an aws-ec2 integration (follow these steps https://github.com/chef/automate/blob/master/dev-docs/adding-data/adding_test_data.md#adding-cloud-integrations) - this will add a number of nodes to the system, associating them with that nodemanager so that scan jobs can be executed
  2. go to Compliance > Scan Jobs > Nodes Added ==> Note the nodes added to the system as a result.
  3. go back to Settings > Integrations, and delete the nodemanager that you created in step 1.
  4. go to back Compliance > Scan Jobs > Nodes Added
  5. you will see all the nodes previously created are now deleted as well

✅ Checklist

📷 Screenshots, if applicable

DeleteAssociatedNodes

@netlify
Copy link

netlify bot commented Sep 15, 2020

Deploy preview for chef-automate ready!

Built with commit ec2de86

https://deploy-preview-4348--chef-automate.netlify.app

Comment on lines 75 to 82
<h2 id="delete-prompt-title" slot="title">Delete Integration?</h2>
<div>
<p>Deleting this node manager will also delete all of its associated nodes</p>
<div class="options">
<chef-button label="confirm" primary caution (click)="handleDelete()">Delete</chef-button>
<chef-button label="cancel" tertiary (click)="resetModal()">Cancel</chef-button>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonong1972 - Suggestions for wording on this Modal prompt for deleting a Node Manager (Integration)?

Copy link

@jonong1972 jonong1972 Sep 16, 2020

Choose a reason for hiding this comment

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

Delete Integration?

This will remove all of its associated nodes and they will not be able to be scanned by scan jobs. To add them back you will need to create a new node integration.

Historical reported data on nodes will still be stored.

@SEAjamieD Try that.

@SEAjamieD SEAjamieD self-assigned this Sep 16, 2020
@SEAjamieD SEAjamieD marked this pull request as ready for review September 16, 2020 19:35
@SEAjamieD SEAjamieD requested review from jonong1972, a team and susanev September 16, 2020 19:36
seajamied added 6 commits September 16, 2020 14:31
…onent

Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
…hings

Signed-off-by: seajamied <jdegnan@chef.io>
…e changed

Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
@SEAjamieD SEAjamieD force-pushed the jamie/4335-modal-to-nodemanager-deletion branch from da326a7 to 20126fd Compare September 16, 2020 21:32
@@ -67,3 +67,19 @@ <h2 class="display4 strong">Cloud Platform Providers</h2>
</main>
</div>
</div>

<chef-modal
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use a <app-delete-object-modal> here instead of <chef-modal> if possible. It should have the flexibility to support your use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually used this, and then reverted it. With the current wording, using <app-delete-object-modal> does not have enough flexibility, unfortunately.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be a straightforward change to get what you need by (a) adding a custom flag to the <app-delete-object-modal>, then display <ng-content> if custom is true, otherwise display the standard verbiage. Let's chat Monday if you need more details.

seajamied added 3 commits September 18, 2020 09:22
Signed-off-by: seajamied <jdegnan@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
…roperty, cleanup

Signed-off-by: seajamied <jdegnan@chef.io>
@SEAjamieD SEAjamieD requested review from msorens and a team September 21, 2020 18:53
</chef-button>
<chef-button tertiary (click)="closeEvent()">Cancel</chef-button>
</div>
<ng-container *ngIf="custom === false">
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that this works for all the other components? IIRC when you just use custom as an HTML attribute without a value, it is undefined rather than false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this works because I placed a default of false for custom on the delete-object-modal component

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good. Unrelated, more standard to test boolean without explicit constants:

Suggested change
<ng-container *ngIf="custom === false">
<ng-container *ngIf="!custom">

Signed-off-by: seajamied <jdegnan@chef.io>
@SEAjamieD SEAjamieD requested a review from a team September 21, 2020 23:12
@susanev susanev merged commit 12c2e4c into master Sep 30, 2020
@susanev susanev deleted the jamie/4335-modal-to-nodemanager-deletion branch September 30, 2020 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add modal to nodemanager deletion to delete associated nodes
4 participants