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.

Sign up for GitHub

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

Jump to bottom

refactor(business_profile): change id for business profile #5748

Merged
merged 8 commits into from
Sep 2, 2024
Merged

refactor(business_profile): change id for business profile #5748

merged 8 commits into from
Sep 2, 2024

Conversation

Narayanbhat166
Copy link
Member

@Narayanbhat166 Narayanbhat166 commented Aug 29, 2024 •
edited
Loading

Type of Change

  • Refactoring

Description

This PR drops the id column and creates it again for v2 with varchar. Also ApiVersion enum is added.

Additional Changes

  • This PR modifies the database schema

Motivation and Context

How did you test it?

This is a v2 specific change only. For business profiles table

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code

@Narayanbhat166 Narayanbhat166 self-assigned this Aug 29, 2024
@Narayanbhat166 Narayanbhat166 requested review from a team as code owners August 29, 2024 19:19
@semanticdiff-com SemanticDiff.com
Copy link

semanticdiff-com bot commented Aug 29, 2024 •
edited
Loading

Review changes with SemanticDiff.

Analyzed 22 of 27 files.

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

File Information
Filename Status
❔ v2_migrations/2024-07-31-100300_business_profile_add_v2_columns_drop_v1_columns/down.sql Unsupported file format
❔ v2_migrations/2024-07-31-100300_business_profile_add_v2_columns_drop_v1_columns/up.sql Unsupported file format
❔ migrations/2024-08-29-135449_add_api_version_to_business_profile/down.sql Unsupported file format
❔ migrations/2024-08-29-135449_add_api_version_to_business_profile/up.sql Unsupported file format
✔️ crates/router/src/workflows/outgoing_webhook_retry.rs 24.25% smaller
✔️ crates/router/src/utils/user/sample_data.rs 19.34% smaller
✔️ crates/router/src/types/domain.rs 4.35% smaller
✔️ crates/router/src/types/api/admin.rs 97.01% smaller
✔️ crates/router/src/db/business_profile.rs 37.27% smaller
✔️ crates/router/src/core/admin.rs 96.64% smaller
✔️ crates/router/src/core/authentication.rs 24.25% smaller
✔️ crates/router/src/core/routing.rs 12.44% smaller
✔️ crates/router/src/core/user.rs 21.95% smaller
✔️ crates/router/src/core/utils.rs 4.89% smaller
✔️ crates/router/src/core/webhooks/incoming.rs 24.25% smaller
✔️ crates/router/src/core/webhooks/outgoing.rs 4.63% smaller
✔️ crates/router/src/core/webhooks/webhook_events.rs 2.22% smaller
✔️ crates/router/src/core/routing/helpers.rs 24.25% smaller
✔️ crates/router/src/core/payments/helpers.rs Analyzed
✔️ crates/router/src/core/payments/routing.rs 5.31% smaller
✔️ crates/router/src/core/payment_methods/cards.rs 24.25% smaller
❔ crates/router/src/core/authentication/utils.rs Unsupported file format
✔️ crates/hyperswitch_domain_models/src/business_profile.rs 1.24% smaller
✔️ crates/diesel_models/src/business_profile.rs Analyzed
✔️ crates/diesel_models/src/schema.rs Analyzed
✔️ crates/diesel_models/src/schema_v2.rs 23.43% smaller
✔️ crates/diesel_models/src/query/business_profile.rs Analyzed

@hyperswitch-bot hyperswitch-bot bot added the M-database-changes Metadata: This PR involves database schema changes label Aug 29, 2024
sai-harsha-vardhan
sai-harsha-vardhan reviewed Aug 30, 2024
View reviewed changes
crates/diesel_models/src/query/business_profile.rs
@@ -46,7 +46,7 @@ impl BusinessProfile {
) -> StorageResult<Self> {
generics::generic_find_one::<<Self as HasTable>::Table, _, _>(
conn,
dsl::profile_id.eq(profile_id.to_owned()),
dsl_identifier.eq(profile_id.to_owned()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have separate v1 and v2 functions, for more clarity as there's no profile_id in v2

jarnura
jarnura approved these changes Aug 31, 2024
View reviewed changes
Copy link
Member

@jarnura jarnura left a comment

Choose a reason for hiding this comment

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

LGTM

ThisIsMani
ThisIsMani approved these changes Sep 2, 2024
View reviewed changes
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.

@likhinbopanna likhinbopanna added this pull request to the merge queue Sep 2, 2024
Merged via the queue into main with commit 8e5c33e Sep 2, 2024
18 checks passed
@likhinbopanna likhinbopanna deleted the change_id_for_business_profile branch September 2, 2024 10:19
pixincreate added a commit that referenced this pull request Sep 3, 2024
@pixincreate
Merge branch 'main' of github.com:juspay/hyperswitch into fiservemea-…
7843706 
…config-fix

* 'main' of github.com:juspay/hyperswitch:
  feat: add profile_id authentication for business profile update and list (#5673)
  chore(version): 2024.09.03.0
  feat(user): implement invitations api (#5769)
  feat(connector): [Adyenplatform] add webhooks for payout (#5749)
  refactor(v2_migrations): re-organize v2 migrations (#5760)
  chore: add wasm support for connector additional details (#5712)
  refactor(connector): Move globepay, powertranz, tsys, worldline to hyperswitch_connectors (#5758)
  fix(cypress): fix cypress throwing error when `connectorId` is not passed and miscellaneous fixes (#5746)
  chore: fix typos (#5766)
  refactor(business_profile): change id for business profile (#5748)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThisIsMani ThisIsMani ThisIsMani approved these changes

@prajjwalkumar17 prajjwalkumar17 prajjwalkumar17 approved these changes

@sai-harsha-vardhan sai-harsha-vardhan sai-harsha-vardhan approved these changes

@Aprabhat19 Aprabhat19 Aprabhat19 approved these changes

@SanchithHegde SanchithHegde SanchithHegde approved these changes

@jarnura jarnura jarnura approved these changes

Assignees

@Narayanbhat166 Narayanbhat166

Labels
api-v2 M-database-changes Metadata: This PR involves database schema changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Narayanbhat166 @jarnura @SanchithHegde @Aprabhat19 @sai-harsha-vardhan @prajjwalkumar17 @ThisIsMani @likhinbopanna