fix: fixes after @berhalak and @fflorent reviews

app/gen-server/lib/homedb/UsersManager.ts

@@ -343,12 +343,7 @@ export class UsersManager {
     email: string,
     manager?: EntityManager
   ): Promise<User|undefined> {
-    const normalizedEmail = normalizeEmail(email);
-    return await (manager || this._connection).createQueryBuilder()
-      .select('user')
-      .from(User, 'user')
-      .leftJoinAndSelect('user.logins', 'logins')
-      .where('email = :email', {email: normalizedEmail})
+    return await this._buildExistingUsersByLoginRequest([email], manager)
       .getOne() || undefined;
   }
 
@@ -359,12 +354,7 @@ export class UsersManager {
     emails: string[],
     manager?: EntityManager
   ): Promise<User[]> {
-    const normalizedEmails = emails.map(email=> normalizeEmail(email));
-    return await (manager || this._connection).createQueryBuilder()
-      .select('user')
-      .from(User, 'user')
-      .leftJoinAndSelect('user.logins', 'logins')
-      .where('email IN (:...emails)', {emails: normalizedEmails})
+    return await this._buildExistingUsersByLoginRequest(emails, manager)
       .getMany();
   }
 
@@ -494,6 +484,13 @@ export class UsersManager {
     return userByLogin;
   }
 
+  /*
+   * This function is an alias of getUserByLogin
+   * Its purpose is to be more expressive and avoid confusion when reading code.
+   * FIXME :In the future it may be used to split getUserByLogin in two distinct functions
+   * One for creation
+   * the other for retrieving users in order to make it more maintainable
+   */
   public async createUser(email: string, options: GetUserOptions = {}): Promise<User|undefined> {
     return await this.getUserByLogin(email, options);
   }
@@ -583,15 +580,15 @@ export class UsersManager {
       const emailMap = delta.users;
       const emails = Object.keys(emailMap);
       const existingUsers = await this.getExistingUsersByLogin(emails, transaction);
-      const emailsExistingUsers = existingUsers.map(user=>user.loginEmail);
-      const emailsUsersToCreate = emails.filter(email => ! emailsExistingUsers.includes(email));
-      for (const email of emailsUsersToCreate){
+      const emailsExistingUsers = existingUsers.map(user => user.loginEmail);
+      const emailsUsersToCreate = emails.filter(email => !emailsExistingUsers.includes(email));
+      const emailUsers = [...existingUsers];
+      for (const email of emailsUsersToCreate) {
         const user = await this.createUser(email, {manager: transaction});
         if (user !== undefined) {
-          existingUsers.push(user);
+          emailUsers.push(user);
         }
       }
-      const emailUsers = [...existingUsers];
       emails.forEach((email, i) => {
         const userIdAffected = emailUsers[i]!.id;
         // Org-level sharing with everyone would allow serious spamming - forbid it.
@@ -790,4 +787,16 @@ export class UsersManager {
     }
     delta.users = users;
   }
+
+  private _buildExistingUsersByLoginRequest(
+    emails: string[],
+    manager?: EntityManager
+  ) {
+    const normalizedEmails = emails.map(email=> normalizeEmail(email));
+    return (manager || this._connection).createQueryBuilder()
+      .select('user')
+      .from(User, 'user')
+      .leftJoinAndSelect('user.logins', 'logins')
+      .where('email IN (:...emails)', {emails: normalizedEmails});
+  }
 }