Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: add encryption support to payment attempt domain model #5882

Merged

Conversation

SanchithHegde
Copy link
Member

Type of Change

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

Description

This PR introduces encryption support for the payment attempt domain model by adding implementations for the Conversion and ReverseConversion traits, and adds new methods in the PaymentAttemptInterface trait for v2 insert, update and find operations which internally call the Conversion / ReverseConversion implementations. For now, I've ignored the KV implementations for the new v2 PaymentAttemptInterface methods, they just call the underlying PSQL based implementations.

In addition, this PR involves the following changes:

  • Moves the incorrectly placed implementation of Conversion / ReverseConversion traits on PaymentIntent to the correct file.
  • Addresses some clippy lints.
  • Updates payment attempt diesel models to use MinorUnit instead of i64 for amount values.
  • Updates some of the usages of find / insert / update payment attempt methods in code to call v1 or v2 implementations based on the enabled feature flags, while some of the other usages have been replaced with a todo!() for the v2 implementations.

Motivation and Context

This allows for adding new fields in payment attempt types for payments v2, which may need encryption support.

How did you test it?

The new code added in this PR cannot be tested via any APIs, since there are no payments v2 APIs. As for the existing code that was modified, it must be ensured that the existing Postman / Cypress tests should not be affected.

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

@SanchithHegde SanchithHegde added A-framework Area: Framework S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor api-v2 labels Sep 13, 2024
@SanchithHegde SanchithHegde added this to the September 2024 Release milestone Sep 13, 2024
@SanchithHegde SanchithHegde self-assigned this Sep 13, 2024
@SanchithHegde SanchithHegde requested review from a team as code owners September 13, 2024 10:28
Copy link

semanticdiff-com bot commented Sep 13, 2024

Review changes with SemanticDiff.

Analyzed 29 of 30 files.

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

File Information
Filename Status
✔️ crates/storage_impl/src/payments/payment_attempt.rs 20.97% smaller
✔️ crates/storage_impl/src/mock_db/payment_attempt.rs Analyzed
✔️ crates/router/src/utils.rs 12.39% smaller
✔️ crates/router/src/utils/user/sample_data.rs 54.41% smaller
✔️ crates/router/src/types/transformers.rs Analyzed
✔️ crates/router/src/types/storage/payment_attempt.rs 54.44% smaller
✔️ crates/router/src/types/domain/user.rs Analyzed
✔️ crates/router/src/types/api/admin.rs Analyzed
✔️ crates/router/src/routes/app.rs Analyzed
✔️ crates/router/src/db/kafka_store.rs Analyzed
✔️ crates/router/src/db/mandate.rs Analyzed
✔️ crates/router/src/db/payment_method.rs Analyzed
✔️ crates/router/src/core/customers.rs 98.92% smaller
✔️ crates/router/src/core/fraud_check.rs Analyzed
✔️ crates/router/src/core/payments/helpers.rs 34.52% smaller
✔️ crates/router/src/core/payments/retry.rs 60.6% smaller
✔️ crates/router/src/core/payments/routing.rs Analyzed
✔️ crates/router/src/core/payments/operations/payment_create.rs 11.36% smaller
✔️ crates/router/src/core/payments/operations/payment_response.rs 37.97% smaller
✔️ crates/router/src/core/payment_methods/cards.rs Analyzed
✔️ crates/router/src/core/fraud_check/operation/fraud_check_post.rs 20.57% smaller
✔️ crates/hyperswitch_domain_models/src/mandates.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/payments/payment_attempt.rs 0.72% smaller
✔️ crates/hyperswitch_domain_models/src/payments/payment_intent.rs 2.78% smaller
✔️ crates/diesel_models/src/enums.rs Analyzed
✔️ crates/diesel_models/src/kv.rs Analyzed
✔️ crates/diesel_models/src/payment_attempt.rs 12.12% smaller
✔️ crates/diesel_models/src/user/sample_data.rs Analyzed
✔️ crates/diesel_models/src/query/payment_attempt.rs Analyzed
.github/workflows/CI-pr.yml Unsupported file format

@SanchithHegde SanchithHegde changed the title Add encryption support payment attempt domain model refactor: add encryption support to payment attempt domain model Sep 13, 2024
crates/diesel_models/src/kv.rs Outdated Show resolved Hide resolved
crates/router/src/db/kafka_store.rs Show resolved Hide resolved
conn,
dsl::attempt_id
.eq(self.attempt_id.to_owned())
.and(dsl::merchant_id.eq(self.merchant_id.to_owned())),
Copy link
Member

Choose a reason for hiding this comment

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

later this would be only based on attempt id

async fn find_payment_attempt_by_attempt_id_merchant_id(
&self,
attempt_id: &str,
merchant_id: &id_type::MerchantId,
storage_scheme: storage_enums::MerchantStorageScheme,
) -> error_stack::Result<PaymentAttempt, errors::StorageError>;

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

Choose a reason for hiding this comment

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

TODO: remove merchant_id from here. Just to confirm, if id is a global id then we do not need merchant_id in the query right @jarnura?

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Sep 17, 2024
Merged via the queue into main with commit f72abe4 Sep 17, 2024
14 checks passed
@Gnanasundari24 Gnanasundari24 deleted the add-encryption-support-payment-attempt-domain-model branch September 17, 2024 11:21
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Sep 18, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-framework Area: Framework api-v2 C-refactor Category: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants