Skip to content

Log face match results to events.log (LG-2495)#4018

Merged
zachmargolis merged 3 commits intomasterfrom
margolis-selfie-step-log-result
Aug 7, 2020
Merged

Log face match results to events.log (LG-2495)#4018
zachmargolis merged 3 commits intomasterfrom
margolis-selfie-step-log-result

Conversation

@zachmargolis
Copy link
Copy Markdown
Contributor

(Builds on previous work in #4007 and #4001 that update logging to the database)

This PR adds logging to events.log, it's stringing together a few different files so here's what's going on:

  • We have Flows and Steps, and FormResponses.
  • Before
    • If Step#call returns a FormResponse, we would log it
    • Our Acuant::Response objects duck-type FormResponses, but are not actually form responses so the previous check to log would fail
    • The Step#call was not returning the result object in most cases at all anyways
  • After
    • We now do a duck type check that allows Acuant::Response objects to be logged (the response objects are already pretty well-behaved about PII)
    • Updates specific steps to make sure they log results (not all)
  • Open Questions
    • Should we audit all steps now and make sure they log? For the purposes of this ticket and this PR, I think no, but it would probably match expectations later down the line

I also added a matcher for FakeAnalytics and to make testing this a little cleaner

Copy link
Copy Markdown
Contributor

@stevegsa stevegsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Copy Markdown
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Copy link
Copy Markdown
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zachmargolis zachmargolis merged commit b0fb3ec into master Aug 7, 2020
@zachmargolis zachmargolis deleted the margolis-selfie-step-log-result branch August 7, 2020 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants