add deletedAt to User model, updated db calls to work with this field (…

…#2250)

packages/server/src/models/user/user.test.ts

@@ -16,9 +16,10 @@ const prepareUserData = (): IUser => {
     options: {
       defaultLanguage: EntityEnums.Language.English,
       defaultTerritory: "",
-      hideStatementElementsOrderTable: false,
+      defaultStatementLanguage: EntityEnums.Language.English,
       searchLanguages: [],
     },
+    verified: true,
     rights: [],
     role: UserEnums.Role.Viewer,
     storedTerritories: [],
@@ -214,4 +215,32 @@ describe("models/user", function () {
       });
     });
   });
+
+  describe("User.delete", function () {
+    let db: Db;
+    const user1 = prepareUser();
+
+    beforeAll(async () => {
+      db = new Db();
+      await db.initDb();
+      await user1.save(db.connection);
+    });
+
+    afterAll(async () => {
+      await clean(db);
+    });
+
+    it("should set deletedAt to current date", async () => {
+      const result = await user1.delete(db.connection)
+      expect(result.replaced).toBe(1);
+
+      // deleted
+      const user1After = await User.findUserById(db.connection, user1.id);
+      expect(user1After).toBeFalsy();
+
+      const thrashedUser1 = await User.findUserByLogin(db.connection, user1.email, false)
+      expect(thrashedUser1).not.toBeNull();
+      expect(thrashedUser1!.deletedAt).toBeTruthy();
+    });
+  });
 });

packages/server/src/models/user/user.ts

@@ -114,6 +114,8 @@ export default class User implements IUser, IDbModel {
 
   hash?: string = "";
 
+  deletedAt?: Date;
+
   static table = "users";
 
   constructor(data: Partial<IUser>) {
@@ -162,8 +164,15 @@ export default class User implements IUser, IDbModel {
       .run(dbInstance);
   }
 
+  /**
+   * Soft delete operation - set deletedAt to current date, value indicated deleted user
+   * @param dbInstance 
+   * @returns 
+   */
   delete(dbInstance: Connection): Promise<WriteResult> {
-    return rethink.table(User.table).get(this.id).delete().run(dbInstance);
+    return rethink.table(User.table).get(this.id).update({
+      deletedAt: new Date()
+    }).run(dbInstance);
   }
 
   isValid(): boolean {
@@ -211,24 +220,42 @@ export default class User implements IUser, IDbModel {
     return this.hash;
   }
 
+  /**
+   * Finds user by 'id' field
+   * Ignores thrashed entries
+   * @param dbInstance 
+   * @param id 
+   * @returns 
+   */
   static async findUserById(
     dbInstance: Connection | undefined,
     id: string
   ): Promise<User | null> {
     const data = await rethink.table(User.table).get(id).run(dbInstance);
-    if (data) {
-      delete data.password;
-      return new User(data);
+    if (!data || (data as IUser).deletedAt) {
+      return null;
     }
-    return null;
+
+    delete data.password;
+    return new User(data);
   }
 
+  /**
+   * Finds user identified by 'email' field
+   * Ignores thrashed entries
+   * @param dbInstance 
+   * @param email 
+   * @returns 
+   */
   static async getUserByEmail(
     dbInstance: Connection | undefined,
     email: string
   ): Promise<User | null> {
     const data = await rethink
       .table(User.table)
+      .filter(function(user: any) {
+        return rethink.not(user.hasFields("deletedAt"))
+      })
       .filter({ email })
       .limit(1)
       .run(dbInstance);
@@ -238,12 +265,22 @@ export default class User implements IUser, IDbModel {
     return null;
   }
 
+  /**
+   * Finds user identified by 'hash' field
+   * Ignores thrashed entries
+   * @param dbInstance 
+   * @param hash 
+   * @returns 
+   */
   static async getUserByHash(
     dbInstance: Connection | undefined,
     hash: string
   ): Promise<User | null> {
     const data = await rethink
       .table(User.table)
+      .filter(function(user: any) {
+        return rethink.not(user.hasFields("deletedAt"))
+      })
       .filter({ hash })
       .run(dbInstance);
     if (data && data.length) {
@@ -252,65 +289,80 @@ export default class User implements IUser, IDbModel {
     return null;
   }
 
+  /**
+   * Returns all users
+   * Ignores thrashed entries
+   * @param dbInstance 
+   * @returns 
+   */
   static async findAllUsers(
     dbInstance: Connection | undefined
   ): Promise<User[]> {
     const data = await rethink
       .table(User.table)
+      .filter(function(user: any) {
+        return rethink.not(user.hasFields("deletedAt"))
+      })
       .orderBy(rethink.asc("role"), rethink.asc("name"))
       .run(dbInstance);
     return data.map((d) => new User(d));
   }
 
   /**
-   * Method searches for user by email or username (login)
+   * Method searches for user by email or username (login).
+   * Optionally includes also thrashed entries (for keeping uniqueness across emails/logins)
    * @param dbInstance
    * @param label
+   * @param includeThrashed
    * @returns
    */
   static async findUserByLogin(
     dbInstance: Db,
-    login: string
+    login: string,
+    includeThrashed: boolean
   ): Promise<User | null> {
-    const data = await rethink
-      .table(User.table)
-      .filter(function (user: any) {
+    let req = await rethink
+      .table(User.table);
+
+    if (!includeThrashed) {
+      req = req.filter(function(user: any) {
+        return rethink.not(user.hasFields("deletedAt"))
+      });
+    }
+
+    const data = await req.filter(function (user: any) {
         return rethink.or(
           rethink.row("name").eq(login),
           rethink.row("email").eq(login)
         );
       })
       .limit(1)
       .run(dbInstance.connection);
+
     return data.length == 0 ? null : new User(data[0]);
   }
 
+  /**
+   * Returns users by cleaned label (for name / email).
+   * Does not return thrashed users.
+   * @param dbInstance 
+   * @param label 
+   * @returns 
+   */
   static async findUsersByLabel(
     dbInstance: Connection | undefined,
     label: string
   ): Promise<User[]> {
     const data = await rethink
       .table(User.table)
-      .filter(function (user: any) {
-        return rethink.or(
-          rethink
-            .row("name")
-            .downcase()
-            .match(`${regExpEscape(label.toLowerCase())}`)
-            .or(),
-          rethink
-            .row("email")
-            .downcase()
-            .match(`${regExpEscape(label.toLowerCase())}`)
-            .or()
-        );
-      })
+
       .run(dbInstance);
-    return data.map((d) => new User(d));
+    return (data as IUser[]).map((d) => new User(d));
   }
 
   /**
    * Searches for users associated with bookmarked entity
+   * Returns also thrashed users
    * @param db
    * @param entityId
    * @returns array of IUser interfaces
@@ -333,6 +385,7 @@ export default class User implements IUser, IDbModel {
 
   /**
    * Searches for users associated with stored territory
+   * Returns also thrashed users
    * @param db
    * @param territoryId
    * @returns array of IUser interfaces
@@ -356,6 +409,7 @@ export default class User implements IUser, IDbModel {
 
   /**
    * Removed bookmarks with entityId from all users
+   * Uses findByBookmarkedEntity which returns also thrashed users
    * @param db
    * @param entityId
    */
@@ -373,6 +427,7 @@ export default class User implements IUser, IDbModel {
 
   /**
    * Removed stored territory with territoryId from all users
+   * Uses findByStoredTerritory which returns also thrashed users
    * @param db
    * @param territoryId
    */

packages/server/src/modules/users/index.ts

@@ -95,7 +95,7 @@ export default Router()
           throw new UserAlreadyActivated();
         }
 
-        if (await User.findUserByLogin(request.db, username)) {
+        if (await User.findUserByLogin(request.db, username, true)) {
           throw new UserNotUnique(
             "Username is already used. Please select a new one."
           );
@@ -339,7 +339,8 @@ export default Router()
         throw new BadParams("login and password have to be set");
       }
 
-      const user = await User.findUserByLogin(request.db, login);
+      const user = await User.findUserByLogin(request.db, login, false);
+      console.log(user)
       if (!user) {
         throw new BadCredentialsError("wrong email / username");
       }
@@ -514,11 +515,11 @@ export default Router()
 
         await request.db.lock();
 
-        if (await User.findUserByLogin(request.db, userData.email)) {
+        if (await User.findUserByLogin(request.db, userData.email, true)) {
           throw new UserNotUnique("email is in use");
         }
         if (userData.name) {
-          if (await User.findUserByLogin(request.db, userData.name)) {
+          if (await User.findUserByLogin(request.db, userData.name, true)) {
             throw new UserNotUnique("username is aready used");
           }
         }
@@ -610,14 +611,14 @@ export default Router()
         await req.db.lock();
 
         if (data.email) {
-          const existingEmail = await User.findUserByLogin(req.db, data.email);
+          const existingEmail = await User.findUserByLogin(req.db, data.email, true);
           if (existingEmail && existingEmail.id !== existingUser.id) {
             throw new UserNotUnique("email is in use");
           }
         }
 
         if (data.name) {
-          const existingName = await User.findUserByLogin(req.db, data.name);
+          const existingName = await User.findUserByLogin(req.db, data.name, true);
           if (existingName && existingName.id !== existingUser.id) {
             throw new UserNotUnique("username is already used");
           }
@@ -708,8 +709,8 @@ export default Router()
         }
 
         const result = await existingUser.delete(request.db.connection);
-        if (!result.deleted) {
-          throw new InternalServerError(`user ${userId} could not be removed`);
+        if (!result.replaced) {
+          throw new InternalServerError(`user ${userId} could not be deleted`);
         }
 
         return {

packages/server/src/modules/users/users.delete.test.ts

@@ -38,20 +38,28 @@ describe("Users delete", function () {
     });
   });
   describe("ok data", () => {
-    it("should return a 200 code with successful response", async () => {
-      const db = new Db();
+    const testUserId = Math.random().toString();
+    const db = new Db();
+
+    beforeAll(async () => {
       await db.initDb();
-      const testUserId = Math.random().toString();
 
       const user = new User({ id: testUserId });
       await user.save(db.connection);
+    })
 
+    it("should return a 200 code with successful response", async () => {
       await request(app)
         .delete(`${apiPath}/users/${testUserId}`)
         .set("authorization", "Bearer " + supertestConfig.token)
         .expect("Content-Type", /json/)
         .expect(successfulGenericResponse)
         .expect(200);
     });
+
+    it("should not return this user after delete operation", async () => {
+      const user = await User.findUserById(db.connection, testUserId)
+      expect(user).toBeNull();
+    });
   });
 });

packages/shared/types/user.ts

@@ -13,6 +13,8 @@ export interface IUser {
   rights: IUserRight[];
   active: boolean; // enabled/disabled - set to true in activation, but can be toggled in admin
   verified: boolean; // email verified - set to true in activation
+
+  deletedAt?: Date
 }
 
 export interface IUserRight {