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

Edit a verification request #508

Merged
merged 30 commits into from
Sep 25, 2023
Merged

Edit a verification request #508

merged 30 commits into from
Sep 25, 2023

Conversation

tortila
Copy link
Contributor

@tortila tortila commented Aug 24, 2023

This PR aims to enable editing of submitted verification requests by their authors, using the same multi-step wizard form.

Scope of changes:

  • New URL path to the edit view: /requests/{id}/edit which is only accessible for admins and authors, and only when the request is open. We re-use the same WizardView but this time pass an instance_dict as an argument, which contains model instances as expected by consecutive steps.
  • All forms used by the wizard are now either ModelForm or ModelFormSets. This allows to inject either an instance of queryset with existing objects and delegate handling the updating/deletion of objects to Django magic.
  • Status "Open" now renamed to "More info required", added a relevant migration.
  • New button on the detail page of a verification request: "Update this request" will be visible to request's author and admins when the request has status "More info required". The button redirects to the edit view.
  • Some workarounds for MultiModelForms and ConvenientFormSets for the whole machinery to work properly (all workarounds documented in the code with examples).
  • New action added to the admin panel: requesting more information about a ProviderRequest object will mark its status to OPEN.
  • Clean up models for representing consent (last step of the verification request form): now consent fields are a part of the ProviderRequest model and not a separate one. Added a relevant migration and changed the model forms.
  • Small change in the CSS: added a rule to always mark elements with attribute "hidden" as display :none. This is a workaround so that evidence rows can be deleted, this didn't work before because the flexbox overrides the "hidden" attribute behavior.
  • Added some tests, including test_editing_pr_updates_original_submission which covers an end-to-end journey of updating a verification requests.

Out of scope:

  • attaching notes/comments by staff to opened verification requests,
  • automatic email communication with the author in case a verification request is re-opened.

After merging:
Remember to run the migrations!

@tortila tortila force-pushed the oz-edit-provider-request branch 2 times, most recently from e18c830 to 7025854 Compare August 30, 2023 12:57
@gitpod-io
Copy link

gitpod-io bot commented Sep 6, 2023

@mrchrisadams
Copy link
Member

hi @tortila thanks for taking me through this on the call just now - I feel like I have a good understanding of the changes now, and the last few bits before we merge it in. Have a nice week away :)

@tortila tortila changed the title [WIP] editing verification requests Edit a verification request Sep 21, 2023
@tortila tortila force-pushed the oz-edit-provider-request branch from 6d4d2d9 to e50a9ee Compare September 21, 2023 11:41
@tortila tortila marked this pull request as ready for review September 21, 2023 11:41
@tortila tortila force-pushed the oz-edit-provider-request branch 3 times, most recently from a75a88f to a7aa367 Compare September 21, 2023 12:14
@tortila tortila force-pushed the oz-edit-provider-request branch from a7aa367 to 926b589 Compare September 21, 2023 12:27
@tortila tortila temporarily deployed to staging September 21, 2023 12:36 — with GitHub Actions Inactive
Copy link
Member

@mrchrisadams mrchrisadams left a comment

Choose a reason for hiding this comment

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

Oliwia and I reviewed this code synchronously on a video call, and again in person. I'm happy to merge this in.

@mrchrisadams mrchrisadams merged commit e011d92 into master Sep 25, 2023
3 checks passed
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