vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(temporary-notes): tree clock marker updates without reload + mobile-friendly full-width create buttons #225

Merged
vvzvlad merged 2 commits from fix/temporary-notes-ui into develop 2026-06-27 01:39:10 +03:00
Conversation 1 Commits 2 Files Changed 15 +374 -79
Ghost commented 2026-06-27 00:30:33 +03:00
Copy Link

Что

Два UI-бага фичи временных заметок (#201/#215, уже влита в develop):

1. Маркер-часики в дереве не обновлялся до релоада

Баннер на странице обновлялся сразу, а оранжевый 🕐 на узле сайдбара — только после reload. Три причины, все исправлены (маркер обновляется без перезагрузки):

  • Toggle (header-меню / баннер): syncTemporaryExpiresInCache() патчил только кэш страницы, не treeDataAtom. Добавлен getDefaultStore() + treeModel.update(...).
  • Create-as-temporary (главный виновник): invalidateOnCreatePage() строил newPage без temporaryExpiresAt, и перестройка дерева (buildTree→mergeRootTrees) затирала оптимистичный/сокетный маркер на undefined. Добавлено поле.
  • Другие клиенты / гонка вставки: temporaryExpiresAt проброшен через серверный broadcast addTreeNode (TreeNodeSnapshot, PAGE_CREATED в page.repo, broadcastPageCreated); idempotency-guard handleCreate дополняет дедлайн, если broadcast выиграл гонку.

2. Кнопки «New note» / «New temporary note» не влезали на мобиле

В new-note-button.tsx и space-create-note-buttons.tsx: side-by-side Group grow → вертикальный Stack, обе fullWidth, временная кнопка color="orange", обычная gray (чуть разные цвета). Текст «New temporary note» больше не обрезается.

Проверка (Playwright, живой стенд)

  • Маркер: create→есть; make-permanent (баннер)→нет; make-temporary (меню)→есть; make-permanent (меню)→нет — всё без reload, PASS.
  • Кнопки (390×844): обе full-width (w=370), стопкой, текст не обрезан, цвета разные (gray rgb(73,80,87) vs orange rgb(253,126,20)).
  • tsc exit 0; tree-socket-reducers 16/16, ws-tree.service+page-ws.listener 25/25.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

## Что Два UI-бага фичи временных заметок (#201/#215, уже влита в develop): ### 1. Маркер-часики в дереве не обновлялся до релоада Баннер на странице обновлялся сразу, а оранжевый 🕐 на узле сайдбара — только после reload. Три причины, все исправлены (маркер обновляется без перезагрузки): - **Toggle (header-меню / баннер):** `syncTemporaryExpiresInCache()` патчил только кэш страницы, не `treeDataAtom`. Добавлен `getDefaultStore()` + `treeModel.update(...)`. - **Create-as-temporary (главный виновник):** `invalidateOnCreatePage()` строил `newPage` без `temporaryExpiresAt`, и перестройка дерева (`buildTree`→`mergeRootTrees`) затирала оптимистичный/сокетный маркер на `undefined`. Добавлено поле. - **Другие клиенты / гонка вставки:** `temporaryExpiresAt` проброшен через серверный broadcast `addTreeNode` (`TreeNodeSnapshot`, `PAGE_CREATED` в `page.repo`, `broadcastPageCreated`); idempotency-guard `handleCreate` дополняет дедлайн, если broadcast выиграл гонку. ### 2. Кнопки «New note» / «New temporary note» не влезали на мобиле В `new-note-button.tsx` и `space-create-note-buttons.tsx`: side-by-side `Group grow` → вертикальный `Stack`, обе `fullWidth`, временная кнопка `color="orange"`, обычная `gray` (чуть разные цвета). Текст «New temporary note» больше не обрезается. ## Проверка (Playwright, живой стенд) - Маркер: create→есть; make-permanent (баннер)→нет; make-temporary (меню)→есть; make-permanent (меню)→нет — **всё без reload, PASS**. - Кнопки (390×844): обе full-width (w=370), стопкой, текст не обрезан, цвета разные (gray `rgb(73,80,87)` vs orange `rgb(253,126,20)`). - `tsc` exit 0; `tree-socket-reducers` 16/16, `ws-tree.service`+`page-ws.listener` 25/25. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-06-27 00:30:33 +03:00
fix(temporary-notes): live sidebar clock marker + stacked mobile create buttons 12ff76fb89 
Issue 1 — the sidebar tree's temporary-note clock marker did not appear/
disappear until a page reload when a note's temporary state changed.

- Make/unmake permanent from the page header menu and the in-page banner went
  through syncTemporaryExpiresInCache(), which patched the page query cache but
  never touched treeDataAtom, so the sidebar node kept its stale
  temporaryExpiresAt. Patch the tree node there too (via jotai's default store),
  so the marker updates without a reload.
- Creating a note as temporary showed no marker until reload: the create flow's
  cache write (invalidateOnCreatePage) omitted temporaryExpiresAt, so the tree
  rebuild (buildTree -> mergeRootTrees) overwrote the optimistic/socket node's
  marker with undefined. Carry temporaryExpiresAt in that cached entry.
- Thread temporaryExpiresAt through the server addTreeNode broadcast (PAGE_CREATED
  snapshot -> TreeNodeSnapshot -> broadcastPageCreated) so OTHER clients watching
  the space also render the marker immediately, and harden handleCreate's
  idempotency guard to patch the deadline if the broadcast won the insert race.

Issue 2 — the home and space-overview "New note" / "New temporary note" buttons
sat side-by-side and the temporary label clipped on narrow mobile widths. Lay
them out full-width, stacked vertically, and tint the temporary button orange
(matching the clock marker + banner) while the regular one stays neutral gray.

Tests: extend tree-socket-reducers.test.ts (addTreeNode carries
temporaryExpiresAt). Verified live with Playwright: marker appears on create and
toggles both ways with no reload; mobile buttons are stacked, full-width,
unclipped, and differently colored.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vvzvlad commented 2026-06-27 00:43:05 +03:00
Owner
Copy Link

Code review — multi-aspect (8 аспектов параллельно)

Вердикт: Approve with comments (одобрить с замечаниями).

Изменение узкое, корректное и хорошо прокомментированное. 6 из 8 аспектов — чисто (security, stability, conventions, documentation, regressions, simplification). Логику гонки независимо подтвердили рецензенты по stability и regressions: патч только добавляет дедлайн и никогда не затирает существующий. Блокирующих находок нет. Единственное замечание — ядро багфикса (ветка race-guard в handleCreate) уезжает без юнит-теста, хотя ручная Playwright-проверка и тест socket-пути в PR есть.

Область ревью: git diff --merge-base develop (база develop @ e9409e24, голова fix/temporary-notes-ui @ 12ff76fb), 10 файлов, +92/−14. Лок-файлов/генерёжки нет.

Must fix before merge

  • [warning][test coverage] Покрыть тестом ветку race-guard в handleCreate — apps/client/src/features/page/tree/hooks/use-tree-mutation.ts:176-191
    Это ядро багфикса, но оно не покрыто ни одним тестом. У ветки два различимых поведения, которые ничто не проверяет: (а) когда серверный addTreeNode-броадкаст выиграл гонку и вставил узел без temporaryExpiresAt — дописать дедлайн из ответа на создание (newNode.temporaryExpiresAt && !existing.temporaryExpiresAt → treeModel.update); (б) явный guard «НЕ затирать», когда у существующего узла дедлайн уже есть. Существующий блок «handleCreate optimistic-insert idempotency» в tree-model.test.ts воспроизводит лишь старую логику find-then-skip и новую ветку не включает; единственный добавленный тест в tree-socket-reducers.test.ts покрывает другой путь (applyAddTreeNode). Условие «не затирать» — ровно тот guard, который будущий рефакторинг может молча сломать без срабатывания тестов.
    Fix: добавить тест (по образцу блока idempotency в tree-model.test.ts), прогоняющий обе под-ветки: (1) узел уже есть без temporaryExpiresAt + ответ несёт дедлайн → у узла появляется temporaryExpiresAt; (2) узел уже есть с temporaryExpiresAt → значение не перезаписывается, prev возвращается по ссылке.

  • [suggestion][test coverage] Расширить тест broadcastPageCreated, проверив перенос temporaryExpiresAt (и нормализацию ?? null) — apps/server/src/ws/ws-tree.service.spec.ts:69
    Новая строка temporaryExpiresAt: page.temporaryExpiresAt ?? null в ws-tree.service.ts:41-44 — серверная половина фикса, но существующий тест проверяет data через expect.objectContaining, а фикстура snapshot поля не содержит. Именно поэтому сьют «проходит 25/25», а новое поле и нормализация absent→null остаются непроверенными.
    Fix: добавить в фикстуру вариант с temporaryExpiresAt и проверить, что броадкаст его несёт; добавить второй кейс без поля и проверить data.temporaryExpiresAt === null, зафиксировав ?? null.

  • [suggestion][test coverage] Покрыть патч treeDataAtom в syncTemporaryExpiresInCache — apps/client/src/features/page-embed/queries/page-embed-query.ts:37-49
    Новый патч содержит ветвление: чтение treeDataAtom из дефолтного стора jotai, treeModel.update, запись только при изменении ссылки (if (nextTree !== prevTree) store.set(...)). Оба пути (страница есть/нет в дереве) — новое поведение, движущее «маркер без релоада», теста у функции нет. Легче предыдущего, т.к. сам treeModel.update хорошо покрыт.
    Fix: vitest на оба случая — узел в дереве получает temporaryExpiresAt; id отсутствует в дереве → значение атома остаётся той же ссылкой.

Test coverage

Новой логики совсем без тестов по существу две единицы: ветка race-guard в handleCreate (warning) и патч treeDataAtom в syncTemporaryExpiresInCache (suggestion). Поле-«плимбинг» (invalidateOnCreatePage, снапшот в page.repo.ts, поле в TreeNodeSnapshot) — однострочный перенос без ветвлений, тестов не требует. UI-изменения (Group→Stack, fullWidth, color) презентационные, ниже планки тестирования проекта. Добавленный тест на applyAddTreeNode — корректный и соразмерный для покрытого им socket-пути.

Architecture & design

1. Клиентский tree-sync: пять мест ручной сборки IPage → SpaceTreeNode.
Один и тот же набор полей вручную копируется в 5 местах: buildTree (utils.ts — канонический маппер, уже несёт temporaryExpiresAt), оптимистичный узел в handleCreate, newPage в invalidateOnCreatePage (этот PR добавил поле), nodeData в useRestorePageMutation (не несёт) и treeNodeData в handleDuplicatePage (не несёт). Это структурная причина класса багов «забыли пронести поле X — маркер пропадает до релоада». Пути restore/duplicate не сломаны сегодня только по совпадению: restore явно сбрасывает temporaryExpiresAt: null на сервере, а duplicate дедлайн не копирует, так что обе делают заметки постоянными.
единый pageToTreeNode(page) маппер*, через который ходят buildTree, handleCreate, restore и duplicate (effort: small). Pros: новое поле добавляется в одном месте, убирает 4 почти-дубликата, закрывает корень класса багов. Cons: у площадок есть мелкие отличия (name: title || "Untitled", canEdit: true) → «map + overrides», а invalidateOnCreatePage строит запись IPage-кэша, не SpaceTreeNode.

2. Серверный снапшот броадкаста: два литерала против TreeNodeSnapshot.
Узел addTreeNode собирается двумя ручными литералами — в page.repo.ts и ws-tree.service.ts, оба обязаны соответствовать интерфейсу TreeNodeSnapshot. Мягче клиентской: литерала всего два и они уже делят общий тип (TS ловит пропуск обязательного поля; слепое пятно — только опциональные).
хелпер toTreeNodeSnapshot(page)*, вызываемый из обоих мест (effort: small). Cons: площадки чуть различаются (hasChildren: false, children: []) → нужны overrides.

## Code review — multi-aspect (8 аспектов параллельно) **Вердикт: Approve with comments (одобрить с замечаниями).** Изменение узкое, корректное и хорошо прокомментированное. 6 из 8 аспектов — чисто (security, stability, conventions, documentation, regressions, simplification). Логику гонки независимо подтвердили рецензенты по stability и regressions: патч только *добавляет* дедлайн и никогда не затирает существующий. Блокирующих находок нет. Единственное замечание — ядро багфикса (ветка race-guard в `handleCreate`) уезжает без юнит-теста, хотя ручная Playwright-проверка и тест socket-пути в PR есть. Область ревью: `git diff --merge-base develop` (база `develop` @ e9409e24, голова `fix/temporary-notes-ui` @ 12ff76fb), 10 файлов, +92/−14. Лок-файлов/генерёжки нет. ### Must fix before merge - **[warning][test coverage] Покрыть тестом ветку race-guard в `handleCreate`** — `apps/client/src/features/page/tree/hooks/use-tree-mutation.ts:176-191` Это ядро багфикса, но оно не покрыто ни одним тестом. У ветки два различимых поведения, которые ничто не проверяет: (а) когда серверный `addTreeNode`-броадкаст выиграл гонку и вставил узел *без* `temporaryExpiresAt` — дописать дедлайн из ответа на создание (`newNode.temporaryExpiresAt && !existing.temporaryExpiresAt` → `treeModel.update`); (б) явный guard «НЕ затирать», когда у существующего узла дедлайн уже есть. Существующий блок «handleCreate optimistic-insert idempotency» в `tree-model.test.ts` воспроизводит лишь старую логику find-then-skip и новую ветку не включает; единственный добавленный тест в `tree-socket-reducers.test.ts` покрывает другой путь (`applyAddTreeNode`). Условие «не затирать» — ровно тот guard, который будущий рефакторинг может молча сломать без срабатывания тестов. **Fix:** добавить тест (по образцу блока idempotency в `tree-model.test.ts`), прогоняющий обе под-ветки: (1) узел уже есть без `temporaryExpiresAt` + ответ несёт дедлайн → у узла появляется `temporaryExpiresAt`; (2) узел уже есть *с* `temporaryExpiresAt` → значение не перезаписывается, `prev` возвращается по ссылке. - **[suggestion][test coverage] Расширить тест `broadcastPageCreated`, проверив перенос `temporaryExpiresAt` (и нормализацию `?? null`)** — `apps/server/src/ws/ws-tree.service.spec.ts:69` Новая строка `temporaryExpiresAt: page.temporaryExpiresAt ?? null` в `ws-tree.service.ts:41-44` — серверная половина фикса, но существующий тест проверяет `data` через `expect.objectContaining`, а фикстура `snapshot` поля не содержит. Именно поэтому сьют «проходит 25/25», а новое поле и нормализация absent→null остаются непроверенными. **Fix:** добавить в фикстуру вариант с `temporaryExpiresAt` и проверить, что броадкаст его несёт; добавить второй кейс без поля и проверить `data.temporaryExpiresAt === null`, зафиксировав `?? null`. - **[suggestion][test coverage] Покрыть патч `treeDataAtom` в `syncTemporaryExpiresInCache`** — `apps/client/src/features/page-embed/queries/page-embed-query.ts:37-49` Новый патч содержит ветвление: чтение `treeDataAtom` из дефолтного стора jotai, `treeModel.update`, запись только при изменении ссылки (`if (nextTree !== prevTree) store.set(...)`). Оба пути (страница есть/нет в дереве) — новое поведение, движущее «маркер без релоада», теста у функции нет. Легче предыдущего, т.к. сам `treeModel.update` хорошо покрыт. **Fix:** vitest на оба случая — узел в дереве получает `temporaryExpiresAt`; id отсутствует в дереве → значение атома остаётся той же ссылкой. ### Test coverage Новой логики совсем без тестов по существу две единицы: ветка race-guard в `handleCreate` (warning) и патч `treeDataAtom` в `syncTemporaryExpiresInCache` (suggestion). Поле-«плимбинг» (`invalidateOnCreatePage`, снапшот в `page.repo.ts`, поле в `TreeNodeSnapshot`) — однострочный перенос без ветвлений, тестов не требует. UI-изменения (`Group`→`Stack`, `fullWidth`, `color`) презентационные, ниже планки тестирования проекта. Добавленный тест на `applyAddTreeNode` — корректный и соразмерный для покрытого им socket-пути. ### Architecture & design **1. Клиентский tree-sync: пять мест ручной сборки `IPage → SpaceTreeNode`.** Один и тот же набор полей вручную копируется в 5 местах: `buildTree` (utils.ts — канонический маппер, уже несёт `temporaryExpiresAt`), оптимистичный узел в `handleCreate`, `newPage` в `invalidateOnCreatePage` (этот PR добавил поле), `nodeData` в `useRestorePageMutation` (**не** несёт) и `treeNodeData` в `handleDuplicatePage` (**не** несёт). Это структурная причина класса багов «забыли пронести поле X — маркер пропадает до релоада». Пути restore/duplicate не сломаны сегодня только по совпадению: restore явно сбрасывает `temporaryExpiresAt: null` на сервере, а duplicate дедлайн не копирует, так что обе делают заметки постоянными. единый `pageToTreeNode(page)` маппер*, через который ходят `buildTree`, `handleCreate`, restore и duplicate (effort: small). Pros: новое поле добавляется в одном месте, убирает 4 почти-дубликата, закрывает корень класса багов. Cons: у площадок есть мелкие отличия (`name: title || "Untitled"`, `canEdit: true`) → «map + overrides», а `invalidateOnCreatePage` строит запись IPage-кэша, не `SpaceTreeNode`. **2. Серверный снапшот броадкаста: два литерала против `TreeNodeSnapshot`.** Узел `addTreeNode` собирается двумя ручными литералами — в `page.repo.ts` и `ws-tree.service.ts`, оба обязаны соответствовать интерфейсу `TreeNodeSnapshot`. Мягче клиентской: литерала всего два и они уже делят общий тип (TS ловит пропуск *обязательного* поля; слепое пятно — только опциональные). хелпер `toTreeNodeSnapshot(page)`*, вызываемый из обоих мест (effort: small). Cons: площадки чуть различаются (`hasChildren: false`, `children: []`) → нужны overrides.
Ghost added 1 commit 2026-06-27 00:58:57 +03:00
test(temporary-notes): cover the create race-guard, broadcast deadline + cache patch; unify page->tree-node mappers d181b5c4ff 
Address review comment 2159 on the temporary-notes UI work.

Tests:
- tree-model: cover handleCreate's race-guard temporaryExpiresAt patch — (a)
  server node inserted WITHOUT a deadline + create response carries one => node
  gains the deadline; (b) node already has a deadline => not overwritten, prev
  returned by reference.
- ws-tree.service.spec: broadcastPageCreated now asserts the deadline is carried
  when present and pinned to null (`?? null`) when absent.
- page-embed-query (new spec): syncTemporaryExpiresInCache patches the in-tree
  node's temporaryExpiresAt, and leaves the atom value at the same reference when
  the id is absent from the loaded tree (no write).

Refactor (closes the drift bug-class at the root):
- Client: extract one canonical pageToTreeNode(page, overrides) mapper in
  tree/utils and route buildTree, handleCreate's optimistic insert, the restore
  mutation and the duplicate handler through it. Restore stays permanent (server
  nulls temporaryExpiresAt) and duplicate stays permanent (server arms no timer)
  — both now reflect the server without a reload, where before they dropped the
  field entirely.
- Server: extract one toTreeNodeSnapshot(page) helper called by both the
  PAGE_CREATED event enrichment (page.repo) and the addTreeNode broadcast
  (ws-tree.service), so the optional temporaryExpiresAt can't drift between the
  two literals.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vvzvlad merged commit 109ab10fc5 into develop 2026-06-27 01:39:10 +03:00
vvzvlad deleted branch fix/temporary-notes-ui 2026-06-27 01:48:04 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#225
Delete Branch "fix/temporary-notes-ui"

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?