Skip to content

Commit

Permalink
Feat: analytics for multiple claims on a flow (#2479)
Browse files Browse the repository at this point in the history
  • Loading branch information
lalver1 authored Oct 31, 2024
2 parents 6bc10bf + 63a154b commit 82d6604
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 21 deletions.
18 changes: 18 additions & 0 deletions benefits/core/migrations/0030_enrollmentevent_extra_claims.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 5.1.2 on 2024-10-29 22:11

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("core", "0029_add_extra_claims"),
]

operations = [
migrations.AddField(
model_name="enrollmentevent",
name="extra_claims",
field=models.TextField(blank=True, null=True),
),
]
1 change: 1 addition & 0 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ class EnrollmentEvent(models.Model):
verified_by = models.TextField()
enrollment_datetime = models.DateTimeField(default=timezone.now)
expiration_datetime = models.DateTimeField(blank=True, null=True)
extra_claims = models.TextField(blank=True, null=True)

def __str__(self):
dt = timezone.localtime(self.enrollment_datetime)
Expand Down
15 changes: 14 additions & 1 deletion benefits/core/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,23 @@ def oauth_token(request):


def oauth_claims(request):
"""Get the oauth claim from the request's session, or None"""
"""Get the oauth claims from the request's session, or None"""
return request.session.get(_OAUTH_CLAIMS)


def oauth_extra_claims(request):
"""Get the extra oauth claims from the request's session, or None"""
claims = oauth_claims(request)
if claims:
f = flow(request)
if f and f.uses_claims_verification:
claims.remove(f.claims_eligibility_claim)
return claims
raise Exception("Oauth claims but no flow")
else:
return None


def origin(request):
"""Get the origin for the request's session, or default to the index route."""
return request.session.get(_ORIGIN, reverse(routes.INDEX))
Expand Down
20 changes: 16 additions & 4 deletions benefits/enrollment/analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,18 @@
class ReturnedEnrollmentEvent(core.Event):
"""Analytics event representing the end of transit processor enrollment request."""

def __init__(self, request, status, error=None, enrollment_group=None, enrollment_method=models.EnrollmentMethods.DIGITAL):
def __init__(
self,
request,
status,
error=None,
enrollment_group=None,
enrollment_method=models.EnrollmentMethods.DIGITAL,
extra_claims=None,
):
super().__init__(request, "returned enrollment", enrollment_method)
if str(status).lower() in ("error", "retry", "success"):
self.update_event_properties(status=status, error=error)
self.update_event_properties(status=status, error=error, extra_claims=extra_claims)
if enrollment_group is not None:
self.update_event_properties(enrollment_group=enrollment_group)

Expand All @@ -35,11 +43,15 @@ def returned_retry(request, enrollment_method: str = models.EnrollmentMethods.DI
core.send_event(ReturnedEnrollmentEvent(request, status="retry", enrollment_method=enrollment_method))


def returned_success(request, enrollment_group, enrollment_method: str = models.EnrollmentMethods.DIGITAL):
def returned_success(request, enrollment_group, enrollment_method: str = models.EnrollmentMethods.DIGITAL, extra_claims=None):
"""Send the "returned enrollment" analytics event with a success status."""
core.send_event(
ReturnedEnrollmentEvent(
request, status="success", enrollment_group=enrollment_group, enrollment_method=enrollment_method
request,
status="success",
enrollment_group=enrollment_group,
enrollment_method=enrollment_method,
extra_claims=extra_claims,
)
)

Expand Down
9 changes: 8 additions & 1 deletion benefits/enrollment/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,22 @@ def index(request):
agency = session.agency(request)
flow = session.flow(request)
expiry = session.enrollment_expiry(request)
oauth_extra_claims = session.oauth_extra_claims(request)
# EnrollmentEvent expects a string value for extra_claims
if oauth_extra_claims:
str_extra_claims = ", ".join(oauth_extra_claims)
else:
str_extra_claims = ""
event = models.EnrollmentEvent.objects.create(
transit_agency=agency,
enrollment_flow=flow,
enrollment_method=models.EnrollmentMethods.DIGITAL,
verified_by=flow.eligibility_verifier,
expiration_datetime=expiry,
extra_claims=str_extra_claims,
)
event.save()
analytics.returned_success(request, flow.group_id)
analytics.returned_success(request, flow.group_id, extra_claims=oauth_extra_claims)
return success(request)

case Status.SYSTEM_ERROR:
Expand Down
7 changes: 3 additions & 4 deletions benefits/oauth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def authorize(request):
flow_claims = flow.claims_all_claims
stored_claims = []

error_claim = None
error_claim = {}

if flow_claims:
userinfo = token.get("userinfo")
Expand All @@ -141,9 +141,8 @@ def authorize(request):
elif claim_value == 1:
# if userinfo contains our claim and the flag is 1 (true), store the *claim*
stored_claims.append(claim)
elif claim_value >= 10 and claim == flow.claims_eligibility_claim:
# error_claim is only set if claim is the eligibility claim
error_claim = claim_value
elif claim_value >= 10:
error_claim[claim] = claim_value

session.update(request, oauth_token=id_token, oauth_claims=stored_claims)
analytics.finished_sign_in(request, error=error_claim)
Expand Down
5 changes: 5 additions & 0 deletions tests/pytest/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,8 @@ def mocked_session_reset(mocker):
@pytest.fixture
def mocked_session_update(mocker):
return mocker.patch("benefits.core.session.update")


@pytest.fixture
def mocked_session_oauth_extra_claims(mocker):
return mocker.patch("benefits.core.session.oauth_extra_claims")
33 changes: 33 additions & 0 deletions tests/pytest/core/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,36 @@ def test_update_flow(app_request):
@pytest.mark.django_db
def test_flow_default(app_request):
assert session.flow(app_request) is None


@pytest.mark.django_db
def test_oauth_extra_claims(app_request, model_EnrollmentFlow_with_scope_and_claim):

app_request.session[session._FLOW] = model_EnrollmentFlow_with_scope_and_claim.id
app_request.session[session._OAUTH_CLAIMS] = [
model_EnrollmentFlow_with_scope_and_claim.claims_eligibility_claim,
"extra_claim",
]

assert session.oauth_extra_claims(app_request) == ["extra_claim"]


@pytest.mark.django_db
def test_oauth_extra_claims_no_claims(app_request, model_EnrollmentFlow_with_scope_and_claim):

app_request.session[session._FLOW] = model_EnrollmentFlow_with_scope_and_claim.id
app_request.session[session._OAUTH_CLAIMS] = []

assert session.oauth_extra_claims(app_request) is None


@pytest.mark.django_db
def test_oauth_extra_claims_claims_no_flow(app_request):

app_request.session[session._OAUTH_CLAIMS] = [
"eligibility_claim",
"extra_claim",
]

with pytest.raises(Exception, match="Oauth claims but no flow"):
session.oauth_extra_claims(app_request)
20 changes: 19 additions & 1 deletion tests/pytest/enrollment/test_analytics.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,28 @@
import pytest

from benefits.enrollment.analytics import FailedAccessTokenRequestEvent
from benefits.enrollment.analytics import FailedAccessTokenRequestEvent, ReturnedEnrollmentEvent


@pytest.mark.django_db
def test_FailedAccessTokenRequestEvent_sets_status_code(app_request):
event = FailedAccessTokenRequestEvent(app_request, 500)

assert event.event_properties["status_code"] == 500


@pytest.mark.django_db
def test_ReturnedEnrollmentEvent_without_error(app_request, mocker):

key1 = "enrollment_flows"
key2 = "extra_claims"
mock_flow = mocker.Mock()
mock_flow.system_name = "flow_1"
mocker.patch("benefits.core.session.flow", return_value=mock_flow)

mock_claims = ["eligibility_claim", "extra_claim"]
mocker.patch("benefits.core.session.oauth_claims", return_value=mock_claims)

event = ReturnedEnrollmentEvent(app_request, status="success")
assert "error_code" not in event.event_properties
assert key1 in event.event_properties
assert key2 in event.event_properties
6 changes: 6 additions & 0 deletions tests/pytest/enrollment/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,9 @@ def test_index_eligible_post_valid_form_success_claims(
mocked_analytics_module,
model_TransitAgency,
model_EnrollmentFlow_with_scope_and_claim,
mocked_session_oauth_extra_claims,
):
mocked_session_oauth_extra_claims.return_value = ["claim_1", "claim_2"]
mocker.patch("benefits.enrollment.views.enroll", return_value=(Status.SUCCESS, None))
spy = mocker.spy(benefits.enrollment.views.models.EnrollmentEvent.objects, "create")

Expand All @@ -318,6 +320,7 @@ def test_index_eligible_post_valid_form_success_claims(
enrollment_method=models.EnrollmentMethods.DIGITAL,
verified_by=model_EnrollmentFlow_with_scope_and_claim.claims_provider.client_name,
expiration_datetime=None,
extra_claims="claim_1, claim_2",
)

assert response.status_code == 200
Expand All @@ -335,7 +338,9 @@ def test_index_eligible_post_valid_form_success_eligibility_api(
mocked_analytics_module,
model_TransitAgency,
model_EnrollmentFlow_with_eligibility_api,
mocked_session_oauth_extra_claims,
):
mocked_session_oauth_extra_claims.return_value = ["claim_1", "claim_2"]
mocker.patch("benefits.enrollment.views.enroll", return_value=(Status.SUCCESS, None))
spy = mocker.spy(benefits.enrollment.views.models.EnrollmentEvent.objects, "create")

Expand All @@ -348,6 +353,7 @@ def test_index_eligible_post_valid_form_success_eligibility_api(
enrollment_method=models.EnrollmentMethods.DIGITAL,
verified_by=model_EnrollmentFlow_with_eligibility_api.eligibility_api_url,
expiration_datetime=None,
extra_claims="claim_1, claim_2",
)

assert response.status_code == 200
Expand Down
62 changes: 52 additions & 10 deletions tests/pytest/oauth/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,31 +237,62 @@ def test_authorize_success(

@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_analytics_module")
@pytest.mark.parametrize(
"extra_claims,userinfo,oauth_claims",
[
(None, {"claim": 1}, ["claim"]),
("", {"claim": 1}, ["claim"]),
("extra_claim", {"claim": 1, "extra_claim": 1}, ["claim", "extra_claim"]),
(
"extra_claim_1 extra_claim_2",
{"claim": 1, "extra_claim_1": 1, "extra_claim_2": 1},
["claim", "extra_claim_1", "extra_claim_2"],
),
],
)
def test_authorize_success_with_claim_true(
app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client
app_request,
mocked_session_flow_uses_claims_verification,
mocked_oauth_client_or_error_redirect__client,
extra_claims,
userinfo,
oauth_claims,
):
flow = mocked_session_flow_uses_claims_verification.return_value
flow.claims_extra_claims = ""
flow.claims_extra_claims = extra_claims
mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "1"}}
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo}

result = authorize(app_request)

mocked_oauth_client.authorize_access_token.assert_called_with(app_request)
assert session.oauth_claims(app_request) == ["claim"]
assert session.oauth_claims(app_request) == oauth_claims
assert result.status_code == 302
assert result.url == reverse(routes.ELIGIBILITY_CONFIRM)


@pytest.mark.django_db
@pytest.mark.usefixtures("mocked_analytics_module")
@pytest.mark.parametrize(
"extra_claims,userinfo",
[
(None, {"claim": 0}),
("", {"claim": 0}),
("extra_claim", {"claim": 0, "extra_claim": 0}),
("extra_claim_1 extra_claim_2", {"claim": 0, "extra_claim_1": 0, "extra_claim_2": 0}),
],
)
def test_authorize_success_with_claim_false(
app_request, mocked_session_flow_uses_claims_verification, mocked_oauth_client_or_error_redirect__client
app_request,
mocked_session_flow_uses_claims_verification,
mocked_oauth_client_or_error_redirect__client,
extra_claims,
userinfo,
):
flow = mocked_session_flow_uses_claims_verification.return_value
flow.claims_extra_claims = ""
flow.claims_extra_claims = extra_claims
mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "0"}}
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo}

result = authorize(app_request)

Expand All @@ -272,21 +303,32 @@ def test_authorize_success_with_claim_false(


@pytest.mark.django_db
@pytest.mark.parametrize(
"extra_claims,userinfo",
[
(None, {"claim": 10}),
("", {"claim": 10}),
("extra_claim", {"claim": 10, "extra_claim": 10}),
("extra_claim_1 extra_claim_2", {"claim": 10, "extra_claim_1": 10, "extra_claim_2": 10}),
],
)
def test_authorize_success_with_claim_error(
app_request,
mocked_session_flow_uses_claims_verification,
mocked_oauth_client_or_error_redirect__client,
mocked_analytics_module,
extra_claims,
userinfo,
):
flow = mocked_session_flow_uses_claims_verification.return_value
flow.claims_extra_claims = ""
flow.claims_extra_claims = extra_claims
mocked_oauth_client = mocked_oauth_client_or_error_redirect__client.return_value
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": {"claim": "10"}}
mocked_oauth_client.authorize_access_token.return_value = {"id_token": "token", "userinfo": userinfo}

result = authorize(app_request)

mocked_oauth_client.authorize_access_token.assert_called_with(app_request)
mocked_analytics_module.finished_sign_in.assert_called_with(app_request, error=10)
mocked_analytics_module.finished_sign_in.assert_called_with(app_request, error=userinfo)
assert session.oauth_claims(app_request) == []
assert result.status_code == 302
assert result.url == reverse(routes.ELIGIBILITY_CONFIRM)
Expand Down

0 comments on commit 82d6604

Please sign in to comment.