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

refactor(connector): [Square] change error message from NotSupported to NotImplemented #2875

Merged

Conversation

nain-F49FF806
Copy link
Contributor

@nain-F49FF806 nain-F49FF806 commented Nov 15, 2023

Type of Change

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

Description

Consistent error messages for not implemented payment method.

Motivation and Context

Resolves #2861

How did you test it?

payment connector create

{
    "connector_type": "fiz_operations",
    "connector_name": "volt",
    "connector_account_details": {
        "auth_type": "MultiAuthKey",
        "api_key": "{{connector_api_key}}",
        "api_secret": "{{connector_api_secret}}",
        "key1": "{{connector_key1}}",
        "key2": "{{connector_key2}}"
    },
    "test_mode": false,
    "disabled": false,
    "payment_methods_enabled": [
                {
            "payment_method": "bank_redirect",
            "payment_method_types": [
                 {
                    "payment_method_type": "open_banking_uk",
                    "minimum_amount": 1,
                    "maximum_amount": 68607706,
                    "recurring_enabled": false,
                    "installment_payment_enabled": false
                    // "accepted_currencies": {
                    //     "type": "enable_only",
                    //     "list": [
                    //         "EUR"
                    //     ]
                    // },
                    // "accepted_countries": {
                    //     "type": "enable_only",
                    //     "list": [
                    //         "DE"
                    //     ]
                    // }
                }
            ]
        }
    ],
    "metadata": {
        "city": "NY",
        "unit": "245"
    },
    "connector_webhook_details": {
        "merchant_secret": "dce1d767-7c28-429c-b0fe-9e0b9e91f962"
    },
    "business_country": "US",
    "business_label": "food"
}

create a payment which is not implemented
Payment create

{
    "amount": 6540,
    "currency": "BRL",
    "confirm": true,
    "capture_method": "automatic",
    "capture_on": "2022-09-10T10:11:12Z",
    "amount_to_capture": 6540,
    "customer_id": "StripeCustomer",
    "email": "[email protected]",
    "name": "John Doe",
    "phone": "999999999",
    "phone_country_code": "+1",
    "description": "Its my first payment request",
    "authentication_type": "no_three_ds",
    "return_url": "https://duck.com",
     "payment_method": "bank_transfer",
    "payment_method_type": "pix",
    "payment_method_data": {
        "bank_transfer": {
            "pix": {}
        }
    },
    "routing": {
        "type": "single",
        "data": "adyen"
    },
    "billing": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "BR",
            "first_name": "PiX"
        }
    },
    "shipping": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "BR",
            "first_name": "PiX"
        }
    },
    "statement_descriptor_name": "joseph",
    "statement_descriptor_suffix": "JS",
    "metadata": {
        "udf1": "value1",
        "new_customer": "true",
        "login_date": "2019-09-10T10:11:12Z"
    }
}

Make any payment for Square for any PM which is not implemented, and see for the error message - it should be payment method not implemented

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

@nain-F49FF806 nain-F49FF806 requested a review from a team as a code owner November 15, 2023 11:06
@swangi-kumari
Copy link
Contributor

Hey @nain-F49FF806 , Thanks for PR.
But we'd prefer that contributors comment on an issue before opening a PR for it, so that other contributors are aware that you are working on it. This would also help reduce duplicate effort towards solving the same issue. Please ensure you follow it moving forward.

@nain-F49FF806
Copy link
Contributor Author

nain-F49FF806 commented Nov 15, 2023

@swangi-kumari Noted.

Saw no one seemed assigned, so just went ahead to have a look at the code before making a comment.
I didn't know if I was going to take it, until I had already finished. Sorry. 😅

On a more fortunate note, I only laid eyes on this today, So hopefully not much time wasted. 🤞
Will make sure to add a comment next time.

Regards.

@swangi-kumari
Copy link
Contributor

Hey @nain-F49FF806 ,
It's fine if you missed. But can you go to respective issue for which you raised this PR and comment on that issue, so that I can assign it to you.

@swangi-kumari
Copy link
Contributor

Hey @nain-F49FF806 ,
Please resolve the conflicts from your branch.

@nain-F49FF806 nain-F49FF806 force-pushed the refactor-square-not-implemented-error branch from 81cc996 to 62db8fd Compare November 29, 2023 13:45
@nain-F49FF806
Copy link
Contributor Author

@swangi-kumari Have rebased onto the most recent main.
Please have a look and let me know if there's anything else needed.

@nain-F49FF806 nain-F49FF806 force-pushed the refactor-square-not-implemented-error branch from 1f7be06 to 5b34b38 Compare December 1, 2023 09:27
Comment on lines 21 to 30
match bank_debit_data {
BankDebitData::AchBankDebit { .. } => Err(errors::ConnectorError::NotImplemented(
"Payment Method".to_string(),
utils::get_unimplemented_payment_method_error_message("Square"),
))
.into_report(),

BankDebitData::SepaBankDebit { .. }
| BankDebitData::BecsBankDebit { .. }
| BankDebitData::BacsBankDebit { .. } => Err(errors::ConnectorError::NotSupported {
message: format!("{:?}", item.request.payment_method_data),
connector: "Square",
})?,
| BankDebitData::BacsBankDebit { .. } => Err(errors::ConnectorError::NotImplemented(
utils::get_unimplemented_payment_method_error_message("Square"),
Copy link
Contributor

Choose a reason for hiding this comment

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

We can club all the match arms and propagate the expected common error.
Please use | and throw the NotImplemented error in the end. No need for it to be done twice

Copy link
Contributor Author

@nain-F49FF806 nain-F49FF806 Feb 1, 2024

Choose a reason for hiding this comment

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

I had assumed the two blocks were kept separate on purpose to have the AchBank arm generate a report, and the others not.

On second look, it seems the conversion happens automatically for the others anyway, to keep the return type identical.

I shall club them all and also remove the .into_report altogether. Let me know if that is not as intended.

crates/router/src/connector/square/transformers.rs Outdated Show resolved Hide resolved
crates/router/src/connector/square/transformers.rs Outdated Show resolved Hide resolved
crates/router/src/connector/square/transformers.rs Outdated Show resolved Hide resolved
@likhinbopanna likhinbopanna added this pull request to the merge queue Feb 27, 2024
Merged via the queue into juspay:main with commit 0626ca9 Feb 27, 2024
18 checks passed
pixincreate added a commit that referenced this pull request Feb 28, 2024
…stman-runner

* 'main' of github.com:juspay/hyperswitch: (22 commits)
  chore(version): 2024.02.28.0
  chore(postman): update Postman collection files
  fix(connector): [AUTHORIZEDOTNET] Fix status mapping (#3845)
  refactor(router): added logs health and deep health (#3780)
  feat(roles): Change list roles, get role and authorization info api to respond with groups (#3837)
  fix(core): validate amount_to_capture in payment update (#3830)
  refactor(connector): [Square] change error message from NotSupported to NotImplemented (#2875)
  feat(router): add connector mit related columns to the payment methods table (#3764)
  ci(postman): refactor NMI postman collection (#3805)
  refactor(connector): [Braintree] Mask PII data (#3759)
  refactor(connector): [Forte] Mask PII data (#3824)
  refactor(compatibility): added compatibility layer request logs (#3774)
  refactor(payment_methods): introduce `locker_id` column in `payment_methods` table (#3760)
  feat(connector): mask pii information in connector request and response for stripe, aci, adyen, airwallex  and authorizedotnet (#3678)
  chore(version): 2024.02.27.0
  fix(core): do not construct request if it is already available (#3826)
  refactor: incorporate `hyperswitch_interface` into router (#3669)
  feat: add unique constraint restriction for KV (#3723)
  feat(connector): [Payme] Add Void flow to Payme (#3817)
  refactor(connector): [Cybersource] Mask PII data  (#3786)
  ...
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-refactor Category: Refactor good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REFACTOR] : [Square] Error Message For Connector Implementation
5 participants