Feat: analytics for multiple claims on a flow

[lalver1]

Closes #2355
This PR is a follow-up to #2463. It includes the following changes:

• Updating the returned enrollment analytics event to include the "extra claims" that came back as verified from IdG
• Storing the error codes returned from IdG in a dictionary since multiple claims may be returned
• Adding a column to EnrollmentEvent that holds the "extra claims"

Note that even though the coverage report for #2463 looked ok, this PR updates a few tests associated with #2463 because they felt a bit incomplete. For example

benefits/tests/pytest/oauth/test_views.py

Line 252 in 43492c6

def test_authorize_success_with_claim_true(

and

benefits/tests/pytest/oauth/test_views.py

Line 283 in 43492c6

def test_authorize_success_with_claim_false(

Reviewing

First run the database migrations since a new field was added to EnrollmentEvent. Since the changes involve the enrollment application and the claims that are returned from IdG, it's a bit tricky to review this PR. The tests seem to capture the behavior that has changed due to this PR, but you can also hard code a few lines to review the error code changes and EnrollmentEvent changes.

To review the error codes part, set claim_value = "10" in

benefits/benefits/oauth/views.py

Line 136 in 43492c6

claim_value = userinfo.get(claim)

and confirm that the error codes show up in the finished # event.

To see a new instance of an EnrollmentEvent that has multiple claims you can set, for example, extra_claims = "claim_1, claim_2" in

benefits/benefits/enrollment/views.py

Line 74 in 727a4c1

extra_claims = ", ".join(session.oauth_extra_claims(request))

to see how multiple claims are stored in the local database.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/core
models.py
session.py
benefits/enrollment
analytics.py
views.py					79
benefits/oauth
views.py
Project Total

This report was generated by python-coverage-comment-action

[angela-tran]

@lalver1 Thanks for the thorough notes in the PR description. I'm just now starting to review this PR and am a little confused by this note:

To see a new instance of an EnrollmentEvent in the local database, remove the EligibleSessionRequired decorator ...

Shouldn't we be able to just do a successful enrollment locally in order to see a new instance of an EnrollmentEvent in the local database? I'm not following why we need to do all these steps to fake it.

[lalver1]

Thanks for the review @angela-tran, yep, if you go through a successful enrollment you'll see the new instance of an EnrollmentEvent, but I think it only covers the case when IdG returns one claim. In the 'Reviewing' notes I was more thinking about being able to see how multiple claims would be shown on the local database in the new column since currently we don't really have a way to see this path when we go through a successful enrollment, so that's why I was faking it but there may be a cleaner way to test it out.

[angela-tran]

@lalver1 Oh ok, I see. So in the steps for faking it, should there also be a step where we make it so the EnrollmentEvent will have some extra_claims? I see that it's around line 74 in enrollment.views.index that we read extra claims.

[lalver1]

Exactly! I'm re-reading the EnrollmentEvent section of 'Reviewing' and actually all that stuff isn't really necessary @angela-tran. Like you mentioned, you really you only need to overwrite line 74 and set, for example, extra_claims = "claim_1, claim_2" to see how multiple claims are stored in the local database. Thanks for the comment, I'll update the description.

[lalver1]

Update on this PR:

• Updated the function

  benefits/benefits/core/session.py

  Line 164 in 0f3ec5c

  def oauth_extra_claims(request, eligibility_claim, oauth_claims):

  that returns only the "extra claims", now it checks for the claims_eligibility_claim instead of assuming it is the first element of the list (but after going back and forth writing this function, I'm realizing that this may not need to be in session.py since it's not using the first argument, request)
• Used existing pattern of calling helper function to simplify this PR's original code in

  benefits/benefits/enrollment/analytics.py

  Line 22 in 0f3ec5c

  self.update_event_properties(status=status, error=error, extra_claims=extra_claims)

Currently addressing the missing coverage due to the new code in session.py

[thekaveman]

It's getting closer, but still could use a bit of code cleanup

[lalver1]

One more update on this PR, we worked with @thekaveman on cleaning up the code some more earlier today and I ran through older adult with the debugger (mostly to make sure types made sense when making calls to .remove() and .join() for example) one last time just to feel a bit more comfortable with the changes introduced.

[thekaveman]

Nice job 🎉