feat(user_roles): get user role details by apoorvdixit88 · Pull Request #5777 · juspay/hyperswitch

apoorvdixit88 · 2024-09-02T20:08:54Z

Type of Change

Description

Add support to get user role details

Additional Changes

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

Motivation and Context

Closes #5776

How did you test it?

Request:

curl --location 'http://localhost:8080/user/user/v2' \
--header 'Content-Type: application/json' \
--header 'Authorization: Bearer JWT' \
--data-raw '{
    "email": "some_email"
}'

Response

[
    {
        "role_id": "org_admin",
        "org": {
            "name": null,
            "id": "org_SDzPXs082bpEQt3jbPlE"
        },
        "merchant": null,
        "profile": null,
        "status": "Active",
        "entity_type": "organization"
    }
]

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

semanticdiff-com · 2024-09-02T20:08:57Z

Review changes with SemanticDiff.

Analyzed 5 of 5 files.

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

	Filename	Status
✔️	crates/router/src/routes/app.rs	Analyzed
✔️	crates/router/src/routes/user.rs	Analyzed
✔️	crates/router/src/core/user.rs	Analyzed
✔️	crates/api_models/src/user.rs	Analyzed
✔️	crates/api_models/src/events/user.rs	60.26% smaller

ThisIsMani · 2024-09-03T11:38:49Z

crates/router/src/core/user.rs

+
+    let mut role_details_list = Vec::new();
+
+    for user_role in user_roles_set {


Parallelize.

ThisIsMani · 2024-09-03T11:45:20Z

crates/router/src/core/user.rs

+        let role_info = if let Some(role) =
+            roles::predefined_roles::PREDEFINED_ROLES.get(&user_role.role_id.as_str())
+        {
+            role.clone()
+        } else {
+            state
+                .store
+                .find_role_by_role_id_in_merchant_scope(
+                    &user_role.role_id,
+                    &user_role
+                        .merchant_id
+                        .ok_or(UserErrors::InternalServerError)
+                        .attach_printable("Merchant id not found for custom role")?,
+                    &user_from_token.org_id,
+                )
+                .await
+                .change_context(UserErrors::InternalServerError)
+                .attach_printable("Failed to get custom role")?
+                .into()
+        };


Why not from_role_id()?

coz merchant id is non option param in that, he we may or may not get it.

Make a function in roles which takes user role and gives back a role info.

If the role_id is present in the predefined roles it will not do any db calls.
Else if the entity type is org, then throw internal server error (As we don't have any org level custom roles.)
Else if the entity type is merchant, then get merchant id from the user role (if it is not there throw 500) and find the custom role.
Else if the entity type is merchant or profile, then get merchant id from the user role (if it is not there throw 500) and find the custom role.

You can change the from_role_id() function if it is possible.

ThisIsMani · 2024-09-03T11:46:04Z

crates/router/src/routes/user.rs

+    req: HttpRequest,
+    payload: web::Query<user_api::GetUserRoleDetailsRequest>,
+) -> HttpResponse {
+    let flow = Flow::GetUserRoleDetails;


Yes we can have, but this flow was not getting used anywhere

ThisIsMani

Some suggestions.

ThisIsMani · 2024-09-04T14:07:29Z

crates/router/src/core/user.rs

+            let merchant = match entity_type.ok_or(UserErrors::InternalServerError)? {
+                EntityType::Internal => {
+                    return Err(UserErrors::InvalidRoleOperationWithMessage(
+                        "Internal roles are not allowed for this operation".to_string(),
+                    ));
+                }
+                EntityType::Organization => None,
+                EntityType::Merchant | EntityType::Profile => {
+                    if let Some(merchant_id) = &user_role.merchant_id {
+                        Some(NameIdUnit {
+                            id: merchant_id.clone(),
+                            name: merchant_map
+                                .get(merchant_id)
+                                .ok_or(UserErrors::InternalServerError)?
+                                .to_owned(),
+                        })
+                    } else {
+                        return Err(UserErrors::InternalServerError);
+                    }
+                }
+            };
+
+            let profile = match entity_type.ok_or(UserErrors::InternalServerError)? {
+                EntityType::Internal => {
+                    return Err(UserErrors::InvalidRoleOperationWithMessage(
+                        "Internal roles are not allowed for this operation".to_string(),
+                    ));
+                }
+                EntityType::Organization | EntityType::Merchant => None,
+                EntityType::Profile => {
+                    if let Some(profile_id) = &user_role.profile_id {
+                        Some(NameIdUnit {
+                            id: profile_id.clone(),
+                            name: profile_map
+                                .get(profile_id)
+                                .ok_or(UserErrors::InternalServerError)?
+                                .to_owned(),
+                        })
+                    } else {
+                        return Err(UserErrors::InternalServerError);
+                    }
+                }
+            };


Can be done in single match statement.

ThisIsMani · 2024-09-04T14:07:58Z

crates/router/src/core/user.rs

+                    if let Some(merchant_id) = &user_role.merchant_id {
+                        Some(NameIdUnit {
+                            id: merchant_id.clone(),
+                            name: merchant_map
+                                .get(merchant_id)
+                                .ok_or(UserErrors::InternalServerError)?
+                                .to_owned(),
+                        })
+                    } else {
+                        return Err(UserErrors::InternalServerError);
+                    }


Use ok_or() instead.

SanchithHegde · 2024-09-04T18:22:13Z

crates/router/src/core/user.rs

+    };
+
+    let (merchant_ids, merchant_profile_ids) = user_roles_set.iter().try_fold(
+        (Vec::new(), Vec::new()),


You can probably use something like Vec::with_capacity(user_roles_set.len()) here, if the size of the Vec is known beforehand, and is same as user_roles_set.

No the size is not fix, we will be only considering those use roles, where the entity type is merchant and profile.
And ignoring the internal and organisation in this use case

* 'main' of github.com:juspay/hyperswitch: feat(customer_v2): Add customer V2 delete api (#5518) chore(version): 2024.09.05.0 feat(user_roles): get user role details (#5777) feat(users): Add profile level invites (#5793) refactor(router): profile based routes for payouts (#5794) Feat(connector): [Fiuu] Add Card Flows (#5786) fix(cypress): fix fiservemea configs for cypress (#5772) fix(cypress): `api_key` check in cypress (#5787) feat(payment_methods_v2): Implemented Diesel and Domain models for v2 (#5700) fix(payout): query for getting a list of active payout IDs (#5771) refactor(router): remove admin v2 intermediate features (#5780) feat(revert): populate payment method details in payments response (#5785) chore(version): 2024.09.04.0 fix(connector): skip 3DS in `network_transaction_id` flow for cybersource (#5781) refactor(euclid): check the authenticity of profile_id being used (#5647) feat(analytics): refactor and introduce analytics APIs to accommodate OrgLevel, MerchantLevel and ProfileLevel authentication (#5729) fix(router): make customer details None in the `Psync` flow if the customer is deleted (#5732) feat(connector): [DEUTSCHE] Add template code (#5774) chore(version): 2024.09.03.1 fix(router): send post message to window.parent instead of window.top in external 3ds flow (#5778)

feat(user_roles): get user role details

c8530c8

apoorvdixit88 added C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-users Area: Users labels Sep 2, 2024

apoorvdixit88 self-assigned this Sep 2, 2024

apoorvdixit88 requested review from a team as code owners September 2, 2024 20:08

apoorvdixit88 marked this pull request as draft September 2, 2024 20:22

fix: get unique list

d318245

apoorvdixit88 marked this pull request as ready for review September 3, 2024 10:27

apoorvdixit88 requested review from ThisIsMani, lsampras and racnan September 3, 2024 10:27

apoorvdixit88 added 4 commits September 3, 2024 16:22

Merge branch 'main' into get-user-role-details-v2

5d0b950

fix: get entity type

7ac5fae

fix: handle internal entity

3ac4e02

fix: rename route function

d6dd9bd

ThisIsMani reviewed Sep 3, 2024

View reviewed changes

apoorvdixit88 added 3 commits September 4, 2024 16:35

fix: change core and api to post

a9f4de6

fix: resolve clippy

3727d92

fix: correct business logic

4a4e5f6

ThisIsMani previously approved these changes Sep 4, 2024

View reviewed changes

apoorvdixit88 added 2 commits September 4, 2024 21:24

Merge branch 'main' into get-user-role-details-v2

6cc0778

fix: resolve suggestions

bf1ec86

apoorvdixit88 dismissed ThisIsMani’s stale review via bf1ec86 September 4, 2024 17:12

ThisIsMani approved these changes Sep 4, 2024

View reviewed changes

SanchithHegde reviewed Sep 4, 2024

View reviewed changes

SanchithHegde approved these changes Sep 4, 2024

View reviewed changes

likhinbopanna added this pull request to the merge queue Sep 4, 2024

Merged via the queue into main with commit eae8d89 Sep 4, 2024

likhinbopanna deleted the get-user-role-details-v2 branch September 4, 2024 20:38

SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Sep 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(user_roles): get user role details#5777

feat(user_roles): get user role details#5777
likhinbopanna merged 11 commits intomainfrom
get-user-role-details-v2

apoorvdixit88 commented Sep 2, 2024 •

edited

Loading

Uh oh!

semanticdiff-com bot commented Sep 2, 2024 •

edited

Loading

Uh oh!

ThisIsMani Sep 3, 2024

Uh oh!

ThisIsMani Sep 3, 2024

Uh oh!

apoorvdixit88 Sep 3, 2024

Uh oh!

ThisIsMani Sep 3, 2024

Uh oh!

ThisIsMani Sep 3, 2024

Uh oh!

apoorvdixit88 Sep 3, 2024

Uh oh!

ThisIsMani left a comment

Uh oh!

ThisIsMani Sep 4, 2024

Uh oh!

ThisIsMani Sep 4, 2024

Uh oh!

SanchithHegde Sep 4, 2024

Uh oh!

apoorvdixit88 Sep 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		let mut role_details_list = Vec::new();

		for user_role in user_roles_set {

Conversation

apoorvdixit88 commented Sep 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

Uh oh!

semanticdiff-com bot commented Sep 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThisIsMani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

apoorvdixit88 commented Sep 2, 2024 •

edited

Loading

semanticdiff-com bot commented Sep 2, 2024 •

edited

Loading