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: insert merchant account and merchant key store in a db transaction #2663

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Azanul
Copy link
Contributor

@Azanul Azanul commented Oct 22, 2023

Type of Change

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

Description

Merchant account creation and respective key store creation now happen in a DB transaction

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?

  • Create a merchant account.
  • Check if both merchant account and merchant key store exists
SELECT * from merchant_account;
SELECT * from merchant_key_store;

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

Azanul and others added 10 commits October 11, 2023 20:08
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
Signed-off-by: Azanul <azanulhaque@gmail.com>
@Azanul Azanul marked this pull request as ready for review October 23, 2023 06:37
@Azanul Azanul requested review from a team as code owners October 23, 2023 06:37
@SanchithHegde SanchithHegde added A-framework Area: Framework A-core Area: Core flows S-waiting-on-author Status: This PR is incomplete or needs to address review comments C-refactor Category: Refactor hacktoberfest Issues that are up for grabs for Hacktoberfest participants labels Oct 24, 2023
crates/router/src/db/merchant_account.rs Outdated Show resolved Hide resolved
crates/storage_impl/src/errors.rs Outdated Show resolved Hide resolved
crates/router/src/db/merchant_account.rs Outdated Show resolved Hide resolved
@dracarys18 dracarys18 changed the title feat: Merchant account & key store db transaction feat: insert merchant account and merchant key store in a db transaction Oct 25, 2023
Signed-off-by: Azanul <azanulhaque@gmail.com>
.await
.map_err(Into::into)
.into_report()?
.convert(merchant_key_store.key.get_inner())
Copy link
Member

Choose a reason for hiding this comment

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

this should be self.get_master_key()

Signed-off-by: Azanul <azanulhaque@gmail.com>
dracarys18
dracarys18 previously approved these changes Oct 26, 2023
SanchithHegde
SanchithHegde previously approved these changes Oct 26, 2023
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Thanks for the PR, @Azanul!

@SanchithHegde
Copy link
Member

@Azanul Could you please address the failing CI checks?

@dracarys18
Copy link
Member

@Azanul can you please resolve the compilation errors, Other than that LGTM

@Azanul
Copy link
Contributor Author

Azanul commented Oct 26, 2023

@dracarys18 @SanchithHegde How do I fix this?
Screenshot 2023-10-27 at 12 56 04 AM

Signed-off-by: Azanul <azanulhaque@gmail.com>
@Azanul Azanul dismissed stale reviews from SanchithHegde and dracarys18 via b3a12ae October 26, 2023 19:34
Signed-off-by: Azanul <azanulhaque@gmail.com>
@dracarys18
Copy link
Member

dracarys18 commented Oct 27, 2023

@dracarys18 @SanchithHegde How do I fix this? Screenshot 2023-10-27 at 12 56 04 AM

You are returning Report<StorageError> from transaction_async which does not implement From<DieselError>. You should return DatabaseError which implements From<DieselError>

@Azanul
Copy link
Contributor Author

Azanul commented Oct 27, 2023

You should return DatabaseError which implements From<DieselError>

How?

@dracarys18
Copy link
Member

dracarys18 commented Oct 31, 2023

You should return DatabaseError which implements From<DieselError>

How?

into_report() converts error to Report instead of that you can use .map_err(err| err.current_context())

@swangi-kumari
Copy link
Contributor

Hey @Azanul ,
Thanks for your interest in contributing to hyperswitch.
Let us know if you need any assistance from our end.
Also, even if hacktoberfest is over, we should celebrate open source everyday and we are open for more contributions from you.
We would still be rewarding folks with goodies even if the PR gets merged post hacktoberfest.
May the Source be with you!

@Azanul
Copy link
Contributor Author

Azanul commented Nov 2, 2023

Hey @Azanul , Thanks for your interest in contributing to hyperswitch. Let us know if you need any assistance from our end. Also, even if hacktoberfest is over, we should celebrate open source everyday and we are open for more contributions from you. We would still be rewarding folks with goodies even if the PR gets merged post hacktoberfest. May the Source be with you!

I'm looking forward to contributing to hyperswitch even after Hacktoberfest.

Regarding the current PR, I don't think I understand the codebase enough or I'm not that good with Rust yet to see this through. Although I keep trying to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows A-framework Area: Framework C-refactor Category: Refactor hacktoberfest Issues that are up for grabs for Hacktoberfest participants S-waiting-on-author Status: This PR is incomplete or needs to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Add Create Merchant and Create Merchant Key Store in a DB transaction
4 participants