LG-8423 log cancel analytics from barcode page#7838
Conversation
|
Do we need a separate event for this, or can we use the I think we need to change the |
@aduth We could potentially change the step name in order to distinguish "the ready to verify page"/barcode page and the "verify your info" page. The additional information this event seeks to track though is if a user opts to cancel their enrollment or not, and information about their enrollment. This seems slightly different from the current @daviddsilvanava thoughts? update: Chatted w/ David and we decided to go with modifying the following existing events so they track information about the enrollment and user's action re:cancelling |
Yeah I think that'd be great if we can use the existing events and add any extra details we want specific to IPP. |
| analytics.idv_cancellation_go_back(step: params[:step]) | ||
| analytics.idv_cancellation_go_back( | ||
| step: params[:step], | ||
| extra: barcode_step ? extra_analytics_attributes : nil, |
There was a problem hiding this comment.
The way the methods are set up in AnalyticsEvents to receive **extra arguments, it's not necessary to explicitly assign an extra key, we could choose to merge the extra values directly into top-level arguments.
| extra: barcode_step ? extra_analytics_attributes : nil, | |
| **extra_analytics_attributes, |
(This suggestion would require moving barcode_step consideration to extra_analytics_attributes:)
def extra_analytics_attributes
extra = {}
if barcode_step?
extra.merge!(
cancelled_enrollment: false,
# ...
)
end
extra
end| def barcode_step | ||
| return params[:step] == 'barcode' | ||
| end | ||
|
|
||
| def enrollment | ||
| return InPersonEnrollment.where(user_id: current_user.id).first |
There was a problem hiding this comment.
Couple minor thoughts:
- If this method returns a boolean value, conventionally I'd expect it to end with
? - The explicit
returnisn't necessary since the last statement of the method is implicitly returned
| def barcode_step | |
| return params[:step] == 'barcode' | |
| end | |
| def enrollment | |
| return InPersonEnrollment.where(user_id: current_user.id).first | |
| def barcode_step? | |
| params[:step] == 'barcode' | |
| end | |
| def enrollment | |
| InPersonEnrollment.where(user_id: current_user.id).first |
| enrollment_code: enrollment.enrollment_code, | ||
| enrollment_id: enrollment.id, |
There was a problem hiding this comment.
We probably want to add some checks that enrollment exists, since we don't validate that anywhere. For example, I was able to trigger a 500 error by;
- Sign in
- Go to http://localhost:3000/verify/cancel?step=barcode
- Click "No, keep going"
| cancelled_enrollment: false, | ||
| enrollment_code: enrollment.enrollment_code, | ||
| enrollment_id: enrollment.id, | ||
| service_provider: decorated_session.sp_name || APP_NAME, |
There was a problem hiding this comment.
Do we need to include this, or can we rely on it being provided in the default event attributes?
identity-idp/app/services/analytics.rb
Line 73 in f226bdd
| service_provider: decorated_session.sp_name || APP_NAME, |
There was a problem hiding this comment.
you're right, we do get that in the default - i'll remove this addition
| cancelled_enrollment: true, | ||
| enrollment_code: enrollment.enrollment_code, | ||
| enrollment_id: enrollment.id, | ||
| service_provider: decorated_session.sp_name || APP_NAME, |
There was a problem hiding this comment.
Same:
| service_provider: decorated_session.sp_name || APP_NAME, |
* LG-8796: Add recaptcha disclosure form at bottom of phone setup screen for new and existing accounts (#7827) * changelog: Upcoming Features, Authentication, Add recaptcha disclosure form at bottom of phone setup screen * styling fixes and dont hardcode login.gov * hide and show other movile service * add captcha submit button to add new phone * add to hide grecaptcha-badge * use recaptcha disclosure shared * remove empty space * lint recaptcha badge * Skip devices database query when identifier is null (#7856) changelog: Internal, Performance, Skip devices database query when identifier is null * Removed analytics check from password visibility toggle spec. (#7860) The test was flaky and the flag is unused, per Andrew Duthie. [skip changelog] * Removed use of Faker email creation in user factory primary email (#7861) * Removed use of Faker email creation in user factory primary email We suspect that using Faker email, rather than explicitly setting sequential emails, was occasionally creating collisions and leading to flaky test failures. [skip changelog] * Add 2023 SAML certs (#7862) (symlink 2022 ones to have non-blank files there) changelog: Internal, Certificates, 2023 SAML Certificate rotation * LG-8215 Update Event: IDV Reproof using Address verify by Mail (#7859) changelog: Internal, Attempts API, Update in tracking event * LG-8604 - Optimize s3 by streaming data w/ feature flag (#7843) * WIP - optimize s3 by streaming data changelog: Internal, Attempts API, Scalability Optimization * shifted a line - rubocop apparently wasn't picky about this * added feature flag. updated spec to reflect nested conditionals * cleaned up some comments. * broke out s3 response code into its own method. * cleaned up code based on feedback * updated default buffer size and made some clarifying changes based on feedback changelog: Internal, Attempts Api, Optimization: S3 Streaming * cleaned up more test descriptions. changelog: Internal, Attempts Api, Feature: S3 Streaming * Add logging ahead of removing Analytics logic (#7864) * Add Rails.logger.info logging near proposed call sites changelog: Internal, Logging, Add logging ahead of streamlining analytics events * Remove unused Authentication Presenter data (#7868) * Remove unused Authentication Presenter data changelog: Internal, Maintenance, Remove unused methods and variables * remove unused pivcac email data * remove more unused methods * Disable notify for good_job (#7870) changelog: Internal, Background Jobs, Update GoodJob and disable NOTIFY * LG-8423 log cancel analytics from barcode page (#7838) * analytics logged when user cancels from barcode pg * update event and add spec for for event in sessions controller * event and spec added for cancellations controller * changelog: Internal, Analytics Events, add cancel from barcode page analytics event * fix lint issue w/ spec * change step to barcode to make unique * use cancellation go back w/ extra attributes when user comes from barcode pg * remove new event and update session controller * update spec * change enrollment key * update cancel spec * check for enrollment and merge extra attributes and update tests * use pending enrollment and remove sp as it is redundant * update spec to create pending enrollment * LG-8901: New Hybrid Handoff (upload step) with feature flag (#7849) * Adding initial fields and new icons * Deleting useless hybrid handoff mobile tests * Pulling in send_link step functionality to upload_step * Adding feature flag for new combined hybrid handoff step * Adding additional checking for 50/50 conditions -- What As @matthinz pointed out, the current code does not account well for the following 50/50 states: 1. User is shown the new combined upload view, but submits the phone number to an instance with the feature flag off; 2. User is shown the original upload view, but clicks the phone button submitting to an instance where the feature flag is now on We handle this case by adding a request param _only_ to the form rendered by the new combined view. That view is only shown to users who have the feature flag enabled. The upload step will now check for that param -- called :combined -- and can determine regardless of the current state of the feature flag, that the user actually has submitted a phone number to the upload step itself, and that the send link step should be skipped. Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> changelog: User-Facing Improvements, Hybrid Handoff Screen, Combining upload and send link steps into one screen * Unpack a dynamic send to make code easier to trace (#7869) * Remove now-unused constant changelog: Internal, Refactor, Small update to remove dynamic send in 2FA mixin * LG-8875 / LG-8877: Try to avoid race condition in document-capture-welcome (#7842) * "Convert" document-capture-welcome to typescript (It was already valid TS) * Rework camera check to block form submission Don't let the form submit until the promise we use to check for the presence / nonpresence of a camera has resolved. * Disable submit buttons while we wait * TEMP: Allow injecting delay into hasCamera() Set HAS_CAMERA_DELAY_MS in localStorage to configure. * Update app/javascript/packs/document-capture-welcome.ts Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Use SpinnerButtonComponent for welcome/agreement * Simplify cameraCheckPromise usage Eliminate additional local variable and just race the promises * Wrap agreement spinner button in <div> Adding margin-top-4 to button does not get applied to the spinner dots * changelog: User-Facing Improvements, Identity verification, help prevent mobile users from ending up in hybrid handoff flow * Log event from frontend for mobile/camera checks Ideally, we want to see if this code actually fixes anything. (Relates to LG-8877) Co-authored-by: Eric Gade <eric.gade@gsa.gov> * Refactor perf measuring slightly Pull out to a function that just augments the promise with perf data * Add spin_on_click to SpinnerButtonComponent Allow setting spin-on-click as needed. * Toggle spinner on form submission Just in case there is a form validation issue, don't start spinning until we're sure we can actually make the submission. * await trackEvent that could occur during form submission * Add idv_mobile_device_and_camera_check to AnalyticsEvents Document the event and make it so it's not prefixed with "Frontend:" * Revert "TEMP: Allow injecting delay into hasCamera()" This reverts commit 96cbc3e. * Don't use performance.measure() Complicated API, don't need it for what we're doing. --------- Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <eric.gade@gsa.gov> * LG-7836 Automatically refresh USPS auth token when necessary (#7808) * implement retry behavior with repetitive code * remove begin/rescue blocks and get failing tests * add retry options to faraday. tests still fail. * refactor implementation & spec * changelog:Bug Fixes, In-person proofing, refresh USPS auth token when request fails * attempt broader retry behavior. tests still fail. * attempt to refresh token before pinging USPS api. more tests fail * got first test to pass, need to get other expired token tests passing * get all expired token tests to pass * DRY up proofer test setup * add buffer to expires_at conditional * fix broken tests by adding redis cache setup * fix broken enrollment helper tests by adding caching setup * respond to feedback * remove no-op * Fixing missed grid gap sizing (follow up to LG-8901) (#7873) * Fixing missed grid gap sizing h/t @aduth changelog: User-Facing Improvements, Hybrid Handoff Screen, Combining upload and send link steps into one screen * LG-9025 Go to new SsnController from hybrid flow when feature flag is set (#7875) From the DocumentCaptureStep, we only want to redirect to the new SsnController if we are in the DocAuthFlow. This step is reused from the Hybrid Flow, and we don't want to go to the SsnController on mobile. In hybrid flow, we don't return to the DocumentCaptureStep, so redirect to SsnController from LinkSentStep when the feature flag is true as well. Duplicate the hybrid flow test with doc_auth_ssn_controller_enabled flag set to true and make sure it passes. Note that on my machine even on main without the feature flag I have to set a binding.pry and delay the mobile browser part of the test to get it to pass. changelog: Internal, refactor, fix hybrid flow with new SsnController (feature flagged) Co-authored-by: eric-gade <eric.gade@gsa.gov> --------- Co-authored-by: Malick Diarra <malick.diarra@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> Co-authored-by: John Maxwell <john.maxwell@gsa.gov> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Osman Latif <109746710+olatifflexion@users.noreply.github.com> Co-authored-by: Rwolfe-Nava <87499456+Rwolfe-Nava@users.noreply.github.com> Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Eric Gade <eric.gade@gsa.gov> Co-authored-by: eileen-nava <80347702+eileen-nava@users.noreply.github.com> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
🎫 Ticket
Lg-8423
🛠 Summary of changes
Adjusts current analytics event that track user's selection from cancel page after arriving there from the barcode page. Additional information logged in events includes whether the user cancelled their enrollment, service provider and enrollment info.
Updates step name for the barcode page so that it is unique and be used to determine if the
extra_analytics_attributesshould be included.idv_cancellation_go_backevent logged from thecancellations_controllerrepresents a user who has clicked theno, keep goingbutton and therefore declines to cancel their enrollment.idv_start_overevent logged from thesession_controllerrepresents a user who has clicked theStart overbutton and therefore cancels their enrollment.📜 Testing Plan
Provide a checklist of steps to confirm the changes.
cancel your barcodebutton at bottom of "ready to verify page"no, keep goingbuttonevents.logand that the value of thecancelled_enrollmentfield is false, step isbarcodeand extra attributes are not nilcancel your barcodebutton at bottom of "ready to verify page"Start overbuttonevents.logand that the value of thecancelled_enrollmentfield is true, step isbarcodeand extra attributes are not nil