Merge pull request #5838 from weseek/support/93148-testcode-for-user-group

support: User Group Relation test

packages/app/src/server/routes/apiv3/user-group.js

@@ -1,6 +1,6 @@
-import loggerFactory from '~/utils/logger';
-import { excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 import UserGroup from '~/server/models/user-group';
+import { excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
+import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
 
@@ -12,12 +12,10 @@ const router = express.Router();
 
 const { body, param, query } = require('express-validator');
 const { sanitizeQuery } = require('express-validator');
-
 const mongoose = require('mongoose');
 
-const ErrorV3 = require('../../models/vo/error-apiv3');
-
 const { serializeUserSecurely } = require('../../models/serializers/user-serializer');
+const ErrorV3 = require('../../models/vo/error-apiv3');
 const { toPagingLimit, toPagingOffset } = require('../../util/express-validator/sanitizer');
 
 const { ObjectId } = mongoose.Types;
@@ -699,21 +697,13 @@ module.exports = (crowi) => {
    *                      description: the associative entity between user and userGroup
    */
   router.delete('/:id/users/:username', loginRequiredStrictly, adminRequired, validator.users.delete, apiV3FormValidator, async(req, res) => {
-    const { id, username } = req.params;
+    const { id: userGroupId, username } = req.params;
 
     try {
-      const [userGroup, user] = await Promise.all([
-        UserGroup.findById(id),
-        User.findUserByUsername(username),
-      ]);
-
-      const groupsOfRelationsToDelete = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
-      const relatedGroupIdsToDelete = groupsOfRelationsToDelete.map(g => g._id);
-
-      const deleteManyRes = await UserGroupRelation.deleteMany({ relatedUser: user._id, relatedGroup: { $in: relatedGroupIdsToDelete } });
-      const serializedUser = serializeUserSecurely(user);
+      const removedUserRes = await crowi.userGroupService.removeUserByUsername(userGroupId, username);
+      const serializedUser = serializeUserSecurely(removedUserRes.user);
 
-      return res.apiv3({ user: serializedUser, deletedGroupsCount: deleteManyRes.deletedCount });
+      return res.apiv3({ user: serializedUser, deletedGroupsCount: removedUserRes.deletedGroupsCount });
     }
     catch (err) {
       const msg = 'Error occurred while removing the user from groups.';

packages/app/src/server/service/user-group.ts

@@ -1,8 +1,11 @@
 import mongoose from 'mongoose';
 
-import loggerFactory from '~/utils/logger';
-import UserGroup, { UserGroupDocument } from '~/server/models/user-group';
+
+import { IUser } from '~/interfaces/user';
+import { ObjectIdLike } from '~/server/interfaces/mongoose-utils';
+import UserGroup from '~/server/models/user-group';
 import { excludeTestIdsFromTargetIds, isIncludesObjectId } from '~/server/util/compare-objectId';
+import loggerFactory from '~/utils/logger';
 
 const logger = loggerFactory('growi:service:UserGroupService'); // eslint-disable-line no-unused-vars
 
@@ -76,7 +79,7 @@ class UserGroupService {
 
     // throw if parent was in self and its descendants
     const descendantsWithTarget = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
-    if (isIncludesObjectId(descendantsWithTarget, parent._id)) {
+    if (isIncludesObjectId(descendantsWithTarget.map(d => d._id), parent._id)) {
       throw Error('It is not allowed to choose parent from descendant groups.');
     }
 
@@ -129,6 +132,22 @@ class UserGroupService {
     return deletedGroups;
   }
 
+  async removeUserByUsername(userGroupId: ObjectIdLike, username: string): Promise<{user: IUser, deletedGroupsCount: number}> {
+    const User = this.crowi.model('User');
+
+    const [userGroup, user] = await Promise.all([
+      UserGroup.findById(userGroupId),
+      User.findUserByUsername(username),
+    ]);
+
+    const groupsOfRelationsToDelete = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
+    const relatedGroupIdsToDelete = groupsOfRelationsToDelete.map(g => g._id);
+
+    const deleteManyRes = await UserGroupRelation.deleteMany({ relatedUser: user._id, relatedGroup: { $in: relatedGroupIdsToDelete } });
+
+    return { user, deletedGroupsCount: deleteManyRes.deletedCount };
+  }
+
 }
 
 module.exports = UserGroupService;

packages/app/test/integration/service/user-groups.test.ts

@@ -5,6 +5,7 @@ import { getInstance } from '../setup-crowi';
 
 describe('UserGroupService', () => {
   let crowi;
+  let User;
   let UserGroup;
   let UserGroupRelation;
 
@@ -16,14 +17,26 @@ describe('UserGroupService', () => {
   const groupId6 = new mongoose.Types.ObjectId();
   const groupId7 = new mongoose.Types.ObjectId();
   const groupId8 = new mongoose.Types.ObjectId();
+  const groupId9 = new mongoose.Types.ObjectId();
+  const groupId10 = new mongoose.Types.ObjectId();
+  const groupId11 = new mongoose.Types.ObjectId();
+  const groupId12 = new mongoose.Types.ObjectId();
 
   const userId1 = new mongoose.Types.ObjectId();
 
   beforeAll(async() => {
     crowi = await getInstance();
+    User = mongoose.model('User');
     UserGroup = mongoose.model('UserGroup');
     UserGroupRelation = mongoose.model('UserGroupRelation');
 
+    await User.insertMany([
+      // ug -> User Group
+      {
+        _id: userId1, name: 'ug_test_user1', username: 'ug_test_user1', email: 'ug_test_user1@example.com',
+      },
+    ]);
+
 
     // Create Groups
     await UserGroup.insertMany([
@@ -76,6 +89,28 @@ describe('UserGroupService', () => {
         name: 'v5_group8',
         description: 'description8',
       },
+      {
+        _id: groupId9,
+        name: 'v5_group9',
+        description: 'description9',
+      },
+      {
+        _id: groupId10,
+        name: 'v5_group10',
+        description: 'description10',
+        parent: groupId9,
+      },
+      {
+        _id: groupId11,
+        name: 'v5_group11',
+        description: 'descriptio11',
+      },
+      {
+        _id: groupId12,
+        name: 'v5_group12',
+        description: 'description12',
+        parent: groupId11,
+      },
     ]);
 
     // Create UserGroupRelations
@@ -92,6 +127,22 @@ describe('UserGroupService', () => {
         relatedGroup: groupId8,
         relatedUser: userId1,
       },
+      {
+        relatedGroup: groupId9,
+        relatedUser: userId1,
+      },
+      {
+        relatedGroup: groupId10,
+        relatedUser: userId1,
+      },
+      {
+        relatedGroup: groupId11,
+        relatedUser: userId1,
+      },
+      {
+        relatedGroup: groupId12,
+        relatedUser: userId1,
+      },
     ]);
 
   });
@@ -192,4 +243,41 @@ describe('UserGroupService', () => {
     expect(userGroupRelation8AfterUpdate).not.toBeNull();
   });
 
+  test('Should throw an error when trying to choose parent from descendant groups.', async() => {
+    const userGroup9 = await UserGroup.findOne({ _id: groupId9, parent: null });
+    const userGroup10 = await UserGroup.findOne({ _id: groupId10, parent: groupId9 });
+
+    const userGroupRelation9BeforeUpdate = await UserGroupRelation.findOne({ relatedGroup:  userGroup9._id, relatedUser: userId1 });
+    const userGroupRelation10BeforeUpdate = await UserGroupRelation.findOne({ relatedGroup:  userGroup10._id, relatedUser: userId1 });
+    expect(userGroupRelation9BeforeUpdate).not.toBeNull();
+    expect(userGroupRelation10BeforeUpdate).not.toBeNull();
+
+    const result = crowi.userGroupService.updateGroup(
+      userGroup9._id, userGroup9.name, userGroup9.description, userGroup10._id,
+    );
+    await expect(result).rejects.toThrow('It is not allowed to choose parent from descendant groups.');
+  });
+
+  test('User should be deleted from child groups when the user excluded from the parent group', async() => {
+    const userGroup11 = await UserGroup.findOne({ _id: groupId11, parent: null });
+    const userGroup12 = await UserGroup.findOne({ _id: groupId12, parent: groupId11 });
+
+    // Both groups have user1
+    const userGroupRelation11BeforeRemove = await UserGroupRelation.findOne({ relatedGroup:  userGroup11._id, relatedUser: userId1 });
+    const userGroupRelation12BeforeRemove = await UserGroupRelation.findOne({ relatedGroup:  userGroup12._id, relatedUser: userId1 });
+    expect(userGroupRelation11BeforeRemove).not.toBeNull();
+    expect(userGroupRelation12BeforeRemove).not.toBeNull();
+
+    // remove user1 from the parent group
+    await crowi.userGroupService.removeUserByUsername(
+      userGroup11._id, 'ug_test_user1',
+    );
+
+    // Both groups have not user1
+    const userGroupRelation11AfterRemove = await UserGroupRelation.findOne({ relatedGroup:  userGroup11._id, relatedUser: userId1 });
+    const userGroupRelation12AfterRemove = await UserGroupRelation.findOne({ relatedGroup:  userGroup12._id, relatedUser: userId1 });
+    await expect(userGroupRelation11AfterRemove).toBeNull();
+    await expect(userGroupRelation12AfterRemove).toBeNull();
+  });
+
 });