Refactor: analytics for claims_provider
#2401
Conversation
github-actions
bot
added
deployment-dev
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.update_event_properties(auth_provider=verifier.claims_provider.client_name) | ||
| flow = session.flow(request) | ||
| if flow and flow.uses_claims_verification: | ||
| self.update_event_properties(claims_provider=flow.claims_provider.client_name) |
There was a problem hiding this comment.
This change was the actual point of this PR
|
Preview url: https://benefits-2401--cal-itp-previews.netlify.app |
thekaveman
force-pushed
the
refactor/claimsprovider-amplitude
branch
from
fd51471
to
c73ff13
Compare
There was a problem hiding this comment.
This looks good to me! Thanks for catching those old variable names / docs that needed to be updated from verifier to flow. I did a global search of "auth_provider", "auth provider", and "verifier" to double-check it as well.
Question (maybe more for #2247 than this PR): will there be commits in cal-itp/data-infra for updating the old references to auth_provider? e.g. in the SQL, YAML, etc.
Yes, see cal-itp/data-infra#3468 -- I figured we could just use one big PR over there to make all these changes once we're done on the Benefits side. Definitely open to smaller PRs for each individual fix too, just didn't want to burden that team with a ton of PR/reviews. I'm going to wait to merge this until @lalver1 merges #2402 since there will be conflicts, and it will probably be easier to resolve in this PR. |
update some variable naming as well
thekaveman
force-pushed
the
refactor/claimsprovider-amplitude
branch
from
c73ff13
to
87b29cf
Compare
thekaveman
force-pushed
the
refactor/claimsprovider-amplitude
branch
from
87b29cf
to
5f56d3d
Compare
Part of #2247
Included a number of small renames/docs updates that I missed during #2379.