Skip to content
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.

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

LG-15379 removes post office closure alerts from barcode page and email #11733

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jennyverdeyen
Copy link
Member

🎫 Ticket

Link to the relevant ticket:
LG-15379

🛠 Summary of changes

Updates barcode page and email to remove the temporary alerts about the January 9th post office closure.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Enter the application via Sinatra and perform the in-person proofing flow all the way to the barcode page.
  • Inspect the barcode page and ensure this alert is not present:
image
  • Inspect the email page and ensure this alert is not present:
image

changelog: User-Facing Improvements, In-person proofing, removes post office closure alerts from barcode page and email
@@ -194,7 +194,6 @@ in_person_outage_message_enabled: false
in_person_proofing_enabled: false
in_person_proofing_enforce_tmx: false
in_person_proofing_opt_in_enabled: false
in_person_proofing_post_office_closed_alert_enabled: true
Copy link
Member Author

Choose a reason for hiding this comment

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

After discussing with @eileen-nava we decided to remove the flag while we're cleaning up the other temporary code.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is another line in this file that needs to be deleted

@jennyverdeyen jennyverdeyen requested review from a team, kellular and eileen-nava and removed request for a team January 10, 2025 18:17
@gina-yamada
Copy link
Contributor

gina-yamada commented Jan 10, 2025

@jennyverdeyen I did not test this yet but I think there are a few more areas to clean up just by looking at the code that went in here and here. I am unclear if we are deleting banner but keeping email or deleting all.

  1. app/mailers/user_mailer.rb
  2. lib/identity_config.rb
  3. spec/controllers/idv/in_person/ready_to_verify_controller_spec.rb
  4. I don't think we can delete all of those translations unless we delete app/mailers/user_mailer.rb and app/views/user_mailer/in_person_post_office_closed.html.erb (and more from Shane's PR)

Copy link
Member

@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.

Should this be a direct revert of #11707 ? I see there's some changes added there which aren't removed here yet (+145/-0 vs. +0/-134), including a few references to configuration in UserMailer.

Edit: I see @gina-yamada already mentioned most of these, sorry for missing that! (I can't delete the review comment)

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