LG-13408 Standardize logging for opted in ipp true/false#10983
Merged
jennyverdeyen merged 3 commits intomainfrom Jul 30, 2024
Merged
LG-13408 Standardize logging for opted in ipp true/false#10983jennyverdeyen merged 3 commits intomainfrom
jennyverdeyen merged 3 commits intomainfrom
Conversation
changelog: Internal, In-person proofing, Standardize logging for opted in ipp true/false values
6e481d3 to
cf1bdf4
Compare
aduth
reviewed
Jul 24, 2024
Contributor
Author
|
@gina-yamada You'll notice I removed the old approach I was discussing with you yesterday in favor of making the conversion from the frontend. Doing it this way also simplified the testing approach since the value is expectedly false instead of nil for the cases in analytics_spec.rb. I considered keeping the logger conversion as a future safeguard but ended up removing it in favor of DRYer code... I reasoned that it's unlikely there are other string versions of the value occurring by that point, and the analytics tests could catch that if it were the case. Noting this for posterity, and in case anyone feels the logger conversion is worth keeping. |
76e329d to
20b3a9c
Compare
eileen-nava
approved these changes
Jul 30, 2024
Contributor
eileen-nava
left a comment
There was a problem hiding this comment.
I tested and this looks good. Nice work, Jenny.
| use_alternate_sdk: useAlternateSdk, | ||
| acuant_version: acuantVersion, | ||
| opted_in_to_in_person_proofing: optedInToInPersonProofing, | ||
| opted_in_to_in_person_proofing: optedInToInPersonProofing === 'true', |
mitchellhenke
pushed a commit
that referenced
this pull request
Jul 31, 2024
* LG-13408 Standardize logging for opted in ipp true/false changelog: Internal, In-person proofing, Standardize logging for opted in ipp true/false values
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to the relevant ticket:
LG-13408
🛠 Summary of changes
Analytics logging for certain events were returning strings "true"/"false" for the property
opted_in_to_in_person_proofingwhereas that property is logged with a boolean value in other events. This change ensures the property is logged as a boolean in these events:IdV: in person proofing location submitted
IdV: in person proofing location visited
IdV: in person proofing prepare submitted
IdV: in person proofing prepare visited
Provide a checklist of steps to confirm the changes.
Start the event log watch with
make watch_eventsin your terminalTrigger each event and observe that
opted_in_to_in_person_proofingis true (Boolean) and not "true" (String), when opted_in_to_in_person_proofing is trueopted_in_to_in_person_proofingwill be true"IdV: in person proofing prepare submitted"and"IdV: in person proofing location visited"events will be logged. Verify that event_properties.opted_in_to_in_person_proofing is true (Boolean) for both events."IdV: in person proofing location submitted"event will be logged as a result. Verify that event_properties.opted_in_to_in_person_proofing is true (Boolean)"IdV: in person proofing prepare visited"is sometimes inconsistent to trigger with opt-in; the page doesn't consistently show up in the flow. One way I have triggered it is by getting to the PO location page, then replacing "#location" with "#prepare" in the url so it reads "http://localhost:3000/verify/document_capture#prepare" - this works for me from the location page, but not necessarily by simply entering that URL from anywhere else. Observe the event is triggered and verify that event_properties.opted_in_to_in_person_proofing is true (Boolean)Trigger each event and observe that
opted_in_to_in_person_proofingis false (Boolean) and not "false" (String), when opted_in_to_in_person_proofing is falseopted_in_to_in_person_proofingwill now be false"IdV: in person proofing prepare visited"will be triggered. Verify that event_properties.opted_in_to_in_person_proofing is false (Boolean)"IdV: in person proofing prepare submitted"and"IdV: in person proofing location visited"events will be logged. Verify that event_properties.opted_in_to_in_person_proofing is false (Boolean) for both events."IdV: in person proofing location submitted"event will be logged as a result. Verify that event_properties.opted_in_to_in_person_proofing is false (Boolean)Screenshots:
Before:
After: