Fixing tests and update user bug fixes

Author: david-rocca

src/controller/org.controller/org.controller.js

@@ -552,6 +552,19 @@ async function updateUser (req, res, next) {
       return res.status(404).json(error.orgDnePathParam(shortNameParams))
     }
 
+    if (shortNameParams !== requesterShortName && !isRequesterSecretariat) {
+      logger.info({ uuid: req.ctx.uuid, message: `${shortNameParams} organization data can only be modified by users of the same organization or the Secretariat.` })
+      await session.abortTransaction()
+      return res.status(403).json(error.notSameOrgOrSecretariat())
+    }
+
+    // Specific check for org_short_name (Secretariat only)
+    if (queryParametersJson.org_short_name && !isRequesterSecretariat) {
+      logger.info({ uuid: req.ctx.uuid, message: 'Only Secretariat can reassign user organization.' })
+      await session.abortTransaction()
+      return res.status(403).json(error.notAllowedToChangeOrganization())
+    }
+
     if (!isRequesterSecretariat && !isAdmin) {
       if (targetUserUUID !== requesterUUID) {
         if (!targetUserUUID) {
@@ -565,29 +578,41 @@ async function updateUser (req, res, next) {
       }
     }
 
-    if (!targetUserUUID) {
-      logger.info({ uuid: req.ctx.uuid, message: 'User DNE' })
-      await session.abortTransaction()
-      return res.status(404).json(error.userDne(usernameParams))
+    const newOrgShortNameToMoveTo = queryParametersJson.org_short_name
+
+    if (newOrgShortNameToMoveTo) {
+      if (newOrgShortNameToMoveTo === shortNameParams) {
+        logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} is already in organization ${newOrgShortNameToMoveTo}.` })
+        await session.abortTransaction()
+        return res.status(403).json(error.alreadyInOrg(newOrgShortNameToMoveTo, usernameParams))
+      }
+
+      const newTargetRegistryOrgUUID = await orgRepo.getOrgUUID(newOrgShortNameToMoveTo, { session })
+
+      if (!newTargetRegistryOrgUUID) {
+        logger.info({ uuid: req.ctx.uuid, message: `New target organization ${newOrgShortNameToMoveTo} does not exist.` })
+        await session.abortTransaction()
+        return res.status(404).json(error.orgDne(newOrgShortNameToMoveTo, 'org_short_name', 'query'))
+      }
     }
 
-    if (shortNameParams !== requesterShortName && !isRequesterSecretariat) {
-      logger.info({ uuid: req.ctx.uuid, message: `${shortNameParams} organization data can only be modified by users of the same organization or the Secretariat.` })
-      await session.abortTransaction()
-      return res.status(403).json(error.notSameOrgOrSecretariat())
+    if (queryParametersJson.active) {
+      if (requesterUUID === targetUserUUID) {
+        await session.abortTransaction()
+        return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
+      }
     }
 
-    if (await userRepo.orgHasUser(shortNameParams, targetUserUUID, { session })) {
-      logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} does not exist for ${shortNameParams} organization.` })
+    if (!targetUserUUID) {
+      logger.info({ uuid: req.ctx.uuid, message: 'User DNE' })
       await session.abortTransaction()
       return res.status(404).json(error.userDne(usernameParams))
     }
 
-    // Specific check for org_short_name (Secretariat only)
-    if (queryParametersJson.org_short_name && !isRequesterSecretariat) {
-      logger.info({ uuid: req.ctx.uuid, message: 'Only Secretariat can reassign user organization.' })
+    if (!await userRepo.orgHasUserByUUID(shortNameParams, targetUserUUID, { session })) {
+      logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} does not exist for ${shortNameParams} organization.` })
       await session.abortTransaction()
-      return res.status(403).json(error.notAllowedToChangeOrganization())
+      return res.status(404).json(error.userDne(usernameParams))
     }
 
     // General permission check for fields requiring admin/secretariat
@@ -609,13 +634,6 @@ async function updateUser (req, res, next) {
       }
     }
 
-    if (queryParametersJson.active) {
-      if (requesterUUID === targetUserUUID) {
-        await session.abortTransaction()
-        return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
-      }
-    }
-
     // This is a special case, and needs to be handled in the controller, and not in the repository
     const rolesFromQuery = queryParametersJson['active_roles.remove'] ?? []
     const removeRolesCollector = []
@@ -633,24 +651,6 @@ async function updateUser (req, res, next) {
       }
     }
 
-    const newOrgShortNameToMoveTo = queryParametersJson.org_short_name
-
-    if (newOrgShortNameToMoveTo) {
-      if (newOrgShortNameToMoveTo === shortNameParams) {
-        logger.info({ uuid: req.ctx.uuid, message: `User ${usernameParams} is already in organization ${newOrgShortNameToMoveTo}.` })
-        await session.abortTransaction()
-        return res.status(403).json(error.alreadyInOrg(newOrgShortNameToMoveTo, usernameParams))
-      }
-
-      const newTargetRegistryOrgUUID = await orgRepo.getOrgUUID(newOrgShortNameToMoveTo, { session })
-
-      if (!newTargetRegistryOrgUUID) {
-        logger.info({ uuid: req.ctx.uuid, message: `New target organization ${newOrgShortNameToMoveTo} does not exist.` })
-        await session.abortTransaction()
-        return res.status(404).json(error.orgDne(newOrgShortNameToMoveTo, 'org_short_name', 'query'))
-      }
-    }
-
     const payload = await userRepo.updateUser(usernameParams, shortNameParams, queryParametersJson, { session })
     await session.commitTransaction()
     return res.status(200).json({ message: `${usernameParams} was successfully updated.`, updated: payload })

src/repositories/baseUserRepository.js

@@ -43,6 +43,16 @@ class BaseUserRepository extends BaseRepository {
   }
 
   // Check if an org has a user by username
+  async orgHasUserByUUID (orgShortName, uuid, options = {}, isLegacyObject = false) {
+    const org = await BaseOrgModel.findOne({ short_name: orgShortName }, null, options)
+    if (!org || !Array.isArray(org.users)) {
+      return false
+    }
+
+    // 4. Check if any UUID is present in org.users
+    return org.users.includes(uuid)
+  }
+
   async orgHasUser (orgShortName, username, options = {}, isLegacyObject = false) {
     // 1. Find all users with this username
     const users = await BaseUser.find({ username }, null, options)