Skip to content

Conversation

bsayak03
Copy link
Contributor

@bsayak03 bsayak03 commented Jun 25, 2025

Type of Change

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

Description

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

Ref Doc : https://developer.santander.com.br/api/documentacao/pix-visao-geral#/api/documentacao/pix-visao-geral/
https://developer.santander.com.br/api/user-guide/pix-visao-geral

How did you test it?

This PR cannot be tested because we are still awaiting on creds

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

@bsayak03 bsayak03 requested review from a team as code owners June 25, 2025 14:45
Copy link

semanticdiff-com bot commented Jun 25, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  crates/hyperswitch_connectors/src/connectors/santander.rs  10% smaller
  crates/hyperswitch_connectors/src/connectors/santander/transformers.rs  8% smaller
  Cargo.lock Unsupported file format
  api-reference/v1/openapi_spec_v1.json  0% smaller
  api-reference/v2/openapi_spec_v2.json  0% smaller
  config/config.example.toml Unsupported file format
  config/deployments/integration_test.toml Unsupported file format
  config/deployments/production.toml Unsupported file format
  config/deployments/sandbox.toml Unsupported file format
  config/development.toml Unsupported file format
  config/docker_compose.toml Unsupported file format
  crates/api_models/src/payments.rs  0% smaller
  crates/common_enums/src/connector_enums.rs  0% smaller
  crates/connector_configs/src/connector.rs  0% smaller
  crates/connector_configs/toml/development.toml Unsupported file format
  crates/connector_configs/toml/production.toml Unsupported file format
  crates/connector_configs/toml/sandbox.toml Unsupported file format
  crates/hyperswitch_connectors/Cargo.toml Unsupported file format
  crates/payment_methods/src/configs/payment_connector_required_fields.rs  0% smaller
  crates/router/src/core/admin.rs  0% smaller
  crates/router/src/core/payments/transformers.rs  0% smaller
  crates/router/src/types/api.rs  0% smaller
  crates/router/src/types/transformers.rs  0% smaller
  loadtest/config/development.toml Unsupported file format

@bsayak03 bsayak03 self-assigned this Jun 25, 2025
@bsayak03 bsayak03 closed this Jun 25, 2025
@bsayak03 bsayak03 reopened this Jun 25, 2025
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Jun 26, 2025
router_data.request.integrity_object = Some(response_integrity_object);
router_data
})
.change_context(errors::ConnectorError::ResponseHandlingFailed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested fix (NIT)
Apply change_context before calling Map()
new_router_data
.change_context(errors::ConnectorError::ResponseHandlingFailed)
.map(|mut router_data| {
router_data.request.integrity_object = Some(response_integrity_object);
router_data
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no map only works on Result and Options and once we do change_context the Result is unwrapped so we wont be able to use .map then

Copy link
Contributor

Choose a reason for hiding this comment

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

Result is not upwrapped unless you apply ?
.change_context(errors::ConnectorError::ResponseHandlingFailed)
will only change the error type. It will still be Result
.change_context() does NOT unwrap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, made the relevant changes

router_data.request.integrity_object = Some(response_integrity_object);
router_data
})
.change_context(errors::ConnectorError::ResponseHandlingFailed)
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

router_data.request.integrity_object = Some(response_integrity_object);
router_data
})
.change_context(errors::ConnectorError::ResponseHandlingFailed)
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

) -> CustomResult<String, errors::ConnectorError> {
Err(errors::ConnectorError::NotImplemented("get_url method".to_string()).into())
let connector_metadata = req.request.connector_metadata.clone();
let end_to_end_id = match &connector_metadata {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
We can use .and_then() instead of nested match cases

pub struct SantanderValue {
pub original: StringMajorUnit,
}

Copy link
Contributor

Choose a reason for hiding this comment

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

SantanderValue and SantanderValueResponse both are same structs
can we use a single struct instead

@@ -78,6 +78,7 @@ pub struct PaymentsAuthorizeData {
pub additional_payment_method_data: Option<AdditionalPaymentData>,
pub merchant_account_id: Option<Secret<String>>,
pub merchant_config_currency: Option<storage_enums::Currency>,
pub santander_pix_qr_expiration_time: Option<api_models::payments::SantanderBillingType>,
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 confirm with core folks if we should be passing connector specific config here
@SanchithHegde

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing this

@deepanshu-iiitu
Copy link
Contributor

Add test cases in PR description

@bsayak03
Copy link
Contributor Author

Add test cases in PR description

Cannot be tested because we are still awaiting on creds from Zurich team

@bsayak03 bsayak03 requested a review from a team as a code owner June 30, 2025 12:36
deepanshu-iiitu
deepanshu-iiitu previously approved these changes Jul 1, 2025
@deepanshu-iiitu deepanshu-iiitu removed the request for review from a team July 1, 2025 08:51
@deepanshu-iiitu deepanshu-iiitu removed the request for review from a team July 1, 2025 08:51
Chethan-rao
Chethan-rao previously approved these changes Jul 1, 2025
AnuthaDev
AnuthaDev previously approved these changes Jul 1, 2025
@bsayak03 bsayak03 dismissed stale reviews from AnuthaDev, Chethan-rao, and deepanshu-iiitu via b4373fd July 1, 2025 10:21
@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 1, 2025
@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 1, 2025
Merged via the queue into main with commit 28d6357 Jul 1, 2025
17 of 20 checks passed
@likhinbopanna likhinbopanna deleted the santander/pix-qr branch July 1, 2025 14:51
@@ -18,6 +18,8 @@ dummy_connector = [
]

[dependencies]
chrono = "0.4"
Copy link
Member

Choose a reason for hiding this comment

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

@bsayak03 @awasthi21 Prefer to use the time crate over chrono since we're using it everywhere else in our codebase...

Copy link
Contributor Author

@bsayak03 bsayak03 Jul 3, 2025

Choose a reason for hiding this comment

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

sure, i'll replace it in the next PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-api-contract-changes Metadata: This PR involves API contract changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants