feat(user): add support to delete user #3374

apoorvdixit88 · 2024-01-17T13:43:09Z

Type of Change

Description

Add add new endpoint /user/user/delete that deletes the user present in the merchant account

Here we have two scenarios

When user is related to more than one merchant accounts: Delete the user role for that particular merchant account. User will be present in other accounts.
When user is part of one merchant account: Delete user role and user completely.

Additional Changes

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

Motivation and Context

Currently there is no endpoint to remove the invited users

How did you test it?

Singup/Singin
Use the following curl to invite users:

curl --location 'http://localhost:8080/user/user/invite' \
--header 'Authorization: Bearer JWT' \
--header 'Content-Type: application/json' \
--data-raw '{
    "email": "[email protected]",
    "name": "user2",
    "role_id": "merchant_admin"
}

Use the following curl to delete invited user:

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

Cases to handle:

only users with permission UsersWrite can delete
org_admin cannot be deleted
internal users cannot be deleted
user cannot delete himself

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

racnan · 2024-01-18T13:51:32Z

crates/diesel_models/src/query/dashboard_metadata.rs

@@ -104,4 +104,18 @@ impl DashboardMetadata {
        )
        .await
    }
+
+    pub async fn delete_user_scoped_dashboard_metadata(


rename to delete_user_scoped_dashboard_metadata_by_merchant_id

racnan · 2024-01-18T13:53:10Z

crates/router/src/core/errors/user.rs

+            Self::UserNotExist => AER::BadRequest(ApiError::new(
+                sub_code,
+                30,
+                "User does not exist in records",
+                None,
+            )),


racnan · 2024-01-18T13:58:13Z

crates/router/src/core/user.rs

+        .find(|&role| role.merchant_id == user_from_token.merchant_id.as_str())
+    {
+        Some(user_role) => {
+            let _ = can_delete_user_role(&user_role.role_id);


error handling ?

racnan · 2024-01-18T14:01:53Z

crates/router/src/utils/user.rs

+        consts::user_role::ROLE_ID_ORGANIZATION_ADMIN
+        | consts::user_role::ROLE_ID_INTERNAL_ADMIN
+        | consts::user_role::ROLE_ID_INTERNAL_VIEW_ONLY_USER
+        | consts::user_role::INTERNAL_USER_MERCHANT_ID => {


racnan · 2024-01-19T11:10:14Z

Description indicates you're deleting user from org but looks like you're deleting user from merchant account.

racnan · 2024-01-19T11:17:03Z

crates/router/src/core/user_role.rs

+        .find(|&role| role.merchant_id == user_from_token.merchant_id.as_str())
+    {
+        Some(user_role) => {
+            utils::user::validate_deletion_permission_for_role_id(&user_role.role_id)?;


user_role util instead of user util

racnan · 2024-01-19T11:32:09Z

crates/router/src/utils/user.rs

+pub fn validate_deletion_permission_for_role_id(role_id: &str) -> UserResult<()> {
+    match role_id {
+        consts::user_role::ROLE_ID_ORGANIZATION_ADMIN
+        | consts::user_role::ROLE_ID_INTERNAL_ADMIN
+        | consts::user_role::ROLE_ID_INTERNAL_VIEW_ONLY_USER => {
+            Err(UserErrors::InvalidDeleteOperation.into())
+                .attach_printable("Deletion not allowed for users with specific role id")
+        }
+        _ => Ok(()),
+    }
+}


make this property of predefined permission.

racnan

Please test locally.

ThisIsMani · 2024-01-22T10:21:57Z

crates/router/src/routes/lock_utils.rs

@@ -176,6 +176,7 @@ impl From<Flow> for ApiIdentifier {
            | Flow::ForgotPassword
            | Flow::ResetPassword
            | Flow::InviteUser
+            | Flow::DeleteUser


Put this in UserRoles

ThisIsMani

LGTM

feat(user): add support to delete user

cf45556

apoorvdixit88 added the A-users Area: Users label Jan 17, 2024

apoorvdixit88 self-assigned this Jan 17, 2024

apoorvdixit88 requested review from a team as code owners January 17, 2024 13:43

apoorvdixit88 marked this pull request as draft January 17, 2024 13:43

hyperswitch-bot bot and others added 2 commits January 17, 2024 13:43

chore: run formatter

3dee527

add helper function

1758e55

apoorvdixit88 force-pushed the delete-user branch from 4f70c35 to 1758e55 Compare January 18, 2024 13:07

Merge branch 'main' into delete-user

d4403be

apoorvdixit88 removed the request for review from a team January 18, 2024 13:09

apoorvdixit88 added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-feature Category: Feature request or enhancement M-api-contract-changes Metadata: This PR involves API contract changes labels Jan 18, 2024

apoorvdixit88 marked this pull request as ready for review January 18, 2024 13:13

apoorvdixit88 requested review from racnan and ThisIsMani January 18, 2024 13:14

apoorvdixit88 linked an issue Jan 18, 2024 that may be closed by this pull request

feat: delete / deactivate user #3390

Closed

racnan requested changes Jan 18, 2024

View reviewed changes

apoorvdixit88 added 4 commits January 18, 2024 21:09

resolve review comments

1147107

sanity check for email

8505300

change delete_user to delete_user_role

6d64e92

Merge branch 'main' into delete-user

81088f7

apoorvdixit88 requested a review from racnan January 19, 2024 10:56

racnan requested changes Jan 19, 2024

View reviewed changes

add is_deletable in permission

3744821

ivor11 previously approved these changes Jan 19, 2024

View reviewed changes

apoorvdixit88 added 2 commits January 22, 2024 14:47

Merge branch 'main' into delete-user

54e3678

resolve conflicts

a4a600a

apoorvdixit88 dismissed ivor11’s stale review via a4a600a January 22, 2024 10:01

racnan approved these changes Jan 22, 2024

View reviewed changes

ThisIsMani reviewed Jan 22, 2024

View reviewed changes

ThisIsMani approved these changes Jan 22, 2024

View reviewed changes

ivor11 approved these changes Jan 23, 2024

View reviewed changes

Merge branch 'main' into delete-user

1474c7f

Gnanasundari24 added this pull request to the merge queue Jan 25, 2024

Merged via the queue into main with commit 7777710 Jan 25, 2024
10 checks passed

Gnanasundari24 deleted the delete-user branch January 25, 2024 07:22

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(user): add support to delete user #3374

feat(user): add support to delete user #3374

apoorvdixit88 commented Jan 17, 2024 •

edited

Loading

racnan Jan 18, 2024

racnan Jan 18, 2024

racnan Jan 18, 2024

racnan Jan 18, 2024

racnan commented Jan 19, 2024

racnan Jan 19, 2024

racnan Jan 19, 2024

racnan left a comment

ThisIsMani Jan 22, 2024

ThisIsMani left a comment

feat(user): add support to delete user #3374

feat(user): add support to delete user #3374

Conversation

apoorvdixit88 commented Jan 17, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

racnan Jan 18, 2024

Choose a reason for hiding this comment

racnan Jan 18, 2024

Choose a reason for hiding this comment

racnan Jan 18, 2024

Choose a reason for hiding this comment

racnan Jan 18, 2024

Choose a reason for hiding this comment

racnan commented Jan 19, 2024

racnan Jan 19, 2024

Choose a reason for hiding this comment

racnan Jan 19, 2024

Choose a reason for hiding this comment

racnan left a comment

Choose a reason for hiding this comment

ThisIsMani Jan 22, 2024

Choose a reason for hiding this comment

ThisIsMani left a comment

Choose a reason for hiding this comment

apoorvdixit88 commented Jan 17, 2024 •

edited

Loading