-
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(router): add db interface for /relay
#6879
Conversation
…ay/db-interface
bbef5a3
to
e7db9e0
Compare
crates/router/src/db/relay.rs
Outdated
_merchant_key_store: &domain::MerchantKeyStore, | ||
_new: hyperswitch_domain_models::relay::Relay, | ||
) -> CustomResult<hyperswitch_domain_models::relay::Relay, errors::StorageError> { | ||
Err(errors::StorageError::MockDbError)? |
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 no throw error here, and call diesel store implementations from here
crates/router/src/core/relay.rs
Outdated
.await | ||
.to_refund_failed_response()?; | ||
|
||
let relay_response = match router_data_res.response { |
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.
+1
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.
api-reference/api-reference/relay/relay-retrieve.mdx
Renaming this would require changes in mint.json, Can you please revert this change? @ShankarSinghC
impl common_utils::events::ApiEventMetric for RelayRequest {} | ||
|
||
impl common_utils::events::ApiEventMetric for RelayResponse {} |
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.
Why are we not raising any API events here?
pub profile_id: common_utils::id_type::ProfileId, | ||
pub merchant_id: common_utils::id_type::MerchantId, | ||
pub relay_type: storage_enums::RelayType, | ||
pub request_data: Option<pii::SecretSerdeValue>, |
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 have an option to store structured data (serialized as JSON) in the request_data
and response_data
fields?
#[derive(Debug, Clone, Deserialize, Serialize)] | ||
pub struct Relay { |
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 need the Serialize
and Deserialize
derives as of today?
) -> Self { | ||
let relay_id = id_type::RelayId::generate(); | ||
Self { | ||
id: relay_id.clone(), |
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.
Why the clone, looks like we aren't using the relay_id
variable anywhere else?
.request_data | ||
.map(|data| { | ||
serde_json::to_value(data).change_context(ValidationError::InvalidValue { | ||
message: "Failed while decrypting business profile data".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.
If we're still going to use serde_json::Value
on the diesel model, then update the error message. Otherwise, update the code as required.
.map(|data| { | ||
serde_json::from_value(data.expose()).change_context( | ||
ValidationError::InvalidValue { | ||
message: "Failed while decrypting business profile data".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.
Same here.
.request_data | ||
.map(|data| { | ||
serde_json::to_value(data).change_context(ValidationError::InvalidValue { | ||
message: "Failed while decrypting business profile data".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.
And here.
let webhook_url = Some(payments::helpers::create_webhook_url( | ||
&state.base_url.clone(), | ||
merchant_id, | ||
connector_name, | ||
)); |
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 should consider using merchant connector account ID instead of connector name.
We can take this change up in a separate PR.
created_at TIMESTAMP NOT NULL DEFAULT now()::TIMESTAMP, | ||
modified_at TIMESTAMP NOT NULL DEFAULT now()::TIMESTAMP, |
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.
Remove the defaults here.
-- Your SQL goes here | ||
CREATE TYPE "RelayStatus" AS ENUM ('created', 'pending', 'failure', 'success'); | ||
|
||
CREATE TYPE "RelayType" AS ENUM ('refund'); |
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.
Should we consider using a string instead of enum for the relay_type
field? This is considering that adding support for new relay types would involve database migrations to add enum variants each time.
This is open for discussion, we need not take it up now.
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.
@SanchithHegde as this is a priority pr, can I address all of your comments in my next pr ?
…ete-pm * 'main' of github.com:juspay/hyperswitch: chore(version): 2024.12.23.0 feat(connector): [JPMORGAN] add Payment flows for cards (#6668) refactor(grpc): send `x-tenant-id` and `x-request-id` in grpc headers (#6904) feat(payment_methods_v2): Added Ephemeral auth for v2 (#6813) chore(cypress): payout - fix test cases for adyenplatform bank (#6887) refactor(connector): [Airwallex] add device_data in payment request (#6881) feat(router): add db interface for `/relay` (#6879) feat(payments_v2): implement payments capture v2 (#6722) feat(router): add /relay endpoint (#6870)
Type of Change
Description
/relay is a api that will be used to just forward the request sent by merchant without performing any application logic on it. In this pr the changes is particular to support the refunds for the payments that was directly performed with the connector (example stripe) with out hyperswitch involved during the payment flow.
Flow
Additional Changes
Motivation and Context
How did you test it?
-> Create a merchant connector account
-> Make a refund request with invalid
connector_resource_id
-> Make a refund request with valid
connector_resource_id
Checklist
cargo +nightly fmt --all
cargo clippy