Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

1016 frontend ability for users to manually enter pii to an in progress subject request #1377

Conversation

chriscalhoun1974
Copy link
Contributor

@chriscalhoun1974 chriscalhoun1974 commented Sep 23, 2022

1016.-.Frontend.ability.for.users.to.manually.enter.pii.to.an.in.progress.subject.request.webm

Purpose

For a given Subject Request record with a status of Requires Input, this UI feature allows a user to enter required manual input data to one or more Manual connections via the Subject Request detail screen (Manual Processing section). Once all of the given Manual connection(s) have the required manual input data entered, each Manual connection will be in a Review status. There will be a Complete DSR button displayed located below the last Manual connection record. Upon clicked, this will update the Subject Request record to a status of In Progress and the user will be redirected to the Subject Requests landing page.

Figma designs
https://www.figma.com/file/MTbZohIGvB4bq2f2wkiDHO/Fidesops-Management-UI?node-id=3458%3A48952

Changes

  • Display Manual Processing section via the Subject Request detail screen based on a given Subject Request record with a status of Requires Input
  • When a user clicks the Begin manual input or Review button, a sliding panel will be displayed via the left hand side. This panel lists each DSR Package Label defined for a given Manual connection. The user can enter the manual input data for a given Manual connection.
  • After each Manual connection has been reviewed, the Subject Request record can be submitted via clicking the Complete DSR button. This will change the status from Requires Input to In Progress.

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #1016

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

Really nice UI Chris, the drawer on the side works nicely. Mostly edge cases called out here, the main thing is I'd like the user to be able to submit an empty form if you agree.

loadingText="Submitting"
size="sm"
variant="solid"
type="submit"
Copy link
Contributor

Choose a reason for hiding this comment

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

On first load, it seems like you can't submit the drawer with empty fields. You can submit it with text, save, and then come back a second time and edit it to a empty string and then save but I'd think you should be able to submit originally with all empty fields.

In my understanding it's okay if all of the fields are empty and they submit. The backend just cares that they made a submission, even if they are all empty. It's possible that they looked for fields and none existed.

Screen Shot 2022-09-23 at 12 56 51 PM

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 have updated the drawer to have the Submit button disabled until the user has entered value(s) which are not deeply equal from the initial form values. Let me know if this makes sense.

Screen Shot 2022-09-25 at 5 52 45 PM
Screen Shot 2022-09-25 at 5 53 14 PM

_hover={{ bg: "primary.400" }}
_active={{ bg: "primary.500" }}
>
Complete DSR
Copy link
Contributor

Choose a reason for hiding this comment

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

Wanted to point out an edge case. If someone goes and edits the original dsr package labels for the manual webhook (can only be done through the API currently), and we've already saved values for the webhook for a given privacy request here, we will not be able to complete the DSR here.

In other words, if we've changed which fields we've said we want to upload to the user, but we have a different set of fields saved, it errors rather than return these different fields to the user. A 429 is thrown when you try to resume.

Ideally, I think they'd need to reupload the correct fields for the offending collection, this might be a combination of backend and frontend changes to resolve.

Screen Shot 2022-09-23 at 2 51 57 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

A related issue, if you refresh this page and there is a mismatch between the values saved for the privacy request and the dsr_package_labels for the webhook, it looks like you can't open the subject request detail page at all.
The detail I return for this particular error is pretty indicative of where the error is. I'd rework to still allow this page to reload if this occurs and maybe show them an empty set of fields that need to be rewritten.

I can chat about potential backend changes too to help with this state if needed.

Screen Shot 2022-09-23 at 3 05 09 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out this user scenario. Let's discuss the edge case scenario(s) with @seanpreston tomorrow if possible. Once manual input data has been uploaded, not sure if a user should be able to edit the DSR Package Label value accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed in person, we'll put in this backend ticket to resolve #1389

Comment on lines +230 to +231
You don‘t have any Manual Webhook connections
set up yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is another edge case. I created a webhook, ran a privacy request (which put this in requires_input) and then went back to my connectors page and deleted the webhook. Now there's no way to resume this privacy request.

I wonder if we should show different text here, and allow them to resume. They did have a webhook, but now it's gone. It can be resumed from a backend perspective.

Screen Shot 2022-09-23 at 3 48 17 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One possible solution for this edge case is to disable the feature for a user to be able to Delete a Webhook connection if one or more Subject Request records have manual input data entered. Let's discuss this further.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed in person, we'll put in this backend ticket to resolve #1388

{key}
</FormLabel>
<Input
{...field}
Copy link
Contributor

Choose a reason for hiding this comment

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

Input field works well, tested things like " and ✊🐝 Ꮆя𝐞EtⓘŇg𝓢 ℱ𝔯Øϻ σᑌ𝓉𝕖𝕣 ѕρᗩ匚ᵉ 🍭 empty fields and random code.

Wanted to get your opinion, should we limit the text we can upload here? Since you need to be able to login to fidesops I doubt people would paste novels into this block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pattisdr Great suggestion. What are the current DB field constraints for these? Once I have the character limitiations, I can implement the client side validation accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't add DB constraints on these particular fields, I think if we did have limits they should be fairly high.

@pattisdr pattisdr force-pushed the 1016-frontend-ability-for-users-to-manually-enter-pii-to-an-in-progress-subject-request branch from 0893fbe to 273e110 Compare September 24, 2022 01:28
@chriscalhoun1974 chriscalhoun1974 force-pushed the 1016-frontend-ability-for-users-to-manually-enter-pii-to-an-in-progress-subject-request branch from 5584b77 to 19c68d7 Compare September 26, 2022 14:31
Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

Allowing fields to be saved even if they aren't dirty helped align the FE and BE so we can save fields with no input. Thanks for these changes!

Backend edge cases will be addressed here
#1394

@pattisdr pattisdr merged commit b2aa8a6 into main Sep 27, 2022
@pattisdr pattisdr deleted the 1016-frontend-ability-for-users-to-manually-enter-pii-to-an-in-progress-subject-request branch September 27, 2022 18:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontend - ability for users to manually enter PII to an IN PROGRESS subject request
2 participants