Skip to content

LG-11083: Enable USPS Public Endpoint#9355

Merged
allthesignals merged 5 commits intomainfrom
wmg/11083-enable-usps-public-endpoint
Oct 19, 2023
Merged

LG-11083: Enable USPS Public Endpoint#9355
allthesignals merged 5 commits intomainfrom
wmg/11083-enable-usps-public-endpoint

Conversation

@allthesignals
Copy link
Contributor

@allthesignals allthesignals commented Oct 11, 2023

🎫 Ticket

🛠 Summary of changes

Enables USPS search endpoint. Switches to mock data when usps_mock_fallback flag is enabled.

📜 Testing Plan

To set this up locally:

In IDP, make sure you have USPS credentials set up in your application.yml. Then, set:
usps_mock_fallback: false

Run the IDP locally.

In identity-sites, create a file _config.dev.yml and add these values in it:

show_po_search: true
po_search_address_search_url: http://127.0.0.1:3000/api/address_search
po_search_locations_search_url: http://127.0.0.1:3000/api/usps_locations
skip_idp_translations: false

Run identity sites and visit this.

@allthesignals allthesignals changed the title changelog: Internal, In-Person Proofing, Enable public USPS post offi… LG-11083: Enable USPS Public Endpoint Oct 11, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have to require the relevant spec file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, or put them in shared examples 😇

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to add a shared example like the one you have here: https://github.com/18F/identity-idp/pull/9355/files#diff-75163b44a581d7a43f027aa8db08c525173b4d280a566720cde848c1abb0f569R43

I'm not sure if there's an easier way to share that code between these specs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the idea here to move all of these parameters into some bit of code that can be shared between this controller and the smaller controller you created for the Help Center?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right

Copy link
Contributor

Choose a reason for hiding this comment

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

Just confirming but we don't want to collect any analytics on the Help Center search, correct>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I'm not sure. It's not mentioned in the ticket buuuut we should definitely check!

Copy link
Contributor

Choose a reason for hiding this comment

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

@allthesignals has this been followed up on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JackRyan1989 I'll make another ticket to add that. It doesn't come for free with the USPS service. I do wonder if that's something might be baked into the USPS service instead of an afterthought on the controllers? Or a Faraday mixin called "Analyzable" or something...

@allthesignals allthesignals marked this pull request as ready for review October 13, 2023 17:32
Copy link
Contributor

Choose a reason for hiding this comment

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

is this being included to use methods, or to shorten the namespace for things like EnrollmentHelper? IMO it's more confusing to use it that way, easier to just use the long names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Updated.

I wish we could lint against includes like this. They make sense when they're used like mixins for sharing controller hooks (RenderConditionConcern). But when used directly, it's confusing. There are some violations/inconsistencies floating around.

@allthesignals allthesignals force-pushed the wmg/11083-enable-usps-public-endpoint branch from 46aa665 to c785ad6 Compare October 18, 2023 15:36
@allthesignals allthesignals requested review from a team, JackRyan1989 and gina-yamada October 18, 2023 16:13
Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gina-yamada
Copy link
Contributor

I observe mocks in the Help Center. I observe mocks in idp when running locally.

Screenshot 2023-10-18 at 12 58 22 PM

Screenshot 2023-10-18 at 12 58 07 PM

@gina-yamada
Copy link
Contributor

@allthesignals Can you show me how you are getting Help Center dev to use the code in this branch in your testing plan?

@allthesignals
Copy link
Contributor Author

@gina-yamada yeah! make sure you have usps_mock_fallback: false in your application.yml. I'll update the PR text

@allthesignals allthesignals merged commit 9c17164 into main Oct 19, 2023
@allthesignals allthesignals deleted the wmg/11083-enable-usps-public-endpoint branch October 19, 2023 17:42
@mdiarra3 mdiarra3 mentioned this pull request Oct 23, 2023
mdiarra3 added a commit that referenced this pull request Oct 24, 2023
* LG-11083: Enable USPS Public Endpoint (#9355)

* changelog: Internal, In-Person Proofing, Enable public USPS post office search

* Use EnrollmentHelper to switch between mock/real thing

* Try behaves_like

* Revert shared examples for now

* Use full name

* Update report mailer preview to be more realistic (#9419)


**How**: stubs CloudwatchClient

changelog: Internal, Reporting, Updates report preview to use live code

* Add analytics section to frontend documentation (#9421)

* Add analytics section to frontend documentation

changelog: Internal, Documentation, Add analytics frontend documentation

* link to correct javascript package

* LG-11101: Support multiple valid MFA to satisfy authentication request (#9335)

changelog: User-Facing Improvements, MFA, Avoid prompting for MFA in some scenarios where a recent MFA satisfies the requirement

* LG-11148 | Adds monthly report on total verified users (#9376)

changelog: Internal, Reporting, Monthly report now includes total verified users

Also incorporates LG-11150

Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>

* Remove second MFA prompt exception for strict MFA requirement (#9422)

changelog: User-Facing Improvements, MFA Setup, Add second MFA reminder screen for single-MFA accounts when signing in at AAL2

* LG-11126 Update Start over verifying your identity screen (#9313)

* change text for start over verify screen

* add translations for page

* add changelog

changelog: User-Facing Improvements, IdV By Mail, update text in start
over verifying identity screen

* remove unused i18n

* create new translation with question mark added

* current step indicator for user not in gpo flow yet

* a missing period

* Restore deleted translations, and rename start_over to start_over_new_address

Co-authored-by: Doug Price <douglas.price@gsa.gov>

* New template for confirm start over from request_letter

Add source param to indicate whether referer is request_letter

* Update specs to check for correct template

Co-authored-by: Doug Price <douglas.price@gsa.gov>

* Add before_letter route for new screen, don't use it yet

And analytics

* Lint, unused arg in analytics_events

* alphabetization lint

* Add suggested comment

Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>

* lints

---------

Co-authored-by: Douglas Price <douglas.price@gsa.gov>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>

* LG-11198: Update address text (#9420)

Update address text

changelog: User-Facing Improvements, IdV, Update text for address

* LG-10922: Display new headings for Hybrid Handoff page on AB test (#9316)

* changelog: User-Facing Improvements, Doc Auth, Display new headings for Hybrid Handoff page on AB test

Adds:

* Conditional headers depending on which flag is on
* Hybrid handoff show view test
* Translations

* LG-11235: Rename double address verification as ipp_enrollment_in_progress (#9390)

* Removed double address verification replaced with ipp_enrollment_in_progress

* changelog: Internal, In-person Proofing, change DAV references to reflect reality

* Change test description to be closer to what is being changed in the controller

* Addressing 50/50 state concerns in proofer and adjudicator

* Addressing linter issues

* Set missing initial value for dav

* Moving arg with default value to end of list

* Apply suggestions from code review

Adding proper input to job_arguments hash.

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Adding note about existing ticket for work post 50/50 state

* Resolving Shannon's comments

* Adding back in test for dav, need reader on adjudicator

* Adding back in test for dav, need reader on adjudicator

---------

Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Add --deflate option to data-pull and action-account scripts (#9424)


changelog: Internal, Scripts, Add --deflate option to data-pull and action-account scripts

---------

Co-authored-by: Matt Gardner <wilburnforce@gmail.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Matt Wagner <mattwagner@navapbc.com>
Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>
Co-authored-by: Douglas Price <douglas.price@gsa.gov>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>
Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com>
Co-authored-by: Brittany Greaner <35475380+night-jellyfish@users.noreply.github.com>
Co-authored-by: Jack Ryan <jackryan@navapbc.com>
Co-authored-by: jack.ryan@gsa.gov <johnaryan@fcoh2j-f4t79kf4.myfiosgateway.com>
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.

4 participants