Skip to content

feat(payments_v2): payment intent diesel and domain models changes v2#5783

Merged
likhinbopanna merged 38 commits intomainfrom
payment-intent-diesel-and-domail-models-changes-v2
Sep 13, 2024
Merged

feat(payments_v2): payment intent diesel and domain models changes v2#5783
likhinbopanna merged 38 commits intomainfrom
payment-intent-diesel-and-domail-models-changes-v2

Conversation

@Narayanbhat166
Copy link
Contributor

@Narayanbhat166 Narayanbhat166 commented Sep 3, 2024

Type of Change

  • Refactoring

Description

This PR refactors the codebase to include all the payment related operations under the v1 feature flag.

This also includes changes to the payment intent diesel and domain models to add new fields that are relevant to the v2 version of application.

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

@semanticdiff-com
Copy link

semanticdiff-com bot commented Sep 3, 2024

Review changes with SemanticDiff.

Analyzed 51 of 59 files.

Overall, the semantic diff is 11% smaller than the GitHub diff.

File Information
Filename Status
v2_migrations/2024-08-28-081847_drop_v1_columns/down.sql Unsupported file format
v2_migrations/2024-08-28-081847_drop_v1_columns/up.sql Unsupported file format
v2_migrations/2024-08-28-081838_update_v2_primary_key_constraints/down.sql Unsupported file format
v2_migrations/2024-08-28-081838_update_v2_primary_key_constraints/up.sql Unsupported file format
v2_migrations/2024-08-28-081747_recreate_ids_for_v2/down.sql Unsupported file format
v2_migrations/2024-08-28-081747_recreate_ids_for_v2/up.sql Unsupported file format
v2_migrations/2024-08-28-081721_add_v2_columns/down.sql Unsupported file format
v2_migrations/2024-08-28-081721_add_v2_columns/up.sql Unsupported file format
✔️ crates/storage_impl/src/lib.rs Analyzed
✔️ crates/storage_impl/src/payments/payment_intent.rs 30.93% smaller
✔️ crates/storage_impl/src/mock_db/payment_intent.rs 40.28% smaller
✔️ crates/router/tests/payments.rs Analyzed
✔️ crates/router/tests/payments2.rs 39.2% smaller
✔️ crates/router/src/core.rs Analyzed
✔️ crates/router/src/lib.rs 44.18% smaller
✔️ crates/router/src/routes.rs Analyzed
✔️ crates/router/src/utils.rs 6.0% smaller
✔️ crates/router/src/workflows.rs Analyzed
✔️ crates/router/src/workflows/payment_sync.rs Analyzed
✔️ crates/router/src/utils/user/sample_data.rs Analyzed
✔️ crates/router/src/services/kafka/payment_intent.rs 0.2% smaller
✔️ crates/router/src/services/kafka/payment_intent_event.rs 0.2% smaller
✔️ crates/router/src/routes/app.rs 85.09% smaller
✔️ crates/router/src/routes/fraud_check.rs Analyzed
✔️ crates/router/src/routes/mandates.rs Analyzed
✔️ crates/router/src/routes/payments.rs Analyzed
✔️ crates/router/src/routes/routing.rs Analyzed
✔️ crates/router/src/routes/user.rs Analyzed
✔️ crates/router/src/db/kafka_store.rs 9.4% smaller
✔️ crates/router/src/core/disputes.rs Analyzed
✔️ crates/router/src/core/fraud_check.rs Analyzed
✔️ crates/router/src/core/mandate.rs Analyzed
✔️ crates/router/src/core/payment_link.rs Analyzed
✔️ crates/router/src/core/payment_methods.rs 16.91% smaller
✔️ crates/router/src/core/payments.rs 54.48% smaller
✔️ crates/router/src/core/pm_auth.rs Analyzed
✔️ crates/router/src/core/verification/utils.rs 32.55% smaller
✔️ crates/router/src/core/user/sample_data.rs Analyzed
✔️ crates/router/src/core/payments/helpers.rs Analyzed
✔️ crates/router/src/core/payments/operations.rs 14.01% smaller
✔️ crates/router/src/core/payments/retry.rs 30.31% smaller
✔️ crates/router/src/core/payments/routing.rs Analyzed
✔️ crates/router/src/core/payments/transformers.rs Analyzed
✔️ crates/router/src/core/payments/operations/payment_response.rs Analyzed
✔️ crates/router/src/core/payment_methods/cards.rs 42.33% smaller
✔️ crates/router/src/core/mandate/helpers.rs Analyzed
✔️ crates/router/src/core/fraud_check/operation/fraud_check_post.rs 0.34% smaller
✔️ crates/router/src/core/fraud_check/operation/fraud_check_pre.rs 6.06% smaller
✔️ crates/router/src/core/files/helpers.rs Analyzed
✔️ crates/router/src/bin/scheduler.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/payments.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/payments/payment_attempt.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/payments/payment_intent.rs 2.93% smaller
✔️ crates/diesel_models/src/payment_intent.rs 0.24% smaller
✔️ crates/diesel_models/src/schema_v2.rs 5.66% smaller
✔️ crates/diesel_models/src/query/payment_intent.rs Analyzed
✔️ crates/diesel_models/src/query/user/sample_data.rs Analyzed
✔️ crates/common_utils/src/id_type/global_id.rs Analyzed
✔️ crates/common_utils/src/id_type/payment.rs 0.09% smaller

@hyperswitch-bot hyperswitch-bot bot added the M-database-changes Metadata: This PR involves database schema changes label Sep 3, 2024
@Narayanbhat166 Narayanbhat166 marked this pull request as ready for review September 8, 2024 06:26
@Narayanbhat166 Narayanbhat166 requested review from a team as code owners September 8, 2024 06:26
@Narayanbhat166 Narayanbhat166 added this to the August 2024 Release milestone Sep 9, 2024
}

#[cfg(all(feature = "v2", feature = "payment_v2"))]
pub async fn find_optional_by_merchant_id_merchant_reference_id(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub async fn find_optional_by_merchant_id_merchant_reference_id(
pub async fn find_optional_by_merchant_reference_id_merchant_id(

pub client_secret: Option<&'a String>,
pub active_attempt_id: String,
pub attempt_count: i16,
pub profile_id: Option<&'a id_type::ProfileId>,
Copy link
Member

Choose a reason for hiding this comment

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

We can make this mandatory

Comment on lines +335 to +339
vec![format!(
"pi_{}_{}",
self.merchant_id.get_string_repr(),
self.merchant_reference_id
)]
Copy link
Member

Choose a reason for hiding this comment

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

This is not required add a note to remove this, id itself will give us unique_constraints

@@ -24,11 +23,7 @@ pub struct PaymentIntent {
pub description: Option<String>,
pub return_url: Option<String>,
pub metadata: Option<serde_json::Value>,
Copy link
Member

Choose a reason for hiding this comment

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

Use pii secret value for metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we have all the serde json types as Strict types in diesel and domain models?

@@ -24,11 +23,7 @@ pub struct PaymentIntent {
pub description: Option<String>,
pub return_url: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

We can have url::Url type here for return_url

pub payment_id: common_utils::id_type::PaymentId,
pub merchant_id: common_utils::id_type::MerchantId,
pub status: storage_enums::IntentStatus,
pub amount: MinorUnit,
Copy link
Member

Choose a reason for hiding this comment

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

  • Make currency also as mandatory field
  • Amount captured move to attempt instead of intent

pub organization_id: common_utils::id_type::OrganizationId,
pub tax_details: Option<TaxDetails>,
pub skip_external_tax_calculation: Option<bool>,
pub merchant_reference_id: String,
Copy link
Member

Choose a reason for hiding this comment

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

Add todo to make this a domain type(PaymentMerchantReferenceId)

pub surcharge_amount: Option<MinorUnit>,
pub tax_on_surcharge: Option<MinorUnit>,
// TODO: change this to global id
pub id: common_utils::id_type::PaymentId,
Copy link
Member

@jarnura jarnura Sep 11, 2024

Choose a reason for hiding this comment

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

Fields needs to be added intent table

  • routing_algorithm_id
  • We include a field customer_present, which replaces off_session. Use Enum instead of a boolean
  • apply_mit_exemption, type is boolean and default value is false
  • introduce surcharge_details,
Struct SurchargeDetails {
    surcharge_amount: MinorUnit,
    tax_on_surcharge: Option<MinorUnit>
}

surcharge_details: Option<Surcharge_details>

pub connector_id: Option<String>,
pub shipping_address_id: Option<String>,
pub billing_address_id: Option<String>,
pub statement_descriptor_name: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

This field should be renamed to statement_descriptor but in database we won't rename instead we add new field

pub business_country: Option<storage_enums::CountryAlpha2>,
pub business_label: Option<String>,
#[diesel(deserialize_as = super::OptionalDieselArray<pii::SecretSerdeValue>)]
pub order_details: Option<Vec<pii::SecretSerdeValue>>,
Copy link
Member

Choose a reason for hiding this comment

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

  • Order details can be type instead of a SecretSerdeValue
  • connector_metadata as SecretSerdeValue
  • feature_metadata as SecretSerdeValue
  • allowed_payment_method_types as SecretSerdeValue
  • introduce a field called merchant_profile_id and make it mandatory
  • introduce a field called payment_link_enabled
  • introduce a field called payment_link_config and type as PaymentCreatePaymentLinkConfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is merchant_profile_id we already have a field called profile_id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

introduce a field called payment_link_enabled

Instead of this field being called as payment_link_enabled, how about enable_payment_link since this captures the intent of the merchant.

pub capture_method: Option<storage_enums::CaptureMethod>,
pub authentication_type: Option<common_enums::AuthenticationType>,
pub amount_to_capture: Option<MinorUnit>,
pub prerouting_algorithm: Option<serde_json::Value>,
Copy link
Member

Choose a reason for hiding this comment

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

Is prerouting required? and why this needs to a Value? where we are moving to a routing id based approach.

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, we need this to store the pre routing results when payment methods list call is made

pub shipping_address: Option<Encryption>,
pub capture_method: Option<storage_enums::CaptureMethod>,
pub authentication_type: Option<common_enums::AuthenticationType>,
pub amount_to_capture: Option<MinorUnit>,
Copy link
Member

@jarnura jarnura Sep 11, 2024

Choose a reason for hiding this comment

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

This will be in attempt

@@ -39,40 +34,45 @@ pub struct PaymentIntent {
pub off_session: Option<bool>,
Copy link
Member

Choose a reason for hiding this comment

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

This off_session we shouldn't use, instead our logic will be based on apply_mit_exemption

jarnura
jarnura previously approved these changes Sep 12, 2024
@Narayanbhat166
Copy link
Contributor Author

All the comments will be addressed in the upcoming PR. We can get this PR merged as there are some dependencies

apoorvdixit88
apoorvdixit88 previously approved these changes Sep 12, 2024
Copy link
Contributor

@apoorvdixit88 apoorvdixit88 left a comment

Choose a reason for hiding this comment

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

Dashboard specific changes look fine.

Aprabhat19
Aprabhat19 previously approved these changes Sep 12, 2024
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Sep 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 12, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Sep 13, 2024
Merged via the queue into main with commit 10ac089 Sep 13, 2024
@likhinbopanna likhinbopanna deleted the payment-intent-diesel-and-domail-models-changes-v2 branch September 13, 2024 08:06
pixincreate added a commit that referenced this pull request Sep 13, 2024
* 'main' of github.com:juspay/hyperswitch: (51 commits)
  feat(connector): [DEUTSCHEBANK] Integrate SEPA Payments (#5826)
  feat(payments_v2): payment intent diesel and domain models changes v2 (#5783)
  feat(connector): [Fiuu] ADD Wasm Configs (#5874)
  chore(version): 2024.09.13.0
  refactor(core): Update shipping_cost and order_tax_amount to net_amount of payment_attempt (#5844)
  refactor: return optional request body from build_request_v2 in ConnectorIntegrationV2 trait (#5865)
  feat(refunds): Refunds aggregate api (#5795)
  refactor: handle redirections for iframed content (#5591)
  refactor(payment_links): Update API contract for dynamic transaction details and upgrade UI (#5849)
  fix(router): add payment_method check in `get_mandate_type` (#5828)
  fix(connector): [ZSL] compare consr_paid_amt with the total amount for identifying partial payments (#5873)
  feat(connector): [Novalnet] add Payment flows for cards (#5726)
  chore(version): 2024.09.12.0
  fix(router): return `collect_billing_details_from_wallet_connector` if `always_collect_billing_details_from_wallet_connector ` is false in merchant payment method list (#5854)
  feat(opensearch): add profile_id and organization_id to /search APIs (#5705)
  build(deps): bump `sqlx` to `0.8.2` (#5859)
  refactor: Remove unwanted commented lines (#5851)
  feat(payments): add support for profile aggregates (#5845)
  Feat(connector): [Fiuu] Add DuitNow/FPX PaymentMethod (#5841)
  chore: remove Connectors enum dependency from ConnectorIntegrationV2 trait (#5840)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-v2 M-database-changes Metadata: This PR involves database schema changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants