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

[#1733] Add option to remove Court Mandates #1802

Merged

Conversation

rhian-cs
Copy link
Contributor

@rhian-cs rhian-cs commented Mar 2, 2021

What github issue is this PR for, if any?

Resolves #1733

What changed, and why?

When implementing #1795 I added an option to add and edit Court Mandates in the CASA Case Edit page.
Now I've implemented a way to delete Court Mandates that have already been saved in the database.

  • The saved court mandates in the CASA Case Edit page now have a (-) (minus) button beside them.
  • Clicking on it will show a dialog box with the requested confirmation text
  • Deviating a little bit from what was requested, instead of adding a "Remove" and a "Return" button I added a "Delete" and "Go back" button instead.
  • Clicking in the "Go back" button or the X button will close the dialog box and do nothing.
  • Clicking in the "Delete" button will send an AJAX request to the server to delete the specified Court Mandate
  • Instead of showing a notice (which most likely would require the page to be reloaded) I opted for showing another dialog box showing success (or failure, in case of any errors)
  • After that, the court mandate disappears from the edit page

To "delete" a court mandate that has not been saved yet, the process remains the same: just empty the field.

How will this affect user permissions?

  • Volunteer permissions: Cannot delete court mandates
  • Supervisor permissions: Can delete court mandates
  • Admin permissions: Can delete court mandates

How is this tested? (please write tests!) 💖💪

Automated tests

Request specs

For each type of user in the system (volunteers, admins and supervisors) I tested:

  • Whether or not they could actually delete a court mandate
  • Correct reponse from the server
Frontend specs
  • The delete button shows a dialog box
  • Clicking "Delete" removes the court mandate from the page
  • The "success" dialog box shows up
  • Clicking on Update CASA Case works perfectly, without any errors

Manual testing

  • Log-in as admin/supervisor
  • Go to a CASA case, add some court mandates and click Update to save the data
  • Click on the minus button
  • Check if the dialog box shows up and displays the correct message and buttons
  • Click on the "Delete" button
  • Check if the success dialog box shows up
  • Check if the court mandate is actually gone in the page
  • Update the page, check if no errors occurred and if the court mandate is actually gone

Screenshots please :)

Overview
01-overview
Dialog box
02-dialog-box-delete
Success message
03-dialog-box-success
After deletion
04-after
Mobile view
05-mobile-view
Mobile view of the dialog box
06-dialog-box-mobile
Mobile view of the success dialog box
07-dialog-box-success-mobile

@github-actions github-actions bot added javascript for use by Github Labeler to mark pull requests that update Javascript code ruby Pull requests that update Ruby code Tests! 🎉💖👏 labels Mar 2, 2021

RSpec.describe "/case_court_mandates", type: :request do
let(:case_court_mandate) { build(:case_court_mandate) }
let(:delete_request) { delete case_court_mandate_url(case_court_mandate) }
Copy link

@Diego-Thomaz Diego-Thomaz Mar 3, 2021

Choose a reason for hiding this comment

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

this let should be a subject

Comment on lines 308 to 313
let(:casa_case) { create(:casa_case) }
let(:mandate) { build(:case_court_mandate) }

before do
casa_case.case_court_mandates << mandate
end

Choose a reason for hiding this comment

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

This logic is repeated in your tests. It'd be nice to avoid this repetition by using a trait within your factory.

@case_court_mandate.destroy
end

def set_case_court_mandate

Choose a reason for hiding this comment

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

This method should be private

Copy link

@Diego-Thomaz Diego-Thomaz left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -10,6 +10,57 @@ id="casa_case_case_court_mandates_attributes_1_mandate_text">\
$(list).children(':last').trigger('focus')
}

function remove_mandate_with_confirmation () {
Swal.fire({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Swal!

Copy link
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

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

Nice :)
Yay tests!

@compwron compwron merged commit abc08a7 into rubyforgood:main Mar 3, 2021
@rhian-cs rhian-cs deleted the 1733-add-option-to-remove-court-mandates branch May 27, 2021 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript for use by Github Labeler to mark pull requests that update Javascript code ruby Pull requests that update Ruby code Tests! 🎉💖👏
Projects
None yet
Development

Successfully merging this pull request may close these issues.

court mandates can be edited/removed
3 participants