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

Allow certain users to delete a site and all its data #1380

Merged
merged 4 commits into from
Sep 11, 2023

Conversation

CristinaRO
Copy link
Contributor

@CristinaRO CristinaRO commented Aug 31, 2023

Trello

Allows users granted the newly introduced "Site Manager" permission to delete a site via the interface.

I've opted for a two-step, JavaScript-free process, for simplicity.

I've also opted for a GitHub-style confirmation prompt, where rather than a yes/no question, the user has to input some text (the site slug AKA abbreviation in our case) to confirm a destructive action.

UI changes

"Danger" link

Screenshot 2023-09-06 at 16 58 43

Confirmation form

Screenshot 2023-09-06 at 16 58 59

Confirmation form with mismatched confirmation

Screenshot 2023-09-06 at 17 00 11 Again, there's not much to lean on in terms of prior art.

Deletion confirmation

Screenshot 2023-09-04 at 12 53 01
Redirects to the organisation dashboard.

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@CristinaRO CristinaRO force-pushed the 816-improve-the-process-for-deleting-a-site branch 2 times, most recently from ded06da to 45e8c72 Compare September 4, 2023 11:44
@CristinaRO CristinaRO changed the title Allow admins to delete a site and all its data Allow certain users to delete a site and all its data Sep 4, 2023
@CristinaRO CristinaRO marked this pull request as ready for review September 4, 2023 11:59
@CristinaRO CristinaRO force-pushed the 816-improve-the-process-for-deleting-a-site branch 2 times, most recently from daaee65 to ee20177 Compare September 8, 2023 12:51
Comment on lines 123 to 144

Scenario: Deleting a site as a Site Manager
Given I have logged in as a Site Manager
And a site bis exists
And I visit this site page
When I delete this site
Then I should be prompted to confirm the deletion
When I fail to confirm the deletion
Then I should see "The confirmation did not match"
And I should be prompted to confirm the deletion
When I confirm the deletion
Then I should be redirected to the organisation dashboard
And I should see the deletion confirmation message

Scenario: Deleting a site as a non-Site Manager
Given I have logged in as a member of DCLG
And a site dclg exists
And I visit this site page
Then I should not see "Delete"
When I visit the path /sites/dclg/confirm_destroy
Then I should be redirected to the site dashboard
And I should see "Only certain users can access that."
Copy link
Contributor

Choose a reason for hiding this comment

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

Really like this test walk through, thank you :)

Copy link
Contributor

@JonathanHallam JonathanHallam left a comment

Choose a reason for hiding this comment

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

Awesome thank you, really like how this is looking

Allows users with the "Site Manager" permission to delete a site via
the interface.

I've opted for a two-step, JavaScript-free process, for simplicity.

I've also opted for a GitHub-style confirmation prompt, where rather
than a yes/no question, the user has to input some text (the site slug
AKA abbreviation in our case) to confirm a destructive action.
There's no `highlight-danger` style in the govuk-admin-template, which
this site still uses.

Adding proper support for govuk-components would be a huge amount of
work, so we've decided to add the least amount of style customisation to
make this feature viable.
@CristinaRO CristinaRO force-pushed the 816-improve-the-process-for-deleting-a-site branch from ee20177 to a17f142 Compare September 11, 2023 15:34
@CristinaRO CristinaRO merged commit 41175d6 into main Sep 11, 2023
5 checks passed
@CristinaRO CristinaRO deleted the 816-improve-the-process-for-deleting-a-site branch September 11, 2023 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants