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

feat(connector): [Stancer] Add authorize flow for Stancer #2876

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

beaujean
Copy link

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Stancer has a clear API with great documentation available here https://wearestancer.gitlab.io/doc/en/API.html

Additional Changes

  • This PR modifies the API contract
  • This PR is a first step to adds a new PSP integration
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

Stancer is a new PSP and it desserve its connector in hyperswitch as it is a simple, reliable and cost efficient and transparent payment API. Stancer provides card and SEPA Direct Debit payments methods.
See https://www.stancer.com/

How did you test it?

Tested with the unit tested provided in the project

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@beaujean beaujean requested review from a team as code owners November 15, 2023 11:31
@github-actions github-actions bot added the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Nov 15, 2023
@prasunna09 prasunna09 changed the title Stancer feat(connector): [Stancer] Add authorize flow for Stancer. Nov 16, 2023
@github-actions github-actions bot removed the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Nov 16, 2023
@prasunna09 prasunna09 changed the title feat(connector): [Stancer] Add authorize flow for Stancer. feat(connector): [Stancer] Add authorize flow for Stancer Nov 16, 2023
config/config.example.toml Outdated Show resolved Hide resolved
config/development.toml Outdated Show resolved Hide resolved
config/docker_compose.toml Outdated Show resolved Hide resolved
crates/api_models/src/enums.rs Outdated Show resolved Hide resolved
crates/api_models/src/enums.rs Outdated Show resolved Hide resolved
crates/router/tests/connectors/sample_auth.toml Outdated Show resolved Hide resolved
crates/test_utils/src/connector_auth.rs Outdated Show resolved Hide resolved
}

fn get_url(&self, _req: &types::PaymentsAuthorizeRouterData, _connectors: &settings::Connectors,) -> CustomResult<String,errors::ConnectorError> {
Err(errors::ConnectorError::NotImplemented("get_url method".to_string()).into())
Copy link
Contributor

@prasunna09 prasunna09 Nov 16, 2023

Choose a reason for hiding this comment

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

Can you please implement this get_url function, as this will return the end point of the api call.

.change_context(errors::ConnectorError::FailedToObtainAuthType)?;
Ok(vec![(
headers::AUTHORIZATION.to_string(),
format!("Bearer {}", auth.api_key.expose()).into_masked(),
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
format!("Bearer {}", auth.api_key.expose()).into_masked(),
format!("Basic {}", auth.api_key.expose()).into_masked(),

According to documentation, stancer uses Basic Authentication.

}

impl
ConnectorIntegration<api::PSync, types::PaymentsSyncData, types::PaymentsResponseData>
Copy link
Contributor

Choose a reason for hiding this comment

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

@beaujean Are you willing to work on this PSync, as this is required to retrieve payments.
you can refer here - get-payment-data

@prasunna09
Copy link
Contributor

@beaujean Could you please resolve the conflicts?

@prasunna09 prasunna09 added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Nov 16, 2023
Copy link
Contributor

@SamraatBansal SamraatBansal left a comment

Choose a reason for hiding this comment

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

Hey @beaujean !
Thanks for raising this amazing PR and we appreciate your contribution in Hyperswitch.

We have added some comments (Mostly to avoid errors at compile time and follow coding best-practices). Will merge into our codebase once these are addressed.

Also would suggest you to run cargo c to get the compile time warnings and errors in your systems and address those.
Also please make sure the request and response fields type are matching with what Stancer is accepting, e.g: Stancer expects a String in case of amount

Comment on lines 27 to 30
Ok(Self {
amount,
router_data: item,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Since Stancer accepts in String, transform the type using the below utils function

Suggested change
Ok(Self {
amount,
router_data: item,
})
let amount = utils::get_amount_as_string(currency_unit, amount, currency)?;
Ok(Self {
amount,
router_data,
})

currency: String,
auth: Option<StancerAuth>,
card: Option<StancerCard>,
sepa: Option>StancerSepa>,
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
sepa: Option>StancerSepa>,
sepa: Option<StancerSepa>,

// Auth Struct
pub struct StancerAuthType {
pub(super) api_key: Secret<String>
pub(super) api_secret: Secret<String>,
Copy link
Contributor

Choose a reason for hiding this comment

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

No need of this field api_secret, since Stancer only uses 1 key.


#[derive(Default, Debug, Serialize, Eq, PartialEq)]
pub struct StancerPaymentsRequest {
amount: i32,
Copy link
Contributor

Choose a reason for hiding this comment

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

As per Stancer docs it accepts String in PaymentsRequest Body

Suggested change
amount: i32,
amount: String,

Comment on lines 388 to 389
Err(errors::ConnectorError::NotImplemented("get_url method".to_string()).into())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the appropriate endpoint url required to be called for a Refund Request to Stancer

crates/router/src/connector/stancer/transformers.rs Outdated Show resolved Hide resolved
Comment on lines 210 to 212
Ok(Self {
amount: item.amount.to_owned(),
})
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
Ok(Self {
amount: item.amount.to_owned(),
})
Ok(Self {
amount: item.amount.to_owned(),
payment:item.router_data.request.connector_transaction_id.clone(),
})

Comment on lines 219 to 226
pub enum RefundStatus {
Failed,
Not_Honored,
Refund_Sent,
Refunded,
#[default]
To_Refund,
}
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
pub enum RefundStatus {
Failed,
Not_Honored,
Refund_Sent,
Refunded,
#[default]
To_Refund,
}
#[serde(rename_all = 'snake_case']
pub enum RefundStatus {
Failed,
NotHonored,
RefundSent,
Refunded,
#[default]
ToRefund,
}

pub struct RefundResponse {
id: String,
status: RefundStatus
amount: i32,
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
amount: i32,
amount: String,

(cherry picked from commit e5245ec46461582117923822afa0a52ccdde7951)
(cherry picked from commit 2282207dd9a93169cfe9da1daed72d9178f4d05a)
(cherry picked from commit 35fcb6c7ee4af951e8e89415ce061a30e375ce3e)
(cherry picked from commit 1a5e442b45bf28107eec773614f6817c376b8e1c)
(cherry picked from commit 6eda4ca28d00d0325438c6b92bef72451cc66864)
(cherry picked from commit 477a2c5148603421039be81db666be9323d842b1)
(cherry picked from commit 2347c7588d56d8910e03d6b6339ede2c3225b83f)
(cherry picked from commit fa17be77d46f3bfa7caeb8981093284f0efd8576)
(cherry picked from commit 17bb0bec52c4b2e17ad8e411552555f5ce0e0525)
(cherry picked from commit 96ba6ce1cf0618f112ff5b139788b4e6bedafc07)
@rsecob rsecob requested a review from a team as a code owner November 24, 2023 03:12
@rsecob
Copy link

rsecob commented Nov 27, 2023

@SamraatBansal @prasunna09 Could we have a review please 🙏

@@ -220,6 +220,7 @@ shift4.base_url = "https://api.shift4.com/"
signifyd.base_url = "https://api.signifyd.com/"
square.base_url = "https://connect.squareupsandbox.com/"
square.secondary_base_url = "https://pci-connect.squareupsandbox.com/"
stancer.base_url = "https://api.stancer.com/"
Copy link
Contributor

Choose a reason for hiding this comment

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

since prod base url and sbx url is same,
we can add a note in wasm dashboard that api keys for test env starts with stest_xxx and for prod env starts with sprod_xxx to avoid making a actual payment in sbx if prod keys are configured.

Copy link

Choose a reason for hiding this comment

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

Of course, where is the wasm dashboard?

@@ -151,6 +151,7 @@ pub enum RoutableConnectors {
Shift4,
Signifyd,
Square,
Stancer,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we comment this out as this connector is not tested end to end

Copy link

Choose a reason for hiding this comment

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

Does this make this connector unusable?

Comment on lines 318 to 323
Ok(format!(
"{}{}/{}",
self.base_url(_connectors),
"v1/checkout",
req.request.get_connector_transaction_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
Ok(format!(
"{}{}/{}",
self.base_url(_connectors),
"v1/checkout",
req.request.get_connector_transaction_id()?
))
Ok(format!(
"{}v1/checkout/{}",
self.base_url(_connectors),
req.request.get_connector_transaction_id()?
))

status_code: res.status_code,
code: error_type,
message: message.to_string(),
reason: None,
Copy link
Contributor

Choose a reason for hiding this comment

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

populate the error reason with error message.

message: message.to_string(),
reason: None,
attempt_status: None,
connector_transaction_id: None,
Copy link
Contributor

Choose a reason for hiding this comment

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

do they send connector_transaction_id in error_response? if yes, pls update this.

_req: &types::RefundsRouterData<api::Execute>,
_connectors: &settings::Connectors,
) -> CustomResult<String, errors::ConnectorError> {
Ok(format!("{}{}", self.base_url(_connectors), "v1/refunds"))
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
Ok(format!("{}{}", self.base_url(_connectors), "v1/refunds"))
Ok(format!("{}v1/refunds", self.base_url(_connectors)))

Comment on lines 556 to 561
Ok(format!(
"{}{}/{}",
self.base_url(connectors),
"v1/refunds",
req.request.get_connector_refund_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
Ok(format!(
"{}{}/{}",
self.base_url(connectors),
"v1/refunds",
req.request.get_connector_refund_id()?
))
Ok(format!(
"{}v1/refunds/{}",
self.base_url(connectors),
req.request.get_connector_refund_id()?
))

Comment on lines 575 to 577
.set_body(types::RefundSyncType::get_request_body(
self, req, connectors,
)?)
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this set_body method since there is no get_request_body func for rsync. by default it would give an empty object instead of body being None

) -> CustomResult<types::RefundSyncRouterData, errors::ConnectorError> {
let response: Refund = res
.response
.parse_struct("stancer Refund")
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
.parse_struct("stancer Refund")
.parse_struct("stancer RefundSync")

headers::AUTHORIZATION.to_string(),
format!(
"Basic {}",
consts::BASE64_ENGINE.encode(format!("{}:", auth.api_key.peek()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this expected to encode api_key:, if this : is not required please remove this

@@ -151,6 +151,7 @@ pub enum RoutableConnectors {
Shift4,
Signifyd,
Square,
// Stancer,
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to comment, it is not a template draft PR, we have the implementation of the connector.

Suggested change
// Stancer,
Stancer,

Comment on lines +209 to +240
async fn execute_pretasks(
&self,
router_data: &mut types::PaymentsAuthorizeRouterData,
app_state: &routes::AppState,
) -> CustomResult<(), errors::ConnectorError> {
if let (api::PaymentMethodData::Card { .. }, enums::AuthenticationType::ThreeDs) = (
&router_data.request.payment_method_data,
&router_data.auth_type,
) {
let types::RouterData { response, .. } = services::execute_connector_processing_step(
app_state,
Box::new(self),
router_data,
payments::CallConnectorAction::Trigger,
None,
)
.await?;
let response: types::PaymentsResponseData = response.map_err(|err| {
errors::ConnectorError::UnexpectedResponseError(err.message.into())
})?;

if let types::PaymentsResponseData::ThreeDSEnrollmentResponse {
enrolled_v2: three_ds_enrolled,
..
} = response
{
router_data.request.enrolled_for_3ds = three_ds_enrolled;
}
}

Ok(())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

To run execute_pretask() function, it is suggested to execute InitPayment flow please refer to airwallex.rs or shift4.rs

@@ -335,6 +337,7 @@ default_imp_for_create_customer!(
connector::Shift4,
connector::Signifyd,
connector::Square,
// connector::Stancer,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this line all together rather than commenting it out.

Suggested change
// connector::Stancer,

@SamraatBansal
Copy link
Contributor

Hey @rsecob !

We have commented a set of changes that are required, Can you please address those.

Thanks and Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants