Skip to content

LG-9641 GPO Resend Confirm Interstitial#8527

Merged
eric-gade merged 6 commits intomainfrom
eric-lg-9641
Jun 2, 2023
Merged

LG-9641 GPO Resend Confirm Interstitial#8527
eric-gade merged 6 commits intomainfrom
eric-lg-9641

Conversation

@eric-gade
Copy link
Contributor

@eric-gade eric-gade commented Jun 1, 2023

🎫 Ticket

LG-9641

🛠 Summary of changes

Instead of creating a "true" new page (controller, view, tests, etc), we found that re-using the current GPO page (ie the "Want a letter?" page) with a presenter that checks whether or not the user already has a letter pending proved sufficient.

📜 Testing Plan

  • Complete IDV up until the phone verification page
  • Select the GPO option and send the letter
  • On the account page, click the enter your code link
  • On the Welcome back page, click the link to send another letter
  • You should see the "Send another letter?" variant specified in this ticket
  • Click the back link, you should be on the welcome back page again
  • Re-click the send another letter link, you should arrive back on the Send another letter variant
  • Click the button to re-send a letter
  • GPO should work as normal

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

English:

Screen Shot 2023-06-01 at 4 28 00 PM

Spanish:

Screen Shot 2023-06-01 at 4 28 09 PM

French:

Screen Shot 2023-06-01 at 4 28 16 PM

eric-gade added 3 commits June 1, 2023 14:59
We are reusing the gpo_controller aka "Want a letter?" code to create
this interstitial page.

changelog: User-Facing Improvements, Confirmation page, add
interstitial confirming that the user wants another letter sent
@eric-gade eric-gade requested a review from a team June 1, 2023 19:27
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍


<%= render AlertComponent.new(message: t('idv.messages.gpo.info_alert'), class: 'margin-bottom-4') %>
<% if @presenter.resend_requested? %>
<%= render AlertIconComponent.new(icon_name: :warning, class: 'display-block margin-bottom-2') %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since all of the content varies depending on this condition, I'd wonder if it'd be better to use StatusPageComponent rather than constructing its parts.

i.e.

<% if @presenter.resend_requested? %>
  <%= render StatusPageComponent.new(status: :warning) do |c| %>
    <% c.with_header { @presenter.title } %>
    <p>
      <%= t('idv.messages.gpo.resend_timeframe') %>
    </p>
    <p>
      <%= t('idv.messages.gpo.resend_code_warning') %>
    </p>
  <% end %>
<% else %>
  <%= render AlertComponent.new(message: t('idv.messages.gpo.info_alert'), class: 'margin-bottom-4') %>
  <%= render PageHeadingComponent.new.with_content(@presenter.title) %>
  <p>
    <%= t('idv.messages.gpo.address_on_file_html') %>
    <%= t('idv.messages.gpo.timeframe_html') %>
  </p>
<% end %>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great call, done with 13a8200

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

I tested this out and it all works beautifully. I logged out and back in before entering the code.

I saw an email on the resend, but none on the first letter request. I assume that was existing behavior since I didn't see it in the code. Looks like the presenter was already there - convenient!

@eric-gade eric-gade merged commit e7cb0a0 into main Jun 2, 2023
@eric-gade eric-gade deleted the eric-lg-9641 branch June 2, 2023 17:11
@solipet solipet mentioned this pull request Jun 6, 2023
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.

3 participants