-
Notifications
You must be signed in to change notification settings - Fork 1
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] Implement applicant deletion #310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think we should schedule a meeting to discuss schema updates in the near future.
isOpen={isDeleteApplicantModalOpen} | ||
applicantId={id} | ||
refetch={() => { | ||
/* Do not refetch, redirect to permit holders page */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we redirect it seems to repopulate the correct list without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without redirecting we'd stay on the info page of the permit holder who'd just been deleted. The redirect returns us to the page with the table of all permit holders.
…n records when deleting applicant
adfe04b
to
c368174
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
Thanks @ChinemeremChigbo for your help with testing, appreciate the thoroughness! 🙌 I verified the hypothesis I shared on Discord and it seems to be correct. To summarize, when running the seed script a second time after deleting all applicants, new applicants are created. These new applicants have IDs different from those of the original applicants (auto-incremented by the DB). Applicants after running the seed script again (IDs were initially 1-4):
However, the seed script assumes that applicants have IDs 1-4, so when inserting applications when seeding the second time, we get an error as the previous applicants no longer exist
The |
This makes sense. Thanks for the explanation!!! LGTM. |
* Create deleteApplicant resolver * Improve error message in deleteApplicant resolver * Implement delete applicant UI * Remove TODO comment * Change delete permit holder button text * Add doc string for deleteApplicant resolver * Cleanup newApplication, renewalApplication, and replacementApplication records when deleting applicant
* [Fix] Save poaFormS3ObjectKey when updating guardian information in permit holders page (#319) * [Fix] Postal Code Space Issue (#316) * Set other gender field on permit holders page (#318) * Add .nvmrc file (#317) * [Feature] Implement applicant deletion (#310) * [Feature] Implement application deletion (#315) * Display other gender field on permit holders page (#314) * Move wallet card task in application processing (#313) * [Feature] Tax receipt (#311) * [Fix] Fix expiry date for in-progress replacement applications (#274) * [Fix] Fix APP history permit type badge (#273) * [Improvement] Adjust spacing of address in invoice (#272) * [Fix] Show validation error messages on form mount (#271)
* Create deleteApplicant resolver * Improve error message in deleteApplicant resolver * Implement delete applicant UI * Remove TODO comment * Change delete permit holder button text * Add doc string for deleteApplicant resolver * Cleanup newApplication, renewalApplication, and replacementApplication records when deleting applicant
* Create deleteApplicant resolver * Improve error message in deleteApplicant resolver * Implement delete applicant UI * Remove TODO comment * Change delete permit holder button text * Add doc string for deleteApplicant resolver * Cleanup newApplication, renewalApplication, and replacementApplication records when deleting applicant
* [Feature] Create setEmployeeAsActive GraphQL endpoint (#322) * [Fix] Save poaFormS3ObjectKey when updating guardian information in permit holders page (#319) * [Fix] Postal Code Space Issue (#316) * Set other gender field on permit holders page (#318) * Add .nvmrc file (#317) * [Feature] Implement applicant deletion (#310) * [Feature] Implement application deletion (#315) * Display other gender field on permit holders page (#314) * Move wallet card task in application processing (#313) * [Feature] Tax receipt (#311) * [Fix] Fix expiry date for in-progress replacement applications (#274) * [Fix] Fix APP history permit type badge (#273) * [Improvement] Adjust spacing of address in invoice (#272) * [Fix] Show validation error messages on form mount (#271)
Notion ticket link
Implement applicant deletion
Implementation description
Applicant deletion feature (note: applicant is called "Permit Holder" in user-facing text)
deleteApplicant
GraphQL mutationNotes
Largely follows conventions used for
setApplicantAsInactive
/setApplicantAsActive
anddeleteEmployee
.Test Plan
Delete from table
Before deletion:
After deletion:
Delete from permit holder page
Before deletion:
After deletion:
Checklist
[Feature]
,[Improvement]
or[Fix]
,