refactor(user_role): Remove V1 insertion for user_roles and allow Invites for org_admins (#6185)

crates/diesel_models/src/query/user_role.rs

@@ -5,7 +5,6 @@ use diesel::{
     BoolExpressionMethods, ExpressionMethods, QueryDsl,
 };
 use error_stack::{report, ResultExt};
-use router_env::logger;
 
 use crate::{
     enums::{UserRoleVersion, UserStatus},
@@ -23,21 +22,6 @@ impl UserRoleNew {
 }
 
 impl UserRole {
-    pub async fn insert_multiple_user_roles(
-        conn: &PgPooledConn,
-        user_roles: Vec<UserRoleNew>,
-    ) -> StorageResult<Vec<Self>> {
-        let query = diesel::insert_into(<Self>::table()).values(user_roles);
-
-        logger::debug!(query = %debug_query::<Pg,_>(&query).to_string());
-
-        query
-            .get_results_async(conn)
-            .await
-            .change_context(errors::DatabaseError::Others)
-            .attach_printable("Error while inserting user_roles")
-    }
-
     pub async fn find_by_user_id(
         conn: &PgPooledConn,
         user_id: String,

crates/router/src/core/user.rs

@@ -67,7 +67,6 @@ pub async fn signup_with_merchant_id(
             state.clone(),
             common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN.to_string(),
             UserStatus::Active,
-            None,
         )
         .await?;
 
@@ -146,7 +145,6 @@ pub async fn signup_token_only_flow(
             state.clone(),
             common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN.to_string(),
             UserStatus::Active,
-            None,
         )
         .await?;
 
@@ -247,7 +245,6 @@ pub async fn connect_account(
                 state.clone(),
                 common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN.to_string(),
                 UserStatus::Active,
-                None,
             )
             .await?;
 
@@ -657,7 +654,7 @@ async fn handle_existing_user_invitation(
                     org_id: user_from_token.org_id.clone(),
                     merchant_id: user_from_token.merchant_id.clone(),
                 })
-                .insert_in_v1_and_v2(state)
+                .insert_in_v2(state)
                 .await?
         }
         EntityType::Profile => {
@@ -767,14 +764,21 @@ async fn handle_new_user_invitation(
     };
 
     let _user_role = match role_info.get_entity_type() {
-        EntityType::Organization => return Err(UserErrors::InvalidRoleId.into()),
+        EntityType::Organization => {
+            user_role
+                .add_entity(domain::OrganizationLevel {
+                    org_id: user_from_token.org_id.clone(),
+                })
+                .insert_in_v2(state)
+                .await?
+        }
         EntityType::Merchant => {
             user_role
                 .add_entity(domain::MerchantLevel {
                     org_id: user_from_token.org_id.clone(),
                     merchant_id: user_from_token.merchant_id.clone(),
                 })
-                .insert_in_v1_and_v2(state)
+                .insert_in_v2(state)
                 .await?
         }
         EntityType::Profile => {
@@ -1128,7 +1132,7 @@ pub async fn create_internal_user(
             org_id: internal_merchant.organization_id,
             merchant_id: internal_merchant_id,
         })
-        .insert_in_v1_and_v2(&state)
+        .insert_in_v2(&state)
         .await
         .change_context(UserErrors::InternalServerError)?;
 
@@ -1142,28 +1146,11 @@ pub async fn create_merchant_account(
 ) -> UserResponse<()> {
     let user_from_db = user_from_token.get_user_from_db(&state).await?;
 
-    let new_user = domain::NewUser::try_from((user_from_db, req, user_from_token))?;
+    let new_merchant = domain::NewUserMerchant::try_from((user_from_db, req, user_from_token))?;
-    let new_merchant = new_user.get_new_merchant();
     new_merchant
         .create_new_merchant_and_insert_in_db(state.to_owned())
         .await?;
 
-    let role_insertion_res = new_user
-        .insert_org_level_user_role_in_db(
-            state.clone(),
-            common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN.to_string(),
-            UserStatus::Active,
-            Some(UserRoleVersion::V1),
-        )
-        .await;
-    if let Err(e) = role_insertion_res {
-        let _ = state
-            .store
-            .delete_merchant_account_by_merchant_id(&new_merchant.get_merchant_id())
-            .await;
-        return Err(e);
-    }
-
     Ok(ApplicationResponse::StatusOk)
 }
 

crates/router/src/db/kafka_store.rs

@@ -37,10 +37,7 @@ use super::{
     user::{sample_data::BatchSampleDataInterface, UserInterface},
     user_authentication_method::UserAuthenticationMethodInterface,
     user_key_store::UserKeyStoreInterface,
-    user_role::{
+    user_role::{ListUserRolesByOrgIdPayload, ListUserRolesByUserIdPayload, UserRoleInterface},
-        InsertUserRolePayload, ListUserRolesByOrgIdPayload, ListUserRolesByUserIdPayload,
-        UserRoleInterface,
-    },
 };
 #[cfg(feature = "payouts")]
 use crate::services::kafka::payout::KafkaPayout;
@@ -3018,8 +3015,8 @@ impl RedisConnInterface for KafkaStore {
 impl UserRoleInterface for KafkaStore {
     async fn insert_user_role(
         &self,
-        user_role: InsertUserRolePayload,
+        user_role: storage::UserRoleNew,
-    ) -> CustomResult<Vec<user_storage::UserRole>, errors::StorageError> {
+    ) -> CustomResult<user_storage::UserRole, errors::StorageError> {
         self.diesel_store.insert_user_role(user_role).await
     }
 

crates/router/src/db/user_role.rs

@@ -13,21 +13,6 @@ use crate::{
     services::Store,
 };
 
-pub enum InsertUserRolePayload {
-    OnlyV1(storage::UserRoleNew),
-    OnlyV2(storage::UserRoleNew),
-    V1AndV2(Box<[storage::UserRoleNew; 2]>),
-}
-
-impl InsertUserRolePayload {
-    fn convert_to_vec(self) -> Vec<storage::UserRoleNew> {
-        match self {
-            Self::OnlyV1(user_role) | Self::OnlyV2(user_role) => vec![user_role],
-            Self::V1AndV2(user_roles) => user_roles.to_vec(),
-        }
-    }
-}
-
 pub struct ListUserRolesByOrgIdPayload<'a> {
     pub user_id: Option<&'a String>,
     pub org_id: &'a id_type::OrganizationId,
@@ -51,8 +36,8 @@ pub struct ListUserRolesByUserIdPayload<'a> {
 pub trait UserRoleInterface {
     async fn insert_user_role(
         &self,
-        user_role: InsertUserRolePayload,
+        user_role: storage::UserRoleNew,
-    ) -> CustomResult<Vec<storage::UserRole>, errors::StorageError>;
+    ) -> CustomResult<storage::UserRole, errors::StorageError>;
 
     async fn find_user_role_by_user_id_and_lineage(
         &self,
@@ -98,11 +83,12 @@ impl UserRoleInterface for Store {
     #[instrument(skip_all)]
     async fn insert_user_role(
         &self,
-        user_role: InsertUserRolePayload,
+        user_role: storage::UserRoleNew,
-    ) -> CustomResult<Vec<storage::UserRole>, errors::StorageError> {
+    ) -> CustomResult<storage::UserRole, errors::StorageError> {
         let conn = connection::pg_connection_write(self).await?;
 
-        storage::UserRole::insert_multiple_user_roles(&conn, user_role.convert_to_vec())
+        user_role
+            .insert(&conn)
             .await
             .map_err(|error| report!(errors::StorageError::from(error)))
     }
@@ -217,44 +203,38 @@ impl UserRoleInterface for Store {
 impl UserRoleInterface for MockDb {
     async fn insert_user_role(
         &self,
-        user_role: InsertUserRolePayload,
+        user_role: storage::UserRoleNew,
-    ) -> CustomResult<Vec<storage::UserRole>, errors::StorageError> {
+    ) -> CustomResult<storage::UserRole, errors::StorageError> {
         let mut db_user_roles = self.user_roles.lock().await;
 
-        user_role
+        if db_user_roles
-            .convert_to_vec()
+            .iter()
-            .into_iter()
+            .any(|user_role_inner| user_role_inner.user_id == user_role.user_id)
-            .map(|user_role| {
+        {
-                if db_user_roles
+            Err(errors::StorageError::DuplicateValue {
-                    .iter()
+                entity: "user_id",
-                    .any(|user_role_inner| user_role_inner.user_id == user_role.user_id)
+                key: None,
-                {
+            })?
-                    Err(errors::StorageError::DuplicateValue {
+        }
-                        entity: "user_id",
+        let user_role = storage::UserRole {
-                        key: None,
+            id: i32::try_from(db_user_roles.len())
-                    })?
+                .change_context(errors::StorageError::MockDbError)?,
-                }
+            user_id: user_role.user_id,
-                let user_role = storage::UserRole {
+            merchant_id: user_role.merchant_id,
-                    id: i32::try_from(db_user_roles.len())
+            role_id: user_role.role_id,
-                        .change_context(errors::StorageError::MockDbError)?,
+            status: user_role.status,
-                    user_id: user_role.user_id,
+            created_by: user_role.created_by,
-                    merchant_id: user_role.merchant_id,
+            created_at: user_role.created_at,
-                    role_id: user_role.role_id,
+            last_modified: user_role.last_modified,
-                    status: user_role.status,
+            last_modified_by: user_role.last_modified_by,
-                    created_by: user_role.created_by,
+            org_id: user_role.org_id,
-                    created_at: user_role.created_at,
+            profile_id: None,
-                    last_modified: user_role.last_modified,
+            entity_id: None,
-                    last_modified_by: user_role.last_modified_by,
+            entity_type: None,
-                    org_id: user_role.org_id,
+            version: enums::UserRoleVersion::V1,
-                    profile_id: None,
+        };
-                    entity_id: None,
+        db_user_roles.push(user_role.clone());
-                    entity_type: None,
+        Ok(user_role)
-                    version: enums::UserRoleVersion::V1,
-                };
-                db_user_roles.push(user_role.clone());
-                Ok(user_role)
-            })
-            .collect::<Result<Vec<_>, _>>()
     }
 
     async fn find_user_role_by_user_id_and_lineage(

crates/router/src/services/authorization/roles/predefined_roles.rs

@@ -83,9 +83,9 @@ pub static PREDEFINED_ROLES: Lazy<HashMap<&'static str, RoleInfo>> = Lazy::new(|
             role_name: "organization_admin".to_string(),
             scope: RoleScope::Organization,
             entity_type: EntityType::Organization,
-            is_invitable: false,
+            is_invitable: true,
-            is_deletable: false,
+            is_deletable: true,
-            is_updatable: false,
+            is_updatable: true,
             is_internal: false,
         },
     );

crates/router/src/types/domain/user.rs

@@ -30,7 +30,7 @@ use crate::{
         admin,
         errors::{UserErrors, UserResult},
     },
-    db::{user_role::InsertUserRolePayload, GlobalStorageInterface},
+    db::GlobalStorageInterface,
     routes::SessionState,
     services::{self, authentication::UserFromToken},
     types::transformers::ForeignFrom,
@@ -661,26 +661,17 @@ impl NewUser {
         state: SessionState,
         role_id: String,
         user_status: UserStatus,
-        version: Option<UserRoleVersion>,
     ) -> UserResult<UserRole> {
         let org_id = self
             .get_new_merchant()
             .get_new_organization()
             .get_organization_id();
-        let merchant_id = self.get_new_merchant().get_merchant_id();
 
         let org_user_role = self
             .get_no_level_user_role(role_id, user_status)
-            .add_entity(OrganizationLevel {
+            .add_entity(OrganizationLevel { org_id });
-                org_id,
-                merchant_id,
-            });
 
-        match version {
+        org_user_role.insert_in_v2(&state).await
-            Some(UserRoleVersion::V1) => org_user_role.insert_in_v1(&state).await,
-            Some(UserRoleVersion::V2) => org_user_role.insert_in_v2(&state).await,
-            None => org_user_role.insert_in_v1_and_v2(&state).await,
-        }
     }
 }
 
@@ -1088,8 +1079,6 @@ pub struct NoLevel;
 #[derive(Clone)]
 pub struct OrganizationLevel {
     pub org_id: id_type::OrganizationId,
-    // Keeping this to allow insertion of org_admins in V1
-    pub merchant_id: id_type::MerchantId,
 }
 
 #[derive(Clone)]
@@ -1143,32 +1132,46 @@ pub struct EntityInfo {
     entity_type: EntityType,
 }
 
-impl<E> NewUserRole<E>
+impl From<OrganizationLevel> for EntityInfo {
-where
+    fn from(value: OrganizationLevel) -> Self {
-    E: Clone,
+        Self {
-{
+            entity_id: value.org_id.get_string_repr().to_owned(),
-    fn convert_to_new_v1_role(
+            entity_type: EntityType::Organization,
-        self,
+            org_id: value.org_id,
-        org_id: id_type::OrganizationId,
+            merchant_id: None,
-        merchant_id: id_type::MerchantId,
-    ) -> UserRoleNew {
-        UserRoleNew {
-            user_id: self.user_id,
-            role_id: self.role_id,
-            status: self.status,
-            created_by: self.created_by,
-            last_modified_by: self.last_modified_by,
-            created_at: self.created_at,
-            last_modified: self.last_modified,
-            org_id: Some(org_id),
-            merchant_id: Some(merchant_id),
             profile_id: None,
-            entity_id: None,
-            entity_type: None,
-            version: UserRoleVersion::V1,
         }
     }
+}
 
+impl From<MerchantLevel> for EntityInfo {
+    fn from(value: MerchantLevel) -> Self {
+        Self {
+            entity_id: value.merchant_id.get_string_repr().to_owned(),
+            entity_type: EntityType::Merchant,
+            org_id: value.org_id,
+            profile_id: None,
+            merchant_id: Some(value.merchant_id),
+        }
+    }
+}
+
+impl From<ProfileLevel> for EntityInfo {
+    fn from(value: ProfileLevel) -> Self {
+        Self {
+            entity_id: value.profile_id.get_string_repr().to_owned(),
+            entity_type: EntityType::Profile,
+            org_id: value.org_id,
+            merchant_id: Some(value.merchant_id),
+            profile_id: Some(value.profile_id),
+        }
+    }
+}
+
+impl<E> NewUserRole<E>
+where
+    E: Clone + Into<EntityInfo>,
+{
     fn convert_to_new_v2_role(self, entity: EntityInfo) -> UserRoleNew {
         UserRoleNew {
             user_id: self.user_id,
@@ -1187,116 +1190,15 @@ where
         }
     }
 
-    async fn insert_v1_and_v2_in_db_and_get_v2(
-        state: &SessionState,
-        v1_role: UserRoleNew,
-        v2_role: UserRoleNew,
-    ) -> UserResult<UserRole> {
-        let inserted_roles = state
-            .store
-            .insert_user_role(InsertUserRolePayload::V1AndV2(Box::new([v1_role, v2_role])))
-            .await
-            .change_context(UserErrors::InternalServerError)?;
-
-        inserted_roles
-            .into_iter()
-            .find(|role| role.version == UserRoleVersion::V2)
-            .ok_or(report!(UserErrors::InternalServerError))
-    }
-}
-
-impl NewUserRole<OrganizationLevel> {
-    pub async fn insert_in_v1(self, state: &SessionState) -> UserResult<UserRole> {
-        let entity = self.entity.clone();
-
-        let new_v1_role = self
-            .clone()
-            .convert_to_new_v1_role(entity.org_id.clone(), entity.merchant_id.clone());
-
-        state
-            .store
-            .insert_user_role(InsertUserRolePayload::OnlyV1(new_v1_role))
-            .await
-            .change_context(UserErrors::InternalServerError)?
-            .pop()
-            .ok_or(report!(UserErrors::InternalServerError))
-    }
-
     pub async fn insert_in_v2(self, state: &SessionState) -> UserResult<UserRole> {
         let entity = self.entity.clone();
 
-        let new_v2_role = self.convert_to_new_v2_role(EntityInfo {
+        let new_v2_role = self.convert_to_new_v2_role(entity.into());
-            org_id: entity.org_id.clone(),
+
-            merchant_id: None,
-            profile_id: None,
-            entity_id: entity.org_id.get_string_repr().to_owned(),
-            entity_type: EntityType::Organization,
-        });
         state
             .store
-            .insert_user_role(InsertUserRolePayload::OnlyV2(new_v2_role))
+            .insert_user_role(new_v2_role)
             .await
-            .change_context(UserErrors::InternalServerError)?
+            .change_context(UserErrors::InternalServerError)
-            .pop()
-            .ok_or(report!(UserErrors::InternalServerError))
-    }
-
-    pub async fn insert_in_v1_and_v2(self, state: &SessionState) -> UserResult<UserRole> {
-        let entity = self.entity.clone();
-
-        let new_v1_role = self
-            .clone()
-            .convert_to_new_v1_role(entity.org_id.clone(), entity.merchant_id.clone());
-
-        let new_v2_role = self.clone().convert_to_new_v2_role(EntityInfo {
-            org_id: entity.org_id.clone(),
-            merchant_id: None,
-            profile_id: None,
-            entity_id: entity.org_id.get_string_repr().to_owned(),
-            entity_type: EntityType::Organization,
-        });
-
-        Self::insert_v1_and_v2_in_db_and_get_v2(state, new_v1_role, new_v2_role).await
-    }
-}
-
-impl NewUserRole<MerchantLevel> {
-    pub async fn insert_in_v1_and_v2(self, state: &SessionState) -> UserResult<UserRole> {
-        let entity = self.entity.clone();
-
-        let new_v1_role = self
-            .clone()
-            .convert_to_new_v1_role(entity.org_id.clone(), entity.merchant_id.clone());
-
-        let new_v2_role = self.clone().convert_to_new_v2_role(EntityInfo {
-            org_id: entity.org_id.clone(),
-            merchant_id: Some(entity.merchant_id.clone()),
-            profile_id: None,
-            entity_id: entity.merchant_id.get_string_repr().to_owned(),
-            entity_type: EntityType::Merchant,
-        });
-
-        Self::insert_v1_and_v2_in_db_and_get_v2(state, new_v1_role, new_v2_role).await
-    }
-}
-
-impl NewUserRole<ProfileLevel> {
-    pub async fn insert_in_v2(self, state: &SessionState) -> UserResult<UserRole> {
-        let entity = self.entity.clone();
-
-        let new_v2_role = self.convert_to_new_v2_role(EntityInfo {
-            org_id: entity.org_id.clone(),
-            merchant_id: Some(entity.merchant_id.clone()),
-            profile_id: Some(entity.profile_id.clone()),
-            entity_id: entity.profile_id.get_string_repr().to_owned(),
-            entity_type: EntityType::Profile,
-        });
-        state
-            .store
-            .insert_user_role(InsertUserRolePayload::OnlyV2(new_v2_role))
-            .await
-            .change_context(UserErrors::InternalServerError)?
-            .pop()
-            .ok_or(report!(UserErrors::InternalServerError))
     }
 }