-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(connector): [Novalnet] add webhooks for card #6033
Conversation
Review changes with SemanticDiff. Analyzed 2 of 5 files. Overall, the semantic diff is 10% smaller than the GitHub diff.
|
pub vendor: u32, | ||
pub project: Option<i64>, | ||
pub project_name: Option<String>, | ||
pub project_url: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the significance of this url, Can we also have this type as url::Url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is related to the merchant's project url, it should be configurable in the novalnet dashboard
let notif = get_webhook_object_from_body(request.body) | ||
.change_context(errors::ConnectorError::WebhookReferenceIdNotFound)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please throw appropriate error here
.change_context(errors::ConnectorError::WebhookEventTypeNotFound)?; | ||
Ok(Box::new(notif)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.change_context(errors::ConnectorError::WebhookEventTypeNotFound)?; | |
Ok(Box::new(notif)) | |
.change_context(errors::ConnectorError::WebhookResourceObjectNotFound)?; | |
Ok(Box::new(notif)) |
.unwrap_or("".to_string()); | ||
|
||
let secret_auth = String::from_utf8(connector_webhook_secrets.secret.to_vec()) | ||
.change_context(errors::ConnectorError::WebhookSourceVerificationFailed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.change_context(errors::ConnectorError::WebhookSourceVerificationFailed) | |
.change_context(errors::ConnectorError::WebhookVerificationSecretInvalid) |
} | ||
#[derive(Serialize, Deserialize, Debug)] | ||
pub struct NovalnetWebhookNotificationResponse { | ||
pub customer: Option<NovalnetResponseCustomer>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we consume all the fields in response ? like customer from webhooks body, if not can you remove them?
api_models::webhooks::RefundIdType::ConnectorRefundId(parent_tid.to_string()), | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is parent_tid here ? connector refund id or connector transaction id?
#[derive(Debug, Serialize, Deserialize, Clone)] | ||
#[serde(untagged)] | ||
pub enum NovalnetWebhookTransactionData { | ||
OtherTransactionData(NovalnetSyncResponseTransactionData), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this field better ?
NovalnetTransactionStatus::OnHold => { | ||
IncomingWebhookEvent::PaymentIntentAuthorizationSuccess | ||
} | ||
NovalnetTransactionStatus::Pending | NovalnetTransactionStatus::Progress => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get progress incase of authentication pending in payments response, are you sure you want to update this to Processing ?
crates/router/src/core/utils.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made changes in this file due to warning thrown by cargo clippy
@@ -311,7 +312,8 @@ pub enum NovalnetTransactionStatus { | |||
} | |||
|
|||
#[derive(Debug, Copy, Display, Clone, Default, Serialize, Deserialize, PartialEq)] | |||
#[serde(rename_all = "SCREAMING_SNAKE_CASE")] | |||
#[strum(serialize_all = "UPPERCASE")] | |||
#[serde(rename_all = "UPPERCASE")] | |||
pub enum NovalnetAPIStatus { | |||
Success, | |||
#[default] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also remove defaults in status enums
crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs
Outdated
Show resolved
Hide resolved
ab4d2df
to
d3d34d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to implement get_dispute_details
for chargeback webhooks. Please test the dispute webhooks too cc: @srujanchikke
fn get_webhook_object_from_body( | ||
body: &[u8], | ||
) -> CustomResult<novalnet::NovalnetWebhookNotificationResponse, errors::ConnectorError> { | ||
let novalnet_webhook_notification_response = std::str::from_utf8(body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we instead use parse_struct
from common_utils here?
29d2e30
to
97fe21d
Compare
Type of Change
Description
Additional Changes
Motivation and Context
https://github.com/juspay/hyperswitch-cloud/issues/6806
How did you test it?
Tested the change manually for the following flows
@Gnanasundari24 @likhinbopanna
For setting up source verificiation key, MCA cURL:
curl --location --request POST 'http://localhost:8080/account/merchant_1728025936/connectors' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: test_admin' \ --data-raw '{ "connector_type": "payment_processor", "connector_name": "novalnet", "connector_account_details": { "auth_type": "SignatureKey", "api_key": "7ibc7ob5|tuJEH3gNbeWJfIHah||nbobljbnmdli0poys|doU3HJVoym7MQ44qf7cpn7pc", "key1": "a87ff679a2f3e71d9181a67b7542122c", "api_secret": "10004" }, "test_mode": true, "disabled": false, "payment_methods_enabled": [ { "payment_method": "card", "payment_method_types": [ { "payment_method_type": "credit", "card_networks": [ "Visa", "Mastercard" ], "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true }, { "payment_method_type": "debit", "card_networks": [ "Visa", "Mastercard" ], "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true } ] }, { "payment_method": "wallet", "payment_method_types": [ { "payment_method_type": "paypal", "payment_experience": "redirect_to_url", "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": false, "installment_payment_enabled": false }, { "payment_method_type": "google_pay", "payment_experience": "redirect_to_url", "card_networks": null, "accepted_currencies": null, "accepted_countries": null, "minimum_amount": 0, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": false } ] } ], "connector_webhook_details": { "merchant_secret": "a87ff679a2f3e71d9181a67b7542122c", "additional_secret": null }, "metadata": { "city": "Musterhausen", "unit": "245", "brand_id": "001" }, "business_country": "DE", "business_label": "food" }'
Outgoing webhooks body
1. Automatic Capture
2. Manual Capture
3. Refund
4. Cancel
Checklist
cargo +nightly fmt --all
cargo clippy