Skip to content

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

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.

Sign up for GitHub

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

Merged
aduth merged 3 commits into from
Sep 6, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a60158
LG-7205: Log click on in-person troubleshooting option
aduth Sep 2, 2022
ea768ac
Update specs to reflect option rendered as link
aduth Sep 2, 2022
7f52ff8
Update specs to reflect option rendered as link
aduth Sep 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions app/assets/stylesheets/components/_block-submit-button.scss
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion app/assets/stylesheets/components/all.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
@import 'account-header';
@import 'banner';
@import 'block-link';
@import 'block-submit-button';
@import 'btn';
@import 'card';
@import 'container';
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/frontend_log_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ class FrontendLogController < ApplicationController
before_action :check_user_authenticated
before_action :validate_parameter_types

# rubocop:disable Layout/LineLength
EVENT_MAP = {
'IdV: verify in person troubleshooting option clicked' => :idv_verify_in_person_troubleshooting_option_clicked,
'IdV: forgot password visited' => :idv_forgot_password,
'IdV: password confirm visited' => :idv_review_info_visited,
'IdV: password confirm submitted' => proc do |analytics|
Expand All @@ -22,6 +24,7 @@ class FrontendLogController < ApplicationController
}.transform_values do |method|
method.is_a?(Proc) ? method : AnalyticsEvents.instance_method(method)
end.freeze
# rubocop:enable Layout/LineLength

def create
event = log_params[:event]
Expand Down
11 changes: 3 additions & 8 deletions app/javascript/packages/components/block-link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import Link, { LinkProps } from './link';
import BlockLinkArrow from './block-link-arrow';

export interface BlockLinkProps extends LinkProps {
/**
* Link destination.
*/
href: string;
}
export interface BlockLinkProps extends LinkProps {}

function BlockLink({ href, children, className, ...linkProps }: BlockLinkProps) {
function BlockLink({ children, className, ...linkProps }: BlockLinkProps) {
const classes = ['block-link', className].filter(Boolean).join(' ');

return (
<Link href={href} {...linkProps} className={classes}>
<Link {...linkProps} className={classes}>
{children}
<BlockLinkArrow />
</Link>
Expand Down
30 changes: 0 additions & 30 deletions app/javascript/packages/components/block-submit-button.spec.tsx
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions app/javascript/packages/components/block-submit-button.tsx
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion app/javascript/packages/components/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export { default as Accordion } from './accordion';
export { default as Alert } from './alert';
export { default as BlockLink } from './block-link';
export { default as BlockSubmitButton } from './block-submit-button';
export { default as Button } from './button';
export { default as FullScreen } from './full-screen';
export { default as Icon } from './icon';
Expand Down
14 changes: 5 additions & 9 deletions app/javascript/packages/components/troubleshooting-options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { ReactNode } from 'react';
import { BlockLink, BlockSubmitButton } from '@18f/identity-components';
import { BlockLink } from '@18f/identity-components';
import { useI18n } from '@18f/identity-react-i18n';
import { BlockLinkProps } from './block-link';

export type TroubleshootingOption = Omit<BlockLinkProps, 'href'> & {
url?: string;
url: string;

text: ReactNode;

Expand Down Expand Up @@ -48,13 +48,9 @@ function TroubleshootingOptions({
<ul className="troubleshooting-options__options">
{options.map(({ url, text, ...extraProps }, index) => (
<li key={`tso-${index}`}>
{url !== undefined ? (
<BlockLink {...extraProps} href={url}>
{text}
</BlockLink>
) : (
<BlockSubmitButton>{text}</BlockSubmitButton>
)}
<BlockLink {...extraProps} href={url}>
{text}
</BlockLink>
</li>
))}
</ul>
Expand Down
60 changes: 43 additions & 17 deletions ...-capture-troubleshooting-options-spec.jsx → ...-capture-troubleshooting-options.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import sinon from 'sinon';
import { render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { ComponentType } from 'react';
import {
MarketingSiteContextProvider,
ServiceProviderContextProvider,
} from '@18f/identity-document-capture';
import { FlowContext } from '@18f/identity-verify-flow';
import DocumentCaptureTroubleshootingOptions from '@18f/identity-document-capture/components/document-capture-troubleshooting-options';
import { FlowContext, FlowContextValue } from '@18f/identity-verify-flow';
import AnalyticsContext from '../context/analytics';
import DocumentCaptureTroubleshootingOptions from './document-capture-troubleshooting-options';
import type { ServiceProviderContext } from '../context/service-provider';

describe('DocumentCaptureTroubleshootingOptions', () => {
const helpCenterRedirectURL = 'https://example.com/redirect/';
const inPersonURL = 'https://example.com/some/idv/ipp/url';
const serviceProviderContext = {
const serviceProviderContext: ServiceProviderContext = {
name: 'Example SP',
failureToProofURL: 'http://example.test/url/to/failure-to-proof',
isLivenessRequired: false,
getFailureToProofURL: () => '',
};
const wrappers = {
const wrappers: Record<string, ComponentType> = {
MarketingSiteContext: ({ children }) => (
<MarketingSiteContextProvider helpCenterRedirectURL={helpCenterRedirectURL}>
{children}
Expand All @@ -33,7 +40,7 @@ describe('DocumentCaptureTroubleshootingOptions', () => {
wrapper: wrappers.MarketingSiteContext,
});

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

@NavaTim NavaTim Sep 2, 2022

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
Copy Markdown
Contributor Author

@aduth aduth Sep 6, 2022

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.


expect(links).to.have.lengthOf(2);
expect(links[0].textContent).to.equal(
Expand All @@ -58,7 +65,7 @@ describe('DocumentCaptureTroubleshootingOptions', () => {
wrapper: wrappers.helpCenterAndServiceProviderContext,
});

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

expect(links).to.have.lengthOf(3);
expect(links[0].textContent).to.equal(
Expand Down Expand Up @@ -93,7 +100,7 @@ describe('DocumentCaptureTroubleshootingOptions', () => {
},
);

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

expect(links[0].href).to.equal(
'https://example.com/redirect/?category=verify-your-identity&article=how-to-add-images-of-your-state-issued-id&location=custom',
Expand Down Expand Up @@ -136,29 +143,48 @@ describe('DocumentCaptureTroubleshootingOptions', () => {
});

context('hasErrors and inPersonURL', () => {
const wrapper = ({ children }) => (
<FlowContext.Provider value={{ inPersonURL }}>{children}</FlowContext.Provider>
const wrapper: ComponentType = ({ children }) => (
<FlowContext.Provider value={{ inPersonURL } as FlowContextValue}>
{children}
</FlowContext.Provider>
);

it('has link to IPP flow', () => {
const { getByText, getAllByRole } = render(
const { getByText, getByRole } = render(
<DocumentCaptureTroubleshootingOptions hasErrors />,
{ wrapper },
);

expect(getByText('components.troubleshooting_options.new_feature')).to.exist();

const buttons = getAllByRole('button');
const ippButton = buttons.find(
({ textContent }) => textContent === 'idv.troubleshooting.options.verify_in_person',
const link = getByRole('link', { name: 'idv.troubleshooting.options.verify_in_person' });

expect(link).to.exist();
});

it('logs an event when clicking the troubleshooting option', async () => {
const trackEvent = sinon.stub();
const { getByRole } = render(
<AnalyticsContext.Provider value={{ trackEvent }}>
<DocumentCaptureTroubleshootingOptions hasErrors />
</AnalyticsContext.Provider>,
{ wrapper },
);

const link = getByRole('link', { name: 'idv.troubleshooting.options.verify_in_person' });
await userEvent.click(link);

expect(trackEvent).to.have.been.calledWith(
'IdV: verify in person troubleshooting option clicked',
);
expect(ippButton).to.exist();
});
});

context('hasErrors and inPersonURL but showInPersonOption is false', () => {
const wrapper = ({ children }) => (
<FlowContext.Provider value={{ inPersonURL }}>{children}</FlowContext.Provider>
const wrapper: ComponentType = ({ children }) => (
<FlowContext.Provider value={{ inPersonURL } as FlowContextValue}>
{children}
</FlowContext.Provider>
);

it('does not have link to IPP flow', () => {
Expand Down Expand Up @@ -191,7 +217,7 @@ describe('DocumentCaptureTroubleshootingOptions', () => {
},
);

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

expect(links).to.have.lengthOf(1);
expect(links[0].getAttribute('href')).to.equal(
Expand Down
10 changes: 9 additions & 1 deletion ...ascript/packages/document-capture/components/document-capture-troubleshooting-options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useI18n } from '@18f/identity-react-i18n';
import type { TroubleshootingOption } from '@18f/identity-components/troubleshooting-options';
import ServiceProviderContext from '../context/service-provider';
import MarketingSiteContext from '../context/marketing-site';
import AnalyticsContext from '../context/analytics';

interface DocumentCaptureTroubleshootingOptionsProps {
/**
Expand Down Expand Up @@ -43,6 +44,7 @@ function DocumentCaptureTroubleshootingOptions({
const { t } = useI18n();
const { inPersonURL } = useContext(FlowContext);
const { getHelpCenterURL } = useContext(MarketingSiteContext);
const { trackEvent } = useContext(AnalyticsContext);
const { name: spName, getFailureToProofURL } = useContext(ServiceProviderContext);

return (
Expand Down Expand Up @@ -81,7 +83,13 @@ function DocumentCaptureTroubleshootingOptions({
<TroubleshootingOptions
isNewFeatures
heading={t('idv.troubleshooting.headings.are_you_near')}
options={[{ text: t('idv.troubleshooting.options.verify_in_person') }]}
options={[
{
url: '#location',
Copy link
Copy Markdown
Contributor Author

@aduth aduth Sep 2, 2022

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:

identity-idp/app/javascript/packages/components/troubleshooting-options.tsx

Lines 52 to 56 in 93a5f05

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

text: t('idv.troubleshooting.options.verify_in_person'),
onClick: () => trackEvent('IdV: verify in person troubleshooting option clicked'),
},
]}
/>
)}
</>
Expand Down
5 changes: 5 additions & 0 deletions app/services/analytics_events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,11 @@ def idv_come_back_later_visit
track_event('IdV: come back later visited')
end

# The user clicked the troubleshooting option to start in-person proofing
def idv_verify_in_person_troubleshooting_option_clicked
track_event('IdV: verify in person troubleshooting option clicked')
end

# The user visited the "ready to verify" page for the in person proofing flow
def idv_in_person_ready_to_verify_visit
track_event('IdV: in person ready to verify visited')
Expand Down
2 changes: 1 addition & 1 deletion spec/features/idv/in_person_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
mock_doc_auth_attention_with_barcode
attach_and_submit_images

click_button t('idv.troubleshooting.options.verify_in_person')
click_link t('idv.troubleshooting.options.verify_in_person')

bethesda_location = page.find_all('.location-collection-item')[1]
bethesda_location.click_button(t('in_person_proofing.body.location.location_button'))
Expand Down
2 changes: 1 addition & 1 deletion spec/support/features/in_person_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def begin_in_person_proofing(_user = nil)
mock_doc_auth_attention_with_barcode
attach_and_submit_images

click_button t('idv.troubleshooting.options.verify_in_person')
click_link t('idv.troubleshooting.options.verify_in_person')
end

def complete_location_step(_user = nil)
Expand Down