feat(page): temporary notes — auto-trash after X hours unless made permanent (#201) #215
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#215
Reference in New Issue
Block a user
Delete Branch "feat/201-temporary-notes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #201
"Temporary notes" — a note with a death timer. Created via a dedicated hourglass button in the space-tree header; after a configurable X hours (default 24) it auto-moves to Trash via the same soft-delete as a manual "Move to trash". To survive it needs an explicit "Make permanent". Motto: structure or die.
Reused mechanisms (mirrors)
is_template: nullable columnpages.temporary_expires_at(NULL = permanent; non-NULL = frozen deadline) -> tree icon -> menu toggle -> toggle endpoint.@Intervalservice modelled onTrashCleanupService.PageRepo.removePage(children ride along to trash; emitsPAGE_SOFT_DELETED).Server
20260626T130000-page-temporary-notes:pages.temporary_expires_at timestamptz NULL+ partial indexpages_temporary_expires_at_idx(armed & not-yet-trashed);workspaces.temporary_note_hours int8 NULL. Default in code:DEFAULT_TEMPORARY_NOTE_HOURS = 24.create-pageDTOtemporary?: boolean; the deadline is frozen at creation, so changing the workspace setting never reschedules existing notes.POST /pages/toggle-temporary(mirror oftoggle-template): arm/clear the timer, CASL-guarded viavalidateCanEdit, cross-workspaceNotFounddefense-in-depth.TemporaryNoteCleanupService: hourly@Intervalsweep (temporary-note-cleanup, every 60 min) that soft-deletes expired notes throughremovePage, attributed to the creator; idempotent (deletedAt IS NULLon both the SELECT and removePage).restorePageclearstemporary_expires_atso a restored note is not instantly re-trashed.temporaryNoteHours(audit-tracked) + repo baseFields.Client
IconClockHour4indicator in the tree row.Migration name
20260626T130000-page-temporary-notes.tsCleanup-job cadence
@Interval('temporary-note-cleanup', 60 * 60 * 1000)— every hour (hourly granularity; a note may live up to ~1h past its deadline, by design).Tests (unit only)
page-temporary.controller.spec.ts: toggle/explicit/permission/cross-workspace +ToggleTemporaryDtovalidation.temporary-note-cleanup.service.spec.ts: selection filters, per-noteremovePage(attributed to creator), error isolation, no-op.20260626T130000-page-temporary-notes.spec.ts: migration up/down sanity (columns + partial index).Verify
pnpm --filter server exec tsc --noEmit— clean.pnpm --filter client exec tsc -b— clean.jest src/core/page src/core/workspace— 234 passed (29 suites).test:intnot run (shared DB).🤖 Generated with Claude Code
"Temporary notes" with a death timer: created via a dedicated hourglass button in the space-tree header, a note auto-moves to Trash after a configurable X hours (default 24) unless explicitly made permanent ("structure or die"). Reuses existing mechanisms, mirroring is_template and the trash-cleanup job: - New nullable column pages.temporary_expires_at (NULL = permanent; non-NULL = frozen deadline) + partial index for the sweep; workspace column temporary_note_hours (default via DEFAULT_TEMPORARY_NOTE_HOURS = 24). - create-page DTO `temporary` flag; the deadline is frozen at creation so later setting changes never reschedule existing notes. - POST /pages/toggle-temporary (mirror of toggle-template): arm/clear the timer, CASL-guarded via validateCanEdit, cross-workspace NotFound defense-in-depth. - TemporaryNoteCleanupService: hourly @Interval sweep that soft-deletes expired notes through the exact PageRepo.removePage path (recursive over children, emits PAGE_SOFT_DELETED), attributed to the creator; idempotent via deletedAt IS NULL filters. - restorePage clears temporary_expires_at so a restored note can't be re-trashed. - Workspace setting temporary_note_hours (audit-tracked) + a hours editor in workspace General settings. - Client: second create button, orange tree icon, tree + page-header menu toggle ("Make temporary"/"Make permanent"), an open-note banner with a rescue action, and en/ru i18n. Tests (unit): toggle-temporary controller (toggle/explicit/permission/cross-ws + DTO validation), cleanup-job sweep (selection filters, per-note removePage, error isolation), and a migration up/down sanity. Server tsc, client tsc -b, and the page+workspace jest suites are green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review — PR #215: временные заметки — авто-перенос в корзину через X часов, если не сделать постоянной
Вердикт: Approve with comments. Блокирующих дефектов в логике нет: data-loss-race на «make permanent во время свипа» восстановим (restore разоружает таймер — проверено), поэтому это warning, а не блокер. Единственное обязательное к мержу — фид user-facing фичи в CHANGELOG (нарушение установленной конвенции репо) и тесты на новую data-loss-чувствительную логику (frozen-deadline в
create()и сброс таймера вrestorePage), которые сейчас отсутствуют.Объём: дифф
develop…feat/201-temporary-notes(merge-base3ddc329b), 35 файлов, +1063/−17. Прогнаны параллельные аспектные ревьюеры (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) + judge-проход.Must fix before merge
[documentation] Добавить запись в CHANGELOG
[Unreleased] / ### Addedпро временные заметки (#201) —CHANGELOG.md:11-13Репо ведёт CHANGELOG в формате Keep-a-Changelog с живой секцией
[Unreleased]/Added, где каждая недавняя user-facing фича документируется со ссылкой на issue (#143, #168, #180, #183 и т.д.); AGENTS.md и предыдущие review-фиксы (f80276d4) делают обновление changelog частью merge-процесса. Дифф не трогает CHANGELOG.md (подтверждено пустым диффом), хотя PR добавляет новый workspace-сеттинг, эндпойнтPOST /pages/toggle-temporary, миграцию БД, фоновый свип и баннер — поведение «заметка сама уходит в корзину через X часов» операторы обязаны знать. Fix: добавить буллет под## [Unreleased]→### Added: временные заметки авто-уходят в корзину через настраиваемый workspace-лайфтайм (дефолтDEFAULT_TEMPORARY_NOTE_HOURS= 24ч), если не сделать постоянной; дедлайн фиксируется при создании; restore разоружает таймер; ссылка (#201), формат как у существующих записей.[stability] Перепроверять
temporaryExpiresAtв момент удаления, чтобы свип не перетирал параллельный «Make permanent» —apps/server/src/core/page/services/temporary-note-cleanup.service.ts:29-58Свип делает нетранзакционный SELECT всех просроченных заметок, затем в цикле вызывает
pageRepo.removePage().removePageудаляет поidс фильтром толькоdeletedAt IS NULLи НЕ перечитываетtemporaryExpiresAt(проверено в page.repo.ts:297-355). Если пользователь нажимает «Make permanent» (toggleTemporaryставитtemporaryExpiresAt = null) в окне между SELECT и конкретнымremovePage, заметку всё равно мягко удалит в корзину. Комментарий сервиса аргументирует идемпотентность только для двойного свипа (deletedAt-фильтр), но не для гонки keep-vs-trash. Не блокирует: восстановимо через restore, который сбрасывает таймер (подтверждено), окно узкое. Fix: сделать удаление в свипе условным — либо отдельный guarded soft-delete сWHERE temporary_expires_at IS NOT NULL AND temporary_expires_at < now AND deleted_at IS NULL(event/children — только при реально затронутой строке), либо в цикле перечитывать строку и пропускатьremovePage, еслиtemporaryExpiresAtсталnull. Добавить тест на это переплетение.[stability] Ограничить выборку просроченных заметок (LIMIT/батчи), чтобы не грузить неограниченный бэклог в память —
apps/server/src/core/page/services/temporary-note-cleanup.service.ts:29-37SELECT тянет все просроченные заметки по всем workspace без LIMIT, затем последовательно делает рекурсивное удаление + emit события на каждую. При коротком
temporaryNoteHoursи массовом создании (например после простоя) бэклог может быть большим. Повторяет паттернTrashCleanupService, поэтому не регрессия. Fix: добавить разумный LIMIT и дренировать остаток на следующем часовом прогоне, либо обрабатывать фиксированными батчами.[simplification] Вынести дублированный пост-toggle sync кэша страниц в общий хелпер —
apps/client/src/features/page/components/header/page-header-menu.tsx:175-189Тот же блок обновления кэша (
for (const key of [page.slugId, page.id]) setQueryData(['pages', key], {...cached, temporaryExpiresAt})+invalidateQueries('sidebar-pages')) скопирован дословно вtemporary-note-banner.tsx:44-56. Две копии плумбинга ключей кэша легко разъезжаются. Fix: вынести вsyncTemporaryExpiresInCache(queryClient, page, temporaryExpiresAt)и вызывать из меню-хедера и баннера.[simplification] Убрать или утончить тавтологичный спек миграции, мокающий всю БД —
apps/server/src/database/migrations/spec/20260626T130000-page-temporary-notes.spec.ts:1-88Единственный migration-спек в репо (конвенции нет); он стабит весь Kysely schema builder и
sql-тег, затем проверяет, что миграция вызывает те же билдер-методы, что видны строкой ниже. Без реальной БД он не ловит реальный дефект миграции (тип колонки, nullability, семантика предиката индекса, порядок) — только повторяет реализацию, добавляя стоимость поддержки. Fix: удалить спек, либо заменить на реальный интеграционный тест (применить up/down к тестовому Postgres и проверить итоговые колонки/индекс с правильными типами).Test coverage
Свип (
TemporaryNoteCleanupService) покрыт. Две новые data-loss-чувствительные ветки покрытия не имеют — это first-class находки:[test-coverage] Добавить тест
create()сtemporary, проверяющий frozentemporaryExpiresAtв payloadinsertPage—apps/server/src/core/page/services/page.service.ts:127-138, 173Новая ветка условно читает
workspaces.temporaryNoteHours, падает вDEFAULT_TEMPORARY_NOTE_HOURS(24) при NULL, считаетnow + hours*3600000и пишет на payload — ничего из этого не проверяется (page.service.spec.tsимеет харнессcreate() → insertPage, ноrun()не ставитtemporary). Регрессия в единице/фоллбэке/арифметике уедет зелёной. Проект уже тестирует payloadinsertPage(provenance), так что это в рамках планки репо. Fix: расширить describecreate() → insertPageкейсами с{ title, spaceId, temporary: true }и db-стабом, гдеworkspacesselect резолвит (a) строку с override и (b){ temporaryNoteHours: null }; с jest fake timers проверитьtemporaryExpiresAt === now + hours*60*60*1000(override и DEFAULT на NULL); плюс кейс безtemporary—temporaryExpiresAtundefined и без workspace-lookup.[test-coverage] Добавить регрессионный тест, что
restorePageсбрасываетtemporaryExpiresAt—apps/server/src/database/repos/page/page.repo.ts:426-433restorePageтеперь ставитtemporaryExpiresAt: nullвместе сdeletedById/deletedAt(подтверждено) — комментарий в диффе прямо объясняет: иначе восстановленная заметка с прошедшим дедлайном будет мгновенно ре-затрэшена следующим свипом. Тестов на restore в репо нет, будущий рефактор молча уронит сброс и вернёт баг. Fix: добавить unit-тест (новый restore-спек либо стаб update-билдера), проверяющий, что payload.set(...)включаетtemporaryExpiresAt: null(в дополнение кdeletedById: null, deletedAt: null); chainable-стаб, записывающий аргументset(), по образцуmakeDbStubизtemporary-note-cleanup.service.spec.ts, достаточен.Architecture & design (forward-looking, non-blocking)
Дублирование вычисления дедлайна + DB-зависимость в контроллере —
page.service.ts:127-138(create) vspage-template.controller.ts:122-134(toggleTemporary). Логика «взвести таймер смерти» (lookuptemporaryNoteHours→ fallbackDEFAULT_TEMPORARY_NOTE_HOURS→new Date(Date.now() + hours*60*60*1000)) написана дословно дважды (оба call-site проверены). Чтобы сделать это в контроллере, PR инжектит@InjectKysely()прямо вPageTemplateController— ни один другой контроллер подapps/server/src/coreне инжектит Kysely напрямую, остальные делегируют персистентность сервисам.добавить
PageService.computeTemporaryExpiresAt(workspaceId): Promise<Date>, вызывать изcreate()и контроллера, убрать@InjectKyselyизPageTemplateController. Pros: единый источник формулы и NULL→default фоллбэка; контроллер возвращается к делегированию как у соседей; убирает нетипичный DB-инжект в контроллере; тривиально юнит-тестируется. Cons: ещё один маленький метод на и так большомPageService;toggleTemporaryвсё равно держит page-found/cross-workspace/CASL-проверки в контроллере, переезжает только compute+persist.Code review (re-review) — PR #215: temporary notes — auto-trash after X hours (#201)
Вердикт: Approve with comments. Единственный прошлый блокер (отсутствующая запись в CHANGELOG для #201) закрыт по существу; в дельте новых блокеров нет. Остаётся одна непокрытая ветка race-guard'а — non-blocking.
Ре-ревью дельты
a9b53d27..4f82be6e(9 файлов, +266/−120). Аспекты: stability, conventions, documentation, regressions, test-coverage (параллельные ревьюеры + judge).Статус прошлых блокеров
[Unreleased]/Addedдля #201 — ЗАКРЫТ. ВCHANGELOG.mdпод## [Unreleased]→### Addedдобавлена реальная запись «Temporary notes — auto-move to Trash after a workspace lifetime», корректно описывающая поведение (фиксация дедлайна на момент создания, почасовой sweep, дети едут следом, баннер «Make permanent», disarm при восстановлении из Trash) и завершающаяся(#201). Секция и формулировка верны, не косметика.Must fix before merge
apps/server/src/core/page/services/spec/temporary-note-cleanup.service.spec.ts:95-128Guard вtemporary-note-cleanup.service.tsпропускает заметку при любом из четырёх условий:!current,deletedAt !== null,temporaryExpiresAt === null,new Date(temporaryExpiresAt) >= now. Тесты покрывают made-permanent и already-trashed, но ветку>= now(пользователь disarm-then-re-arm или повторно включил таймер в окне между батч-SELECT и per-row re-read, получив свежий будущий дедлайн) — нет. Дефолтный stub отдаётnew Date(0)(всегда< now), поэтому регрессия, ломающая именно это сравнение (>=→>или потеря обёрткиnew Date(...)), пройдёт весь сьют незамеченной. Это сердце дельты — защита от перевооружения во время sweep'а. Fix: добавить кейс по образцу существующих race-тестов — переопределитьreReadFirstна{ temporaryExpiresAt: new Date(Date.now() + 60*60*1000), deletedAt: null }и проверить, чтоpageRepo.removePageне вызван.83f64583e5to53cbec9354Ghost referenced this pull request2026-06-26 22:33:37 +03:00