feat(router): add outgoing payment webhooks for v2 #6613
Conversation
sai-harsha-vardhan
added
A-core
|
Review changes with Changed Files
|
| } | ||
| } | ||
|
|
||
| fn get_outgoing_webhook_event_content_from_event_metadata( |
There was a problem hiding this comment.
we can use From or impl based function conversion for this.
| metrics::WEBHOOK_OUTGOING_NOT_RECEIVED_COUNT.add( | ||
| &metrics::CONTEXT, | ||
| 1, | ||
| &[metrics::KeyValue::new( | ||
| MERCHANT_ID, | ||
| merchant_id.get_string_repr().to_owned(), | ||
| )], | ||
| ); |
There was a problem hiding this comment.
make this also as a function like increment_webhook_outgoing_received_count
| NoSchedule, | ||
| } | ||
|
|
||
| async fn update_event_if_client_error( |
There was a problem hiding this comment.
In these functions (update_event_in_storage, update_event_if_client_error) the logic can be merged if caller sending OutgoingWebhookResponseContent
There was a problem hiding this comment.
Like
fn get_outgoing_webhook_response_content_for_client_error(..) { ... }
fn get_outgoing_webhook_response_content_for_response(..) { ... }then call update_event with outgoing_webhook_response_content.
Even the these get_outgoing_webhook_response_content can be on impl based function on private structs.
| state.event_handler().log_event(&webhook_event); | ||
| } | ||
|
|
||
| fn get_webhook_url_from_business_profile( |
There was a problem hiding this comment.
this should be me moved to impl function on profile, avoid using business_profile term instead usin profile
| let key_manager_state: &KeyManagerState = &(&state).into(); | ||
| let event_id = event.event_id; | ||
|
|
||
| let error = if let Err(error) = trigger_webhook_result { |
There was a problem hiding this comment.
use trigger_webhook_result.err().map(|..| ...)
| let error = report!(errors::WebhooksFlowError::NotReceivedByMerchant); | ||
| logger::warn!(?error, ?delivery_attempt, status_code, %log_message); | ||
|
|
||
| //TODO: add outgoing webhook retries support |
| logger::debug!(outgoing_webhook_response=?response); | ||
|
|
||
| match delivery_attempt { | ||
| enums::WebhookDeliveryAttempt::InitialAttempt => match response { |
There was a problem hiding this comment.
What is difference between InitialAttempt and ManualRetry?
Instead matching first on delivery_attempt, first match on response. Since current approach has more duplication than the vice-versa
| Err(error) => { | ||
| if error.current_context().is_db_unique_violation() { | ||
| logger::debug!("Event with idempotent ID `{idempotent_event_id}` already exists in the database"); | ||
| return Ok(()); |
There was a problem hiding this comment.
Add comments why this is short circuited
| primary_object_id: String, | ||
| primary_object_type: enums::EventObjectType, | ||
| content: api::OutgoingWebhookContent, | ||
| primary_object_created_at: Option<time::PrimitiveDateTime>, |
There was a problem hiding this comment.
Why this is option? Any cases we won't get primary_object_created_at
| "Outgoing webhooks are disabled in application configuration, or merchant webhook URL \ | ||
| could not be obtained; skipping outgoing webhooks for event" | ||
| ); | ||
| return Ok(()); |
There was a problem hiding this comment.
Add comment for short circuiting.
Type of Change
Description
add outgoing payment webhooks for v2
Additional Changes
Motivation and Context
How did you test it?
Tested Manually
Setup incoming webhooks and trigger a successful payment, to receive an outgoing webhook
Outgoing Payload Response
Checklist
cargo +nightly fmt --allcargo clippy