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

fix(dispute): change dispute currency type to currency enum #6454

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

ImSagnik007
Copy link
Contributor

@ImSagnik007 ImSagnik007 commented Oct 28, 2024

Type of Change

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

Description

Changed Dispute Currency from String to ENUM.

Connector create (Stripe):
curl --location 'http://localhost:8080/account/merchant_1731933435/connectors' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: test_admin' \ --data '{ "connector_type": "payment_processor", "connector_name": "stripe", "connector_account_details": { "auth_type": "HeaderKey", "api_key": "abc" }, "test_mode": true, "disabled": false, "payment_methods_enabled": [ { "payment_method": "card", "payment_method_types": [ { "payment_method_type": "credit", "payment_experience": null, "card_networks": [ "Visa", "Mastercard" ], "accepted_currencies": null, "accepted_countries": null, "minimum_amount": -1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true }, { "payment_method_type": "debit", "payment_experience": null, "card_networks": [ "Visa", "Mastercard" ], "accepted_currencies": null, "accepted_countries": null, "minimum_amount": -1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true } ] }, { "payment_method": "wallet", "payment_method_types": [ { "payment_method_type": "apple_pay", "payment_experience": "invoke_sdk_client", "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true }, { "payment_method_type": "google_pay", "payment_experience": "invoke_sdk_client", "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true } ] }, { "payment_method": "pay_later", "payment_method_types": [ { "payment_method_type": "klarna", "payment_experience": "redirect_to_url", "card_networks": null, "accepted_currencies": null, "accepted_countries": null, "minimum_amount": 1, "maximum_amount": 68607706, "recurring_enabled": true, "installment_payment_enabled": true } ] } ], "metadata": { "apple_pay_combined": { "manual": { "session_token_data": { "initiative": "web", "certificate": "", "display_name": "applepay", "certificate_keys": "", "payment_processing_details_at": "Connector", "initiative_context": "sdk-test-app.netlify.app", "merchant_identifier": "merchant.com.stripe.sang", "merchant_business_country": "US" }, "payment_request_data": { "label": "applepay", "supported_networks": [ "visa", "masterCard", "amex", "discover" ], "merchant_capabilities": [ "supports3DS" ] } } }, "google_pay": { "merchant_info": { "merchant_name": "Stripe" }, "allowed_payment_methods": [ { "type": "CARD", "parameters": { "allowed_auth_methods": [ "PAN_ONLY", "CRYPTOGRAM_3DS" ], "allowed_card_networks": [ "AMEX", "DISCOVER", "INTERAC", "JCB", "MASTERCARD", "VISA" ] }, "tokenization_specification": { "type": "PAYMENT_GATEWAY", "parameters": { "gateway": "stripe", "stripe:version": "2018-10-31", "stripe:publishableKey": "abc" } } } ] } } }'

Payments Create:
curl --location 'http://localhost:8080/payments' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: abc' \ --data-raw '{ "amount": 6540, "currency": "USD", "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://google.com", "payment_method": "card", "payment_method_type": "credit", "payment_method_data": { "card": { "card_number": "4242424242424242", "card_exp_month": "10", "card_exp_year": "25", "card_holder_name": "joseph Doe", "card_cvc": "123" } }, "billing": { "address": { "line1": "1467", "line2": "Harrison Street", "line3": "Harrison Street", "city": "San Fransico", "state": "California", "zip": "94122", "country": "US", "first_name": "joseph", "last_name": "Doe" }, "phone": { "number": "8056594427", "country_code": "+91" } }, "shipping": { "address": { "line1": "1467", "line2": "Harrison Street", "line3": "Harrison Street", "city": "San Fransico", "state": "California", "zip": "94122", "country": "US", "first_name": "joseph", "last_name": "Doe" }, "phone": { "number": "8056594427", "country_code": "+91" } }, "statement_descriptor_name": "joseph", "statement_descriptor_suffix": "JS", "metadata": { "udf1": "value1", "new_customer": "true", "login_date": "2019-09-10T10:11:12Z" } }'

payment_intent of the webhook body should be the connector_transaction_id of the Payments create response.

Webhook testing:
curl --location 'http://localhost:8080/webhooks/merchant_1731037648/mca_X1y5O8y5i29CXmimByPp' \ --header 'stripe-signature: t=1730987258,v1=,v0=' \ --header 'Content-Type: application/json' \ --data '{ "id": "evt_1QIW3iD5R7gDAGffqLkKU7uK", "object": "event", "api_version": "2022-11-15", "created": 1730987257, "data": { "object": { "id": "dp_1QIW3hD5R7gDAGffIDgTp3hd", "object": "dispute", "amount": 100, "balance_transaction": null, "balance_transactions": [], "charge": "ch_3QIW3hD5R7gDAGff1ZNgFNL9", "created": 1730987257, "currency": "usd", "enhanced_eligibility_types": [], "evidence": { "access_activity_log": null, "billing_address": null, "cancellation_policy": null, "cancellation_policy_disclosure": null, "cancellation_rebuttal": null, "customer_communication": null, "customer_email_address": null, "customer_name": null, "customer_purchase_ip": null, "customer_signature": null, "duplicate_charge_documentation": null, "duplicate_charge_explanation": null, "duplicate_charge_id": null, "enhanced_evidence": {}, "product_description": null, "receipt": null, "refund_policy": null, "refund_policy_disclosure": null, "refund_refusal_explanation": null, "service_date": null, "service_documentation": null, "shipping_address": null, "shipping_carrier": null, "shipping_date": null, "shipping_documentation": null, "shipping_tracking_number": null, "uncategorized_file": null, "uncategorized_text": null }, "evidence_details": { "due_by": 1731801599, "enhanced_eligibility": {}, "has_evidence": false, "past_due": false, "submission_count": 0 }, "is_charge_refundable": true, "livemode": false, "metadata": {}, "payment_intent": "pi_3QIjEqD5R7gDAGff1hTCMHyE", "payment_method_details": { "card": { "brand": "visa", "case_type": "inquiry", "network_reason_code": "10" }, "type": "card" }, "reason": "fraudulent", "status": "warning_needs_response" } }, "livemode": false, "pending_webhooks": 12, "request": { "id": "req_LYPj1MWBqlb5QS", "idempotency_key": "7469f2e2-fe9f-4c8d-b863-cf7d4d7e325c" }, "type": "charge.dispute.created" }'

In the dispute table the dispute_currency column should contain actual currency enum like: USD

Connectors that returns uppercase currency but implemented as String: Airwallex, Braintree, Checkout, TrustPay
and lowercase currency("usd"): Stripe

Migration query to be run before running up.sql:
UPDATE dispute SET currency = UPPER(currency);

Additional Changes

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

Motivation and Context

How did you test it?

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

@ImSagnik007 ImSagnik007 requested review from a team as code owners October 28, 2024 13:17
Copy link

semanticdiff-com bot commented Oct 28, 2024

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/connector/braintree.rs  87% smaller
  crates/router/src/types/transformers.rs  54% smaller
  crates/router/src/db/dispute.rs  53% smaller
  crates/router/src/services/kafka/dispute.rs  49% smaller
  crates/router/src/connector/stripe/transformers.rs  44% smaller
  crates/router/src/services/kafka/dispute_event.rs  44% smaller
  crates/api_models/src/disputes.rs  41% smaller
  crates/router/src/types/storage/dispute.rs  30% smaller
  crates/router/src/compatibility/stripe/webhooks.rs  3% smaller
  crates/router/src/connector/utils.rs  1% smaller
  api-reference-v2/openapi_spec.json  0% smaller
  api-reference/openapi_spec.json  0% smaller
  crates/diesel_models/src/dispute.rs  0% smaller
  crates/diesel_models/src/schema.rs  0% smaller
  crates/diesel_models/src/schema_v2.rs  0% smaller
  crates/hyperswitch_connectors/src/connectors/airwallex/transformers.rs  0% smaller
  crates/hyperswitch_connectors/src/connectors/novalnet.rs  0% smaller
  crates/hyperswitch_interfaces/src/disputes.rs  0% smaller
  crates/router/src/connector/adyen.rs  0% smaller
  crates/router/src/connector/bluesnap.rs  0% smaller
  crates/router/src/connector/braintree/transformers.rs  0% smaller
  crates/router/src/connector/checkout/transformers.rs  0% smaller
  crates/router/src/connector/payme.rs  0% smaller
  crates/router/src/connector/paypal.rs  0% smaller
  crates/router/src/connector/rapyd.rs  0% smaller
  crates/router/src/connector/trustpay/transformers.rs  0% smaller
  crates/router/src/core/webhooks/incoming.rs  0% smaller
  crates/router/src/utils/user/sample_data.rs  0% smaller
  migrations/2024-10-28-125949_add_dispute_currency_column_in_dispute_table/down.sql Unsupported file format
  migrations/2024-10-28-125949_add_dispute_currency_column_in_dispute_table/up.sql Unsupported file format

@ImSagnik007 ImSagnik007 self-assigned this Oct 28, 2024
@hyperswitch-bot hyperswitch-bot bot added the M-database-changes Metadata: This PR involves database schema changes label Oct 28, 2024
@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 3346794 to 07c8721 Compare October 29, 2024 08:59
@ImSagnik007 ImSagnik007 requested a review from a team as a code owner October 29, 2024 08:59
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Oct 29, 2024
@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 8f26d2e to e97eede Compare November 8, 2024 03:55
@hyperswitch-bot hyperswitch-bot bot removed the M-api-contract-changes Metadata: This PR involves API contract changes label Nov 8, 2024
@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from e97eede to ed16e72 Compare November 8, 2024 04:03
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Nov 8, 2024
@@ -483,7 +483,7 @@ mod tests {
DisputeNew {
dispute_id: dispute_ids.dispute_id,
amount: "amount".into(),
currency: "currency".into(),
currency: common_enums::Currency::USD,
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 use default in tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,2 @@
-- Your SQL goes here
ALTER TABLE dispute ALTER COLUMN currency TYPE "Currency" USING currency::"Currency";
Copy link
Contributor

Choose a reason for hiding this comment

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

This query would fail, if there's any invalid data. We would need to run a migration on the sandbox and production environments to backfill the currency field in the dispute table, ensuring everything is in uppercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the migration query in the description

Copy link
Contributor

@sai-harsha-vardhan sai-harsha-vardhan Nov 11, 2024

Choose a reason for hiding this comment

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

Please ensure that this up.sql should be run only after deployment both in sandbox and production (along with the required migration to convert the existing data to upper case), you can add the code comments telling that this should be run after deployment

@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 50cbc6e to 72c065a Compare November 11, 2024 05:17
@hyperswitch-bot hyperswitch-bot bot added M-api-contract-changes Metadata: This PR involves API contract changes and removed M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 11, 2024
@@ -412,8 +412,7 @@ pub async fn generate_sample_data(
amount: (amount * 100).to_string(),
currency: payment_intent
.currency
.unwrap_or(common_enums::Currency::USD)
.to_string(),
.unwrap_or(common_enums::Currency::USD),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use default here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,2 @@
-- Your SQL goes here
ALTER TABLE dispute ALTER COLUMN currency TYPE "Currency" USING currency::"Currency";
Copy link
Contributor

@sai-harsha-vardhan sai-harsha-vardhan Nov 11, 2024

Choose a reason for hiding this comment

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

Please ensure that this up.sql should be run only after deployment both in sandbox and production (along with the required migration to convert the existing data to upper case), you can add the code comments telling that this should be run after deployment

@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 4fc1601 to 2b315a1 Compare November 11, 2024 08:18
@hyperswitch-bot hyperswitch-bot bot added M-api-contract-changes Metadata: This PR involves API contract changes and removed M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 11, 2024
@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 9b87865 to 5d07f47 Compare November 11, 2024 09:24
@hyperswitch-bot hyperswitch-bot bot added M-api-contract-changes Metadata: This PR involves API contract changes and removed M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 11, 2024
@@ -0,0 +1,2 @@
-- Your SQL goes here
ALTER TABLE dispute ALTER COLUMN currency TYPE "Currency" USING currency::"Currency"; -- Migration query to be run after deployment before running this query
Copy link
Member

Choose a reason for hiding this comment

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

Won't this lock the entire table, and possibly affect deployments which happen in a staggered manner?

@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 43ad7b4 to 8c724f3 Compare November 13, 2024 20:47
@ImSagnik007 ImSagnik007 requested a review from a team as a code owner November 13, 2024 20:47
@hyperswitch-bot hyperswitch-bot bot added M-api-contract-changes Metadata: This PR involves API contract changes and removed M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 13, 2024
AkshayaFoiger
AkshayaFoiger previously approved these changes Nov 18, 2024
@ShivanshMathurJuspay
Copy link
Contributor

Can you please post a Kafka message which is being pushed as the sample log as well please

@likhinbopanna likhinbopanna added this pull request to the merge queue Nov 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024
@ImSagnik007 ImSagnik007 force-pushed the dispute_currency_enum branch from 17121e1 to d901c8d Compare November 20, 2024 07:49
@hyperswitch-bot hyperswitch-bot bot removed the M-api-contract-changes Metadata: This PR involves API contract changes label Nov 20, 2024
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Nov 20, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Nov 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Nov 20, 2024
Merged via the queue into main with commit 98aa84b Nov 20, 2024
15 of 17 checks passed
@likhinbopanna likhinbopanna deleted the dispute_currency_enum branch November 20, 2024 14:14
bsayak03 pushed a commit that referenced this pull request Nov 26, 2024
Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>
bsayak03 pushed a commit that referenced this pull request Nov 26, 2024
Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>
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 M-database-changes Metadata: This PR involves database schema changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants