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: add an api for toggle KV for all merchants #4600

Merged
merged 7 commits into from May 20, 2024

Conversation

dracarys18
Copy link
Member

Type of Change

  • New feature

Description

Add an API to toggle KV for all merchants

Additional Changes

  • This PR modifies the API contract

Motivation and Context

Add an API to toggle KV for all merchants

How did you test it?

  • Enable KV for all merchants
curl --location 'http://localhost:8080/accounts/kv' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--data '{
    "kv_enabled": true
}'

Check if any merchant_account has postgres_only.

SELECT storage_scheme from merchant_account where storage_scheme='postgres_only'

It should return 0 rows

-Disable KV for all merchants

curl --location 'http://localhost:8080/accounts/kv' \
--header 'Content-Type: application/json' \
--header 'api-key: test_admin' \
--data '{
    "kv_enabled": false
}'

Check if any merchant_account has redis_kv

SELECT storage_scheme from merchant_account where storage_scheme='redis_kv'

It should return 0 rows

Checklist

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

@dracarys18 dracarys18 self-assigned this May 9, 2024
@dracarys18 dracarys18 requested review from a team as code owners May 9, 2024 07:02
SanchithHegde
SanchithHegde previously approved these changes May 9, 2024
Comment on lines +131 to +136
generics::generic_update_with_results::<<Self as HasTable>::Table, _, _, _>(
conn,
dsl::merchant_id.ne_all(vec![""]),
merchant_account,
)
.await
Copy link
Member

Choose a reason for hiding this comment

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

For this specific API, you could have used generic_update() which returns the number of rows updated. But I understand this function is meant to be generic enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Used update_with_results to have a check if all the MerchantAccount returned from the list have desired value or not. But left it because seemed redundant

@dracarys18 dracarys18 force-pushed the toggle_kv_for_all_merchants branch from d5b7857 to 72b559f Compare May 10, 2024 05:41
@dracarys18 dracarys18 added A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels May 10, 2024
@dracarys18 dracarys18 linked an issue May 10, 2024 that may be closed by this pull request
SanchithHegde
SanchithHegde previously approved these changes May 10, 2024
akshay-97
akshay-97 previously approved these changes May 10, 2024
@dracarys18 dracarys18 dismissed stale reviews from akshay-97 and SanchithHegde via c702f09 May 20, 2024 06:49
akshay-97
akshay-97 previously approved these changes May 20, 2024
jarnura
jarnura previously approved these changes May 20, 2024
@dracarys18 dracarys18 dismissed stale reviews from jarnura and akshay-97 via f32d927 May 20, 2024 08:19
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue May 20, 2024
Merged via the queue into main with commit 7f53461 May 20, 2024
10 checks passed
@Gnanasundari24 Gnanasundari24 deleted the toggle_kv_for_all_merchants branch May 20, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] add an API to toggle KV for all the merchants
6 participants