diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 5b5fe6c..c4f661d 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -32,6 +32,9 @@ jobs: NODE_OPTIONS: --max-old-space-size=6144 run: npm run check + - name: Tests + run: npm test + - name: Type check document worker run: | cd server/documents/worker diff --git a/IMPLEMENTATION_LOG.md b/IMPLEMENTATION_LOG.md index 46aba8e..98b7dce 100644 --- a/IMPLEMENTATION_LOG.md +++ b/IMPLEMENTATION_LOG.md @@ -23,3 +23,27 @@ - Что изменено: — (только проверка) - Как проверялось: `git fetch origin` + `git status` (чисто, синхронизировано с origin/main, HEAD `a2b231d`), `npm run check` — зелёный (оба tsc: основной и client-di2). - Влияние на поиск/UX: нет. + +--- + +## [0.15] Сквозная документация изменений (этот файл) + +- Статус: ✅ done (коммит `76a0d85` «chore(docs): add implementation log») +- Зачем: по одному файлу любой ИИ-агент понимает, что сделано, зачем, какие файлы тронуты и как проверялось. +- Что изменено: создан `IMPLEMENTATION_LOG.md` в корне репозитория (шапка-контекст + шаблон записей). +- Влияние на поиск/UX: нет. + +--- + +## [A3 / 1.6 частично] Подключение тестов + починка 5 падающих + +- Статус: ✅ done +- Зачем: тесты существовали (3 файла, vitest), но не запускались ни локально (нет скрипта), ни в CI; 5 из 45 падали. Без зелёных тестов дальнейшие изменения безопасности/производительности — вслепую. +- Что изменено: + - `package.json` — скрипт `"test": "vitest run"`. + - `.github/workflows/pr-check.yml` — шаг «Tests» (`npm test`) после type check. + - `server/utils/userActivity.ts` — **фикс реального бага**: `trackUserActivity` мог кинуть синхронно внутри `authenticateToken`, исключение попадало в catch аутентификации → ложный `403 Недействительный токен` на первом запросе пользователя за минуту. Тело обёрнуто в try/catch, throttle сбрасывается для повторной попытки (соответствие контракту fire-and-forget). + - `tests/auto-transitions.test.ts` — 3 устаревших теста обновлены под намеренное текущее поведение (forward-only по position; синк assignee в task_assignees; системное сообщение заменено аудит-записью «Автопереход»), добавлен мок `delegation.service`. +- Как проверялось: `npx vitest run` — 45/45 зелёные; `npm run check` — чисто. +- Влияние на поиск/UX: нет. +- Подводные камни: баг в `userActivity.ts` проявлялся только на первом аутентифицированном запросе в минуту (throttle) — в проде мог давать sporadic 403. diff --git a/package.json b/package.json index bdaeded..e1a3c39 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "build": "vite build && vite build -c vite.config.di2.ts && esbuild server/index.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", "start": "NODE_ENV=production node dist/index.js", "check": "tsc && tsc -p client-di2/tsconfig.json", + "test": "vitest run", "skins:generate": "tsx scripts/dev/generate-skins.ts", "db:generate": "drizzle-kit generate", "db:migrate": "node scripts/migrate-docker.js", diff --git a/server/utils/userActivity.ts b/server/utils/userActivity.ts index a5ba71a..808a685 100644 --- a/server/utils/userActivity.ts +++ b/server/utils/userActivity.ts @@ -20,15 +20,22 @@ export function trackUserActivity(userId: number): void { lastActivityUpdate.set(userId, now); // Run async without awaiting so the request is not delayed. - db.update(users) - .set({ lastActivityAt: new Date() }) - .where(eq(users.id, userId)) - .then(() => { - // No-op on success - }) - .catch((err) => { - // Reset the throttle timestamp on error so the next request retries. - lastActivityUpdate.delete(userId); - console.error("[trackUserActivity] failed to update user activity:", err); - }); + // Синхронные ошибки (например, недоступный пул соединений) тоже не должны + // срывать аутентификацию — трекинг активности некритичен. + try { + db.update(users) + .set({ lastActivityAt: new Date() }) + .where(eq(users.id, userId)) + .then(() => { + // No-op on success + }) + .catch((err) => { + // Reset the throttle timestamp on error so the next request retries. + lastActivityUpdate.delete(userId); + console.error("[trackUserActivity] failed to update user activity:", err); + }); + } catch (err) { + lastActivityUpdate.delete(userId); + console.error("[trackUserActivity] failed to update user activity:", err); + } } diff --git a/tests/auto-transitions.test.ts b/tests/auto-transitions.test.ts index 0b9d6d8..59144dd 100644 --- a/tests/auto-transitions.test.ts +++ b/tests/auto-transitions.test.ts @@ -15,7 +15,6 @@ const mockStorage = vi.hoisted(() => ({ updateTask: vi.fn(), addTaskAssignee: vi.fn().mockResolvedValue(undefined), addTaskAuditLog: vi.fn().mockResolvedValue(undefined), - createTaskMessage: vi.fn().mockResolvedValue(undefined), getForm: vi.fn().mockResolvedValue(null), })); @@ -58,6 +57,11 @@ vi.mock('../server/routes/task-helpers', () => ({ indexTaskAsync: vi.fn().mockResolvedValue(undefined), })); +vi.mock('../server/services/delegation.service', () => ({ + ensureReviewAssignees: vi.fn().mockResolvedValue(undefined), + cleanupStaleDelegates: vi.fn().mockResolvedValue(undefined), +})); + // ─── Tests ──────────────────────────────────────────────────────────────────── import { evaluateAutoTransitions } from '../server/utils/auto-transitions'; @@ -236,9 +240,10 @@ describe('evaluateAutoTransitions', () => { const result = await evaluateAutoTransitions(1, 1); expect(result.changed).toBe(true); - // 10→20→30, then 30→20 but 20 already visited → stop + // 10→20→30, дальше цепочка обрывается: автопереходы двигают задачу + // только ВПЕРЁД по position (auto-transitions.ts), поэтому отката 30→20 нет expect(result.visitedStatusIds).toEqual([10, 20, 30]); - expect(mockStorage.updateTask.mock.calls.length).toBe(3); + expect(mockStorage.updateTask.mock.calls.length).toBe(2); }); it('should stop at max depth to prevent infinite loops', async () => { @@ -261,7 +266,7 @@ describe('evaluateAutoTransitions', () => { const result = await evaluateAutoTransitions(1, 1, { maxDepth: 3 }); expect(result.changed).toBe(true); - // Should stop after maxDepth transitions (3 updates: A→B→A→B, but maxDepth=3 means 3 iterations) + // Правило «только вперёд» останавливает цепочку на B (10→20), maxDepth не достигается expect(mockStorage.updateTask.mock.calls.length).toBeLessThanOrEqual(3); }); @@ -325,6 +330,9 @@ describe('evaluateAutoTransitions', () => { mockStorage.getTaskFieldValues.mockResolvedValue([ { fieldId: 4, value: 'yes' }, ]); + // Явно пустой список переходов: правил назначения нет, поэтому + // triggeredBy не должен попадать в исполнители задачи + mockStorage.getStatusTransitions.mockResolvedValue([]); mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20 })); await evaluateAutoTransitions(1, 1, { triggeredBy: 1 }); @@ -332,8 +340,10 @@ describe('evaluateAutoTransitions', () => { expect(mockStorage.addTaskAssignee).not.toHaveBeenCalled(); }); - it('should keep authorId=0 in system message for auto-transition', async () => { - mockStorage.getTask.mockResolvedValue(makeTask({ currentStatusId: 10 })); + it('should sync resolved assignee into task_assignees on auto-transition', async () => { + // Намеренное поведение (auto-transitions.ts): основной исполнитель + // синкается в task_assignees, чтобы TaskDetail мог его отобразить + mockStorage.getTask.mockResolvedValue(makeTask({ currentStatusId: 10, assignedTo: null })); mockStorage.getFormStatuses.mockResolvedValue([ makeStatus({ id: 10, name: 'Initial', position: 0 }), makeStatus({ id: 20, name: 'Auto', position: 1, entryConditions: [{ fieldCode: 'field_4', value: 'yes' }] }), @@ -344,14 +354,14 @@ describe('evaluateAutoTransitions', () => { mockStorage.getTaskFieldValues.mockResolvedValue([ { fieldId: 4, value: 'yes' }, ]); - mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20 })); + mockStorage.getStatusTransitions.mockResolvedValue([ + { id: 1, fromStatusId: 10, toStatusId: 20, assigneeUserId: 42 }, + ]); + mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20, assignedTo: 42 })); await evaluateAutoTransitions(1, 1, { triggeredBy: 1 }); - expect(mockStorage.createTaskMessage).toHaveBeenCalledWith( - expect.objectContaining({ authorId: 0 }), - expect.any(Number) - ); + expect(mockStorage.addTaskAssignee).toHaveBeenCalledWith(1, 42, 1); }); it('should use AND logic for entryConditions and not bounce back', async () => {