# Feature/196, 198, 199 Grouping users

* Fix review feedback

lib/models/page-group-relation.js

@@ -105,21 +105,27 @@ class PageGroupRelation {
   }
 
   /**
-   * get if the page has relation for group
+   * find the relation or create(if not exists) for page and group
    *
    * @static
    * @param {Page} page
    * @param {UserGroup} userGroup
-   * @returns {Promise<boolean>} is the relation for page and group exists(or not)
+   * @returns {Promise<PageGroupRelation>}
    * @memberof PageGroupRelation
    */
-  static isExistsRelationForPageAndGroup(page, userGroup) {
+  static findOrCreateRelationForPageAndGroup(page, userGroup) {
     const query = { targetPage: page.id, relatedGroup: userGroup.id };
 
     return this
       .count(query)
       .then((count) => {
-        return (0 < count);
+        // return (0 < count);
+        if (0 < count) {
+          return this.find(query).exec();
+        }
+        else {
+          return this.createRelation(userGroup, page);
+        }
       })
       .catch((err) => {
         debug('An Error occured.', err);
@@ -143,6 +149,41 @@ class PageGroupRelation {
       .exec();
   }
 
+  /**
+   * get is exists granted group for relatedPage and relatedUser
+   *
+   * @static
+   * @param {any} pageData relatedPage
+   * @param {any} userData relatedUser
+   * @returns is exists granted group(or not)
+   * @memberof PageGroupRelation
+   */
+  static isExistsGrantedGroupForPageAndUser(pageData, userData) {
+    var UserGroupRelation = crowi.model('UserGroupRelation');
+
+    return this.findByPage(pageData)
+      .then((pageRelations) => {
+        return pageRelations.map((pageRelation) => {
+          return UserGroupRelation.isRelatedUserForGroup(userData, pageRelation.relatedGroup)
+        });
+      })
+      .then((checkPromises) => {
+        return Promise.all(checkPromises)
+      })
+      .then((checkResults) => {
+        var checkResult = false;
+        checkResults.map((result) => {
+          if (result) {
+            checkResult = true;
+          }
+        });
+        return resolve(checkResult);
+      })
+      .catch((err) => {
+        return reject(err);
+      });
+  }
+
   /**
    * create page and group relation
    *

lib/models/page.js

@@ -454,6 +454,7 @@ module.exports = function(crowi) {
 
   pageSchema.statics.findPageByIdAndGrantedUser = function(id, userData) {
     var Page = this;
+    var PageGroupRelation = crowi.model('PageGroupRelation');
     var pageData = null;
 
     return new Promise(function(resolve, reject) {
@@ -461,7 +462,7 @@ module.exports = function(crowi) {
       .then(function(result) {
         pageData = result;
         if (userData && !pageData.isGrantedFor(userData)) {
-          return Page.isExistsGrantedGroupFor(pageData, userData);
+          return PageGroupRelation.isExistsGrantedGroupForPageAndUser(pageData, userData);
         }
         else {
           return resolve(true);
@@ -482,6 +483,7 @@ module.exports = function(crowi) {
   // find page and check if granted user
   pageSchema.statics.findPage = function(path, userData, revisionId, ignoreNotFound) {
     var self = this;
+    var PageGroupRelation = crowi.model('PageGroupRelation');
 
     return new Promise(function(resolve, reject) {
       self.findOne({path: path}, function(err, pageData) {
@@ -500,9 +502,8 @@ module.exports = function(crowi) {
         }
 
         if (!pageData.isGrantedFor(userData)) {
-          self.isExistsGrantedGroupFor(pageData, userData)
+          PageGroupRelation.isExistsGrantedGroupForPageAndUser(pageData, userData)
             .then(function (checkResult) {
-              debug('isExistsGrantedGroupFor checkResult is ', checkResult);
               if (!checkResult) {
                 return reject(new Error('Page is not granted for the user')); //PAGE_GRANT_ERROR, null);
               } else {
@@ -786,10 +787,9 @@ module.exports = function(crowi) {
     var PageGroupRelation = crowi.model('PageGroupRelation');
     var UserGroupRelation = crowi.model('UserGroupRelation');
     var provGrant = page.grant;
-    var grantUserGroup = null;
 
     if (grant == GRANT_USER_GROUP && grantUserGroupId == null) {
-      throw new Error();
+      throw new Error('grant userGroupId is not specified');
     }
     return new Promise(function(resolve, reject) {
       page.grant = grant;
@@ -806,39 +806,37 @@ module.exports = function(crowi) {
           return reject(err);
         }
 
-        // グループの場合
-        if (grant == GRANT_USER_GROUP) {
-          debug('grant is usergroup', grantUserGroupId);
-          UserGroupRelation.findByGroupIdAndUser(grantUserGroupId, userData)
-          .then((relation) => {
-            debug('userGroupRelation is found : ', relation)
-            if (relation != null) {
-              grantUserGroup = relation.relatedGroup;
-              return PageGroupRelation.isExistsRelationForPageAndGroup(page, grantUserGroup);
-            }
-            else {
-              return reject(new Error('No UserGroup is exists. userGroupId : ', grantUserGroupId));
-            }
-          })
-          .then((isAlreadyExists) => {
-            debug('pageGroupRelation is exists ', isAlreadyExists);
-            if (!isAlreadyExists) {
-              debug('create new Page and Group relations', grantUserGroup);
-              PageGroupRelation.createRelation(grantUserGroup, page)
-                .then((relationData) => {
-                  return resolve();
-                });
-            }
-          });
-        }
-        else {
-          PageGroupRelation.removeAllByPage(page);
-        }
-        return resolve(data);
+        Page.updateGrantUserGroup(page, grant, grantUserGroupId, userData)
+        .then(() => {
+          return resolve(data);
+        });
       });
     });
   };
 
+  pageSchema.statics.updateGrantUserGroup = function (page, grant, grantUserGroupId, userData) {
+
+    // グループの場合
+    if (grant == GRANT_USER_GROUP) {
+      debug('grant is usergroup', grantUserGroupId);
+      UserGroupRelation.findByGroupIdAndUser(grantUserGroupId, userData)
+      .then((relation) => {
+        if (relation == null) {
+          reject(new Error('no relations were exist for group and user.'));
+        }
+        return PageGroupRelation.findOrCreateRelationForPageAndGroup(page, relation.relatedGroup);
+
+      })
+      .catch((err) => {
+        return reject(new Error('No UserGroup is exists. userGroupId : ', grantUserGroupId));
+      });
+    }
+    else {
+      PageGroupRelation.removeAllByPage(page);
+    }
+
+  };
+
   // Instance method でいいのでは
   pageSchema.statics.pushToGrantedUsers = function(page, userData) {
 
@@ -927,10 +925,9 @@ module.exports = function(crowi) {
             }
 
             if (newPage.grant == Page.GRANT_USER_GROUP && grantUserGroupId != null) {
-              PageGroupRelation.createRelation(grantUserGroupId, newPage, function (err, relationData) {
-                if (err) {
-                  return reject(err);
-                }
+              Page.updateGrantUserGroup(newPage, grant, grantUserGroupId, user)
+              .catch((err) => {
+                return reject(err);
               });
             }
             var newRevision = Revision.prepareRevision(newPage, body, user, {format: format});
@@ -1231,35 +1228,6 @@ module.exports = function(crowi) {
     return;
   };
 
-  // 指定ページに紐づくユーザグループに、対象のユーザが含まれるかを確認
-  pageSchema.statics.isExistsGrantedGroupFor = function(pageData, userData) {
-    var PageGroupRelation = crowi.model('PageGroupRelation');
-    var UserGroupRelation = crowi.model('UserGroupRelation');
-
-    debug('isExistsGrantedGroupFor is called.');
-
-    return new Promise(function (resolve, reject) {
-      PageGroupRelation.findByPage(pageData)
-      .then(function (pageRelations) {
-        debug('PageGroupRelation.findByPage result is ', pageRelations);
-        if (pageRelations == null) {
-          debug('isExistsGrantedGroupFor is return resolve(false);');
-          return resolve(false);
-        }
-        return pageRelations.map(pageRelation => UserGroupRelation.isRelatedUserForGroup(userData, pageRelation.relatedGroup)
-        .then(function (checkResult) {
-          return resolve(checkResult);
-        })
-        .catch((err) => {
-          return reject(err);
-        }));
-      })
-      .catch((err) => {
-        return reject(err);
-      });
-    });
-  }
-
   /**
    * return path that added slash to the end for specified path
    */

lib/models/user-group-relation.js

@@ -171,8 +171,9 @@ class UserGroupRelation {
         const relatedUserIds = relations.map((relation) => {
           return relation.relatedUser.id;
         });
-        const query = { _id: { $nin: relatedUserIds } };
+        const query = { _id: { $nin: relatedUserIds }, status: User.STATUS_ACTIVE };
 
+        debug("findUserByNotRelatedGroup ", query);
         return User.find(query).exec();
       });
   }

lib/routes/admin.js

@@ -570,29 +570,26 @@ module.exports = function(crowi, app) {
         targetUserGroup = userGroup;
         if (targetUserGroup == null) {
           req.flash('errorMessage', 'グループがありません');
-          return new Promise();
+          throw new Error('no userGroup is exists. ', name);
         }
         else {
           renderVar.userGroup = targetUserGroup;
 
-          // get all user and group relations
-          return UserGroupRelation.findAllRelationForUserGroup(targetUserGroup);
+          return Promise.all([
+            // get all user and group relations
+            UserGroupRelation.findAllRelationForUserGroup(targetUserGroup),
+            // get all page and group relations
+            PageGroupRelation.findAllRelationForUserGroup(targetUserGroup),
+            // get all not related users for group
+            UserGroupRelation.findUserByNotRelatedGroup(targetUserGroup),
+          ]);
         }
       })
-      .then((relations) => {
-        renderVar.userGroupRelations = relations;
-
-        // get all page and group relations
-        return PageGroupRelation.findAllRelationForUserGroup(targetUserGroup)
-      })
-      .then(function (pageRelations) {
-        renderVar.pageGroupRelations = pageRelations;
-
-        // get all not related users for group
-        return UserGroupRelation.findUserByNotRelatedGroup(targetUserGroup);
-      })
-      .then((notRelatedusers) => {
-        renderVar.notRelatedusers = notRelatedusers;
+      .then((resolves) => {
+        renderVar.userGroupRelations = resolves[0];
+        renderVar.pageGroupRelations = resolves[1];
+        renderVar.notRelatedusers = resolves[2];
+        debug('notRelatedusers', renderVar.notRelatedusers);
 
         return res.render('admin/user-group-detail', renderVar);
       })