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: introduce a domain type for profile ID #5687

Merged
merged 8 commits into from
Aug 27, 2024

Conversation

SanchithHegde
Copy link
Member

Type of Change

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

Description

This PR introduces a domain type for profile ID and refactors the existing code to use the domain type in place of strings being used today.

In addition, this PR has the following changes:

  • Introduces a trait GenerateId for generating IDs.
  • Removes the Default implementation on PayoutsNew and PayoutAttemptNew types defined in the hyperswitch_domain_models crate, since the ProfileId type does not implement Default.

Additional Changes

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

This PR has no new contract changes, this PR only updates the merchant connector account create request in the OpenAPI documentation to include the maximum character length restriction placed on the profile_id field.

Motivation and Context

Using a domain type should help better enforce restrictions like minimum and maximum character restrictions, and should help prevent from incorrect values being passed around (to some extent).

How did you test it?

Sanity testing with Postman.

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 S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor M-api-contract-changes Metadata: This PR involves API contract changes api-v2 labels Aug 25, 2024
@SanchithHegde SanchithHegde added this to the August 2024 Release milestone Aug 25, 2024
@SanchithHegde SanchithHegde self-assigned this Aug 25, 2024
@SanchithHegde SanchithHegde requested review from a team as code owners August 25, 2024 11:37
Comment on lines -888 to -902
if !profile_id.is_empty_after_trim() {
// Validate whether profile_id passed in request is valid and is linked to the merchant
core_utils::validate_and_get_business_profile(
state.store.as_ref(),
key_manager_state,
key_store,
Some(profile_id),
merchant_id,
)
.await?
.map(|business_profile| Some(business_profile.profile_id))
} else {
// If empty, Update profile_id to None in the database
Some(None)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We won't be able to allow the "if empty set default profile to null" behavior anymore if using the ProfileId domain type for the field, since the domain type doesn't allow constructing profile IDs from empty strings.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for pointing this out!

Copy link
Member

Choose a reason for hiding this comment

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

@JeevaRamu0104 Tagging for visibility

jarnura
jarnura previously approved these changes Aug 26, 2024
jarnura
jarnura previously approved these changes Aug 26, 2024
Copy link
Contributor

@ThisIsMani ThisIsMani 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 looks fine.

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Aug 27, 2024
Merged via the queue into main with commit b63d723 Aug 27, 2024
14 checks passed
@Gnanasundari24 Gnanasundari24 deleted the introduce-profile-id-domain-type branch August 27, 2024 08:28
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Aug 27, 2024
pixincreate added a commit that referenced this pull request Aug 27, 2024
* 'main' of github.com:juspay/hyperswitch:
  refactor(open_banking): Added merchant data update in mca update (#5655)
  feat: add test_mode for quickly testing payout links (#5669)
  refactor: introduce a domain type for profile ID (#5687)
  ci(cypress): update paybox configs (#5664)
  feat(openapi):  Add open api routes for routing v2 (#5686)
  feat(connector): [NOVALNET] Add template code (#5670)
  feat(user): business email update (#5674)
  chore(config): add production connector-configs for netcetera external 3ds flow (#5698)
  chore(version): 2024.08.27.0
  refactor(euclid): make the disabled node's relation as negative (#5701)
  feat: populate payment method details in payments response (#5661)
  build(deps): bump `diesel` to `2.2.3` and `sqlx` to `0.8.1` (#5688)
pixincreate added a commit that referenced this pull request Aug 27, 2024
* 'main' of github.com:juspay/hyperswitch: (134 commits)
  refactor(open_banking): Added merchant data update in mca update (#5655)
  feat: add test_mode for quickly testing payout links (#5669)
  refactor: introduce a domain type for profile ID (#5687)
  ci(cypress): update paybox configs (#5664)
  feat(openapi):  Add open api routes for routing v2 (#5686)
  feat(connector): [NOVALNET] Add template code (#5670)
  feat(user): business email update (#5674)
  chore(config): add production connector-configs for netcetera external 3ds flow (#5698)
  chore(version): 2024.08.27.0
  refactor(euclid): make the disabled node's relation as negative (#5701)
  feat: populate payment method details in payments response (#5661)
  build(deps): bump `diesel` to `2.2.3` and `sqlx` to `0.8.1` (#5688)
  feat(customer_v2):  added list customer v2 end point (#5517)
  feat(business_profile): add tax_connector_id column in business_profile table (#5576)
  chore: create v2 route for organization (#5679)
  refactor(payments_response): remove setter from payments response (#5676)
  feat(payment_methods_v2): Payment methods v2 API models (#5564)
  chore(version): 2024.08.26.0
  feat(connector): [Adyen] add dispute flows for adyen connector (#5514)
  chore(version): 2024.08.23.0
  ...
pixincreate added a commit that referenced this pull request Aug 27, 2024
…-key-check

* 'main' of github.com:juspay/hyperswitch:
  feat(core): Add mTLS certificates for each request (#5636)
  refactor(open_banking): Added merchant data update in mca update (#5655)
  feat: add test_mode for quickly testing payout links (#5669)
  refactor: introduce a domain type for profile ID (#5687)
  ci(cypress): update paybox configs (#5664)
  feat(openapi):  Add open api routes for routing v2 (#5686)
  feat(connector): [NOVALNET] Add template code (#5670)
  feat(user): business email update (#5674)
  chore(config): add production connector-configs for netcetera external 3ds flow (#5698)
  chore(version): 2024.08.27.0
  refactor(euclid): make the disabled node's relation as negative (#5701)
  feat: populate payment method details in payments response (#5661)
  build(deps): bump `diesel` to `2.2.3` and `sqlx` to `0.8.1` (#5688)
  feat(customer_v2):  added list customer v2 end point (#5517)
  feat(business_profile): add tax_connector_id column in business_profile table (#5576)
  chore: create v2 route for organization (#5679)
  refactor(payments_response): remove setter from payments response (#5676)
  feat(payment_methods_v2): Payment methods v2 API models (#5564)
  chore(version): 2024.08.26.0
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api-v2 C-refactor Category: Refactor M-api-contract-changes Metadata: This PR involves API contract changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants