Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-14905: socure webhook analytics event updates #11490

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

amirbey
Copy link
Contributor

@amirbey amirbey commented Nov 9, 2024

🎫 Ticket

LG-14905

🛠 Summary of changes

  • add docv_transaction_token to webhook analytics events
  • correct user_id in webhook analytics event to use the user's uuid

📜 Testing Plan

  • complete Socure DocV and verify docVTransactionToken and user_id in event logs

@amirbey amirbey force-pushed the amirbey/LG-14905-socure-webhook-events-update branch from a883e89 to 3459e53 Compare November 9, 2024 02:41
@amirbey amirbey changed the title log docvtransacton token and user's uuid in webhook analytics events LG:14905: socure webhook analytics event updates Nov 9, 2024
Copy link
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.

A few suggestions, but greatly improved!


expect(response).to have_http_status(:unauthorized)
end

it 'returns bad request with no event in the body' do
Copy link
Contributor

Choose a reason for hiding this comment

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

This spec seems out of place. Maybe move to

  • with a valid webhook key (existing context)
    • when event is missing (new context)

Comment on lines 82 to 83
event_type:,
reference_id: reference_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to use the compact form, let's use it whenever we can.

Suggested change
event_type:,
reference_id: reference_id,
event_type:,
reference_id:,

customer_user_id:,
docv_transaction_token: dcs.socure_docv_transaction_token,
event_type:,
reference_id: reference_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
reference_id: reference_id,
reference_id:,

customer_user_id:,
docv_transaction_token: dcs.socure_docv_transaction_token,
event_type:,
reference_id: reference_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
reference_id: reference_id,
reference_id:,

@@ -0,0 +1,11 @@
FactoryBot.define do
factory :document_capture_session do
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay!

@amirbey amirbey marked this pull request as ready for review November 12, 2024 17:12
changelog: Upcoming Features, Document Authentication, Socure webhook event attribute updates
@amirbey amirbey force-pushed the amirbey/LG-14905-socure-webhook-events-update branch from eda4677 to f748595 Compare November 12, 2024 18:44
@amirbey amirbey changed the title LG:14905: socure webhook analytics event updates LG-14905: socure webhook analytics event updates Nov 12, 2024
@amirbey amirbey merged commit fb04482 into main Nov 12, 2024
1 check passed
@amirbey amirbey deleted the amirbey/LG-14905-socure-webhook-events-update branch November 12, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants