Refactor: code quality improvements

- Extract admin.service.js from admin.controller.js (proper layering)
- Remove console.log statements from todo.controller.js
- Fix inconsistent error handling in auth.controller.js (return next)
- Remove logger.debug calls from contact.controller.js
- Add transaction management to contact.service.js addContact()

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/controllers/admin.controller.js

@@ -1,10 +1,5 @@
-import { db } from '../config/database.js';
-import { users, userEmailAccounts, emailAccounts } from '../db/schema.js';
-import { eq, inArray } from 'drizzle-orm';
-import { hashPassword, generateTempPassword } from '../utils/password.js';
+import * as adminService from '../services/admin.service.js';
 import { logUserCreation, logRoleChange } from '../services/audit.service.js';
-import { ConflictError, NotFoundError } from '../utils/errors.js';
-import * as emailAccountService from '../services/email-account.service.js';
 
 /**
  * Vytvorenie nového usera s automatic temporary password (admin only)
@@ -18,83 +13,41 @@ export const createUser = async (req, res, next) => {
   const userAgent = req.headers['user-agent'];
 
   try {
-    // Skontroluj či username už neexistuje
-    const [existingUser] = await db
-      .select()
-      .from(users)
-      .where(eq(users.username, username))
-      .limit(1);
-
-    if (existingUser) {
-      throw new ConflictError('Username už existuje');
-    }
-
-    // Automaticky vygeneruj temporary password
-    const tempPassword = generateTempPassword(12);
-    const hashedTempPassword = await hashPassword(tempPassword);
-
-    // Validuj role - iba 'admin' alebo 'member'
-    const validRole = role === 'admin' ? 'admin' : 'member';
-
-    // Vytvor usera
-    const [newUser] = await db
-      .insert(users)
-      .values({
+    const result = await adminService.createUser(
       username,
-        tempPassword: hashedTempPassword,
-        role: validRole,
-        firstName: firstName || null,
-        lastName: lastName || null,
-        changedPassword: false,
-      })
-      .returning();
-
-    // Ak sú poskytnuté email credentials, vytvor email account (many-to-many)
-    let emailAccountCreated = false;
-    let emailAccountData = null;
-
-    if (email && emailPassword) {
-      try {
-        // Použij emailAccountService ktorý automaticky vytvorí many-to-many link
-        const newEmailAccount = await emailAccountService.createEmailAccount(
-          newUser.id,
+      firstName,
+      lastName,
+      role,
       email,
       emailPassword
     );
 
-        emailAccountCreated = true;
-        emailAccountData = {
-          id: newEmailAccount.id,
-          email: newEmailAccount.email,
-          jmapAccountId: newEmailAccount.jmapAccountId,
-          shared: newEmailAccount.shared,
-        };
-      } catch (emailError) {
-        // Email account sa nepodarilo vytvoriť, ale user bol vytvorený
-        // Admin môže pridať email account neskôr
-        console.error('Failed to create email account:', emailError);
-      }
-    }
-
     // Log user creation
-    await logUserCreation(adminId, newUser.id, username, validRole, ipAddress, userAgent);
+    await logUserCreation(
+      adminId,
+      result.user.id,
+      username,
+      result.user.role,
+      ipAddress,
+      userAgent
+    );
 
     res.status(201).json({
       success: true,
       data: {
         user: {
-          id: newUser.id,
-          username: newUser.username,
-          firstName: newUser.firstName,
-          lastName: newUser.lastName,
-          role: newUser.role,
-          emailSetup: emailAccountCreated,
-          emailAccount: emailAccountData,
-          tempPassword: tempPassword, // Vráti plain text password pre admina aby ho mohol poslať userovi
+          id: result.user.id,
+          username: result.user.username,
+          firstName: result.user.firstName,
+          lastName: result.user.lastName,
+          role: result.user.role,
+          emailSetup: result.emailAccountCreated,
+          emailAccount: result.emailAccountData,
+          tempPassword: result.tempPassword,
         },
       },
-      message: emailAccountCreated
-        ? emailAccountData.shared
+      message: result.emailAccountCreated
+        ? result.emailAccountData.shared
           ? 'Používateľ vytvorený a pripojený k existujúcemu zdieľanému email účtu.'
           : 'Používateľ úspešne vytvorený s novým emailovým účtom.'
         : 'Používateľ úspešne vytvorený. Email môže byť nastavený neskôr.',
@@ -110,18 +63,7 @@ export const createUser = async (req, res, next) => {
  */
 export const getAllUsers = async (req, res, next) => {
   try {
-    const allUsers = await db
-      .select({
-        id: users.id,
-        username: users.username,
-        firstName: users.firstName,
-        lastName: users.lastName,
-        role: users.role,
-        changedPassword: users.changedPassword,
-        lastLogin: users.lastLogin,
-        createdAt: users.createdAt,
-      })
-      .from(users);
+    const allUsers = await adminService.getAllUsers();
 
     res.status(200).json({
       success: true,
@@ -141,36 +83,12 @@ export const getUser = async (req, res, next) => {
   const { userId } = req.params;
 
   try {
-    const [user] = await db
-      .select({
-        id: users.id,
-        username: users.username,
-        firstName: users.firstName,
-        lastName: users.lastName,
-        role: users.role,
-        changedPassword: users.changedPassword,
-        lastLogin: users.lastLogin,
-        createdAt: users.createdAt,
-        updatedAt: users.updatedAt,
-      })
-      .from(users)
-      .where(eq(users.id, userId))
-      .limit(1);
-
-    if (!user) {
-      throw new NotFoundError('Používateľ nenájdený');
-    }
-
-    // Get user's email accounts (cez many-to-many)
-    const userEmailAccounts = await emailAccountService.getUserEmailAccounts(userId);
+    const user = await adminService.getUserById(userId);
 
     res.status(200).json({
       success: true,
       data: {
-        user: {
-          ...user,
-          emailAccounts: userEmailAccounts,
-        },
+        user,
       },
     });
   } catch (error) {
@@ -190,38 +108,14 @@ export const changeUserRole = async (req, res, next) => {
   const userAgent = req.headers['user-agent'];
 
   try {
-    // Získaj starú rolu
-    const [user] = await db
-      .select()
-      .from(users)
-      .where(eq(users.id, userId))
-      .limit(1);
-
-    if (!user) {
-      throw new NotFoundError('Používateľ nenájdený');
-    }
-
-    const oldRole = user.role;
-
-    // Update role
-    await db
-      .update(users)
-      .set({
-        role,
-        updatedAt: new Date(),
-      })
-      .where(eq(users.id, userId));
+    const result = await adminService.changeUserRole(userId, role);
 
     // Log role change
-    await logRoleChange(adminId, userId, oldRole, role, ipAddress, userAgent);
+    await logRoleChange(adminId, userId, result.oldRole, result.newRole, ipAddress, userAgent);
 
     res.status(200).json({
       success: true,
-      data: {
-        userId,
-        oldRole,
-        newRole: role,
-      },
+      data: result,
       message: 'Rola používateľa bola zmenená',
     });
   } catch (error) {
@@ -237,58 +131,12 @@ export const deleteUser = async (req, res, next) => {
   const { userId } = req.params;
 
   try {
-    const [user] = await db
-      .select()
-      .from(users)
-      .where(eq(users.id, userId))
-      .limit(1);
-
-    if (!user) {
-      throw new NotFoundError('Používateľ nenájdený');
-    }
-
-    // Zabraň zmazaniu posledného admina
-    if (user.role === 'admin') {
-      const [adminCount] = await db
-        .select({ count: db.$count(users) })
-        .from(users)
-        .where(eq(users.role, 'admin'));
-
-      if (adminCount.count <= 1) {
-        throw new ConflictError('Nemôžete zmazať posledného administrátora');
-      }
-    }
-
-    // Get user's email account IDs before deletion
-    const userEmailAccountLinks = await db
-      .select({ emailAccountId: userEmailAccounts.emailAccountId })
-      .from(userEmailAccounts)
-      .where(eq(userEmailAccounts.userId, userId));
-
-    const emailAccountIds = userEmailAccountLinks.map(link => link.emailAccountId);
-
-    // Delete user (cascades userEmailAccounts links)
-    await db.delete(users).where(eq(users.id, userId));
-
-    // Delete orphaned email accounts (no users linked)
-    // This will cascade delete contacts and emails
-    let deletedEmailAccounts = 0;
-    for (const emailAccountId of emailAccountIds) {
-      const [remainingLinks] = await db
-        .select({ count: db.$count(userEmailAccounts) })
-        .from(userEmailAccounts)
-        .where(eq(userEmailAccounts.emailAccountId, emailAccountId));
-
-      if (remainingLinks.count === 0) {
-        await db.delete(emailAccounts).where(eq(emailAccounts.id, emailAccountId));
-        deletedEmailAccounts++;
-      }
-    }
+    const result = await adminService.deleteUser(userId);
 
     res.status(200).json({
       success: true,
       message: 'Používateľ bol zmazaný',
-      deletedEmailAccounts,
+      deletedEmailAccounts: result.deletedEmailAccounts,
     });
   } catch (error) {
     return next(error);

src/controllers/auth.controller.js

@@ -57,7 +57,7 @@ export const login = async (req, res, next) => {
     // Log failed login
     await logLoginAttempt(username, false, ipAddress, userAgent, error.message);
 
-    next(error);
+    return next(error);
   }
 };
 
@@ -84,7 +84,7 @@ export const setPassword = async (req, res, next) => {
       message: 'Heslo úspešne nastavené',
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };
 
@@ -114,7 +114,7 @@ export const linkEmail = async (req, res, next) => {
       message: 'Email účet úspešne pripojený a overený',
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };
 
@@ -135,7 +135,7 @@ export const skipEmail = async (req, res, next) => {
       message: 'Email setup preskočený',
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };
 
@@ -164,7 +164,7 @@ export const logout = async (req, res, next) => {
       message: result.message,
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };
 
@@ -183,7 +183,7 @@ export const getSession = async (req, res, next) => {
       },
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };
 
@@ -201,6 +201,6 @@ export const getMe = async (req, res, next) => {
       },
     });
   } catch (error) {
-    next(error);
+    return next(error);
   }
 };

src/controllers/contact.controller.js

@@ -1,7 +1,6 @@
 import * as contactService from '../services/contact.service.js';
 import { discoverContactsFromJMAP, getJmapConfigFromAccount } from '../services/jmap.service.js';
 import * as emailAccountService from '../services/email-account.service.js';
-import { logger } from '../utils/logger.js';
 
 /**
  * Get all contacts for an email account
@@ -47,18 +46,12 @@ export const discoverContacts = async (req, res, next) => {
     const userId = req.userId;
     const { accountId, search = '', limit = 50 } = req.query;
 
-    logger.debug('discoverContacts called', { userId, accountId, search, limit });
-
     // Get email account (or primary if not specified)
     let emailAccount;
     if (accountId) {
-      logger.debug('Getting email account by ID', { accountId });
       emailAccount = await emailAccountService.getEmailAccountWithCredentials(accountId, userId);
-      logger.debug('Email account retrieved', { id: emailAccount.id, email: emailAccount.email });
     } else {
-      logger.debug('No accountId provided, getting primary account', { userId });
       const primaryAccount = await emailAccountService.getPrimaryEmailAccount(userId);
-      logger.debug('Primary account', primaryAccount ? { id: primaryAccount.id, email: primaryAccount.email } : { found: false });
       if (!primaryAccount) {
         return res.status(400).json({
           success: false,
@@ -69,16 +62,9 @@ export const discoverContacts = async (req, res, next) => {
         });
       }
       emailAccount = await emailAccountService.getEmailAccountWithCredentials(primaryAccount.id, userId);
-      logger.debug('Email account retrieved from primary', { id: emailAccount.id, email: emailAccount.email });
     }
 
     const jmapConfig = getJmapConfigFromAccount(emailAccount);
-    logger.debug('JMAP Config created', {
-      server: jmapConfig.server,
-      username: jmapConfig.username,
-      accountId: jmapConfig.accountId,
-      hasPassword: !!jmapConfig.password
-    });
 
     const potentialContacts = await discoverContactsFromJMAP(
       jmapConfig,
@@ -93,7 +79,6 @@ export const discoverContacts = async (req, res, next) => {
       data: potentialContacts,
     });
   } catch (error) {
-    logger.error('ERROR in discoverContacts', { error: error.message, stack: error.stack });
     return next(error);
   }
 };
@@ -106,11 +91,8 @@ export const discoverContacts = async (req, res, next) => {
 export const addContact = async (req, res, next) => {
   try {
     const userId = req.userId;
-    logger.debug('Full req.body', { body: req.body });
     const { email, name = '', notes = '', accountId } = req.body;
 
-    logger.debug('addContact called', { userId, email, name, accountId });
-
     if (!email) {
       return res.status(400).json({
         success: false,
@@ -124,10 +106,8 @@ export const addContact = async (req, res, next) => {
     // Get email account (or primary if not specified)
     let emailAccount;
     if (accountId) {
-      logger.debug('Using provided accountId', { accountId });
       emailAccount = await emailAccountService.getEmailAccountWithCredentials(accountId, userId);
     } else {
-      logger.debug('No accountId provided, using primary account');
       const primaryAccount = await emailAccountService.getPrimaryEmailAccount(userId);
       if (!primaryAccount) {
         return res.status(400).json({
@@ -139,7 +119,6 @@ export const addContact = async (req, res, next) => {
         });
       }
       emailAccount = await emailAccountService.getEmailAccountWithCredentials(primaryAccount.id, userId);
-      logger.debug('Using primary account', { accountId: primaryAccount.id });
     }
 
     const jmapConfig = getJmapConfigFromAccount(emailAccount);

src/controllers/todo.controller.js

@@ -110,7 +110,6 @@ export const createTodo = async (req, res, next) => {
     const userId = req.userId;
     const data = req.body;
 
-    console.log('Backend received todo data:', data);
     const todo = await todoService.createTodo(userId, data);
 
     // Log audit event
@@ -136,7 +135,6 @@ export const updateTodo = async (req, res, next) => {
     const { todoId } = req.params;
     const data = req.body;
 
-    console.log('Backend received update data:', data);
     const todo = await todoService.updateTodo(todoId, data);
 
     res.status(200).json({

src/services/admin.service.js

@@ -0,0 +1,217 @@
+import { db } from '../config/database.js';
+import { users, userEmailAccounts, emailAccounts } from '../db/schema.js';
+import { eq } from 'drizzle-orm';
+import { hashPassword, generateTempPassword } from '../utils/password.js';
+import { ConflictError, NotFoundError } from '../utils/errors.js';
+import * as emailAccountService from './email-account.service.js';
+import { logger } from '../utils/logger.js';
+
+/**
+ * Skontroluj či username už neexistuje
+ */
+export const checkUsernameExists = async (username) => {
+  const [existingUser] = await db
+    .select()
+    .from(users)
+    .where(eq(users.username, username))
+    .limit(1);
+
+  return !!existingUser;
+};
+
+/**
+ * Vytvorenie nového usera s automatic temporary password
+ */
+export const createUser = async (username, firstName, lastName, role, email, emailPassword) => {
+  // Skontroluj či username už neexistuje
+  if (await checkUsernameExists(username)) {
+    throw new ConflictError('Username už existuje');
+  }
+
+  // Automaticky vygeneruj temporary password
+  const tempPassword = generateTempPassword(12);
+  const hashedTempPassword = await hashPassword(tempPassword);
+
+  // Validuj role - iba 'admin' alebo 'member'
+  const validRole = role === 'admin' ? 'admin' : 'member';
+
+  // Vytvor usera
+  const [newUser] = await db
+    .insert(users)
+    .values({
+      username,
+      tempPassword: hashedTempPassword,
+      role: validRole,
+      firstName: firstName || null,
+      lastName: lastName || null,
+      changedPassword: false,
+    })
+    .returning();
+
+  // Ak sú poskytnuté email credentials, vytvor email account (many-to-many)
+  let emailAccountCreated = false;
+  let emailAccountData = null;
+
+  if (email && emailPassword) {
+    try {
+      const newEmailAccount = await emailAccountService.createEmailAccount(
+        newUser.id,
+        email,
+        emailPassword
+      );
+
+      emailAccountCreated = true;
+      emailAccountData = {
+        id: newEmailAccount.id,
+        email: newEmailAccount.email,
+        jmapAccountId: newEmailAccount.jmapAccountId,
+        shared: newEmailAccount.shared,
+      };
+    } catch (emailError) {
+      logger.error('Failed to create email account:', { error: emailError.message });
+    }
+  }
+
+  return {
+    user: newUser,
+    tempPassword,
+    emailAccountCreated,
+    emailAccountData,
+  };
+};
+
+/**
+ * Zoznam všetkých userov
+ */
+export const getAllUsers = async () => {
+  const allUsers = await db
+    .select({
+      id: users.id,
+      username: users.username,
+      firstName: users.firstName,
+      lastName: users.lastName,
+      role: users.role,
+      changedPassword: users.changedPassword,
+      lastLogin: users.lastLogin,
+      createdAt: users.createdAt,
+    })
+    .from(users);
+
+  return allUsers;
+};
+
+/**
+ * Získanie konkrétneho usera
+ */
+export const getUserById = async (userId) => {
+  const [user] = await db
+    .select({
+      id: users.id,
+      username: users.username,
+      firstName: users.firstName,
+      lastName: users.lastName,
+      role: users.role,
+      changedPassword: users.changedPassword,
+      lastLogin: users.lastLogin,
+      createdAt: users.createdAt,
+      updatedAt: users.updatedAt,
+    })
+    .from(users)
+    .where(eq(users.id, userId))
+    .limit(1);
+
+  if (!user) {
+    throw new NotFoundError('Používateľ nenájdený');
+  }
+
+  // Get user's email accounts (cez many-to-many)
+  const userEmailAccountsList = await emailAccountService.getUserEmailAccounts(userId);
+
+  return {
+    ...user,
+    emailAccounts: userEmailAccountsList,
+  };
+};
+
+/**
+ * Zmena role usera
+ */
+export const changeUserRole = async (userId, newRole) => {
+  // Získaj starú rolu
+  const [user] = await db
+    .select()
+    .from(users)
+    .where(eq(users.id, userId))
+    .limit(1);
+
+  if (!user) {
+    throw new NotFoundError('Používateľ nenájdený');
+  }
+
+  const oldRole = user.role;
+
+  // Update role
+  await db
+    .update(users)
+    .set({
+      role: newRole,
+      updatedAt: new Date(),
+    })
+    .where(eq(users.id, userId));
+
+  return { userId, oldRole, newRole };
+};
+
+/**
+ * Zmazanie usera
+ */
+export const deleteUser = async (userId) => {
+  const [user] = await db
+    .select()
+    .from(users)
+    .where(eq(users.id, userId))
+    .limit(1);
+
+  if (!user) {
+    throw new NotFoundError('Používateľ nenájdený');
+  }
+
+  // Zabraň zmazaniu posledného admina
+  if (user.role === 'admin') {
+    const [adminCount] = await db
+      .select({ count: db.$count(users) })
+      .from(users)
+      .where(eq(users.role, 'admin'));
+
+    if (adminCount.count <= 1) {
+      throw new ConflictError('Nemôžete zmazať posledného administrátora');
+    }
+  }
+
+  // Get user's email account IDs before deletion
+  const userEmailAccountLinks = await db
+    .select({ emailAccountId: userEmailAccounts.emailAccountId })
+    .from(userEmailAccounts)
+    .where(eq(userEmailAccounts.userId, userId));
+
+  const emailAccountIds = userEmailAccountLinks.map(link => link.emailAccountId);
+
+  // Delete user (cascades userEmailAccounts links)
+  await db.delete(users).where(eq(users.id, userId));
+
+  // Delete orphaned email accounts (no users linked)
+  let deletedEmailAccounts = 0;
+  for (const emailAccountId of emailAccountIds) {
+    const [remainingLinks] = await db
+      .select({ count: db.$count(userEmailAccounts) })
+      .from(userEmailAccounts)
+      .where(eq(userEmailAccounts.emailAccountId, emailAccountId));
+
+    if (remainingLinks.count === 0) {
+      await db.delete(emailAccounts).where(eq(emailAccounts.id, emailAccountId));
+      deletedEmailAccounts++;
+    }
+  }
+
+  return { deletedEmailAccounts };
+};

src/services/contact.service.js

@@ -3,7 +3,6 @@ import { contacts, emails, companies } from '../db/schema.js';
 import { eq, and, desc, or, ne } from 'drizzle-orm';
 import { NotFoundError, ConflictError } from '../utils/errors.js';
 import { syncEmailsFromSender } from './jmap.service.js';
-import { logger } from '../utils/logger.js';
 
 /**
  * Get all contacts for an email account
@@ -21,6 +20,7 @@ export const getContactsForEmailAccount = async (emailAccountId) => {
 
 /**
  * Add a new contact to an email account
+ * Uses transaction for atomic operations
  */
 export const addContact = async (emailAccountId, jmapConfig, email, name = '', notes = '', addedByUserId = null) => {
   // Check if contact already exists for this email account
@@ -39,8 +39,10 @@ export const addContact = async (emailAccountId, jmapConfig, email, name = '', n
     throw new ConflictError('Kontakt už existuje');
   }
 
+  // Use transaction for atomic contact creation and email reassignment
+  const newContact = await db.transaction(async (tx) => {
     // Create contact
-  const [newContact] = await db
+    const [contact] = await tx
       .insert(contacts)
       .values({
         emailAccountId,
@@ -51,29 +53,18 @@ export const addContact = async (emailAccountId, jmapConfig, email, name = '', n
       })
       .returning();
 
-  // Sync emails from this sender
-  try {
-    await syncEmailsFromSender(jmapConfig, emailAccountId, newContact.id, email);
-  } catch (error) {
-    logger.error('Failed to sync emails for new contact', { error: error.message });
-    // Don't throw - contact was created successfully
-  }
-
     // REASSIGN: Fix any existing emails that belong to this contact but have wrong contactId
-  try {
-    logger.info(`Checking for emails to reassign to contact ${email}...`);
-
     // Find emails where:
-    // - from === newContact.email OR to === newContact.email
-    // - contactId !== newContact.id (belongs to different contact)
+    // - from === contact.email OR to === contact.email
+    // - contactId !== contact.id (belongs to different contact)
     // - emailAccountId matches
-    const emailsToReassign = await db
+    const emailsToReassign = await tx
       .select()
       .from(emails)
       .where(
         and(
           eq(emails.emailAccountId, emailAccountId),
-          ne(emails.contactId, newContact.id),
+          ne(emails.contactId, contact.id),
           or(
             eq(emails.from, email),
             eq(emails.to, email)
@@ -82,25 +73,32 @@ export const addContact = async (emailAccountId, jmapConfig, email, name = '', n
       );
 
     if (emailsToReassign.length > 0) {
-      logger.info(`Found ${emailsToReassign.length} emails to reassign to contact ${email}`);
-
-      // Update contactId for these emails
-      for (const emailToReassign of emailsToReassign) {
-        await db
+      // Bulk update contactId for these emails
+      await tx
         .update(emails)
         .set({
-            contactId: newContact.id,
-            companyId: newContact.companyId || null,
+          contactId: contact.id,
+          companyId: contact.companyId || null,
         })
-          .where(eq(emails.id, emailToReassign.id));
+        .where(
+          and(
+            eq(emails.emailAccountId, emailAccountId),
+            ne(emails.contactId, contact.id),
+            or(
+              eq(emails.from, email),
+              eq(emails.to, email)
+            )
+          )
+        );
     }
 
-      logger.success(`Reassigned ${emailsToReassign.length} emails to contact ${email}`);
-    } else {
-      logger.info(`No emails to reassign for contact ${email}`);
-    }
-  } catch (error) {
-    logger.error('Failed to reassign emails', { error: error.message });
+    return contact;
+  });
+
+  // Sync emails from this sender (outside transaction - external API call)
+  try {
+    await syncEmailsFromSender(jmapConfig, emailAccountId, newContact.id, email);
+  } catch {
     // Don't throw - contact was created successfully
   }