Feat: analytics for in-person eligibility/enrollment #2402
Conversation
github-actions
bot
added
back-end
lalver1
force-pushed
the
feat/in-person-analytics
branch
from
802972c to
579aeda
Compare
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lalver1
force-pushed
the
feat/in-person-analytics
branch
2 times, most recently
from
16fe317 to
abd62bb
Compare
| @@ -30,6 +32,7 @@ def eligibility(request): | |||
| flow_id = form.cleaned_data.get("flow") | |||
| flow = models.EnrollmentFlow.objects.get(id=flow_id) | |||
| session.update(request, flow=flow) | |||
| eligibility_analytics.started_eligibility(request, flow, enrollment_method=models.EnrollmentMethods.IN_PERSON) | |||
There was a problem hiding this comment.
I would think we also want to send the selected enrollment flow event here, right? For consistency with the digital side.
There was a problem hiding this comment.
Yep, good idea. Added it in 1595941
benefits/in_person/views.py
Outdated
| sentry_sdk.capture_exception(exception) | ||
| return redirect(routes.IN_PERSON_SERVER_ERROR) | ||
|
|
||
| case Status.REENROLLMENT_ERROR: | ||
| return redirect(routes.IN_PERSON_ENROLLMENT_REENROLLMENT_ERROR) | ||
| # GET enrollment index | ||
| else: | ||
| flow = session.flow(request) | ||
| eligibility_analytics.returned_success(request, flow, enrollment_method=models.EnrollmentMethods.IN_PERSON) |
There was a problem hiding this comment.
I think in general, we should move away from sending events in response to GET requests where possible.
It becomes easier to spam the events if all you need to do is visit the URL. Especially for these types of events where we do already have a POST-back, I think ideally the event should be sent there.
There was a problem hiding this comment.
Ah that makes sense! Moved it to the POST section in e9f5ead
benefits/in_person/views.py
Outdated
| @@ -137,6 +155,7 @@ def enrollment(request): | |||
|
|
|||
| def reenrollment_error(request): | |||
| """View handler for a re-enrollment attempt that is not yet within the re-enrollment window.""" | |||
| enrollment_analytics.returned_error(request, "Re-enrollment error.", enrollment_method=models.EnrollmentMethods.IN_PERSON) | |||
There was a problem hiding this comment.
Could be moved up into POST-back
There was a problem hiding this comment.
Yep, moved it up in f5a376a
benefits/in_person/views.py
Outdated
| @@ -147,6 +166,7 @@ def reenrollment_error(request): | |||
|
|
|||
| def retry(request): | |||
| """View handler for card verification failure.""" | |||
| enrollment_analytics.returned_retry(request, enrollment_method=models.EnrollmentMethods.IN_PERSON) | |||
There was a problem hiding this comment.
This one can't be moved since the user is sent here from JavaScript.
Would could enforce that this route is POST-only and make the from POST back to here... that doesn't have to happen in this PR though.
There was a problem hiding this comment.
Right 👍
So it turns out that the form's (form_retry) method is already POST, so would it be enough to wrap the analytics call in an if request.method == "POST":?
lalver1
force-pushed
the
feat/in-person-analytics
branch
from
abd62bb to
1595941
Compare
lalver1
force-pushed
the
feat/in-person-analytics
branch
from
60b1401 to
0ca7d95
Compare
Closes #2245
This PR implements in-person analytics events for
started eligibilityreturned eligibilityselected enrollment flowstarted card tokenizationended card tokenizationreturned enrollmentfailed access token requestSince the
started card tokenizationandended card tokenizationevents use the Amplitude browser SDK (instead of the HTTP API used byClientincore/analytics.py), theenrollmentviews andindex.htmltemplate have been updated in aef5527 to include theenrollment_methodfield.