Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion app/assets/images/alert/pending.svg

This file was deleted.

13 changes: 2 additions & 11 deletions app/assets/stylesheets/components/_step-indicator.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
$step-indicator-current-step-border-width: 3px;
$step-indicator-line-height: 4px;
$step-indicator-pending-color: #a8b6c6;

lg-step-indicator {
display: block;
Expand Down Expand Up @@ -106,15 +105,11 @@ lg-step-indicator {
border: $step-indicator-current-step-border-width solid color('success');
}

.step-indicator__step--complete:not(.step-indicator__step--pending)::before {
.step-indicator__step--complete::before {
background-color: color('white');
background-image: url('alert/success.svg');
}

.step-indicator__step--pending::before {
background-image: url('alert/pending.svg');
}

.step-indicator__step:not(:last-child)::after {
background-color: color('base-lighter');
content: '';
Expand All @@ -126,11 +121,7 @@ lg-step-indicator {
width: calc(100% - 1rem - #{$step-indicator-line-height * 2});
}

.step-indicator__step--pending:not(:last-child)::after {
background-color: $step-indicator-pending-color;
}

.step-indicator__step--complete:not(.step-indicator__step--pending):not(:last-child)::after {
.step-indicator__step--complete:not(:last-child)::after {
background-color: color('success');
}

Expand Down
8 changes: 4 additions & 4 deletions app/controllers/idv/personal_key_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ def update
private

def step_indicator_steps
steps = Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS
return steps if idv_session.address_verification_mechanism != 'gpo'
steps.map do |step|
step[:name] == :verify_phone_or_address ? step.merge(status: :pending) : step
if idv_session.address_verification_mechanism == 'gpo'
Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO
else
Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS
end
end

Expand Down
8 changes: 4 additions & 4 deletions app/controllers/idv/review_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ def redirect_to_idv_app_if_enabled
end

def step_indicator_steps
steps = Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS
return steps if idv_session.address_verification_mechanism != 'gpo'
steps.map do |step|
step[:name] == :verify_phone_or_address ? step.merge(status: :pending) : step
if idv_session.address_verification_mechanism == 'gpo'
Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO
else
Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS
end
end

Expand Down
1 change: 1 addition & 0 deletions app/javascript/packages/eslint-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
- `no-useless-constructor`
- React: The following rules are no longer enforced:
- `react/no-array-index-key`
- `prefer-const` is only enforced on destructuring assignment if all variables should be `const` ([`destructuring: 'all'` option](https://eslint.org/docs/latest/rules/prefer-const#destructuring)).

## v2.0.0 (2022-03-14)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const config = {
'object-curly-spacing': 'off',
'operator-linebreak': 'off',
'padded-blocks': 'off',
'prefer-const': ['error', { destructuring: 'all', ignoreReadBeforeAssign: true }],
'quote-props': 'off',
'require-await': 'error',
'rest-spread-spacing': 'off',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('StepIndicatorStep', () => {
expect(status).to.be.ok();
expect(step.classList.contains('step-indicator__step--current')).to.be.true();
expect(step.classList.contains('step-indicator__step--complete')).to.be.false();
expect(step.classList.contains('step-indicator__step--pending')).to.be.false();
expect(status.classList.contains('step-indicator__step-subtitle')).to.be.false();
expect(status.classList.contains('usa-sr-only')).to.be.true();
});
Expand All @@ -32,30 +31,11 @@ describe('StepIndicatorStep', () => {
expect(status).to.be.ok();
expect(step.classList.contains('step-indicator__step--current')).to.be.false();
expect(step.classList.contains('step-indicator__step--complete')).to.be.true();
expect(step.classList.contains('step-indicator__step--pending')).to.be.false();
expect(status.classList.contains('step-indicator__step-subtitle')).to.be.false();
expect(status.classList.contains('usa-sr-only')).to.be.true();
});
});

context('pending step', () => {
it('renders step', () => {
const { getByText } = render(<StepIndicatorStep title="Step" status={StepStatus.PENDING} />);

const title = getByText('Step');
const status = getByText('step_indicator.status.pending');
const step = title.closest('.step-indicator__step')!;

expect(title).to.be.ok();
expect(status).to.be.ok();
expect(step.classList.contains('step-indicator__step--current')).to.be.false();
expect(step.classList.contains('step-indicator__step--complete')).to.be.false();
expect(step.classList.contains('step-indicator__step--pending')).to.be.true();
expect(status.classList.contains('step-indicator__step-subtitle')).to.be.true();
expect(status.classList.contains('usa-sr-only')).to.be.false();
});
});

context('incomplete step', () => {
it('renders step', () => {
const { getByText } = render(
Expand All @@ -70,7 +50,6 @@ describe('StepIndicatorStep', () => {
expect(status).to.be.ok();
expect(step.classList.contains('step-indicator__step--current')).to.be.false();
expect(step.classList.contains('step-indicator__step--complete')).to.be.false();
expect(step.classList.contains('step-indicator__step--pending')).to.be.false();
expect(status.classList.contains('step-indicator__step-subtitle')).to.be.false();
expect(status.classList.contains('usa-sr-only')).to.be.true();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ function StepIndicatorStep({ title, status }: StepIndicatorStepProps) {
'step-indicator__step',
status === CURRENT && 'step-indicator__step--current',
status === COMPLETE && 'step-indicator__step--complete',
status === PENDING && 'step-indicator__step--pending',
]
.filter(Boolean)
.join(' ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,18 @@ describe('VerifyFlowStepIndicator', () => {
});

context('with gpo as address verification method', () => {
it('renders address verification as pending', () => {
const { getByText } = render(
it('revises the flow path to omit address verification and add letter step', () => {
const { queryByText } = render(
<AddressVerificationMethodContextProvider initialMethod="gpo">
<VerifyFlowStepIndicator currentStep="personal_key" />
</AddressVerificationMethodContextProvider>,
);

const previous = getByText('step_indicator.flows.idv.verify_phone_or_address');
expect(previous.closest('.step-indicator__step--pending')).to.exist();
const verifyAddress = queryByText('step_indicator.flows.idv.verify_phone_or_address');
const getALetter = queryByText('step_indicator.flows.idv.get_a_letter');

expect(verifyAddress).to.not.exist();
expect(getALetter).to.exist();
});
});

Expand Down
28 changes: 15 additions & 13 deletions app/javascript/packages/verify-flow/verify-flow-step-indicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ type VerifyFlowStepIndicatorStep =
| 'verify_phone_or_address'
| 'secure_account'
| 'find_a_post_office'
| 'go_to_the_post_office';
| 'go_to_the_post_office'
| 'get_a_letter';

interface VerifyFlowConfig {
/**
Expand Down Expand Up @@ -94,35 +95,35 @@ export function getStepStatus(index, currentStepIndex): StepStatus {
}

/**
* Given contextual details of the current flow path, returns explicit statuses which should be used
* at particular steps.
* Given contextual details of the current flow path, returns the relevant flow configuration.
*
* @param details Flow details
*
* @return Step status overrides.
* @return Flow step configuration.
*/
function getStatusOverrides({
function getFlowStepsConfig({
path,
addressVerificationMethod,
}: {
path: VerifyFlowPath;
addressVerificationMethod: AddressVerificationMethod;
}) {
const statuses: Partial<Record<VerifyFlowStepIndicatorStep, StepStatus>> = {};
}): VerifyFlowConfig {
let { steps, mapping } = FLOW_STEP_PATHS[path];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this the line the eslint-plugin change was for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is this the line the eslint-plugin change was for?

Yep, sorry, I should have highlighted that connection.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With the previous rule enforced, you'd have to do something like...

let { steps } = FLOW_STEPS_PATH[path];
const { mapping } = FLOW_STEPS_PATH[path];

Not only is it kinda garish, I think the general pattern could be problematic in cases such as...

let { foo } = doSomeExpensiveOrMutativeTask();
const { bar } = doSomeExpensiveOrMutativeTask();


if (addressVerificationMethod === 'gpo') {
statuses.verify_phone_or_address = StepStatus.PENDING;
steps = steps.filter((step) => step !== 'verify_phone_or_address').concat('get_a_letter');
}

return statuses;
return { steps, mapping };
}

function VerifyFlowStepIndicator({
currentStep,
path = VerifyFlowPath.DEFAULT,
}: VerifyFlowStepIndicatorProps) {
const { steps, mapping } = FLOW_STEP_PATHS[path];
const currentStepIndex = steps.indexOf(mapping[currentStep]);
const { addressVerificationMethod } = useContext(AddressVerificationMethodContext);
const statusOverrides = getStatusOverrides({ addressVerificationMethod });
const { steps, mapping } = getFlowStepsConfig({ path, addressVerificationMethod });
const currentStepIndex = steps.indexOf(mapping[currentStep]);

// i18n-tasks-use t('step_indicator.flows.idv.getting_started')
// i18n-tasks-use t('step_indicator.flows.idv.verify_id')
Expand All @@ -131,14 +132,15 @@ function VerifyFlowStepIndicator({
// i18n-tasks-use t('step_indicator.flows.idv.secure_account')
// i18n-tasks-use t('step_indicator.flows.idv.find_a_post_office')
// i18n-tasks-use t('step_indicator.flows.idv.go_to_the_post_office')
// i18n-tasks-use t('step_indicator.flows.idv.get_a_letter')

return (
<StepIndicator className="margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4">
{steps.map((step, index) => (
<StepIndicatorStep
key={step}
title={t(`step_indicator.flows.idv.${step}`)}
status={statusOverrides[step] || getStepStatus(index, currentStepIndex)}
status={getStepStatus(index, currentStepIndex)}
/>
))}
</StepIndicator>
Expand Down
8 changes: 8 additions & 0 deletions app/services/idv/flows/doc_auth_flow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ class DocAuthFlow < Flow::BaseFlow
{ name: :secure_account },
].freeze

STEP_INDICATOR_STEPS_GPO = [
{ name: :getting_started },
{ name: :verify_id },
{ name: :verify_info },
{ name: :secure_account },
{ name: :get_a_letter },
].freeze

OPTIONAL_SHOW_STEPS = {
verify_wait: Idv::Steps::VerifyWaitStepShow,
}.freeze
Expand Down
6 changes: 2 additions & 4 deletions app/views/idv/come_back_later/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
<% content_for(:pre_flash_content) do %>
<%= render 'shared/step_indicator', {
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS.map do |step|
step[:name] == :secure_account ? step.merge(status: :complete) : step
end,
current_step: :verify_phone_or_address,
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO,
current_step: :get_a_letter,
locale_scope: 'idv',
class: 'margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4',
} %>
Expand Down
6 changes: 2 additions & 4 deletions app/views/idv/gpo_verify/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
<% content_for(:pre_flash_content) do %>
<%= render 'shared/step_indicator', {
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS.map do |step|
step[:name] == :secure_account ? step.merge(status: :complete) : step
end,
current_step: :verify_phone_or_address,
steps: Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO,
current_step: :get_a_letter,
locale_scope: 'idv',
class: 'margin-x-neg-2 margin-top-neg-4 tablet:margin-x-neg-6 tablet:margin-top-neg-4',
} %>
Expand Down
3 changes: 1 addition & 2 deletions app/views/shared/_step_indicator_step.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ locals:
status = local_assigns[:status] || :not_complete
classes = ['step-indicator__step']
classes << 'step-indicator__step--current' if local_assigns[:status] == :current
classes << 'step-indicator__step--complete' if local_assigns[:status] == :complete
classes << 'step-indicator__step--pending' if local_assigns[:status] == :pending %>
classes << 'step-indicator__step--complete' if local_assigns[:status] == :complete %>

<%#
Using `aria-current="step"` would be the preferred method to indicate the current step, but at the
Expand Down
1 change: 1 addition & 0 deletions config/locales/step_indicator/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ en:
flows:
idv:
find_a_post_office: Find a Post Office
get_a_letter: Get a letter in the mail
getting_started: Getting started
go_to_the_post_office: Go to the Post Office
secure_account: Secure your account
Expand Down
1 change: 1 addition & 0 deletions config/locales/step_indicator/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ es:
flows:
idv:
find_a_post_office: Encontrar una oficina de correos
get_a_letter: Reciba una carta por correo
getting_started: Inicio
go_to_the_post_office: Ir a la oficina de correos
secure_account: Proteje tu cuenta
Expand Down
1 change: 1 addition & 0 deletions config/locales/step_indicator/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ fr:
flows:
idv:
find_a_post_office: Trouver un bureau de poste
get_a_letter: Recevoir une lettre par la poste
getting_started: Démarrer
go_to_the_post_office: Se rendre au bureau de poste
secure_account: Sécurisez votre compte
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/idv/personal_key_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ def index
subject.idv_session.address_verification_mechanism = 'gpo'
end

it 'assigns step indicator steps with pending status' do
it 'assigns step indicator steps with gpo letter step' do
get :show

steps = assigns(:step_indicator_steps)

expect(flash.now[:success]).to eq t('idv.messages.mail_sent')
expect(assigns(:step_indicator_steps)).to include(
hash_including(name: :verify_phone_or_address, status: :pending),
)
expect(steps).to eq(Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO)
end
end
end
Expand Down
14 changes: 7 additions & 7 deletions spec/controllers/idv/review_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ def show
it 'shows steps' do
get :new

expect(subject.view_assigns['step_indicator_steps']).not_to include(
hash_including(name: :verify_phone_or_address, status: :pending),
)
steps = subject.view_assigns['step_indicator_steps']

expect(steps).to eq(Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS)
end

context 'idv app password confirm step is enabled' do
Expand All @@ -236,12 +236,12 @@ def show
idv_session.address_verification_mechanism = 'gpo'
end

it 'shows revises steps to show pending address verification' do
it 'shows gpo steps' do
get :new

expect(subject.view_assigns['step_indicator_steps']).to include(
hash_including(name: :verify_phone_or_address, status: :pending),
)
steps = subject.view_assigns['step_indicator_steps']

expect(steps).to eq(Idv::Flows::DocAuthFlow::STEP_INDICATOR_STEPS_GPO)
end
end

Expand Down
8 changes: 3 additions & 5 deletions spec/features/idv/steps/confirmation_step_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
'.step-indicator__step--complete',
text: t('step_indicator.flows.idv.verify_phone_or_address'),
)
expect(page).not_to have_css('.step-indicator__step--pending')
expect(page).not_to have_content(t('step_indicator.flows.idv.get_a_letter'))
end

it 'allows the user to refresh and still displays the personal key' do
Expand Down Expand Up @@ -70,10 +70,8 @@
it 'shows status content for gpo verification progress' do
expect(page).to have_content(t('idv.messages.mail_sent'))
expect_step_indicator_current_step(t('step_indicator.flows.idv.secure_account'))
expect(page).to have_css(
'.step-indicator__step--pending',
text: t('step_indicator.flows.idv.verify_phone_or_address'),
)
expect(page).to have_content(t('step_indicator.flows.idv.get_a_letter'))
expect(page).not_to have_content(t('step_indicator.flows.idv.verify_phone_or_address'))
end

it_behaves_like 'personal key page', :gpo
Expand Down
8 changes: 2 additions & 6 deletions spec/features/users/verify_profile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@
end

context 'GPO letter' do
it 'shows step indicator progress with current verify step, completed secure account' do
it 'shows step indicator progress with current step' do
sign_in_live_with_2fa(user)

expect_step_indicator_current_step(t('step_indicator.flows.idv.verify_phone_or_address'))
expect(page).to have_css(
'.step-indicator__step--complete',
text: t('step_indicator.flows.idv.secure_account'),
)
expect_step_indicator_current_step(t('step_indicator.flows.idv.get_a_letter'))
end

scenario 'valid OTP' do
Expand Down
Loading