imprv: User group update parent validation (#5084)

* CC to FC

* Improved userGroupTable

* SWRized

* Updated table & home page component

* Removed hard code

* Implemented listing api

* Fixed lint error

* Implemented key serializer for swr

* Impl update

* Omit serialize middleware

* Upgraded swr ^1.0.1 => ^1.1.2

* Improved sync

* Fixed lint errors

* Removed unnecessary code

* Improved nukey

* Improved types

* Improved interface

* Improved type validation

* Implemented delete

* Normalized view

* Improved

* Implemented delete

* Implemented create

* Renamed type

* Removed comments

* Added validation

* Removed unnecessary code

* Removed unnecessary method

* Added a comment

* Typescriptized

* Omitted crowi

* Fixed

* Fixed ci

* Fixed

* Fixed model import

* Deleted user-group.js

* Added setting null feature

* Resolved conflict

* Renamed variables

* Reduced lines

* Renamed utility and added jsdoc

* Renamed

* Renamed

packages/app/src/interfaces/user.ts

@@ -18,7 +18,7 @@ export type IUserGroup = {
   name: string;
   createdAt: Date;
   description: string;
-  parent: Ref<IUserGroup>;
+  parent: Ref<IUserGroup> | null;
 }
 
 export type IUserHasId = IUser & HasObjectId;

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

@@ -1,5 +1,5 @@
 import loggerFactory from '~/utils/logger';
-import { filterIdsByIds } from '~/server/util/compare-objectId';
+import { excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
 
 const logger = loggerFactory('growi:routes:apiv3:user-group'); // eslint-disable-line no-unused-vars
 
@@ -443,7 +443,7 @@ module.exports = (crowi) => {
       const existingRelations = await UserGroupRelation.find({ relatedGroup: { $in: userGroupIds }, relatedUser: user._id });
       const existingGroupIds = existingRelations.map(r => r.relatedGroup);
 
-      const groupIdsOfRelationToCreate = filterIdsByIds(userGroupIds, existingGroupIds);
+      const groupIdsOfRelationToCreate = excludeTestIdsFromTargetIds(userGroupIds, existingGroupIds);
 
       const insertedRelations = await UserGroupRelation.createRelations(groupIdsOfRelationToCreate, user);
       const serializedUser = serializeUserSecurely(user);

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

@@ -2,6 +2,7 @@ import mongoose from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
 import UserGroup from '~/server/models/user-group';
+import { compareObjectId, isIncludesObjectId } from '~/server/util/compare-objectId';
 
 const logger = loggerFactory('growi:service:UserGroupService'); // eslint-disable-line no-unused-vars
 
@@ -26,7 +27,7 @@ class UserGroupService {
 
   // TODO 85062: write test code
   // ref: https://dev.growi.org/61b2cdabaa330ce7d8152844
-  async updateGroup(id, name, description, parentId, forceUpdateParents = false) {
+  async updateGroup(id, name: string, description: string, parentId?: string, forceUpdateParents = false) {
     const userGroup = await UserGroup.findById(id);
     if (userGroup == null) {
       throw new Error('The group does not exist');
@@ -45,9 +46,26 @@ class UserGroupService {
     if (userGroup.parent === parentId) {
       return userGroup.save();
     }
+    // set parent to null and return when parentId is null
+    if (parentId == null) {
+      userGroup.parent = null;
+      return userGroup.save();
+    }
 
     const parent = await UserGroup.findById(parentId);
 
+    if (parent == null) { // it should not be null
+      throw Error('parent does not exist.');
+    }
+
+
+    // throw if parent was in its descendants
+    const descendantsWithTarget = await UserGroup.findGroupsWithDescendantsRecursively([userGroup]);
+    const descendants = descendantsWithTarget.filter(d => compareObjectId(d._id, userGroup._id));
+    if (isIncludesObjectId(descendants, parent._id)) {
+      throw Error('It is not allowed to choose parent from descendant groups.');
+    }
+
     // find users for comparison
     const [targetGroupUsers, parentGroupUsers] = await Promise.all(
       [UserGroupRelation.findUserIdsByGroupId(userGroup._id), UserGroupRelation.findUserIdsByGroupId(parent?._id)], // TODO 85062: consider when parent is null to update the group as the root

packages/app/src/server/util/compare-objectId.ts

@@ -3,14 +3,31 @@ import mongoose from 'mongoose';
 type IObjectId = mongoose.Types.ObjectId;
 const ObjectId = mongoose.Types.ObjectId;
 
-export const filterArr1ByArr2 = (arr1: IObjectId[], arr2: IObjectId[]): IObjectId[] => {
+export const compareObjectId = (id1: IObjectId, id2: IObjectId): boolean => {
+  return id1.toString() === id2.toString();
+};
+
+export const isIncludesObjectId = (arr: IObjectId[], id: IObjectId): boolean => {
+  const _arr = arr.map(i => i.toString());
+  const _id = id.toString();
+
+  return _arr.includes(_id);
+};
+
+/**
+ * Exclude ObjectIds which exist in testIds from targetIds
+ * @param targetIds Array of mongoose.Types.ObjectId
+ * @param testIds Array of mongoose.Types.ObjectId
+ * @returns Array of mongoose.Types.ObjectId
+ */
+export const excludeTestIdsFromTargetIds = (targetIds: IObjectId[], testIds: IObjectId[]): IObjectId[] => {
   // cast to string
-  const arrToBeFiltered = arr1.map(e => e.toString());
-  const arrToFilter = arr2.map(e => e.toString());
+  const arr1 = targetIds.map(e => e.toString());
+  const arr2 = testIds.map(e => e.toString());
 
   // filter
-  const filtered = arrToBeFiltered.filter(e => !arrToFilter.includes(e));
+  const excluded = arr2.filter(e => !arr1.includes(e));
 
   // cast to ObjectId
-  return filtered.map(e => new ObjectId(e));
+  return excluded.map(e => new ObjectId(e));
 };