Merge pull request #5735 from weseek/fix/sending-mail-to-deleted-users

fix: New user notification email is also sent TO: deleted_at_<epoch_time>@deleted

packages/app/src/server/models/user.js

@@ -1,14 +1,15 @@
 /* eslint-disable no-use-before-define */
 import loggerFactory from '~/utils/logger';
 
+const crypto = require('crypto');
+
 const debug = require('debug')('growi:models:user');
+const md5 = require('md5');
 const mongoose = require('mongoose');
 const mongoosePaginate = require('mongoose-paginate-v2');
 const uniqueValidator = require('mongoose-unique-validator');
-const md5 = require('md5');
 
 const ObjectId = mongoose.Schema.Types.ObjectId;
-const crypto = require('crypto');
 
 const { listLocaleIds, migrateDeprecatedLocaleId } = require('~/utils/locale-utils');
 
@@ -393,8 +394,16 @@ module.exports = function(crowi) {
       .sort(sort);
   };
 
-  userSchema.statics.findAdmins = async function() {
-    return this.find({ admin: true });
+  userSchema.statics.findAdmins = async function(option) {
+    const sort = option?.sort ?? { createdAt: -1 };
+
+    let status = option?.status ?? [STATUS_ACTIVE];
+    if (!Array.isArray(status)) {
+      status = [status];
+    }
+
+    return this.find({ admin: true, status: { $in: status } })
+      .sort(sort);
   };
 
   userSchema.statics.findUserByUsername = function(username) {

packages/app/test/integration/models/user.test.js

@@ -8,17 +8,53 @@ describe('User', () => {
   let crowi;
   let User;
 
+  let adminusertestToBeRemovedId;
+
   beforeAll(async() => {
     crowi = await getInstance();
     User = mongoose.model('User');
 
-    await User.create({
-      name: 'Example for User Test',
-      username: 'usertest',
-      email: 'usertest@example.com',
-      password: 'usertestpass',
-      lang: 'en_US',
-    });
+    await User.insertMany([
+      {
+        name: 'Example for User Test',
+        username: 'usertest',
+        email: 'usertest@example.com',
+        password: 'usertestpass',
+        lang: 'en_US',
+      },
+      {
+        name: 'Admin Example Active',
+        username: 'adminusertest1',
+        email: 'adminusertest1@example.com',
+        password: 'adminusertestpass',
+        admin: true,
+        status: User.STATUS_ACTIVE,
+        lang: 'en_US',
+      },
+      {
+        name: 'Admin Example Suspended',
+        username: 'adminusertest2',
+        email: 'adminusertes2@example.com',
+        password: 'adminusertestpass',
+        admin: true,
+        status: User.STATUS_SUSPENDED,
+        lang: 'en_US',
+      },
+      {
+        name: 'Admin Example to delete',
+        username: 'adminusertestToBeRemoved',
+        email: 'adminusertestToBeRemoved@example.com',
+        password: 'adminusertestpass',
+        admin: true,
+        status: User.STATUS_ACTIVE,
+        lang: 'en_US',
+      },
+    ]);
+
+    // delete adminusertestToBeRemoved
+    const adminusertestToBeRemoved = await User.findOne({ username: 'adminusertestToBeRemoved' });
+    adminusertestToBeRemovedId = adminusertestToBeRemoved._id;
+    await adminusertestToBeRemoved.statusDelete();
   });
 
   describe('Create and Find.', () => {
@@ -39,6 +75,47 @@ describe('User', () => {
       });
 
     });
+
+  });
+
+  describe('Delete.', () => {
+
+    describe('Deleted users', () => {
+      test('should have correct attributes', async() => {
+        const adminusertestToBeRemoved = await User.findOne({ _id: adminusertestToBeRemovedId });
+
+        expect(adminusertestToBeRemoved).toBeInstanceOf(User);
+        expect(adminusertestToBeRemoved.name).toBe('');
+        expect(adminusertestToBeRemoved.password).toBe('');
+        expect(adminusertestToBeRemoved.googleId).toBeNull();
+        expect(adminusertestToBeRemoved.isGravatarEnabled).toBeFalsy();
+        expect(adminusertestToBeRemoved.image).toBeNull();
+      });
+    });
+  });
+
+  describe('User.findAdmins', () => {
+    test('should retrieves only active users', async() => {
+      const users = await User.findAdmins();
+      const adminusertestActive = users.find(user => user.username === 'adminusertest1');
+      const adminusertestSuspended = users.find(user => user.username === 'adminusertest2');
+      const adminusertestToBeRemoved = users.find(user => user._id.toString() === adminusertestToBeRemovedId.toString());
+
+      expect(adminusertestActive).toBeInstanceOf(User);
+      expect(adminusertestSuspended).toBeUndefined();
+      expect(adminusertestToBeRemoved).toBeUndefined();
+    });
+
+    test('with \'includesInactive\' option should retrieves suspended users', async() => {
+      const users = await User.findAdmins({ status: [User.STATUS_ACTIVE, User.STATUS_SUSPENDED] });
+      const adminusertestActive = users.find(user => user.username === 'adminusertest1');
+      const adminusertestSuspended = users.find(user => user.username === 'adminusertest2');
+      const adminusertestToBeRemoved = users.find(user => user._id.toString() === adminusertestToBeRemovedId.toString());
+
+      expect(adminusertestActive).toBeInstanceOf(User);
+      expect(adminusertestSuspended).toBeInstanceOf(User);
+      expect(adminusertestToBeRemoved).toBeUndefined();
+    });
   });
 
   describe('User Utilities', () => {