feat(users): add support for profile user delete by apoorvdixit88 · Pull Request #5541 · juspay/hyperswitch

apoorvdixit88 · 2024-08-06T11:03:04Z

Type of Change

Description

Add support to delete profile level users
Delete is backward compatible
Delete to support both V1 and V2 operations accordingly

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 #5540

How did you test it?

Use the curl to delete user role

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

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

ThisIsMani

This might break when org level roles are updatable or deletable.

ThisIsMani · 2024-08-08T13:58:03Z

crates/router/src/core/user_role.rs

+        if !target_role_info.is_deletable()
+            || !utils::user_role::is_valid_entity_operation(
+                deletion_requestor_role_info.get_entity_type(),
+                target_role_info.get_entity_type(),
+            )
+        {


Handle these errors separately.

ThisIsMani · 2024-08-08T13:58:25Z

crates/router/src/core/user_role.rs

+            .attach_printable("User is not associated with the merchant");
+    }
+
+    // If user has no more role associated with it then deleting user


Change this comment or move this to bottom.

apoorvdixit88 · 2024-08-12T08:48:19Z

This might break when org level roles are updatable or deletable.

For V1 org_level roles are neither updatable nor deletable. With V2 it will work fine. Also we won't be introducing org_level roles in V1.

racnan · 2024-08-12T10:06:36Z

crates/diesel_models/src/query/user_role.rs

+                (dsl::org_id
+                    .eq(org_id.clone())
+                    .and(dsl::merchant_id.is_null().and(dsl::profile_id.is_null())))
+                .or(dsl::org_id.eq(org_id.clone()).and(
+                    dsl::merchant_id
+                        .eq(merchant_id.clone())
+                        .and(dsl::profile_id.is_null()),
+                ))
+                .or(dsl::org_id.eq(org_id).and(
+                    dsl::merchant_id
+                        .eq(merchant_id)
+                        .and(dsl::profile_id.eq(profile_id)),
+                )),
+            )


lets have this as a helper function.

racnan · 2024-08-12T10:07:03Z

crates/diesel_models/src/query/user_role.rs

+                .or(dsl::org_id.eq(org_id).and(
+                    dsl::merchant_id
+                        .eq(merchant_id)
+                        .and(dsl::profile_id.eq(profile_id)),
+                )),


put this under if let Some(_).

racnan · 2024-08-12T10:07:19Z

crates/diesel_models/src/query/user_role.rs

+        let predicate = dsl::user_id
+            .eq(user_id)
+            .and(
+                (dsl::org_id
+                    .eq(org_id.clone())
+                    .and(dsl::merchant_id.is_null().and(dsl::profile_id.is_null())))
+                .or(dsl::org_id.eq(org_id.clone()).and(
+                    dsl::merchant_id
+                        .eq(merchant_id.clone())
+                        .and(dsl::profile_id.is_null()),
+                ))
+                .or(dsl::org_id.eq(org_id).and(
+                    dsl::merchant_id
+                        .eq(merchant_id)
+                        .and(dsl::profile_id.eq(profile_id)),
+                )),
+            )
+            .and(dsl::version.eq(version));


same as above

racnan · 2024-08-12T10:32:17Z

crates/router/src/core/user_role.rs

+            UserRoleVersion::V2,
+        )
+        .await
+        .change_context(UserErrors::InternalServerError)


send 400 for not found.

racnan · 2024-08-12T11:05:05Z

crates/router/src/core/user_role.rs

+    if !user_role_deleted_flag {
+        return Err(report!(UserErrors::InvalidDeleteOperation))
+            .attach_printable("User is not associated with the merchant");


500 is not handled.

racnan · 2024-08-12T11:21:16Z

crates/router/src/utils/user_role.rs

+pub fn entity_level(entity_type: EntityType) -> u8 {
+    match entity_type {
+        EntityType::Internal => 3,
+        EntityType::Organization => 2,
+        EntityType::Merchant => 1,
+        EntityType::Profile => 0,
+    }
+}


https://doc.rust-lang.org/std/cmp/trait.Ord.html

ThisIsMani

Please remove the old delete function.

ThisIsMani · 2024-08-14T08:36:47Z

crates/router/src/core/user_role.rs

+        .change_context(UserErrors::InternalServerError)?;
+
+    // If user has no more role associated with him then deleting user
+    if user_roles.is_empty() {


You can check if v2 roles are also empty.

jarnura · 2024-08-14T11:50:43Z

crates/router/src/db/kafka_store.rs

-            .list_user_roles_by_user_id(user_id, version)
-            .await
+    ) -> CustomResult<storage::UserRole, errors::StorageError> {
+        self.find_user_role_by_user_id_and_lineage(


Suggested change

self.find_user_role_by_user_id_and_lineage(

self.diesel_store.find_user_role_by_user_id_and_lineage(

In kakfaStore if we can self.functions will causes stack_overflow. So avoid following that.

jarnura · 2024-08-14T11:51:05Z

crates/router/src/db/kafka_store.rs

+        profile_id: Option<&String>,
+        version: enums::UserRoleVersion,
+    ) -> CustomResult<storage::UserRole, errors::StorageError> {
+        self.delete_user_role_by_user_id_and_lineage(


Suggested change

self.delete_user_role_by_user_id_and_lineage(

self.diesel_store.delete_user_role_by_user_id_and_lineage(

In kakfaStore if we can self.functions will causes stack_overflow. So avoid following that.

apoorvdixit88 added C-feature Category: Feature request or enhancement C-refactor Category: Refactor A-users Area: Users labels Aug 6, 2024

apoorvdixit88 requested review from ThisIsMani and racnan August 6, 2024 11:03

apoorvdixit88 self-assigned this Aug 6, 2024

apoorvdixit88 requested review from a team as code owners August 6, 2024 11:03

feat(users): delete user role

0aee7a6

apoorvdixit88 force-pushed the fix-delete-user-role branch from 32070eb to 0aee7a6 Compare August 8, 2024 13:14

fix: correct spellings

5aa44b5

apoorvdixit88 force-pushed the fix-delete-user-role branch from a4e797c to 5aa44b5 Compare August 8, 2024 13:21

ThisIsMani reviewed Aug 8, 2024

View reviewed changes

apoorvdixit88 and others added 2 commits August 12, 2024 13:06

Merge branch 'main' into fix-delete-user-role

a1fcd39

docs(openapi): re-generate OpenAPI specification

d4c670d

hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Aug 12, 2024

apoorvdixit88 and others added 2 commits August 12, 2024 14:20

fix: resolve suggestions

0a528e4

Merge branch 'main' into fix-delete-user-role

a70447a

racnan requested changes Aug 12, 2024

View reviewed changes

racnan reviewed Aug 12, 2024

View reviewed changes

fix: resolve review comments

00574de

hyperswitch-bot bot removed the M-api-contract-changes Metadata: This PR involves API contract changes label Aug 13, 2024

apoorvdixit88 added 4 commits August 13, 2024 23:40

Merge branch 'main' into fix-delete-user-role

e2c8f10

fix: handle map error

df3035a

fix: change internal server error logic

91cb0d6

fix: add todo

908ee8f

ThisIsMani reviewed Aug 14, 2024

View reviewed changes

apoorvdixit88 and others added 2 commits August 14, 2024 14:14

fix: remove old delete

820b02d

chore: run formatter

977252e

ThisIsMani previously approved these changes Aug 14, 2024

View reviewed changes

jarnura requested changes Aug 14, 2024

View reviewed changes

fix: correct kafka store function

a87358c

apoorvdixit88 dismissed ThisIsMani’s stale review via a87358c August 14, 2024 11:58

jarnura approved these changes Aug 14, 2024

View reviewed changes

racnan approved these changes Aug 14, 2024

View reviewed changes

Gnanasundari24 added this pull request to the merge queue Aug 14, 2024

Merged via the queue into main with commit 19a9180 Aug 14, 2024

Gnanasundari24 deleted the fix-delete-user-role branch August 14, 2024 15:00

	self.find_user_role_by_user_id_and_lineage(
	self.diesel_store.find_user_role_by_user_id_and_lineage(

	self.delete_user_role_by_user_id_and_lineage(
	self.diesel_store.delete_user_role_by_user_id_and_lineage(

Conversation

apoorvdixit88 commented Aug 6, 2024

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

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!

apoorvdixit88 commented Aug 12, 2024

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants