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): [Bluesnap] Metadata to connector metadata mapping #3331

Merged
merged 14 commits into from
Jan 30, 2024

Conversation

AkshayaFoiger
Copy link
Contributor

@AkshayaFoiger AkshayaFoiger commented Jan 11, 2024

Type of Change

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

Description

resolve #3839
Metadata should be in
{String : Value } type where Value is not an object.
In case it is an object an Request Encoding error is thrown

Test Case?

Testing can be done by creating a card/gpay/applepay payment and by passing the following metadata in PAYMENTS-CREATE:

"metadata": {
        "udf1": ["adkjhfa","adjbfka"],
        "new_customer": {"key":"value"},
        "login_date": "2019-09-10T10:11:12Z"
    }
Screenshot 2024-01-12 at 6 24 31 PM

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

@AkshayaFoiger AkshayaFoiger self-assigned this Jan 11, 2024
@AkshayaFoiger AkshayaFoiger requested review from a team as code owners January 11, 2024 12:54
@AkshayaFoiger AkshayaFoiger added A-connector-compatibility Area: Connector compatibility A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement labels Jan 11, 2024
@AkshayaFoiger AkshayaFoiger changed the title feat(connector): [Bluesnap] metadata to connector metadata mapping feat(connector): [Bluesnap] Metadata to connector metadata mapping Jan 11, 2024
srujanchikke
srujanchikke previously approved these changes Jan 12, 2024
srujanchikke
srujanchikke previously approved these changes Jan 17, 2024
@@ -590,6 +621,14 @@ impl TryFrom<&BluesnapRouterData<&types::PaymentsCompleteAuthorizeRouterData>>
.parse_value("BluesnapRedirectionResponse")
.change_context(errors::ConnectorError::ResponseDeserializationFailed)?;

let transaction_meta_data = match item.router_data.request.metadata.as_ref() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to pass meta data in complete authorize as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because transaction call to Bluesnap for 3ds payments happen through complete authorize

@@ -590,6 +621,14 @@ impl TryFrom<&BluesnapRouterData<&types::PaymentsCompleteAuthorizeRouterData>>
.parse_value("BluesnapRedirectionResponse")
.change_context(errors::ConnectorError::ResponseDeserializationFailed)?;

let transaction_meta_data = match item.router_data.request.metadata.as_ref() {
Some(metadata) => Some(BluesnapMetadata {
meta_data: Vec::<RequestMetadata>::foreign_from(metadata.peek().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.

Incase of manual capture and void were we able to see the metadata for those transaction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, metadata sent in auth is visible on dashboard for capture and void calls

Comment on lines +85 to +87
meta_key: Option<String>,
meta_value: Option<String>,
is_visible: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to keep these fields as optional? RequestMetadata should be instructed only incase of payment metadata is present

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These fields are optional at the connector end. Hence made them optional in RequestMetadata struct

@AkshayaFoiger AkshayaFoiger added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Jan 18, 2024
@AkshayaFoiger AkshayaFoiger added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Jan 19, 2024
@AkshayaFoiger
Copy link
Contributor Author

AkshayaFoiger commented Jan 19, 2024

As we are hardcoding is_visible as "Y". The metadata passed by merchant will be visible in invoice. By default bluesnap takes the value of is_visible to "N".
Screenshot 2024-01-19 at 12 40 51 PM

srujanchikke
srujanchikke previously approved these changes Jan 22, 2024
vector.push(RequestMetadata {
meta_key: key,
meta_value: value.map(|field_value| field_value.to_string()),
is_visible: Some("Y".to_string()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please assign this to constant and use it here

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Jan 30, 2024
Merged via the queue into main with commit b2afdc3 Jan 30, 2024
10 of 12 checks passed
@Gnanasundari24 Gnanasundari24 deleted the metadata/bluesnap branch January 30, 2024 09:29
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-compatibility Area: Connector compatibility A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants