Skip to content

LG-15271: Socure webhook repeater#11633

Merged
amirbey merged 18 commits intomainfrom
amirbey/socure-webhook-repeater
Dec 19, 2024
Merged

LG-15271: Socure webhook repeater#11633
amirbey merged 18 commits intomainfrom
amirbey/socure-webhook-repeater

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Dec 12, 2024

🎫 Ticket

LG-15271

🛠 Summary of changes

Broadcast webhooks received from Socure to configure receivers.

📜 Testing Plan

Currently configured in the Timnit Sandbox

  • Verify Socure Dashboard sandbox env has Timnit IdP designated to receive webhooks
  • Complete DocV in the Timnit Sandbox
  • Verify that webhooks appear in Cloudwatch events log for Timnit
  • Verify that webhooks appear in Cloudwatch events log the sandboxes configured socure_docv_webhook_repeat_endpoints to receive repeated webhooks in Timnit config

@amirbey amirbey self-assigned this Dec 13, 2024
@amirbey amirbey marked this pull request as ready for review December 13, 2024 21:18
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.

One spec question but otherwise looks great!

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 doesn't have any expectations, or am I missing something?

@amirbey amirbey force-pushed the amirbey/socure-webhook-repeater branch from 52b7ade to f27c8c2 Compare December 16, 2024 17:16
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a very long timeout! especially for something that inline instead of backgrounded. would we consider making this a background job so that we can have the original webhook return faster?

@amirbey amirbey requested a review from zachmargolis December 17, 2024 18:42
@voidlily
Copy link
Contributor

can you rebase this to pick up fixes for the reviewapps cluster to be less sad?

@amirbey amirbey force-pushed the amirbey/socure-webhook-repeater branch from cd83ab7 to 5c227fb Compare December 17, 2024 20:50
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 couple of small comments, but looks great overall!

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 not going to make this configurable or override it in specs, it could just be a var on line 47.

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
it 'does track state if state tracking is disabled' do
it 'does track state if state tracking is enabled' do

Comment on lines 95 to 108
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 move the expects inside it it it's more obvious what is actually being tested:

Suggested change
endpoints.each do |endpoint|
expect(SocureDocvRepeatWebhookJob).to receive(:perform_later) do |*args|
expect(args.first[:body]).to eq(JSON.parse(repeated_body.to_json))
expect(args.first[:endpoint]).to eq(endpoint)
expect(args.first[:headers]).to eq(headers)
end
end
end
context 'when idv workers are enabled' do
it 'queues SocureDocvRepeatWebhook jobs' do
post :create, params: webhook_body
end
end
end
context 'when idv workers are enabled' do
it 'queues SocureDocvRepeatWebhook jobs' do
endpoints.each do |endpoint|
expect(SocureDocvRepeatWebhookJob).to receive(:perform_later) do |*args|
expect(args.first[:body]).to eq(JSON.parse(repeated_body.to_json))
expect(args.first[:endpoint]).to eq(endpoint)
expect(args.first[:headers]).to eq(headers)
end
end
post :create, params: webhook_body
end
end

@amirbey amirbey force-pushed the amirbey/socure-webhook-repeater branch from 0e03eb1 to e8949c5 Compare December 19, 2024 15:48
@amirbey amirbey merged commit 1cfc19d into main Dec 19, 2024
@amirbey amirbey deleted the amirbey/socure-webhook-repeater branch December 19, 2024 16:12
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.

4 participants