Skip to content

LG-7205: Log click on in-person troubleshooting option#6907

Merged
aduth merged 3 commits intomainfrom
aduth-lg-7205-ipp-option-click
Sep 6, 2022
Merged

LG-7205: Log click on in-person troubleshooting option#6907
aduth merged 3 commits intomainfrom
aduth-lg-7205-ipp-option-click

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Sep 2, 2022

Why: To have better insight into the user's journey through in-person proofing.

Testing Instructions:

  1. Navigate to http://localhost:3000
  2. Sign in
  3. Navigate to http://localhost:3000/verify
  4. Complete proofing flow up to document capture
  5. Upload attention result YAML test documents
  6. Click Submit
  7. Click "Verify your identity in person"
  8. In your terminal, run tail -n2 log/events.log
  9. Observe logged event for "IdV: verify in person troubleshooting option clicked"

**Why**: To have better insight into the user's journey through in-person proofing.

changelog: Upcoming Features, In-person proofing, Improve analytics for in-person proofing actions
options={[{ text: t('idv.troubleshooting.options.verify_in_person') }]}
options={[
{
url: '#location',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit of a cheat, but working as intended: Because FormSteps reflects the step as encoded in the URL hash fragment, we can link directly to the step, rather than using the submit button (BlockSubmitButton) to navigate as a submission of the step.

This was a workaround to an issue where onClick wasn't being called due to the fact that we were only spreading props on the link version of the option, not the button version:

<BlockLink {...extraProps} href={url}>
{text}
</BlockLink>
) : (
<BlockSubmitButton>{text}</BlockSubmitButton>

@aduth aduth requested review from a team and NavaTim September 2, 2022 15:48
});

const links = /** @type {HTMLAnchorElement[]} */ (getAllByRole('link'));
const links = getAllByRole('link') as HTMLAnchorElement[];
Copy link
Contributor

Choose a reason for hiding this comment

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

If we update these tests later, then I'd like to see this refactored to be more white-box or black-box rather than straddling the two by using getAllByRole with white-box checks. Combining the two like this reduces the assurance that the elements will behave as expected.

Probably would be good enough here to add an expect that the fetched elements are instances of HTMLAnchorElement, or to fetch using the tag name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this inconsistency is necessary to appease both Testing Library and TypeScript:

  • Testing Library because it intentionally doesn't provide element querying utilities to encourage testing as the users (and assistive technology) would interact with the application.
  • TypeScript because it wouldn't expect #target to exist on anything other than an anchor element.

It would be nice if there were some way to test to see that the link opens in a new tab without referencing the attribute value directly like this, so we could avoid casting the result of getByRole.

Copy link
Contributor

@NavaTim NavaTim left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@aduth aduth merged commit d46df2c into main Sep 6, 2022
@aduth aduth deleted the aduth-lg-7205-ipp-option-click branch September 6, 2022 13:33
@zachmargolis zachmargolis mentioned this pull request Sep 7, 2022
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