Log GPO letter requested in analytics#6658
Merged
Conversation
jmhooper
reviewed
Aug 1, 2022
kbighorse
changed the title
jmhooper
reviewed
Aug 1, 2022
Changelog: Improvements (LG-6793) categories: - Improvements
aduth
reviewed
Aug 2, 2022
zachmargolis
reviewed
Aug 2, 2022
changelog: Internal, Logging, Log the time an address verification letter is enqueued
…/18F/identity-idp into LG-6793-log-gpo-letter-requested
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
…/18F/identity-idp into LG-6793-log-gpo-letter-requested
jmhooper
reviewed
Aug 3, 2022
| idv_session.complete_session | ||
|
|
||
| if idv_session.address_verification_mechanism == 'gpo' | ||
| analytics.idv_gpo_address_letter_requested(enqueued_at: Time.zone.now) |
Contributor
There was a problem hiding this comment.
Just had the thought that we may want to add a resend attribute or something like that to these. That way we can filter by whether a letter is being resent. That could potentially be a different story though.
note: my understanding that these hash values are currently ignored in this spec
jmhooper
reviewed
Aug 4, 2022
spec/features/idv/analytics_spec.rb
Outdated
| 'IdV: doc auth verify_wait visited' => { flow_path: 'standard', step: 'verify_wait', step_count: 1 }, | ||
| 'IdV: doc auth optional verify_wait submitted' => { success: true, errors: {}, address_edited: false, proofing_results: { messages: [], exception: nil, transaction_id: 'resolution-mock-transaction-id-123', reference: 'aaa-bbb-ccc', timed_out: false, context: { dob_year_only: false, should_proof_state_id: true, stages: { resolution: { client: 'ResolutionMock', errors: {}, exception: nil, success: true, timed_out: false, transaction_id: 'resolution-mock-transaction-id-123', reference: 'aaa-bbb-ccc' }, state_id: { client: 'StateIdMock', errors: {}, success: true, timed_out: false, exception: nil, transaction_id: 'state-id-mock-transaction-id-456', state: 'MT', state_id_jurisdiction: 'ND' } } } }, ssn_is_unique: true, step: 'verify_wait_step_show' }, | ||
| 'IdV: phone of record visited' => {}, | ||
| 'IdV: USPS address letter requested' => { enqueued_at: now }, |
Contributor
There was a problem hiding this comment.
I think freezing time may have actually been the move here. It looks like the controller is still creating the Time object that this is being compared against. As a result they may not be equal if Time advances between the init of this Time object and the controller's Time object.
Around block freezes time so Time.zone.now doesn't drift between initializing and testing
jmhooper
approved these changes
Aug 4, 2022
jmhooper
pushed a commit
that referenced
this pull request
Aug 8, 2022
* Test analytics logs GPO letter requested * Rename new property * Support path to request letter instead of verify phone * Create helper method for gpo step * Test analytics events * Add expected parameter * Pass `enqueued_at` to `track_event` * Modify formatting * Remove unused event * Move test to services analytics spec * Use fake analytics in test * Rename analytics object Changelog: Improvements (LG-6793) categories: - Improvements * Add comment on new method * Rename analytics object changelog: Internal, Logging, Log the time an address verification letter is enqueued * Add comment on new method * Log letter requested when profile is initiated * Log enqueued at in 2 places * Camel case data type Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Support extra parameters * Add trailing comma * Inject current time as a default value * Capture current time once for testing later note: my understanding that these hash values are currently ignored in this spec * Replace now variable with freeze time Around block freezes time so Time.zone.now doesn't drift between initializing and testing Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Merged
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
5 participants
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.
Partially closes https://cm-jira.usa.gov/browse/LG-6793
This pull request adds a DateTime parameter to the analytics event to track that a GPO address letter was requested. Also includes calls from gpo_controller and review_controller and supporting specs.