diff --git a/docs/plans/2026-06-05-project-completion.md b/docs/plans/2026-06-05-project-completion.md deleted file mode 100644 index 7b430cedb9..0000000000 --- a/docs/plans/2026-06-05-project-completion.md +++ /dev/null @@ -1,176 +0,0 @@ -# Project completion experience - -**Date:** 2026-06-05 (rev. after multi-agent review) -**Status:** ✅ Implemented on `feat/completing-projects-48eeb4` — state layer, stats util, service, celebration + resolve dialogs, menu wiring, trophy badge on the archived page, translations, wiki. Verified: unit tests (reducer 34, selectors 4, stats 6) + existing specs (menu 10, service 12, page 5) green; dev build exit 0; eslint + int:test clean. -**Branch:** `feat/completing-projects-48eeb4` -**Scope:** Give projects a rewarding "done" state. The **append/merge** half ("fold a project's tasks into another") was split out to issue **#8032** after review (YAGNI-adjacent + materially heavier than first scoped). - -## Problem - -Two real friction points drive this: - -1. **A complex chunk of work has no good home.** Today the options are a heavyweight permanent project, or a "mega task" with an ever-growing subtask list. The mega task feels bad: the parent is one perpetually-unchecked item hanging over you, and progress is buried inside it instead of visible as moving pieces. -2. **Finishing big work isn't rewarding.** The only end-state for a project is `isArchived` (`project.model.ts:15`, even marked `// TODO remove maybe`). Archiving is _"shove it out of sight"_ — semantically the opposite of celebrating a finish. - -### Key insight — no nesting, no new entity - -Both pains are about **a container you can _finish_**, not about hierarchy. Nesting projects-in-projects works against pain #2 (a sub-project inside a never-ending parent still leaves the parent hanging) and drags in aggregation/cascade/sync cost for little benefit. The lightweight "dump space" people want **is just a regular `Project`** with a missing lifecycle operation: **complete** it → reward + a place to look back. Grouping of related projects is already covered by the menu-tree **folders**; small breakdowns by nested subtasks. So this plan adds **one operation on the existing entity**, not a new type. - -## Non-goals - -- No nested/parent-child projects, no roll-up of time/progress from children. -- No new "mini project" entity or UI concept. -- No append/merge (→ #8032). -- No change to how archiving itself works — completion piggybacks on the `isArchived` flag for menu-hiding, but `isDone` stays a distinct flag so a celebrated finish ≠ a quiet archive. - -## ⚠️ Correction from review — what archiving actually does - -The first draft assumed completing→auto-archiving would run the `ArchiveOperationHandler` and move done tasks into the archive store. **That is false** and was verified against source: - -- `archiveProject` is a pure `isArchived: true` flag flip (`project.reducer.ts:166-177`); the project archive _effects_ are commented out (`project.effects.ts:72`, "CURRENTLY NOT IMPLEMENTED"). -- `archiveProject` is **not** in `ARCHIVE_AFFECTING_ACTION_TYPES` (`archive-operation-handler.service.ts:40-54`). Only `moveToArchive` / `deleteProject` etc. move tasks to IndexedDB. - -**Implications that shape this plan:** - -- A completed project's tasks **stay live** in the NgRx store. Archiving only hides the project from the active menu (via the `!isArchived` filters). -- ⇒ **Stats can be computed live** from the still-live tasks; no snapshot needed (decision below). -- ⇒ **Reopen is trivially safe** — tasks never left, so un-archiving fully restores the project. No archive-restore logic. -- ⚠️ Done tasks of a completed project remain visible in worklog/search/metrics. Acceptable (they're history), but noted. - -## Resolved decisions - -| # | Decision | Resolution | -| --- | --- | --- | -| Q1 | Auto-archive on complete | **Yes** — `completeProject` also sets `isArchived: true` ("complete and out of the way"). This is a flag flip only — menu-hiding, **no** task cleanup (see correction above). | -| Q2 | Unfinished tasks | **Prompt** (a plain confirm), default **Move to Inbox** with the count shown; plus "Mark them done" / "Cancel". | -| Q3 | Stats live vs. snapshot | **Compute live** — no `completionStats` field. The "mandatory snapshot" reason was based on the false archive premise. | -| Q4 | Completion surface | **Split:** `DialogConfirm` for the unfinished-task resolve step, then a **separate** celebration component. | -| Q5 | Trophy view | **No new page.** Add a "Completed on X" badge + live stats + **Reopen** to completed rows of the existing archived-projects page, and improve that page. | -| Q6 | Append/merge | **Deferred → #8032.** | - -### Done vs. Archived — selector wiring (review-critical) - -`isDone` ⇒ also `isArchived`. **Do NOT narrow `selectArchivedProjects`.** It feeds task-list filtering — `selectArchivedProjectIds` is consumed by `task.selectors.ts:104,181` (`selectTaskEntitiesInActiveProjects`, `selectAllTasksInActiveProjects` → Today/Overdue). Narrowing it to `isArchived && !isDone` would **leak completed projects' tasks back into Today/Overdue** (incl. done tasks still carrying `dueDay`/`dueWithTime`, Rule #5). Instead: - -- **Keep** `selectArchivedProjects` = `isArchived` (covers completed too) → task filtering + menu-hiding stay correct, unchanged. -- **Add** `selectCompletedProjects` = `isDone` → highlights/filters completed rows on the trophy page. -- **Add** `selectPlainArchivedProjects` = `isArchived && !isDone` → page-only, if we want to visually separate "finished" from "shelved". -- **Reopen** clears `isDone` + `doneOn` **and** `isArchived: false` (returns to the active menu). - -### Data model - -Add to `ProjectBasicCfg` (`src/app/features/project/project.model.ts`), mirroring `Task` (`isDone` + `doneOn`): - -```ts -export interface ProjectBasicCfg { - title: string; - isArchived?: boolean; - isDone?: boolean; // NEW — completed (also implies isArchived) - doneOn?: number | null; // NEW — completion timestamp (ms) - isHiddenFromMenu?: boolean; - // ... -} -``` - -Both new fields **optional** → forward-compatible for sync (typia accepts missing optional fields; only new _required_ fields / literal-union members break old clients; verified `createValidate` does not reject excess props). Default in `DEFAULT_PROJECT` (`project.const.ts:11`): `isDone: false, doneOn: null`. `INBOX_PROJECT` can never be completed (guard like archive). **plugin-api note:** `ProjectCopy` extends the plugin-api `Project`; the new fields live on the app-side `ProjectBasicCfg` and compile fine without touching `packages/plugin-api`. Plugins won't see completion state — intentional (matches non-goals); revisit only if a plugin needs it. - -### Sync-correctness (CLAUDE.md rules) - -- **`completeProject` / `reopenProject` are plain project Updates** (`OpType.Update`, `entityType:'PROJECT'`), modeled exactly like `archiveProject` (`project.actions.ts:76-100`) → captured by the op-log capture effect automatically via `meta`. **Add `ActionType` enum entries** (`action-types.enum.ts`, section P) — the immutable wire format (review caught this omission). -- **Must NOT** be added to `ARCHIVE_AFFECTING_ACTION_TYPES`. -- **The celebration effect injects `LOCAL_ACTIONS`** (Rule #1) → a remote/replayed `completeProject` never pops a dialog / fires confetti on another device. -- **`doneOn` is computed at the call site** (via `DateService`) and passed as a prop — never `Date.now()` in the reducer (Rule #4). -- **LWW note (accept):** a concurrent remote `updateProject` (e.g. rename) vs local `completeProject` resolves by coarse whole-entity LWW — same as `archiveProject` today; completion has no archive-win protection, so it _can_ be lost to a concurrent unrelated edit. Not a regression; documented. - ---- - -## Implementation - -### 1. State + actions - -- `project.actions.ts`: add `completeProject({ id, doneOn })` and `reopenProject({ id })` (mirror archive, `OpType.Update`). Add matching `ActionType` enum entries. -- `project.reducer.ts` (next to archive cases `:166-189`): - - `completeProject` → `{ isDone: true, doneOn, isArchived: true }`. - - `reopenProject` → `{ isDone: false, doneOn: null, isArchived: false }`. - - Guard `INBOX_PROJECT`. -- `project.selectors.ts`: add `selectCompletedProjects` + `selectPlainArchivedProjects` (see selector wiring above). **Leave `selectArchivedProjects` unchanged.** -- `project.service.ts`: `complete(id)` / `reopen(id)` wrappers (mirror `archive()`/`unarchive()` `:145`). - -### 2. Completion flow - -Trigger: a "Complete project" item in the project context menu (`work-context-menu.component.{ts,html}:79-111`), beside Archive. Order/group it and add microcopy so "Complete" vs "Archive" is legible (both end up `isArchived`; only one celebrates). - -1. **Resolve unfinished tasks** (only if any undone tasks across `taskIds` + `backlogTaskIds`, incl. subtasks). Open a `DialogConfirmComponent`-style prompt showing **the count**, with: - - **Move to Inbox** _(default)_ — safe carry-forward. - - **Mark them done** — "close enough." - - **Cancel.** - - _Bulk mechanic:_ no bulk action exists today. For Part 1, loop the existing per-task action (`moveToOtherProject` / `updateTask isDone`) and apply the **Rule #6 flush** (`await new Promise(r => setTimeout(r, 0))`) after the loop. (A single bulk meta-reducer action is the cleaner upgrade — deferred; note the op-count.) -2. `ProjectService.complete(id)` dispatches `completeProject` (reducer sets done + archived). -3. **Celebrate** (section 3). -4. If the completed project was active, navigate to `/` (archive already does this; also clear any selected-task/detail-panel pointing at the now-hidden project — cf. recent fix `d44cb1138d`). -5. **Undo:** show a snack with **Undo** → `reopenProject` (accidental/instant-regret escape, since the project otherwise vanishes from the menu). - -### 3. Celebration (separate component) - -A small `ProjectCompleteCelebrationComponent` (dialog), reusing the layout language of `focus-mode/focus-mode-session-done` and the "summary-point" grid of `daily-summary`: - -- **Confetti** via `ConfettiService.createConfetti()` — gate on **both** `isDisableAnimations` and `isDisableCelebration` (no confetti → dialog still shows). -- "🎉 Project complete" + project title + the **stats grid** (section 4). -- Primary **Done**; secondary **View completed projects** → the archived page (trophy section). -- Reopen is offered via the post-complete snack (step 2.5) and on the trophy page, not here. - -### 4. Stats (computed live) - -Computed on demand for the celebration and the trophy rows, from the still-live tasks: - -- **Tasks done / total** — count project tasks (`taskIds` + `backlogTaskIds`; decide subtask inclusion, state it consistently) by `isDone`. -- **Hours worked** — sum `task.timeSpent` over the project's **parent** tasks only (a parent's `timeSpent` already includes subtasks — `task.reducer.util.ts:53-72`; summing both double-counts). Alternatively read `TimeTrackingState.project[projectId]`. -- **Days worked** — distinct `timeSpentOnDay` keys across tasks. -- **Finished in N days** — `startedOn`→`doneOn` calendar span. `startedOn` = earliest `timeSpentOnDay` key, fallback `project.created`. **This is the one stat that works with time-tracking off** — feature it. -- `worklog/util/get-time-spent-for-day.util.ts` aggregates per-day; reuse. - -**Degrade gracefully:** many users don't track time. When `timeSpent === 0`, **hide** hours/days rows (don't show "0h over 0 days" — demotivating). Drop "avg per day" (vanity, prone to "0.4h/day"). - -### 5. Trophy view (improve the archived page) - -Completed projects already land on `/archived-projects` (they're `isArchived`). Rather than a new page: - -- On completed rows (`selectCompletedProjects`), show a **trophy/badge + "Completed on `doneOn`"** + the live stats, and offer **Reopen** (`reopenProject`) instead of Unarchive. -- **Improve the page** generally (it's currently a bare list): clearer layout, the stat readout, sort by `doneOn`, and make it more discoverable (the celebration's "View completed projects" links here; consider a findable entry rather than only the visibility menu). -- Optionally use `selectPlainArchivedProjects` to visually separate "Finished" from "Shelved". - -### 6. Testing - -- Reducer: `completeProject` sets `isDone`+`doneOn`+`isArchived:true`; `reopenProject` clears all three; INBOX guarded. -- **Regression (review-critical):** completing a project keeps its (done, `dueDay`-carrying) tasks **out of** Today/Overdue — i.e. `selectArchivedProjects` still includes completed projects and the task-filtering selectors are unchanged. Add an explicit test. -- Selector: `selectCompletedProjects` = `isDone`; `selectPlainArchivedProjects` = `isArchived && !isDone`. -- Stats: live math — no double-count of parent+subtask time; `finished in N days` with time-tracking off. -- Effect: celebration effect uses `LOCAL_ACTIONS` (no confetti/dialog on replayed/remote `completeProject`). -- Translations: `en.json` only, via `T`. User-facing → update docs per `docs/documentation-guide.md`. - -## Risks - -- **Selector leak (mitigated):** the §"Done vs Archived" wiring + the regression test exist specifically to prevent completed tasks reappearing in Today/Overdue. Audit all `selectArchivedProjects`/`selectArchivedProjectIds` consumers (`project.service.ts:83`, `magic-nav-config.service.ts:85`, `archived-projects-page.component.ts:52`, `task.selectors.ts:104,181`, `task-repeat-cfg.selectors.ts:22`). -- **Discoverability:** auto-archive makes a completed project vanish instantly; the reward is a one-shot unless the trophy page is findable. Undo snack + an improved, reachable trophy page mitigate. -- **Inbox flood:** "Move to Inbox" on a big dump-space project can dump many tasks into Inbox — hence showing the count, and offering "Mark done". -- **Live-stat drift (accepted):** if tasks are later deleted/manually-archived, recomputed stats shift. Acceptable for a retrospective view; this is the cost of choosing live-compute over a snapshot. - -## Open items - -- Trophy-page improvement scope (how far to take the redesign). -- Unfinished-task default — confirm Inbox vs. mark-done after seeing it in use. -- Subtask inclusion in tasks-done count (product call). - -## Key files - -| Area | File | -| --- | --- | -| Model / defaults | `src/app/features/project/project.model.ts`, `project.const.ts` | -| Actions / reducer / selectors | `src/app/features/project/store/project.actions.ts`, `project.reducer.ts`, `project.selectors.ts`; `action-types.enum.ts` | -| Service | `src/app/features/project/project.service.ts` | -| Context menu / trigger | `src/app/core-ui/work-context-menu/work-context-menu.component.{ts,html}` | -| Trophy page | `src/app/pages/archived-projects-page/` (enhance) | -| Reward | `src/app/core/confetti/confetti.service.ts`; ref `features/focus-mode/focus-mode-session-done/`, `pages/daily-summary/` | -| Stats | `src/app/features/tasks/store/task.reducer.util.ts` (rollup caveat), `features/time-tracking/time-tracking.model.ts`, `features/worklog/util/get-time-spent-for-day.util.ts` | -| Resolve dialog | `src/app/ui/dialog-confirm/dialog-confirm.component.ts` | -| Append/merge (deferred) | issue **#8032** | diff --git a/docs/wiki/4.06-Project-View.md b/docs/wiki/4.06-Project-View.md index 73481e9d00..3a9bd90e57 100644 --- a/docs/wiki/4.06-Project-View.md +++ b/docs/wiki/4.06-Project-View.md @@ -75,7 +75,7 @@ A completed project becomes fully **dormant** — all its data is preserved, but - Has its repeating tasks suspended — no new instances are generated - Has its reminders suppressed — no notifications fire for its tasks -**Finding and reopening** completed projects: click the eye icon in the Projects section header to open the visibility menu, then navigate to **Archived projects**, where completed projects appear with a trophy badge and their completion date. Use the **Reopen** action there — or **Undo** on the snack shown right after completing — to clear the completed state and bring the project back to the sidebar with all tasks, repeating configs, and reminders active again. +**Finding and reopening** completed projects: click the eye icon in the Projects section header to open the visibility menu, then navigate to **Archived projects**, where completed projects appear with a trophy badge and their completion date. Use the **Reopen** action there to clear the completed state and bring the project back to the sidebar. ## Related diff --git a/packages/shared-schema/src/index.ts b/packages/shared-schema/src/index.ts index abc0ddeee1..6e1f56f25d 100644 --- a/packages/shared-schema/src/index.ts +++ b/packages/shared-schema/src/index.ts @@ -46,12 +46,14 @@ export { SUPER_SYNC_MAX_CLIENT_ID_LENGTH, SUPER_SYNC_MAX_OPS_PER_UPLOAD, SUPER_SYNC_MAX_ENTITY_IDS_PER_OP, + SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP, SUPER_SYNC_OP_TYPES, SUPER_SYNC_IMPORT_REASONS, SUPER_SYNC_SNAPSHOT_REASONS, SUPER_SYNC_SNAPSHOT_OP_TYPES, SuperSyncVectorClockSchema, SuperSyncClientIdSchema, + SuperSyncAffectedEntitySchema, SuperSyncOperationSchema, SuperSyncUploadOpsRequestSchema, SuperSyncDownloadOpsQuerySchema, diff --git a/packages/shared-schema/src/migration.types.ts b/packages/shared-schema/src/migration.types.ts index 0c1bf71383..b8f87dd9fc 100644 --- a/packages/shared-schema/src/migration.types.ts +++ b/packages/shared-schema/src/migration.types.ts @@ -21,10 +21,16 @@ export interface OperationLike { entityType: string; entityId?: string; entityIds?: string[]; + affectedEntities?: AffectedEntityLike[]; payload: unknown; schemaVersion: number; } +export interface AffectedEntityLike { + entityType: string; + entityId: string; +} + /** * Defines a schema migration from one version to the next. * Migrations must be pure functions with no external dependencies. diff --git a/packages/shared-schema/src/migrations/misc-to-tasks-settings-migration-v1-to-v2.ts b/packages/shared-schema/src/migrations/misc-to-tasks-settings-migration-v1-to-v2.ts index 9a3158ba86..cc9966a315 100644 --- a/packages/shared-schema/src/migrations/misc-to-tasks-settings-migration-v1-to-v2.ts +++ b/packages/shared-schema/src/migrations/misc-to-tasks-settings-migration-v1-to-v2.ts @@ -97,6 +97,7 @@ export const MiscToTasksSettingsMigration_v1v2: SchemaMigration = { result.push({ ...op, id: `${op.id}_misc`, + affectedEntities: [{ entityType: 'GLOBAL_CONFIG', entityId: 'misc' }], payload: buildPayload(miscCfg, 'misc'), }); } @@ -106,6 +107,7 @@ export const MiscToTasksSettingsMigration_v1v2: SchemaMigration = { ...op, id: `${op.id}_tasks`, entityId: 'tasks', + affectedEntities: [{ entityType: 'GLOBAL_CONFIG', entityId: 'tasks' }], payload: buildPayload(tasksCfg, 'tasks'), }); } diff --git a/packages/shared-schema/src/supersync-http-contract.ts b/packages/shared-schema/src/supersync-http-contract.ts index d06033ced2..5451fc0927 100644 --- a/packages/shared-schema/src/supersync-http-contract.ts +++ b/packages/shared-schema/src/supersync-http-contract.ts @@ -4,6 +4,7 @@ export const SUPER_SYNC_CLIENT_ID_REGEX = /^[a-zA-Z0-9_-]+$/; export const SUPER_SYNC_MAX_CLIENT_ID_LENGTH = 255; export const SUPER_SYNC_MAX_OPS_PER_UPLOAD = 100; export const SUPER_SYNC_MAX_ENTITY_IDS_PER_OP = 1000; +export const SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP = SUPER_SYNC_MAX_ENTITY_IDS_PER_OP; export const SUPER_SYNC_OP_TYPES = [ 'CRT', @@ -55,6 +56,11 @@ export const SuperSyncClientIdSchema = z 'clientId must be alphanumeric with underscores/hyphens only', ); +export const SuperSyncAffectedEntitySchema = z.object({ + entityType: z.string().min(1).max(255), + entityId: z.string().min(1).max(255), +}); + export const SuperSyncOperationSchema = z.object({ id: z.string().min(1).max(255), clientId: SuperSyncClientIdSchema, @@ -66,6 +72,10 @@ export const SuperSyncOperationSchema = z.object({ .array(z.string().max(255)) .max(SUPER_SYNC_MAX_ENTITY_IDS_PER_OP) .optional(), + affectedEntities: z + .array(SuperSyncAffectedEntitySchema) + .max(SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP) + .optional(), payload: z.unknown(), vectorClock: SuperSyncVectorClockSchema, timestamp: z.number(), diff --git a/packages/shared-schema/tests/migrations/misc-to-tasks-settings-migration-v1-to-v2.spec.ts b/packages/shared-schema/tests/migrations/misc-to-tasks-settings-migration-v1-to-v2.spec.ts index 82b3fb406b..3024cc4a03 100644 --- a/packages/shared-schema/tests/migrations/misc-to-tasks-settings-migration-v1-to-v2.spec.ts +++ b/packages/shared-schema/tests/migrations/misc-to-tasks-settings-migration-v1-to-v2.spec.ts @@ -144,6 +144,9 @@ describe('Migrate MiscConfig to TasksConfig', () => { const miscOp = resultArray.find((r) => r.entityId === 'misc'); expect(miscOp).toBeDefined(); expect(miscOp!.id).toBe('op_1_misc'); + expect(miscOp!.affectedEntities).toEqual([ + { entityType: 'GLOBAL_CONFIG', entityId: 'misc' }, + ]); expect(miscOp!.payload).toEqual({ isMinimizeToTray: false, }); @@ -152,6 +155,9 @@ describe('Migrate MiscConfig to TasksConfig', () => { const tasksOp = resultArray.find((r) => r.entityId === 'tasks'); expect(tasksOp).toBeDefined(); expect(tasksOp!.id).toBe('op_1_tasks'); + expect(tasksOp!.affectedEntities).toEqual([ + { entityType: 'GLOBAL_CONFIG', entityId: 'tasks' }, + ]); expect(tasksOp!.payload).toEqual({ isConfirmBeforeDelete: true, isMarkdownFormattingInNotesEnabled: false, // Inverted from isTurnOffMarkdown: true @@ -178,6 +184,9 @@ describe('Migrate MiscConfig to TasksConfig', () => { const singleOp = result as OperationLike; expect(singleOp.id).toBe('op_2_tasks'); expect(singleOp.entityId).toBe('tasks'); + expect(singleOp.affectedEntities).toEqual([ + { entityType: 'GLOBAL_CONFIG', entityId: 'tasks' }, + ]); expect(singleOp.payload).toEqual({ isAutoAddWorkedOnToToday: true, defaultProjectId: 'project_123', diff --git a/packages/shared-schema/tests/supersync-http-contract.spec.ts b/packages/shared-schema/tests/supersync-http-contract.spec.ts index ea47e5529a..a9ef85af17 100644 --- a/packages/shared-schema/tests/supersync-http-contract.spec.ts +++ b/packages/shared-schema/tests/supersync-http-contract.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP, SUPER_SYNC_MAX_ENTITY_IDS_PER_OP, SUPER_SYNC_MAX_OPS_PER_UPLOAD, SuperSyncDownloadOpsQuerySchema, @@ -35,6 +36,26 @@ describe('SuperSync HTTP contract schemas', () => { expect(SUPER_SYNC_MAX_OPS_PER_UPLOAD).toBe(100); }); + it('preserves affectedEntities during upload parsing', () => { + const parsed = SuperSyncUploadOpsRequestSchema.parse({ + ops: [ + { + ...createValidOperation(), + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], + }, + ], + clientId: 'client_1', + }); + + expect(parsed.ops[0].affectedEntities).toEqual([ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ]); + }); + it('preserves server request behavior by stripping unknown upload fields', () => { const parsed = SuperSyncUploadOpsRequestSchema.parse({ ops: [{ ...createValidOperation(), extraOpField: true }], @@ -63,6 +84,23 @@ describe('SuperSync HTTP contract schemas', () => { ).toThrow(); }); + it('caps affectedEntities per operation', () => { + expect(() => + SuperSyncUploadOpsRequestSchema.parse({ + ops: [ + { + ...createValidOperation(), + affectedEntities: Array.from( + { length: SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP + 1 }, + (_, i) => ({ entityType: 'TASK', entityId: `task-${i}` }), + ), + }, + ], + clientId: 'client_1', + }), + ).toThrow(); + }); + it('rejects invalid client IDs in upload requests', () => { expect(() => SuperSyncUploadOpsRequestSchema.parse({ diff --git a/packages/super-sync-server/prisma/migrations/20260606000000_add_operation_affected_entities/migration.sql b/packages/super-sync-server/prisma/migrations/20260606000000_add_operation_affected_entities/migration.sql new file mode 100644 index 0000000000..8266caba40 --- /dev/null +++ b/packages/super-sync-server/prisma/migrations/20260606000000_add_operation_affected_entities/migration.sql @@ -0,0 +1,118 @@ +CREATE TABLE "operation_affected_entities" ( + "id" BIGSERIAL PRIMARY KEY, + "operation_id" TEXT NOT NULL, + "user_id" INTEGER NOT NULL, + "entity_type" TEXT NOT NULL, + "entity_id" TEXT NOT NULL, + "server_seq" INTEGER NOT NULL, + CONSTRAINT "operation_affected_entities_operation_id_fkey" + FOREIGN KEY ("operation_id") REFERENCES "operations"("id") ON DELETE CASCADE ON UPDATE CASCADE +); + +CREATE UNIQUE INDEX "operation_affected_entities_operation_id_entity_type_entity_id_key" + ON "operation_affected_entities"("operation_id", "entity_type", "entity_id"); + +CREATE INDEX "operation_affected_entities_user_id_entity_type_entity_id_server_seq_idx" + ON "operation_affected_entities"("user_id", "entity_type", "entity_id", "server_seq"); + +INSERT INTO "operation_affected_entities" + ("operation_id", "user_id", "entity_type", "entity_id", "server_seq") +SELECT "id", "user_id", "entity_type", "entity_id", "server_seq" +FROM "operations" +WHERE "entity_id" IS NOT NULL +ON CONFLICT DO NOTHING; + +WITH completion_payloads AS ( + SELECT + "id", + "user_id", + "server_seq", + COALESCE("payload"->'actionPayload', "payload") AS "action_payload" + FROM "operations" + WHERE "action_type" IN ('[Task Shared] completeProject', '[Project] Complete Project') +), +completion_refs AS ( + SELECT + "id", + "user_id", + "server_seq", + CASE + WHEN jsonb_typeof("action_payload"->'taskIdsToMarkDone') = 'array' + THEN "action_payload"->'taskIdsToMarkDone' + ELSE '[]'::jsonb + END AS "task_ids_to_mark_done", + CASE + WHEN jsonb_typeof("action_payload"->'topLevelTaskIdsToMoveToInbox') = 'array' + THEN "action_payload"->'topLevelTaskIdsToMoveToInbox' + ELSE '[]'::jsonb + END AS "top_level_task_ids_to_move_to_inbox", + CASE + WHEN jsonb_typeof("action_payload"->'taskIdsToMoveToInbox') = 'array' + THEN "action_payload"->'taskIdsToMoveToInbox' + ELSE '[]'::jsonb + END AS "task_ids_to_move_to_inbox", + CASE + WHEN jsonb_typeof("action_payload"->'taskIdsToMarkUndone') = 'array' + THEN "action_payload"->'taskIdsToMarkUndone' + ELSE '[]'::jsonb + END AS "task_ids_to_mark_undone" + FROM completion_payloads +) +INSERT INTO "operation_affected_entities" + ("operation_id", "user_id", "entity_type", "entity_id", "server_seq") +SELECT + "id", + "user_id", + 'TASK', + task_refs."task_id", + "server_seq" +FROM completion_refs, +LATERAL jsonb_array_elements_text( + "task_ids_to_mark_done" || + "top_level_task_ids_to_move_to_inbox" || + "task_ids_to_move_to_inbox" || + "task_ids_to_mark_undone" +) AS task_refs("task_id") +ON CONFLICT DO NOTHING; + +WITH completion_payloads AS ( + SELECT + "id", + "user_id", + "server_seq", + COALESCE("payload"->'actionPayload', "payload") AS "action_payload" + FROM "operations" + WHERE "action_type" IN ('[Task Shared] completeProject', '[Project] Complete Project') +), +completion_move_refs AS ( + SELECT + "id", + "user_id", + "server_seq", + CASE + WHEN jsonb_typeof("action_payload"->'topLevelTaskIdsToMoveToInbox') = 'array' + THEN jsonb_array_length("action_payload"->'topLevelTaskIdsToMoveToInbox') + ELSE 0 + END AS "top_level_move_count", + CASE + WHEN jsonb_typeof("action_payload"->'taskIdsToMoveToInbox') = 'array' + THEN jsonb_array_length("action_payload"->'taskIdsToMoveToInbox') + ELSE 0 + END AS "move_count", + CASE + WHEN jsonb_typeof("action_payload"->'taskIdsToMarkDone') = 'array' + THEN jsonb_array_length("action_payload"->'taskIdsToMarkDone') + ELSE 0 + END AS "mark_done_count" + FROM completion_payloads +) +INSERT INTO "operation_affected_entities" + ("operation_id", "user_id", "entity_type", "entity_id", "server_seq") +SELECT "id", "user_id", 'PROJECT', 'INBOX_PROJECT', "server_seq" +FROM completion_move_refs +WHERE "top_level_move_count" > 0 OR "move_count" > 0 +UNION ALL +SELECT "id", "user_id", 'TAG', 'TODAY', "server_seq" +FROM completion_move_refs +WHERE "mark_done_count" > 0 +ON CONFLICT DO NOTHING; diff --git a/packages/super-sync-server/prisma/schema.prisma b/packages/super-sync-server/prisma/schema.prisma index d2bbc7e361..f748daae7d 100644 --- a/packages/super-sync-server/prisma/schema.prisma +++ b/packages/super-sync-server/prisma/schema.prisma @@ -79,6 +79,7 @@ model Operation { syncImportReason String? @map("sync_import_reason") user User @relation(fields: [userId], references: [id], onDelete: Cascade) + affectedEntities OperationAffectedEntity[] @@unique([userId, serverSeq]) @@index([userId, entityType, entityId, serverSeq]) @@ -88,6 +89,21 @@ model Operation { @@map("operations") } +model OperationAffectedEntity { + id BigInt @id @default(autoincrement()) + operationId String @map("operation_id") + userId Int @map("user_id") + entityType String @map("entity_type") + entityId String @map("entity_id") + serverSeq Int @map("server_seq") + + operation Operation @relation(fields: [operationId], references: [id], onDelete: Cascade) + + @@unique([operationId, entityType, entityId]) + @@index([userId, entityType, entityId, serverSeq]) + @@map("operation_affected_entities") +} + model UserSyncState { userId Int @id @map("user_id") lastSeq Int @default(0) @map("last_seq") diff --git a/packages/super-sync-server/src/sync/conflict.ts b/packages/super-sync-server/src/sync/conflict.ts index 44ff53a92f..ca0b297c62 100644 --- a/packages/super-sync-server/src/sync/conflict.ts +++ b/packages/super-sync-server/src/sync/conflict.ts @@ -1,4 +1,4 @@ -import { Prisma } from '@prisma/client'; +import type { Prisma } from '@prisma/client'; import { Logger } from '../logger'; import { BatchUploadCandidate, @@ -14,6 +14,27 @@ import { limitVectorClockSize, } from './sync.types'; +export interface EntityConflictPair { + entityType: string; + entityId: string; +} + +type RawQueryTx = Prisma.TransactionClient & { + $queryRawUnsafe?: (query: string, ...values: unknown[]) => Promise; +}; + +const toTemplateStringsArray = (query: string): TemplateStringsArray => + Object.assign([query], { raw: [query] }) as unknown as TemplateStringsArray; + +const queryRawWithBoundParams = ( + tx: RawQueryTx, + query: string, + ...values: unknown[] +): Promise => + tx.$queryRawUnsafe + ? tx.$queryRawUnsafe(query, ...values) + : tx.$queryRaw(toTemplateStringsArray(query), ...values); + /** * Check if an incoming operation conflicts with existing operations. * Returns conflict info if a concurrent modification is detected. @@ -32,25 +53,51 @@ export const detectConflict = async ( return { hasConflict: false }; } - // Build list of entity IDs to check for conflicts. - // Operations may have either entityId (singular) or entityIds (batch operations). - const rawEntityIdsToCheck = op.entityIds?.length - ? op.entityIds - : op.entityId - ? [op.entityId] - : []; + const entityPairsToCheck = getConflictEntityPairs(op); // Skip if no entity IDs (can't have entity-level conflicts) - if (rawEntityIdsToCheck.length === 0) { + if (entityPairsToCheck.length === 0) { return { hasConflict: false }; } - if (rawEntityIdsToCheck.length === 1) { - return detectConflictForEntity(userId, op, rawEntityIdsToCheck[0], tx); + if (entityPairsToCheck.length === 1) { + const [entity] = entityPairsToCheck; + return detectConflictForEntity(userId, op, entity.entityType, entity.entityId, tx); } - const entityIdsToCheck = Array.from(new Set(rawEntityIdsToCheck)); - return detectConflictForEntities(userId, op, entityIdsToCheck, tx); + return detectConflictForEntityPairs(userId, op, entityPairsToCheck, tx); +}; + +export const detectConflictForEntityPairs = async ( + userId: number, + op: Operation, + entityPairsToCheck: EntityConflictPair[], + tx: Prisma.TransactionClient, +): Promise => { + const latestOpByEntity = await prefetchLatestEntityOpsForBatch( + userId, + entityPairsToCheck, + tx, + ); + + for (const entity of entityPairsToCheck) { + const existingOp = latestOpByEntity.get( + getEntityConflictKey(entity.entityType, entity.entityId), + ); + if (!existingOp) continue; + + const conflictResult = resolveConflictForExistingOp( + op, + entity.entityType, + entity.entityId, + existingOp, + ); + if (conflictResult.hasConflict) { + return conflictResult; + } + } + + return { hasConflict: false }; }; export const detectConflictForEntities = async ( @@ -68,17 +115,27 @@ export const detectConflictForEntities = async ( start, start + CONFLICT_DETECTION_ENTITY_BATCH_SIZE, ); - const latestOps = await tx.$queryRaw` + const entityIdPlaceholders = batchEntityIds + .map((_, index) => `$${index + 3}`) + .join(', '); + + const latestOps = await queryRawWithBoundParams( + tx, + ` SELECT DISTINCT ON (entity_id) entity_id AS "entityId", client_id AS "clientId", vector_clock AS "vectorClock" FROM operations - WHERE user_id = ${userId} - AND entity_type = ${op.entityType} - AND entity_id IN (${Prisma.join(batchEntityIds)}) + WHERE user_id = $1 + AND entity_type = $2 + AND entity_id IN (${entityIdPlaceholders}) ORDER BY entity_id, server_seq DESC - `; + `, + userId, + op.entityType, + ...batchEntityIds, + ); const latestOpByEntityId = new Map(); for (const latestOp of latestOps) { @@ -89,7 +146,12 @@ export const detectConflictForEntities = async ( const existingOp = latestOpByEntityId.get(entityId); if (!existingOp) continue; - const conflictResult = resolveConflictForExistingOp(op, entityId, existingOp); + const conflictResult = resolveConflictForExistingOp( + op, + op.entityType, + entityId, + existingOp, + ); if (conflictResult.hasConflict) { return conflictResult; } @@ -101,6 +163,7 @@ export const detectConflictForEntities = async ( export const resolveConflictForExistingOp = ( op: Operation, + entityType: string, entityId: string, existingOp: { clientId: string; vectorClock: unknown }, ): ConflictResult => { @@ -127,7 +190,7 @@ export const resolveConflictForExistingOp = ( return { hasConflict: true, conflictType: 'equal_different_client', - reason: `Equal vector clocks from different clients for ${op.entityType}:${entityId} (client ${op.clientId} vs ${existingOp.clientId})`, + reason: `Equal vector clocks from different clients for ${entityType}:${entityId} (client ${op.clientId} vs ${existingOp.clientId})`, existingClock, }; } @@ -137,7 +200,7 @@ export const resolveConflictForExistingOp = ( return { hasConflict: true, conflictType: 'concurrent', - reason: `Concurrent modification detected for ${op.entityType}:${entityId}`, + reason: `Concurrent modification detected for ${entityType}:${entityId}`, existingClock, }; } @@ -147,7 +210,7 @@ export const resolveConflictForExistingOp = ( return { hasConflict: true, conflictType: 'superseded', - reason: `Superseded operation: server has newer version of ${op.entityType}:${entityId}`, + reason: `Superseded operation: server has newer version of ${entityType}:${entityId}`, existingClock, }; } @@ -157,7 +220,7 @@ export const resolveConflictForExistingOp = ( return { hasConflict: true, conflictType: 'unknown', - reason: `Unknown vector clock comparison result for ${op.entityType}:${entityId}`, + reason: `Unknown vector clock comparison result for ${entityType}:${entityId}`, existingClock, }; }; @@ -170,31 +233,23 @@ export const resolveConflictForExistingOp = ( export const detectConflictForEntity = async ( userId: number, op: Operation, + entityType: string, entityId: string, tx: Prisma.TransactionClient, ): Promise => { - // Get the latest operation for this entity - const existingOp = await tx.operation.findFirst({ - where: { - userId, - entityType: op.entityType, - entityId, - }, - select: { - clientId: true, - vectorClock: true, - }, - orderBy: { - serverSeq: 'desc', - }, - }); + const latestOpByEntity = await prefetchLatestEntityOpsForBatch( + userId, + [{ entityType, entityId }], + tx, + ); + const existingOp = latestOpByEntity.get(getEntityConflictKey(entityType, entityId)); // No existing operation = no conflict if (!existingOp) { return { hasConflict: false }; } - return resolveConflictForExistingOp(op, entityId, existingOp); + return resolveConflictForExistingOp(op, entityType, entityId, existingOp); }; export const isSameDuplicateOperation = ( @@ -296,23 +351,41 @@ export const getConflictEntityIds = (op: Operation): string[] => { return Array.from(new Set(rawEntityIds)); }; +export const getConflictEntityPairs = (op: Operation): EntityConflictPair[] => { + const rawEntityPairs: EntityConflictPair[] = [ + ...(op.affectedEntities ?? []), + ...getConflictEntityIds(op).map((entityId) => ({ + entityType: op.entityType, + entityId, + })), + ]; + const entityPairs = new Map(); + + for (const entity of rawEntityPairs) { + if (!entity.entityType || !entity.entityId) continue; + entityPairs.set(getEntityConflictKey(entity.entityType, entity.entityId), { + entityType: entity.entityType, + entityId: entity.entityId, + }); + } + + return Array.from(entityPairs.values()); +}; + export const getEntityConflictKey = (entityType: string, entityId: string): string => { return `${entityType}\u0000${entityId}`; }; export const getBatchConflictEntityPairs = ( candidates: BatchUploadCandidate[], -): { entityType: string; entityId: string }[] => { - const entityPairs = new Map(); +): EntityConflictPair[] => { + const entityPairs = new Map(); for (const candidate of candidates) { if (isFullStateOpType(candidate.op.opType)) continue; - for (const entityId of getConflictEntityIds(candidate.op)) { - entityPairs.set(getEntityConflictKey(candidate.op.entityType, entityId), { - entityType: candidate.op.entityType, - entityId, - }); + for (const entity of getConflictEntityPairs(candidate.op)) { + entityPairs.set(getEntityConflictKey(entity.entityType, entity.entityId), entity); } } @@ -321,7 +394,7 @@ export const getBatchConflictEntityPairs = ( export const prefetchLatestEntityOpsForBatch = async ( userId: number, - entityPairs: { entityType: string; entityId: string }[], + entityPairs: EntityConflictPair[], tx: Prisma.TransactionClient, ): Promise> => { const latestByEntity = new Map(); @@ -332,23 +405,37 @@ export const prefetchLatestEntityOpsForBatch = async ( start < entityPairs.length; start += CONFLICT_DETECTION_ENTITY_BATCH_SIZE ) { - const touchedRows = entityPairs - .slice(start, start + CONFLICT_DETECTION_ENTITY_BATCH_SIZE) - .map(({ entityType, entityId }) => Prisma.sql`(${entityType}, ${entityId})`); + const touchedRows = entityPairs.slice( + start, + start + CONFLICT_DETECTION_ENTITY_BATCH_SIZE, + ); + const touchedPlaceholders = touchedRows + .map((_, index) => `($${index * 2 + 2}, $${index * 2 + 3})`) + .join(', '); + const touchedParams = touchedRows.flatMap(({ entityType, entityId }) => [ + entityType, + entityId, + ]); - const latestOps = await tx.$queryRaw` - SELECT DISTINCT ON (o.entity_type, o.entity_id) - o.entity_type AS "entityType", - o.entity_id AS "entityId", + const latestOps = await queryRawWithBoundParams( + tx, + ` + SELECT DISTINCT ON (ae.entity_type, ae.entity_id) + ae.entity_type AS "entityType", + ae.entity_id AS "entityId", o.client_id AS "clientId", o.vector_clock AS "vectorClock" - FROM operations o - JOIN (VALUES ${Prisma.join(touchedRows)}) AS touched(entity_type, entity_id) - ON touched.entity_type = o.entity_type - AND touched.entity_id = o.entity_id - WHERE o.user_id = ${userId} - ORDER BY o.entity_type, o.entity_id, o.server_seq DESC - `; + FROM operation_affected_entities ae + JOIN operations o ON o.id = ae.operation_id + JOIN (VALUES ${touchedPlaceholders}) AS touched(entity_type, entity_id) + ON touched.entity_type = ae.entity_type + AND touched.entity_id = ae.entity_id + WHERE ae.user_id = $1 + ORDER BY ae.entity_type, ae.entity_id, ae.server_seq DESC + `, + userId, + ...touchedParams, + ); for (const latestOp of latestOps) { latestByEntity.set( diff --git a/packages/super-sync-server/src/sync/services/operation-download.service.ts b/packages/super-sync-server/src/sync/services/operation-download.service.ts index 34b079d673..f717c7e5a9 100644 --- a/packages/super-sync-server/src/sync/services/operation-download.service.ts +++ b/packages/super-sync-server/src/sync/services/operation-download.service.ts @@ -4,6 +4,7 @@ * Extracted from SyncService for better separation of concerns. * This service handles operation retrieval with gap detection and snapshot optimization. */ +import type { Prisma } from '@prisma/client'; import { prisma } from '../../db'; import { Operation, @@ -63,7 +64,32 @@ type OperationDownloadRow = { syncImportReason: string | null; }; -const mapOperationRow = (row: OperationDownloadRow): ServerOperation => ({ +type OperationAffectedEntityRow = { + operationId: string; + entityType: string; + entityId: string; +}; + +type Queryable = Pick & { + $queryRawUnsafe?: (query: string, ...values: unknown[]) => Promise; +}; + +const toTemplateStringsArray = (query: string): TemplateStringsArray => + Object.assign([query], { raw: [query] }) as unknown as TemplateStringsArray; + +const queryRawWithBoundParams = ( + tx: Queryable, + query: string, + ...values: unknown[] +): Promise => + tx.$queryRawUnsafe + ? tx.$queryRawUnsafe(query, ...values) + : tx.$queryRaw(toTemplateStringsArray(query), ...values); + +const mapOperationRow = ( + row: OperationDownloadRow, + affectedEntities?: Operation['affectedEntities'], +): ServerOperation => ({ serverSeq: row.serverSeq, op: { id: row.id, @@ -78,6 +104,7 @@ const mapOperationRow = (row: OperationDownloadRow): ServerOperation => ({ timestamp: Number(row.clientTimestamp), isPayloadEncrypted: row.isPayloadEncrypted, syncImportReason: row.syncImportReason ?? undefined, + ...(affectedEntities ? { affectedEntities } : {}), }, receivedAt: Number(row.receivedAt), }); @@ -128,8 +155,13 @@ export class OperationDownloadService { take: limit, select: OPERATION_DOWNLOAD_SELECT, }); + const affectedEntitiesByOpId = await this._getAffectedEntitiesByOpId( + prisma, + userId, + ops.map((op) => op.id), + ); - return ops.map(mapOperationRow); + return ops.map((op) => mapOperationRow(op, affectedEntitiesByOpId.get(op.id))); } /** @@ -284,7 +316,14 @@ export class OperationDownloadService { } } - const mappedOps = ops.map(mapOperationRow); + const affectedEntitiesByOpId = await this._getAffectedEntitiesByOpId( + tx, + userId, + ops.map((op) => op.id), + ); + const mappedOps = ops.map((op) => + mapOperationRow(op, affectedEntitiesByOpId.get(op.id)), + ); const persistedSnapshotVectorClock = latestFullStateOp && seqRow?.latestFullStateSeq === latestFullStateOp.serverSeq ? parsePersistedVectorClock(seqRow.latestFullStateVectorClock) @@ -334,6 +373,45 @@ export class OperationDownloadService { }; } + private async _getAffectedEntitiesByOpId( + tx: Queryable, + userId: number, + opIds: string[], + ): Promise> { + const affectedEntitiesByOpId = new Map(); + if (opIds.length === 0) { + return affectedEntitiesByOpId; + } + + const opIdPlaceholders = opIds.map((_, index) => `$${index + 2}`).join(', '); + const rows = await queryRawWithBoundParams( + tx, + ` + SELECT + operation_id AS "operationId", + entity_type AS "entityType", + entity_id AS "entityId" + FROM operation_affected_entities + WHERE user_id = $1 + AND operation_id IN (${opIdPlaceholders}) + ORDER BY operation_id, id ASC + `, + userId, + ...opIds, + ); + + for (const row of rows) { + const entities = affectedEntitiesByOpId.get(row.operationId) ?? []; + entities.push({ + entityType: row.entityType, + entityId: row.entityId, + }); + affectedEntitiesByOpId.set(row.operationId, entities); + } + + return affectedEntitiesByOpId; + } + private async _computeSnapshotVectorClock( userId: number, latestSnapshotSeq: number, diff --git a/packages/super-sync-server/src/sync/services/operation-upload.service.ts b/packages/super-sync-server/src/sync/services/operation-upload.service.ts index 66b8d24aa8..e1079432f8 100644 --- a/packages/super-sync-server/src/sync/services/operation-upload.service.ts +++ b/packages/super-sync-server/src/sync/services/operation-upload.service.ts @@ -1,4 +1,4 @@ -import { Prisma } from '@prisma/client'; +import type { Prisma } from '@prisma/client'; import { Logger } from '../../logger'; import { computeOpStorageBytes } from '../sync.const'; import { @@ -19,7 +19,7 @@ import { import { detectConflict, getBatchConflictEntityPairs, - getConflictEntityIds, + getConflictEntityPairs, getEntityConflictKey, isSameDuplicateOperation, prefetchLatestEntityOpsForBatch, @@ -53,6 +53,22 @@ const toSafeServerSeq = (value: number | bigint | undefined, userId: number): nu throw new Error(`Invalid last_seq value returned for user ${userId}`); }; +type RawExecuteTx = Prisma.TransactionClient & { + $executeRawUnsafe?: (query: string, ...values: unknown[]) => Promise; +}; + +const toTemplateStringsArray = (query: string): TemplateStringsArray => + Object.assign([query], { raw: [query] }) as unknown as TemplateStringsArray; + +const executeRawWithBoundParams = ( + tx: RawExecuteTx, + query: string, + ...values: unknown[] +): Promise => + tx.$executeRawUnsafe + ? tx.$executeRawUnsafe(query, ...values) + : tx.$executeRaw(toTemplateStringsArray(query), ...values); + export class OperationUploadService { constructor( private readonly validationService: ValidationService, @@ -112,6 +128,51 @@ export class OperationUploadService { }; } + private async insertAffectedEntities( + userId: number, + accepted: Array<{ op: Operation; serverSeq: number }>, + tx: Prisma.TransactionClient, + ): Promise { + const rows = accepted.flatMap((candidate) => + getConflictEntityPairs(candidate.op).map((entity) => ({ + operationId: candidate.op.id, + userId, + entityType: entity.entityType, + entityId: entity.entityId, + serverSeq: candidate.serverSeq, + })), + ); + + if (rows.length === 0) { + return; + } + + const valuePlaceholders = rows + .map( + (_, index) => + `($${index * 5 + 1}, $${index * 5 + 2}, $${index * 5 + 3}, $${index * 5 + 4}, $${index * 5 + 5})`, + ) + .join(', '); + const values = rows.flatMap((row) => [ + row.operationId, + row.userId, + row.entityType, + row.entityId, + row.serverSeq, + ]); + + await executeRawWithBoundParams( + tx, + ` + INSERT INTO operation_affected_entities + (operation_id, user_id, entity_type, entity_id, server_seq) + VALUES ${valuePlaceholders} + ON CONFLICT DO NOTHING + `, + ...values, + ); + } + /** * Aggregate the per-client max vector_clock counter over all operations for * `userId` with `server_seq < beforeServerSeq`. Used at full-state op upload @@ -443,13 +504,18 @@ export class OperationUploadService { const { op } = candidate; if (!isFullStateOpType(op.opType)) { let conflict: ConflictResult | null = null; - for (const entityId of getConflictEntityIds(op)) { + for (const entity of getConflictEntityPairs(op)) { const existingOp = latestOpByEntity.get( - getEntityConflictKey(op.entityType, entityId), + getEntityConflictKey(entity.entityType, entity.entityId), ); if (!existingOp) continue; - const conflictResult = resolveConflictForExistingOp(op, entityId, existingOp); + const conflictResult = resolveConflictForExistingOp( + op, + entity.entityType, + entity.entityId, + existingOp, + ); if (conflictResult.hasConflict) { conflict = conflictResult; break; @@ -487,10 +553,10 @@ export class OperationUploadService { accepted.push(acceptedOperation); if (!isFullStateOpType(op.opType)) { - for (const entityId of getConflictEntityIds(op)) { - latestOpByEntity.set(getEntityConflictKey(op.entityType, entityId), { - entityType: op.entityType, - entityId, + for (const entity of getConflictEntityPairs(op)) { + latestOpByEntity.set(getEntityConflictKey(entity.entityType, entity.entityId), { + entityType: entity.entityType, + entityId: entity.entityId, clientId: op.clientId, vectorClock: op.vectorClock, }); @@ -504,8 +570,8 @@ export class OperationUploadService { /** * Stage 5: reserve a contiguous server_seq range for the accepted ops in one * INSERT ... ON CONFLICT (which also serializes concurrent batches via the - * user_sync_state row lock), assign each op its seq, and bulk-insert. Returns - * the DB round trips consumed (2). + * user_sync_state row lock), assign each op its seq, bulk-insert operations, + * and persist typed affected-entity refs. Returns the DB round trips consumed (3). */ private async reserveSeqAndInsert( userId: number, @@ -550,7 +616,8 @@ export class OperationUploadService { syncImportReason: candidate.op.syncImportReason ?? null, })), }); - return 2; + await this.insertAffectedEntities(userId, accepted, tx); + return 3; } /** @@ -862,6 +929,8 @@ export class OperationUploadService { }; } + await this.insertAffectedEntities(userId, [{ op, serverSeq }], tx); + if (fullStateVectorClock) { // Persist the aggregate of (prior history ∪ this op's clock), not just the // op's own clock. BACKUP_IMPORT uses a fresh `{ clientId: 1 }` by design diff --git a/packages/super-sync-server/src/sync/services/snapshot-generation.service.ts b/packages/super-sync-server/src/sync/services/snapshot-generation.service.ts index 76265cef05..6aa968cab6 100644 --- a/packages/super-sync-server/src/sync/services/snapshot-generation.service.ts +++ b/packages/super-sync-server/src/sync/services/snapshot-generation.service.ts @@ -45,6 +45,8 @@ const REPLAY_OPERATION_SELECT = { isPayloadEncrypted: true, } as const; +const REPEATABLE_READ_ISOLATION_LEVEL = 'RepeatableRead' as const; + export class SnapshotGenerationService { /** * Internal implementation of snapshot generation. @@ -289,7 +291,7 @@ export class SnapshotGenerationService { // Prevent races between `_assertNoEncryptedOps` / `_assertCachedSnapshotBaseReplayable` // and the subsequent `findMany` batches: a concurrent writer must not be able to // slip an encrypted op into the snapshot window after the guards have passed. - isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead, + isolationLevel: REPEATABLE_READ_ISOLATION_LEVEL, }, ); @@ -434,7 +436,7 @@ export class SnapshotGenerationService { }, { timeout: 60000, // Snapshot generation can take time - isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead, + isolationLevel: REPEATABLE_READ_ISOLATION_LEVEL, }, ); } diff --git a/packages/super-sync-server/src/sync/services/validation.service.ts b/packages/super-sync-server/src/sync/services/validation.service.ts index b321b123d5..3419ec73b7 100644 --- a/packages/super-sync-server/src/sync/services/validation.service.ts +++ b/packages/super-sync-server/src/sync/services/validation.service.ts @@ -13,7 +13,7 @@ import { sanitizeVectorClock, validatePayload, } from '../sync.types'; -import { ENTITY_TYPES } from '@sp/shared-schema'; +import { ENTITY_TYPES, SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP } from '@sp/shared-schema'; import { Logger } from '../../logger'; /** @@ -99,6 +99,52 @@ export class ValidationService { }; } } + if (op.affectedEntities !== undefined) { + if (!Array.isArray(op.affectedEntities)) { + return { + valid: false, + error: 'affectedEntities must be an array', + errorCode: SYNC_ERROR_CODES.INVALID_ENTITY_ID, + }; + } + if (op.affectedEntities.length > SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP) { + return { + valid: false, + error: `affectedEntities has too many entries (${op.affectedEntities.length}, max ${SUPER_SYNC_MAX_AFFECTED_ENTITIES_PER_OP})`, + errorCode: SYNC_ERROR_CODES.INVALID_ENTITY_ID, + }; + } + for (const entity of op.affectedEntities) { + if (!entity || typeof entity !== 'object') { + return { + valid: false, + error: 'Invalid affectedEntities entry', + errorCode: SYNC_ERROR_CODES.INVALID_ENTITY_ID, + }; + } + if ( + typeof entity.entityType !== 'string' || + !ALLOWED_ENTITY_TYPES.has(entity.entityType) + ) { + return { + valid: false, + error: `Invalid affected entityType: ${entity.entityType}`, + errorCode: SYNC_ERROR_CODES.INVALID_ENTITY_TYPE, + }; + } + if ( + typeof entity.entityId !== 'string' || + entity.entityId.length > 255 || + entity.entityId.trim().length === 0 + ) { + return { + valid: false, + error: 'Invalid affected entityId format or length', + errorCode: SYNC_ERROR_CODES.INVALID_ENTITY_ID, + }; + } + } + } // Require entityId for regular entity operations. // Full-state operations (SYNC_IMPORT, BACKUP_IMPORT, REPAIR) and bulk entity types diff --git a/packages/super-sync-server/src/sync/sync.const.ts b/packages/super-sync-server/src/sync/sync.const.ts index 7d93c55ce8..659a173681 100644 --- a/packages/super-sync-server/src/sync/sync.const.ts +++ b/packages/super-sync-server/src/sync/sync.const.ts @@ -36,6 +36,7 @@ export const isValidClientId = (cid: unknown): cid is string => * SUM(pg_column_size) DoS that scanning every delta op caused. */ export const APPROX_BYTES_PER_OP = 1024; +const APPROX_BYTES_PER_AFFECTED_ENTITY_ROW = 96; /** * Locally-computed approximation of how many bytes an operation's payload and @@ -54,12 +55,18 @@ export const APPROX_BYTES_PER_OP = 1024; export const computeOpStorageBytes = (op: { payload: unknown; vectorClock: unknown; + affectedEntities?: unknown; }): { bytes: number; fallback: boolean } => { try { + const affectedEntities = Array.isArray(op.affectedEntities) + ? op.affectedEntities + : []; return { bytes: Buffer.byteLength(JSON.stringify(op.payload ?? null), 'utf8') + - Buffer.byteLength(JSON.stringify(op.vectorClock ?? {}), 'utf8'), + Buffer.byteLength(JSON.stringify(op.vectorClock ?? {}), 'utf8') + + Buffer.byteLength(JSON.stringify(affectedEntities), 'utf8') + + affectedEntities.length * APPROX_BYTES_PER_AFFECTED_ENTITY_ROW, fallback: false, }; } catch { diff --git a/packages/super-sync-server/src/sync/sync.service.ts b/packages/super-sync-server/src/sync/sync.service.ts index 53b4f68b80..d26aaa1b87 100644 --- a/packages/super-sync-server/src/sync/sync.service.ts +++ b/packages/super-sync-server/src/sync/sync.service.ts @@ -24,9 +24,23 @@ import { type CacheSnapshotResult, type SnapshotDedupResponse, } from './services'; -const getPrismaP2002TargetTokens = ( - err: Prisma.PrismaClientKnownRequestError, -): string[] => { + +const REPEATABLE_READ_ISOLATION_LEVEL = 'RepeatableRead' as const; + +type PrismaKnownRequestErrorLike = { + code?: string; + meta?: { + target?: unknown; + }; +}; + +const getPrismaErrorCode = (err: unknown): string | undefined => { + if (!err || typeof err !== 'object') return undefined; + const code = (err as { code?: unknown }).code; + return typeof code === 'string' ? code : undefined; +}; + +const getPrismaP2002TargetTokens = (err: PrismaKnownRequestErrorLike): string[] => { const target = err.meta?.target; if (Array.isArray(target)) return target.map(String); if (typeof target === 'string') return [target]; @@ -34,11 +48,11 @@ const getPrismaP2002TargetTokens = ( }; const isRetryableOperationUniqueViolation = (err: unknown): boolean => { - if (!(err instanceof Prisma.PrismaClientKnownRequestError) || err.code !== 'P2002') { + if (getPrismaErrorCode(err) !== 'P2002') { return false; } - const targetTokens = getPrismaP2002TargetTokens(err); + const targetTokens = getPrismaP2002TargetTokens(err as PrismaKnownRequestErrorLike); if (targetTokens.length === 0) return true; const normalizedTargets = targetTokens.map((target) => @@ -266,7 +280,7 @@ export class SyncService { // The serial path also performs the legacy post-sequence conflict re-check. // The batch path serializes accepted writers through the shared // user_sync_state.last_seq row update; see ARCHITECTURE-DECISIONS.md. - isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead, + isolationLevel: REPEATABLE_READ_ISOLATION_LEVEL, }, ); @@ -298,7 +312,7 @@ export class SyncService { // Prisma uses P2034 for "Transaction failed due to a write conflict or a deadlock" // PostgreSQL uses 40001 (serialization_failure) and 40P01 (deadlock_detected) const isSerializationFailure = - (err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2034') || + getPrismaErrorCode(err) === 'P2034' || (this.config.batchUpload && isRetryableOperationUniqueViolation(err)) || errorMessage.includes('40001') || errorMessage.includes('40P01') || diff --git a/packages/super-sync-server/src/sync/sync.types.ts b/packages/super-sync-server/src/sync/sync.types.ts index 3b71e79bae..1e6ffb6d34 100644 --- a/packages/super-sync-server/src/sync/sync.types.ts +++ b/packages/super-sync-server/src/sync/sync.types.ts @@ -168,6 +168,7 @@ export interface Operation { entityType: string; entityId?: string; entityIds?: string[]; // For batch operations + affectedEntities?: AffectedEntityRef[]; payload: unknown; vectorClock: VectorClock; timestamp: number; @@ -176,6 +177,11 @@ export interface Operation { syncImportReason?: string; } +export interface AffectedEntityRef { + entityType: string; + entityId: string; +} + export interface DuplicateOperationCandidate { id: string; userId: number; diff --git a/packages/super-sync-server/tests/conflict-detection.spec.ts b/packages/super-sync-server/tests/conflict-detection.spec.ts index 46d4645d40..c3124d6587 100644 --- a/packages/super-sync-server/tests/conflict-detection.spec.ts +++ b/packages/super-sync-server/tests/conflict-detection.spec.ts @@ -1,6 +1,5 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { uuidv7 } from 'uuidv7'; -import { Prisma } from '@prisma/client'; import { testState, resetTestState } from './sync.service.test-state'; // Mock the database module with Prisma mocks @@ -10,19 +9,32 @@ vi.mock('../src/db', async () => { hasOperationUniqueConflict, testState: state, } = await import('./sync.service.test-state'); - const { Prisma: PrismaModule } = await import('@prisma/client'); + + class PrismaClientKnownRequestError extends Error { + code: string; + clientVersion: string; + meta?: unknown; + + constructor( + message: string, + options: { code: string; clientVersion: string; meta?: unknown }, + ) { + super(message); + this.name = 'PrismaClientKnownRequestError'; + this.code = options.code; + this.clientVersion = options.clientVersion; + this.meta = options.meta; + } + } const createTxMock = () => ({ operation: { create: vi.fn().mockImplementation(async (args: any) => { if (state.operations.has(args.data.id)) { - throw new PrismaModule.PrismaClientKnownRequestError( - 'Unique constraint failed', - { - code: 'P2002', - clientVersion: '5.0.0', - }, - ); + throw new PrismaClientKnownRequestError('Unique constraint failed', { + code: 'P2002', + clientVersion: '5.0.0', + }); } state.serverSeqCounter++; const op = { @@ -42,10 +54,10 @@ vi.mock('../src/db', async () => { if (args.skipDuplicates) { continue; } - throw new PrismaModule.PrismaClientKnownRequestError( - 'Unique constraint failed', - { code: 'P2002', clientVersion: '5.0.0' }, - ); + throw new PrismaClientKnownRequestError('Unique constraint failed', { + code: 'P2002', + clientVersion: '5.0.0', + }); } state.operations.set(row.id, { @@ -161,10 +173,55 @@ vi.mock('../src/db', async () => { return state.users.get(args.where.id) || null; }), }, - // Upload transaction writes the storage counter atomically via $executeRaw. - $executeRaw: vi.fn().mockResolvedValue(0), + // Upload transaction writes the storage counter via $executeRaw and + // affected-entity rows via $executeRawUnsafe. + $executeRaw: vi.fn().mockImplementation(async (strings: unknown) => { + const sql = Array.isArray(strings) ? strings.join('') : String(strings); + return sql.includes('operation_affected_entities') ? 1 : 0; + }), + $executeRawUnsafe: vi + .fn() + .mockImplementation(async (sql: string, ...params: unknown[]) => { + if (!sql.includes('operation_affected_entities')) { + return 0; + } + + for (let i = 0; i < params.length; i += 5) { + const [operationId, userId, entityType, entityId, serverSeq] = params.slice( + i, + i + 5, + ); + const exists = state.operationAffectedEntities.some( + (row: any) => + row.operationId === operationId && + row.entityType === entityType && + row.entityId === entityId, + ); + if (!exists) { + state.operationAffectedEntities.push({ + operationId, + userId, + entityType, + entityId, + serverSeq, + }); + } + } + return state.operationAffectedEntities.length; + }), $queryRaw: vi.fn().mockImplementation(async (strings: any, ...params: unknown[]) => { const sql = Array.isArray(strings) ? strings.join('') : String(strings); + if (sql.includes('INSERT INTO user_sync_state')) { + const [txUserId, incrementBy] = params as [number, number]; + const existing = state.userSyncStates.get(txUserId) ?? { + userId: txUserId, + lastSeq: 0, + }; + const lastSeq = existing.lastSeq + incrementBy; + state.userSyncStates.set(txUserId, { ...existing, lastSeq }); + return [{ lastSeq }]; + } + // Full-state op uploads aggregate prior vector clocks via $queryRaw. if (sql.includes('jsonb_each_text(vector_clock)')) { const [txUserId, beforeServerSeq] = params as [number, number]; @@ -188,39 +245,50 @@ vi.mock('../src/db', async () => { })); } - const [userId, entityType, entityIdsSql] = params as [number, string, Prisma.Sql]; - state.batchConflictQueryCount++; - if (!Array.isArray(entityIdsSql.values)) { - throw new Error( - 'Expected batched conflict query entity IDs to be passed via Prisma.join(...)', - ); - } - const batchEntityIds = new Set( - entityIdsSql.values.filter( - (entityId): entityId is string => typeof entityId === 'string', - ), - ); - const latestByEntityId = new Map(); - const ops = Array.from(state.operations.values()) - .filter( - (op: any) => - op.userId === userId && - op.entityType === entityType && - batchEntityIds.has(op.entityId), - ) - .sort((a: any, b: any) => b.serverSeq - a.serverSeq); - - for (const op of ops) { - if (!op.entityId || latestByEntityId.has(op.entityId)) continue; - latestByEntityId.set(op.entityId, op); - } - - return Array.from(latestByEntityId.values()).map((op: any) => ({ - entityId: op.entityId, - clientId: op.clientId, - vectorClock: op.vectorClock, - })); + return []; }), + $queryRawUnsafe: vi + .fn() + .mockImplementation(async (sql: string, ...params: unknown[]) => { + if (!sql.includes('FROM operation_affected_entities')) { + return []; + } + + state.batchConflictQueryCount++; + const userId = params[0] as number; + const touchedValues = params.slice(1); + const touchedPairs = new Set(); + for (let i = 0; i < touchedValues.length; i += 2) { + const [entityType, entityId] = touchedValues.slice(i, i + 2); + if (typeof entityType === 'string' && typeof entityId === 'string') { + touchedPairs.add(`${entityType}\u0000${entityId}`); + } + } + + const latestByEntity = new Map(); + const rows = [...state.operationAffectedEntities] + .filter( + (row: any) => + row.userId === userId && + touchedPairs.has(`${row.entityType}\u0000${row.entityId}`), + ) + .sort((a: any, b: any) => b.serverSeq - a.serverSeq); + + for (const row of rows) { + const key = `${row.entityType}\u0000${row.entityId}`; + if (latestByEntity.has(key)) continue; + const op = state.operations.get(row.operationId); + if (!op) continue; + latestByEntity.set(key, { row, op }); + } + + return Array.from(latestByEntity.values()).map(({ row, op }: any) => ({ + entityType: row.entityType, + entityId: row.entityId, + clientId: op.clientId, + vectorClock: op.vectorClock, + })); + }), }); return { @@ -228,6 +296,24 @@ vi.mock('../src/db', async () => { $transaction: vi .fn() .mockImplementation(async (callback: any) => callback(createTxMock())), + $queryRaw: vi.fn().mockResolvedValue([]), + $queryRawUnsafe: vi + .fn() + .mockImplementation(async (sql: string, ...params: unknown[]) => { + if (!sql.includes('FROM operation_affected_entities')) { + return []; + } + + const userId = params[0] as number; + const opIds = new Set(params.slice(1)); + return state.operationAffectedEntities + .filter((row: any) => row.userId === userId && opIds.has(row.operationId)) + .map((row: any) => ({ + operationId: row.operationId, + entityType: row.entityType, + entityId: row.entityId, + })); + }), userSyncState: { findUnique: vi.fn().mockImplementation(async (args: any) => { return state.userSyncStates.get(args.where.userId) || null; @@ -694,6 +780,38 @@ describe('Conflict Detection', () => { }); describe('Entity Type Isolation', () => { + it('should detect conflicts through typed affected entities on mixed-entity batches', async () => { + const service = getSyncService(); + + const projectCompletion = createOp({ + entityType: 'PROJECT', + entityId: 'project-1', + actionType: '[Task Shared] completeProject', + opType: 'BATCH', + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], + clientId: clientA, + vectorClock: { [clientA]: 1 }, + }); + expect( + (await service.uploadOps(userId, clientA, [projectCompletion]))[0].accepted, + ).toBe(true); + + const taskUpdate = createOp({ + entityType: 'TASK', + entityId: 'task-1', + clientId: clientB, + vectorClock: { [clientB]: 1 }, + }); + const result = await service.uploadOps(userId, clientB, [taskUpdate]); + + expect(result[0].accepted).toBe(false); + expect(result[0].errorCode).toBe(SYNC_ERROR_CODES.CONFLICT_CONCURRENT); + expect(result[0].error).toContain('TASK:task-1'); + }); + it('should not conflict operations on different entities of same type', async () => { const service = getSyncService(); diff --git a/packages/super-sync-server/tests/conflict.spec.ts b/packages/super-sync-server/tests/conflict.spec.ts index 87f104103b..4fd237c9c3 100644 --- a/packages/super-sync-server/tests/conflict.spec.ts +++ b/packages/super-sync-server/tests/conflict.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { isSameDuplicateOperation, isSameDuplicateTimestamp, + getConflictEntityPairs, pruneVectorClockForStorage, resolveConflictForExistingOp, stableJsonStringify, @@ -132,6 +133,7 @@ describe('conflict helpers', () => { it('classifies concurrent vector clocks as conflicts', () => { const result = resolveConflictForExistingOp( op({ vectorClock: { 'client-a': 1 } }), + 'TASK', 'task-1', { clientId: 'client-b', vectorClock: { 'client-b': 1 } }, ); @@ -146,6 +148,7 @@ describe('conflict helpers', () => { it('classifies less-than vector clocks as superseded', () => { const result = resolveConflictForExistingOp( op({ vectorClock: { 'client-a': 1 } }), + 'TASK', 'task-1', { clientId: 'client-a', vectorClock: { 'client-a': 2 } }, ); @@ -163,6 +166,27 @@ describe('conflict helpers', () => { ); }); + it('unions typed affected entities with legacy entity ids', () => { + expect( + getConflictEntityPairs( + op({ + entityType: 'PROJECT', + entityId: 'project-1', + entityIds: ['project-1', 'project-2'], + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], + }), + ), + ).toEqual([ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'PROJECT', entityId: 'project-2' }, + ]); + }); + it('prunes and mutates vector clocks before storage', () => { const incoming = op({ clientId: 'client-25', diff --git a/packages/super-sync-server/tests/operation-download.service.spec.ts b/packages/super-sync-server/tests/operation-download.service.spec.ts index 4d8ad280c5..4bef2d9157 100644 --- a/packages/super-sync-server/tests/operation-download.service.spec.ts +++ b/packages/super-sync-server/tests/operation-download.service.spec.ts @@ -13,6 +13,7 @@ vi.mock('../src/db', () => ({ findUnique: vi.fn(), }, $queryRaw: vi.fn(), + $queryRawUnsafe: vi.fn(), $transaction: vi.fn(), }, })); @@ -110,6 +111,7 @@ describe('OperationDownloadService', () => { beforeEach(() => { vi.clearAllMocks(); vi.mocked(prisma.$queryRaw).mockResolvedValue([]); + vi.mocked(prisma.$queryRawUnsafe).mockResolvedValue([]); service = new OperationDownloadService(); }); @@ -148,6 +150,22 @@ describe('OperationDownloadService', () => { expect(result[1].serverSeq).toBe(2); }); + it('should include affected entities when present', async () => { + const mockOps = [createMockOpRow(1, 'client-1', { id: 'op-1' })]; + vi.mocked(prisma.operation.findMany).mockResolvedValue(mockOps as any); + vi.mocked(prisma.$queryRawUnsafe).mockResolvedValue([ + { operationId: 'op-1', entityType: 'PROJECT', entityId: 'project-1' }, + { operationId: 'op-1', entityType: 'TASK', entityId: 'task-1' }, + ]); + + const result = await service.getOpsSince(1, 0); + + expect(result[0].op.affectedEntities).toEqual([ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ]); + }); + it('should exclude operations from specified client', async () => { vi.mocked(prisma.operation.findMany).mockResolvedValue([]); @@ -230,6 +248,8 @@ describe('OperationDownloadService', () => { userSyncState: { findUnique: vi.fn(), }, + $queryRaw: vi.fn().mockResolvedValue([]), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return mockFn(mockTx); }); @@ -439,6 +459,7 @@ describe('OperationDownloadService', () => { findUnique: vi.fn().mockResolvedValue({ lastSeq: 60 }), }, $queryRaw: vi.fn(), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(capturedTx); }); @@ -545,6 +566,7 @@ describe('OperationDownloadService', () => { $queryRaw: vi .fn() .mockResolvedValue([{ client_id: 'snapshot-author', max_counter: 1n }]), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); @@ -571,6 +593,7 @@ describe('OperationDownloadService', () => { userSyncState: { findUnique: vi.fn().mockResolvedValue({ lastSeq: 20 }), }, + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); @@ -651,6 +674,7 @@ describe('OperationDownloadService', () => { findUnique: vi.fn().mockResolvedValue({ lastSeq: 50 }), }, $queryRaw: vi.fn().mockResolvedValue([]), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); @@ -675,6 +699,7 @@ describe('OperationDownloadService', () => { userSyncState: { findUnique: vi.fn().mockResolvedValue({ lastSeq: 20 }), }, + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); @@ -738,6 +763,7 @@ describe('OperationDownloadService', () => { { client_id: 'client-2', max_counter: 5n }, { client_id: 'client-3', max_counter: 8n }, ]), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); @@ -1050,6 +1076,7 @@ describe('OperationDownloadService', () => { findUnique: vi.fn().mockResolvedValue({ lastSeq: 70 }), }, $queryRaw: vi.fn(), + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(capturedTx); }); @@ -1094,6 +1121,7 @@ describe('OperationDownloadService', () => { userSyncState: { findUnique: vi.fn().mockResolvedValue({ lastSeq: 70 }), }, + $queryRawUnsafe: vi.fn().mockResolvedValue([]), }; return fn(mockTx); }); diff --git a/packages/super-sync-server/tests/sync.service.test-state.ts b/packages/super-sync-server/tests/sync.service.test-state.ts index e2a1a719be..56931aa240 100644 --- a/packages/super-sync-server/tests/sync.service.test-state.ts +++ b/packages/super-sync-server/tests/sync.service.test-state.ts @@ -6,6 +6,7 @@ export const testState = { operations: new Map(), + operationAffectedEntities: [] as any[], syncDevices: new Map(), userSyncStates: new Map(), users: new Map(), @@ -16,6 +17,7 @@ export const testState = { export function resetTestState(): void { testState.operations = new Map(); + testState.operationAffectedEntities = []; testState.syncDevices = new Map(); testState.userSyncStates = new Map(); testState.users = new Map(); diff --git a/packages/sync-core/src/conflict-resolution.ts b/packages/sync-core/src/conflict-resolution.ts index 5a6f8bd9e7..cf625a7d40 100644 --- a/packages/sync-core/src/conflict-resolution.ts +++ b/packages/sync-core/src/conflict-resolution.ts @@ -2,6 +2,7 @@ import { extractActionPayload, extractEntityFromPayload, extractUpdateChanges, + getOperationAffectedEntities, OpType, } from './operation.types'; import type { EntityConflict, Operation } from './operation.types'; @@ -430,13 +431,10 @@ export const partitionLwwResolutions = < partitions.remoteWinsOps.push(...processRemoteWinnerOps(conflict)); for (const op of conflict.remoteOps) { - const ids = op.entityIds?.length - ? op.entityIds - : op.entityId - ? [op.entityId] - : []; - for (const id of ids) { - partitions.remoteWinnerAffectedEntityKeys.add(toEntityKey(op.entityType, id)); + for (const entity of getOperationAffectedEntities(op)) { + partitions.remoteWinnerAffectedEntityKeys.add( + toEntityKey(entity.entityType, entity.entityId), + ); } } continue; diff --git a/packages/sync-core/src/index.ts b/packages/sync-core/src/index.ts index 66597c9bba..fafcee0ebe 100644 --- a/packages/sync-core/src/index.ts +++ b/packages/sync-core/src/index.ts @@ -5,11 +5,13 @@ export { extractActionPayload, extractEntityFromPayload, extractUpdateChanges, + getOperationAffectedEntities, } from './operation.types'; export type { VectorClock, Operation, OperationLogEntry, + AffectedEntity, EntityConflict, ConflictResult, EntityChange, diff --git a/packages/sync-core/src/operation.types.ts b/packages/sync-core/src/operation.types.ts index ceb37f8edb..c16b222da4 100644 --- a/packages/sync-core/src/operation.types.ts +++ b/packages/sync-core/src/operation.types.ts @@ -74,6 +74,14 @@ export interface Operation { */ entityIds?: string[]; + /** + * Typed entity refs touched by this operation. + * + * Use this when one operation changes multiple entity types. `entityId` and + * `entityIds` stay as the legacy primary scope for backwards compatibility. + */ + affectedEntities?: AffectedEntity[]; + // DATA /** * The actual data change associated with the operation. @@ -110,6 +118,11 @@ export interface Operation { schemaVersion: number; } +export interface AffectedEntity { + entityType: string; + entityId: string; +} + export interface OperationLogEntry = Operation> { /** * Local, monotonic auto-increment integer (IndexedDB primary key). @@ -202,6 +215,31 @@ export interface MultiEntityPayload { entityChanges: EntityChange[]; } +export const getOperationAffectedEntities = ( + op: Pick< + Operation, + 'entityType' | 'entityId' | 'entityIds' | 'affectedEntities' + >, +): AffectedEntity[] => { + const rawEntities: AffectedEntity[] = [ + ...(op.affectedEntities ?? []), + ...(op.entityIds?.length ? op.entityIds : op.entityId ? [op.entityId] : []).map( + (entityId) => ({ entityType: op.entityType, entityId }), + ), + ]; + + const seen = new Set(); + const result: AffectedEntity[] = []; + for (const entity of rawEntities) { + if (!entity.entityType || !entity.entityId) continue; + const key = `${entity.entityType}\u0000${entity.entityId}`; + if (seen.has(key)) continue; + seen.add(key); + result.push(entity); + } + return result; +}; + /** * Type guard to check if a payload is a multi-entity payload. */ diff --git a/packages/sync-providers/src/provider-types.ts b/packages/sync-providers/src/provider-types.ts index 86e4d09035..2d92176e34 100644 --- a/packages/sync-providers/src/provider-types.ts +++ b/packages/sync-providers/src/provider-types.ts @@ -86,6 +86,7 @@ export interface SyncOperation { entityType: string; entityId?: string; entityIds?: string[]; + affectedEntities?: SyncAffectedEntity[]; payload: unknown; vectorClock: VectorClock; timestamp: number; @@ -94,6 +95,11 @@ export interface SyncOperation { syncImportReason?: string; } +export interface SyncAffectedEntity { + entityType: string; + entityId: string; +} + export interface ServerSyncOperation { serverSeq: number; op: SyncOperation; diff --git a/src/app/core-ui/work-context-menu/work-context-menu.component.spec.ts b/src/app/core-ui/work-context-menu/work-context-menu.component.spec.ts index 3b83b1f16a..a25f5173c6 100644 --- a/src/app/core-ui/work-context-menu/work-context-menu.component.spec.ts +++ b/src/app/core-ui/work-context-menu/work-context-menu.component.spec.ts @@ -144,7 +144,38 @@ describe('WorkContextMenuComponent', () => { expect(mockProjectService.complete).toHaveBeenCalledWith( 'project-123', logicalDoneOn, - { topLevelTaskIdsToMoveToInbox: ['t1'] }, + { + topLevelTaskIdsToMoveToInbox: ['t1'], + taskIdsToMoveToInbox: ['t1'], + }, + ); + }); + + it('includes moved subtasks and reopened parents in Inbox completion metadata', async () => { + const info = { + topLevelTasks: [{ id: 't1', isDone: true, subTaskIds: ['s1'] } as any], + allTasks: [ + { id: 't1', isDone: true, subTaskIds: ['s1'] } as any, + { id: 's1', isDone: false, subTaskIds: [] } as any, + ], + unfinishedTasks: [{ id: 's1', isDone: false, subTaskIds: [] } as any], + topLevelTasksWithUnfinishedWork: [ + { id: 't1', isDone: true, subTaskIds: ['s1'] } as any, + ], + }; + mockProjectService.getCompletionInfo.and.returnValue(Promise.resolve(info)); + resolveResult$ = of('inbox'); + + await component.completeProject(); + + expect(mockProjectService.complete).toHaveBeenCalledWith( + 'project-123', + logicalDoneOn, + { + topLevelTaskIdsToMoveToInbox: ['t1'], + taskIdsToMoveToInbox: ['t1', 's1'], + taskIdsToMarkUndone: ['t1'], + }, ); }); @@ -167,7 +198,10 @@ describe('WorkContextMenuComponent', () => { expect(mockProjectService.complete).toHaveBeenCalledWith( 'project-123', logicalDoneOn, - { topLevelTaskIdsToMoveToInbox: ['t2'] }, + { + topLevelTaskIdsToMoveToInbox: ['t2'], + taskIdsToMoveToInbox: ['t2'], + }, ); }); diff --git a/src/app/core-ui/work-context-menu/work-context-menu.component.ts b/src/app/core-ui/work-context-menu/work-context-menu.component.ts index 39ecf71962..dcd4f83943 100644 --- a/src/app/core-ui/work-context-menu/work-context-menu.component.ts +++ b/src/app/core-ui/work-context-menu/work-context-menu.component.ts @@ -31,12 +31,32 @@ import { WorkContextMarkdownService } from '../../features/work-context/work-con import { ShareService, ShareSupport } from '../../core/share/share.service'; import { Store } from '@ngrx/store'; import { TaskSharedActions } from '../../root-store/meta/task-shared.actions'; -import { TaskWithSubTasks } from '../../features/tasks/task.model'; +import { Task, TaskWithSubTasks } from '../../features/tasks/task.model'; import { firstValueFrom, Observable, of } from 'rxjs'; import { AsyncPipe } from '@angular/common'; import type { WorkContextSettingsDialogData } from '../../features/work-context/dialog-work-context-settings/dialog-work-context-settings.component'; import { DateService } from '../../core/date/date.service'; +const getTaskTreeIds = ( + allTasks: Pick[], + rootTasks: Pick[], +): string[] => { + const taskById = new Map(allTasks.map((task) => [task.id, task])); + const seen = new Set(); + const ids: string[] = []; + const collect = (taskId: string): void => { + if (seen.has(taskId)) { + return; + } + seen.add(taskId); + ids.push(taskId); + taskById.get(taskId)?.subTaskIds?.forEach(collect); + }; + + rootTasks.forEach((task) => collect(task.id)); + return ids; +}; + @Component({ selector: 'work-context-menu', templateUrl: './work-context-menu.component.html', @@ -233,11 +253,17 @@ export class WorkContextMenuComponent implements OnInit { this.contextId, doneOn, resolution === 'inbox' - ? { - topLevelTaskIdsToMoveToInbox: currentInfo.topLevelTasksWithUnfinishedWork.map( - (task) => task.id, - ), - } + ? (() => { + const topLevelTasks = currentInfo.topLevelTasksWithUnfinishedWork; + const taskIdsToMarkUndone = topLevelTasks + .filter((task) => task.isDone) + .map((task) => task.id); + return { + topLevelTaskIdsToMoveToInbox: topLevelTasks.map((task) => task.id), + taskIdsToMoveToInbox: getTaskTreeIds(currentInfo.allTasks, topLevelTasks), + ...(taskIdsToMarkUndone.length ? { taskIdsToMarkUndone } : {}), + }; + })() : resolution === 'markDone' ? { taskIdsToMarkDone: currentInfo.unfinishedTasks.map((task) => task.id) } : {}, diff --git a/src/app/core/persistence/operation-log/compact/action-type-codes.spec.ts b/src/app/core/persistence/operation-log/compact/action-type-codes.spec.ts index 71fbe95d61..22cfea5819 100644 --- a/src/app/core/persistence/operation-log/compact/action-type-codes.spec.ts +++ b/src/app/core/persistence/operation-log/compact/action-type-codes.spec.ts @@ -54,6 +54,7 @@ describe('action-type-codes', () => { expect(decodeActionType('HA')).toBe('[Task Shared] addTask'); expect(decodeActionType('GA')).toBe('[Tag] Add Tag'); expect(decodeActionType('PA')).toBe('[Project] Add Project'); + expect(decodeActionType('PCO')).toBe('[Project] Complete Project'); }); it('should return code as-is for unknown codes (assumed to be full action type)', () => { diff --git a/src/app/core/persistence/operation-log/compact/action-type-codes.ts b/src/app/core/persistence/operation-log/compact/action-type-codes.ts index 166b43ef2f..3d6b6e7be6 100644 --- a/src/app/core/persistence/operation-log/compact/action-type-codes.ts +++ b/src/app/core/persistence/operation-log/compact/action-type-codes.ts @@ -147,6 +147,7 @@ export const ACTION_TYPE_TO_CODE: Record = { [ActionType.PROJECT_AUTO_MOVE_TO_BACKLOG]: 'PAB', [ActionType.PROJECT_AUTO_MOVE_FROM_BACKLOG]: 'PAR', [ActionType.PROJECT_MOVE_ALL_BACKLOG]: 'PBA', + [ActionType.PROJECT_COMPLETE]: 'PCO', // TaskRepeatCfg actions (R) [ActionType.REPEAT_CFG_ADD]: 'RA', diff --git a/src/app/core/persistence/operation-log/compact/compact-operation.types.ts b/src/app/core/persistence/operation-log/compact/compact-operation.types.ts index 9981cc88f2..19f6a200b3 100644 --- a/src/app/core/persistence/operation-log/compact/compact-operation.types.ts +++ b/src/app/core/persistence/operation-log/compact/compact-operation.types.ts @@ -11,6 +11,7 @@ import { VectorClock } from '../../../util/vector-clock'; * - e = entityType * - d = entityId * - ds = entityIds + * - ae = affectedEntities * - p = payload * - c = clientId * - v = vectorClock @@ -36,6 +37,9 @@ export interface CompactOperation { /** entityIds for batch (optional) */ ds?: string[]; + /** affectedEntities for mixed-entity batch operations (optional) */ + ae?: Array<{ entityType: string; entityId: string }>; + /** payload - unchanged */ p: unknown; diff --git a/src/app/core/persistence/operation-log/compact/operation-codec.service.spec.ts b/src/app/core/persistence/operation-log/compact/operation-codec.service.spec.ts index 9aa97c72e8..7d6ac29027 100644 --- a/src/app/core/persistence/operation-log/compact/operation-codec.service.spec.ts +++ b/src/app/core/persistence/operation-log/compact/operation-codec.service.spec.ts @@ -31,6 +31,10 @@ describe('operation-codec.service', () => { const mockOperationWithOptionals: Operation = { ...mockOperation, entityIds: ['task-1', 'task-2'], + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], }; describe('encodeOperation', () => { @@ -54,6 +58,10 @@ describe('operation-codec.service', () => { const compact = encodeOperation(mockOperationWithOptionals); expect(compact.ds).toEqual(['task-1', 'task-2']); + expect(compact.ae).toEqual([ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ]); }); }); diff --git a/src/app/core/persistence/operation-log/compact/operation-codec.service.ts b/src/app/core/persistence/operation-log/compact/operation-codec.service.ts index 4de4fab3d3..3d6d07b4cf 100644 --- a/src/app/core/persistence/operation-log/compact/operation-codec.service.ts +++ b/src/app/core/persistence/operation-log/compact/operation-codec.service.ts @@ -32,6 +32,9 @@ export const encodeOperation = (op: Operation): CompactOperation => { if (op.entityIds !== undefined) { compact.ds = op.entityIds; } + if (op.affectedEntities !== undefined) { + compact.ae = op.affectedEntities; + } if (op.syncImportReason !== undefined) { compact.r = op.syncImportReason; } @@ -63,6 +66,9 @@ export const decodeOperation = (compact: CompactOperation): Operation => { if (compact.ds !== undefined) { op.entityIds = compact.ds; } + if (compact.ae !== undefined) { + op.affectedEntities = compact.ae as Operation['affectedEntities']; + } if (compact.r !== undefined) { op.syncImportReason = compact.r as SyncImportReason; } diff --git a/src/app/features/calendar-integration/time-block/time-block-sync.effects.spec.ts b/src/app/features/calendar-integration/time-block/time-block-sync.effects.spec.ts index 35e62cdddb..69014ed714 100644 --- a/src/app/features/calendar-integration/time-block/time-block-sync.effects.spec.ts +++ b/src/app/features/calendar-integration/time-block/time-block-sync.effects.spec.ts @@ -520,4 +520,41 @@ describe('TimeBlockSyncEffects', () => { bulkSub.unsubscribe(); flush(); })); + + it('caps project-completion HTTP fan-out so it does not burst rate limits', fakeAsync(() => { + const projectCompleteSub = effects.upsertOnProjectComplete$.subscribe(); + const taskIds = ['t-1', 't-2', 't-3', 't-4', 't-5']; + + const resolvers: Array<() => void> = []; + upsertEventSpy.and.callFake( + () => + new Promise((resolve) => { + resolvers.push(resolve); + }), + ); + + actions$.next( + TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: Date.now(), + taskIdsToMarkDone: taskIds, + }), + ); + + expect(upsertEventSpy).toHaveBeenCalledTimes(3); + + resolvers[0](); + flushMicrotasks(); + expect(upsertEventSpy).toHaveBeenCalledTimes(4); + + resolvers[1](); + flushMicrotasks(); + expect(upsertEventSpy).toHaveBeenCalledTimes(5); + + resolvers.slice(2).forEach((r) => r()); + flushMicrotasks(); + expect(upsertEventSpy).toHaveBeenCalledTimes(5); + projectCompleteSub.unsubscribe(); + flush(); + })); }); diff --git a/src/app/features/calendar-integration/time-block/time-block-sync.effects.ts b/src/app/features/calendar-integration/time-block/time-block-sync.effects.ts index 5318fbfd28..921a4993a2 100644 --- a/src/app/features/calendar-integration/time-block/time-block-sync.effects.ts +++ b/src/app/features/calendar-integration/time-block/time-block-sync.effects.ts @@ -148,6 +148,37 @@ export class TimeBlockSyncEffects { { dispatch: false }, ); + upsertOnProjectComplete$: Observable = createEffect( + () => + this._actions$.pipe( + ofType(TaskSharedActions.completeProject), + concatMap(({ taskIdsToMarkDone = [], taskIdsToMarkUndone = [] }) => { + const taskIds = Array.from( + new Set([...taskIdsToMarkDone, ...taskIdsToMarkUndone]), + ); + return taskIds.length ? this._queueUpsertTimeBlocks$(taskIds) : EMPTY; + }), + ), + { dispatch: false }, + ); + + private _queueUpsertTimeBlocks$(taskIds: string[]): Observable { + return this._withTimeBlockContext$((ctx) => + from(taskIds).pipe( + mergeMap( + (taskId) => + from( + this._queueTimeBlockOperation(taskId, { + type: 'upsertIfScheduled', + ctx, + }), + ), + MAX_PARALLEL_TIME_BLOCK_HTTP, + ), + ), + ); + } + /** * When a task is unscheduled or transferred (which clears dueWithTime), * delete the time-block event. diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts index 3d28f7e38e..9c08d07bc5 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts @@ -173,8 +173,12 @@ describe('IssueTwoWaySyncEffects', () => { issueProviderId: 'provider-1', issueLastSyncedValues: { status: 'NEEDS-ACTION' }, }); + const taskAfterReducer = { + ...task, + isDone: true, + }; - taskServiceSpy.getByIdOnce$.and.returnValue(of(task)); + taskServiceSpy.getByIdOnce$.and.returnValues(of(task), of(taskAfterReducer)); issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(createMockIssueProvider())); effects.pushFieldsOnTaskUpdate$.subscribe(); @@ -187,6 +191,102 @@ describe('IssueTwoWaySyncEffects', () => { tick(); + expect(adapter.pushChanges).toHaveBeenCalledWith( + 'issue-1', + { status: 'COMPLETED' }, + jasmine.any(Object), + ); + + adapterRegistry.unregister('TEST_PROVIDER'); + })); + + it('should push reopened project-completion tasks to remote issue', fakeAsync(() => { + const adapter = createMockAdapter({ + getFieldMappings: jasmine + .createSpy('getFieldMappings') + .and.returnValue([isDoneFieldMapping]), + getSyncConfig: jasmine.createSpy('getSyncConfig').and.returnValue({}), + fetchIssue: jasmine + .createSpy('fetchIssue') + .and.resolveTo({ status: 'COMPLETED' }), + extractSyncValues: jasmine + .createSpy('extractSyncValues') + .and.returnValue({ status: 'COMPLETED' }), + }); + adapterRegistry.register('TEST_PROVIDER', adapter); + + const task = createMockTask({ + id: 'task-1', + issueType: 'TEST_PROVIDER' as any, + issueId: 'issue-1', + issueProviderId: 'provider-1', + issueLastSyncedValues: { status: 'COMPLETED' }, + isDone: false, + }); + + taskServiceSpy.getByIdOnce$.and.returnValue(of(task)); + issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(createMockIssueProvider())); + + effects.pushDoneStateOnProjectComplete$.subscribe(); + + actions$.next( + TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: Date.now(), + taskIdsToMarkUndone: ['task-1'], + }), + ); + + tick(); + + expect(adapter.pushChanges).toHaveBeenCalledWith( + 'issue-1', + { status: 'NEEDS-ACTION' }, + jasmine.any(Object), + ); + + adapterRegistry.unregister('TEST_PROVIDER'); + })); + + it('should push isDone changes from project completion to remote issue', fakeAsync(() => { + const adapter = createMockAdapter({ + getFieldMappings: jasmine + .createSpy('getFieldMappings') + .and.returnValue([isDoneFieldMapping]), + getSyncConfig: jasmine.createSpy('getSyncConfig').and.returnValue({}), + fetchIssue: jasmine + .createSpy('fetchIssue') + .and.resolveTo({ status: 'NEEDS-ACTION' }), + extractSyncValues: jasmine + .createSpy('extractSyncValues') + .and.returnValue({ status: 'NEEDS-ACTION' }), + }); + adapterRegistry.register('TEST_PROVIDER', adapter); + + const task = createMockTask({ + id: 'task-1', + issueType: 'TEST_PROVIDER' as any, + issueId: 'issue-1', + issueProviderId: 'provider-1', + issueLastSyncedValues: { status: 'NEEDS-ACTION' }, + isDone: true, + }); + + taskServiceSpy.getByIdOnce$.and.returnValue(of(task)); + issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(createMockIssueProvider())); + + effects.pushDoneStateOnProjectComplete$.subscribe(); + + actions$.next( + TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: Date.now(), + taskIdsToMarkDone: ['task-1'], + }), + ); + + tick(); + expect(adapter.pushChanges).toHaveBeenCalled(); adapterRegistry.unregister('TEST_PROVIDER'); diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts index 3c868bb4e7..60df4267e6 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts @@ -182,6 +182,64 @@ export class IssueTwoWaySyncEffects { { dispatch: false }, ); + pushDoneStateOnProjectComplete$: Observable = createEffect( + () => + this._actions$.pipe( + ofType(TaskSharedActions.completeProject), + concatMap((action) => + from([ + ...(action.taskIdsToMarkDone ?? []).map((taskId) => ({ + taskId, + changes: { isDone: true }, + })), + ...(action.taskIdsToMarkUndone ?? []).map((taskId) => ({ + taskId, + changes: { isDone: false }, + })), + ]), + ), + filter(({ taskId }) => { + if (this._syncOriginatedTaskIds.delete(taskId)) { + return false; + } + return true; + }), + concatMap(({ taskId, changes }) => + this._taskService.getByIdOnce$(taskId).pipe( + map((fullTask) => ({ + fullTask, + changes, + })), + ), + ), + filter(({ fullTask }) => { + if ( + !fullTask || + !fullTask.issueType || + !fullTask.issueProviderId || + !fullTask.issueId + ) { + return false; + } + return !!this._getAdapter(fullTask.issueType); + }), + concatMap(({ fullTask, changes }) => + this._pushChanges$(fullTask, changes).pipe( + catchError((err) => { + IssueLog.err('Two-way sync project completion push failed', err); + this._snackService.open({ + type: 'ERROR', + msg: T.F.ISSUE.S.TWO_WAY_SYNC_PUSH_FAILED, + translateParams: { errorMsg: getErrorTxt(err) }, + }); + return EMPTY; + }), + ), + ), + ), + { dispatch: false }, + ); + pushTagChangesAfterTagDelete$: Observable = createEffect( () => this._actions$.pipe( diff --git a/src/app/features/project/project.service.ts b/src/app/features/project/project.service.ts index fa48629ab3..dca0017e72 100644 --- a/src/app/features/project/project.service.ts +++ b/src/app/features/project/project.service.ts @@ -63,6 +63,8 @@ export interface ProjectCompletionInfo { export interface ProjectCompletionResolution { taskIdsToMarkDone?: string[]; topLevelTaskIdsToMoveToInbox?: string[]; + taskIdsToMoveToInbox?: string[]; + taskIdsToMarkUndone?: string[]; } @Injectable({ diff --git a/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.spec.ts b/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.spec.ts index 05111668ef..c33523eda3 100644 --- a/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.spec.ts +++ b/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.spec.ts @@ -1041,6 +1041,34 @@ describe('TaskRepeatCfgEffects - Repeatable Subtasks', () => { }); }); + it('should update repeat-on-complete cfgs when project completion marks a task done', () => { + testScheduler.run(({ hot, expectObservable }) => { + const action = TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: Date.now(), + taskIdsToMarkDone: ['parent-task-id'], + }); + + actions$ = hot('-a', { a: action }); + taskService.getByIdOnce$.and.returnValue(of(mockTask)); + taskRepeatCfgService.getTaskRepeatCfgById$.and.returnValue( + of({ ...mockRepeatCfg, repeatFromCompletionDate: true }), + ); + + const today = getDbDateStr(); + const expectedAction = updateTaskRepeatCfg({ + taskRepeatCfg: { + id: 'repeat-cfg-id', + changes: { startDate: today, lastTaskCreationDay: today }, + }, + }); + + expectObservable(effects.updateStartDateOnProjectComplete$).toBe('-a', { + a: expectedAction, + }); + }); + }); + it('should not emit when task has no repeatCfgId', () => { testScheduler.run(({ hot, expectObservable }) => { const action = TaskSharedActions.updateTask({ diff --git a/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.ts b/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.ts index 2800f0612d..b16f6694de 100644 --- a/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.ts +++ b/src/app/features/task-repeat-cfg/store/task-repeat-cfg.effects.ts @@ -1,5 +1,6 @@ import { inject, Injectable } from '@angular/core'; import { createEffect, ofType } from '@ngrx/effects'; +import { Action } from '@ngrx/store'; import { LOCAL_ACTIONS } from '../../../util/local-actions.token'; import { concatMap, @@ -610,63 +611,16 @@ export class TaskRepeatCfgEffects { this._localActions$.pipe( ofType(TaskSharedActions.updateTask), filter((a) => a.task.changes.isDone === true), - switchMap(({ task }) => - this._taskService.getByIdOnce$(task.id as string).pipe( - switchMap((fullTask) => { - if (fullTask) { - return rxOf(fullTask); - } - return from(this._taskArchiveService.load()).pipe( - map((archive) => archive.entities[task.id as string]), - ); - }), - ), - ), - filter((task): task is Task => !!task?.repeatCfgId), - switchMap((task) => - this._taskRepeatCfgService.getTaskRepeatCfgById$(task.repeatCfgId as string).pipe( - take(1), - map((cfg) => ({ task, cfg })), - ), - ), - filter( - ({ cfg }) => - !!cfg && - (cfg.repeatFromCompletionDate === true || cfg.waitForCompletion === true), - ), - concatMap(({ task, cfg }) => { - const today = this._dateService.todayStr(); - const isLatestInstance = this._isLatestInstance(task, cfg); + switchMap(({ task }) => this._getActionsForCompletedTask(task.id as string)), + mergeMap((actions) => actions), + ), + ); - // For repeatFromCompletionDate, update both startDate AND lastTaskCreationDay - // because getEffectiveRepeatStartDate() prioritizes lastTaskCreationDay when set. - // Without updating lastTaskCreationDay, the recurrence stays anchored to the old date. - if (cfg.repeatFromCompletionDate && isLatestInstance) { - return rxOf([ - updateTaskRepeatCfg({ - taskRepeatCfg: { - id: cfg.id as string, - changes: { - startDate: today, - lastTaskCreationDay: today, - }, - }, - }), - ]); - } - - // For waitForCompletion, probe creation after any repeat instance is - // completed. The service checks whether any live or archived instance - // still blocks the gate, so completing an older final blocker can - // materialize the next due task immediately. - if (cfg.waitForCompletion) { - return from( - this._taskRepeatCfgService._getActionsForTaskRepeatCfg(cfg, Date.now()), - ).pipe(map((nextTaskActions) => nextTaskActions)); - } - - return rxOf([]); - }), + updateStartDateOnProjectComplete$ = createEffect(() => + this._localActions$.pipe( + ofType(TaskSharedActions.completeProject), + mergeMap(({ taskIdsToMarkDone = [] }) => from(taskIdsToMarkDone)), + concatMap((taskId) => this._getActionsForCompletedTask(taskId)), mergeMap((actions) => actions), ), ); @@ -894,6 +848,64 @@ export class TaskRepeatCfgEffects { })); } + private _getActionsForCompletedTask(taskId: string): Observable { + return this._taskService.getByIdOnce$(taskId).pipe( + switchMap((fullTask) => { + if (fullTask) { + return rxOf(fullTask); + } + return from(this._taskArchiveService.load()).pipe( + map((archive) => archive.entities[taskId]), + ); + }), + filter((task): task is Task => !!task?.repeatCfgId), + switchMap((task) => + this._taskRepeatCfgService.getTaskRepeatCfgById$(task.repeatCfgId as string).pipe( + take(1), + map((cfg) => ({ task, cfg })), + ), + ), + filter( + ({ cfg }) => + !!cfg && + (cfg.repeatFromCompletionDate === true || cfg.waitForCompletion === true), + ), + concatMap(({ task, cfg }) => { + const today = this._dateService.todayStr(); + const isLatestInstance = this._isLatestInstance(task, cfg); + + // For repeatFromCompletionDate, update both startDate AND lastTaskCreationDay + // because getEffectiveRepeatStartDate() prioritizes lastTaskCreationDay when set. + // Without updating lastTaskCreationDay, the recurrence stays anchored to the old date. + if (cfg.repeatFromCompletionDate && isLatestInstance) { + return rxOf([ + updateTaskRepeatCfg({ + taskRepeatCfg: { + id: cfg.id as string, + changes: { + startDate: today, + lastTaskCreationDay: today, + }, + }, + }), + ]); + } + + // For waitForCompletion, probe creation after any repeat instance is + // completed. The service checks whether any live or archived instance + // still blocks the gate, so completing an older final blocker can + // materialize the next due task immediately. + if (cfg.waitForCompletion) { + return from( + this._taskRepeatCfgService._getActionsForTaskRepeatCfg(cfg, Date.now()), + ).pipe(map((nextTaskActions) => nextTaskActions)); + } + + return rxOf([]); + }), + ); + } + private _isLatestInstance(task: Task, cfg: TaskRepeatCfgCopy): boolean { const lastCreationDay = getEffectiveLastTaskCreationDay(cfg); if (!lastCreationDay) { diff --git a/src/app/op-log/apply/operation-converter.util.spec.ts b/src/app/op-log/apply/operation-converter.util.spec.ts index 6a3d3c971e..ffbe2815cb 100644 --- a/src/app/op-log/apply/operation-converter.util.spec.ts +++ b/src/app/op-log/apply/operation-converter.util.spec.ts @@ -361,6 +361,24 @@ describe('operation-converter utility', () => { } }); + it('should replay legacy project completion as the current shared action', () => { + const op = createMockOperation({ + actionType: ActionType.PROJECT_COMPLETE, + opType: OpType.Batch, + entityType: 'PROJECT', + entityId: 'project-1', + payload: { id: 'project-1', doneOn: 1_800_000_000_000 }, + }); + + const action = convertOpToAction(op); + + expect(action.type).toBe(ActionType.TASK_SHARED_COMPLETE_PROJECT); + expect(action.meta.entityType).toBe('PROJECT'); + expect(action.meta.entityId).toBe('project-1'); + expect((action as any).id).toBe('project-1'); + expect((action as any).doneOn).toBe(1_800_000_000_000); + }); + describe('legacy planTasksForToday date backfill', () => { it('injects today from the originating operation timestamp when missing', () => { const op = createMockOperation({ diff --git a/src/app/op-log/apply/operation-converter.util.ts b/src/app/op-log/apply/operation-converter.util.ts index d39073eaf6..8a0bc51233 100644 --- a/src/app/op-log/apply/operation-converter.util.ts +++ b/src/app/op-log/apply/operation-converter.util.ts @@ -22,7 +22,7 @@ import { getDbDateStr } from '../../util/get-db-date-str'; * reference the old action type. */ export const ACTION_TYPE_ALIASES: Record = { - // Example: '[Task] Update Task': '[Task] Update', + [ActionType.PROJECT_COMPLETE]: ActionType.TASK_SHARED_COMPLETE_PROJECT, }; /** @@ -218,6 +218,7 @@ export const convertOpToAction = (op: Operation): PersistentAction => { entityType: op.entityType, entityId: op.entityId, entityIds: op.entityIds, + affectedEntities: op.affectedEntities, opType: op.opType, isRemote: true, // Important to prevent re-logging during replay/sync }, diff --git a/src/app/op-log/capture/operation-log.effects.ts b/src/app/op-log/capture/operation-log.effects.ts index fb669a6621..16cf45d4ec 100644 --- a/src/app/op-log/capture/operation-log.effects.ts +++ b/src/app/op-log/capture/operation-log.effects.ts @@ -216,6 +216,7 @@ export class OperationLogEffects implements DeferredLocalActionsPort { entityType: action.meta.entityType, entityId, entityIds, + affectedEntities: action.meta.affectedEntities, payload: multiEntityPayload, clientId: clientId, vectorClock: newClock, diff --git a/src/app/op-log/core/action-types.enum.spec.ts b/src/app/op-log/core/action-types.enum.spec.ts index b751a9fbae..e6aeacbb5a 100644 --- a/src/app/op-log/core/action-types.enum.spec.ts +++ b/src/app/op-log/core/action-types.enum.spec.ts @@ -12,8 +12,8 @@ describe('ActionType enum', () => { const enumValues = Object.values(ActionType) as string[]; const mappingKeys = Object.keys(ACTION_TYPE_TO_CODE); - it('should have exactly 146 members', () => { - expect(enumValues.length).toBe(146); + it('should have exactly 147 members', () => { + expect(enumValues.length).toBe(147); }); it('should have 1:1 correspondence with ACTION_TYPE_TO_CODE', () => { @@ -62,6 +62,7 @@ describe('ActionType enum', () => { it('should have correct Project action types', () => { expect(ActionType.PROJECT_ADD).toBe('[Project] Add Project'); expect(ActionType.PROJECT_UPDATE).toBe('[Project] Update Project'); + expect(ActionType.PROJECT_COMPLETE).toBe('[Project] Complete Project'); }); it('should handle inconsistent spacing in SimpleCounter', () => { diff --git a/src/app/op-log/core/action-types.enum.ts b/src/app/op-log/core/action-types.enum.ts index 506df8c5d1..7019a4233e 100644 --- a/src/app/op-log/core/action-types.enum.ts +++ b/src/app/op-log/core/action-types.enum.ts @@ -126,6 +126,7 @@ export enum ActionType { PROJECT_AUTO_MOVE_TO_BACKLOG = '[Project] Auto Move Task from regular to backlog', PROJECT_AUTO_MOVE_FROM_BACKLOG = '[Project] Auto Move Task from backlog to regular', PROJECT_MOVE_ALL_BACKLOG = '[Project] Move all backlog tasks to regular', + PROJECT_COMPLETE = '[Project] Complete Project', // TaskRepeatCfg actions (R) REPEAT_CFG_ADD = '[TaskRepeatCfg][Task] Add TaskRepeatCfg to Task', diff --git a/src/app/op-log/core/operation.types.ts b/src/app/op-log/core/operation.types.ts index f6384ef052..1881d07cbf 100644 --- a/src/app/op-log/core/operation.types.ts +++ b/src/app/op-log/core/operation.types.ts @@ -7,6 +7,7 @@ import type { VectorClock, Operation as LibOperation, OperationLogEntry as LibOperationLogEntry, + AffectedEntity as LibAffectedEntity, EntityChange as LibEntityChange, EntityConflict as LibEntityConflict, ConflictResult as LibConflictResult, @@ -22,6 +23,7 @@ import { import { ActionType } from './action-types.enum'; export { OpType, extractActionPayload } from '@sp/sync-core'; +export { getOperationAffectedEntities } from '@sp/sync-core'; export type { VectorClock }; export { ENTITY_TYPES, ActionType }; @@ -57,9 +59,13 @@ export type SyncImportReason = * `entityType` to the app's enums and adds the optional `syncImportReason` * field carried on full-state ops. */ -export interface Operation extends Omit { +export interface Operation extends Omit< + LibOperation, + 'actionType' | 'entityType' | 'affectedEntities' +> { actionType: ActionType; entityType: EntityType; + affectedEntities?: AffectedEntity[]; /** * Optional reason for full-state operations (SYNC_IMPORT, BACKUP_IMPORT, REPAIR). * Used in the conflict dialog to explain why the import was created. @@ -72,6 +78,23 @@ export interface OperationLogEntry extends Omit { op: Operation; } +export interface AffectedEntity extends Omit { + entityType: EntityType; +} + +export const toAppAffectedEntities = ( + affectedEntities: + | ReadonlyArray<{ + entityType: string; + entityId: string; + }> + | undefined, +): AffectedEntity[] | undefined => + affectedEntities?.map((entity) => ({ + entityType: entity.entityType as EntityType, + entityId: entity.entityId, + })); + export interface EntityChange extends Omit { entityType: EntityType; } diff --git a/src/app/op-log/core/persistent-action.interface.ts b/src/app/op-log/core/persistent-action.interface.ts index 65d989af07..635d75ca7a 100644 --- a/src/app/op-log/core/persistent-action.interface.ts +++ b/src/app/op-log/core/persistent-action.interface.ts @@ -1,11 +1,12 @@ import { Action } from '@ngrx/store'; -import { EntityType, OpType } from './operation.types'; +import { AffectedEntity, EntityType, OpType } from './operation.types'; export interface PersistentActionMeta { isPersistent?: boolean; // When false, the action is blacklisted and not persisted entityType: EntityType; entityId?: string; // Optional if entityIds is provided entityIds?: string[]; // For batch operations + affectedEntities?: AffectedEntity[]; // Typed refs for mixed-entity batch operations opType: OpType; isRemote?: boolean; // TRUE if from Sync (prevents re-logging) isBulk?: boolean; // TRUE for batch operations diff --git a/src/app/op-log/persistence/operation-log-store.service.ts b/src/app/op-log/persistence/operation-log-store.service.ts index f7a41f6a60..90a10a6aaa 100644 --- a/src/app/op-log/persistence/operation-log-store.service.ts +++ b/src/app/op-log/persistence/operation-log-store.service.ts @@ -10,7 +10,7 @@ import { } from '../core/operation.types'; import { StorageQuotaExceededError } from '../core/errors/sync-errors'; import { toEntityKey } from '../util/entity-key.util'; -import { getOpEntityIds } from '../util/get-op-entity-ids.util'; +import { getOpAffectedEntities } from '../util/get-op-entity-ids.util'; import { encodeOperation, decodeOperation, @@ -808,9 +808,9 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort(); for (const entry of unsynced) { - const ids = getOpEntityIds(entry.op); - for (const id of ids) { - const key = toEntityKey(entry.op.entityType, id); + const affectedEntities = getOpAffectedEntities(entry.op); + for (const entity of affectedEntities) { + const key = toEntityKey(entity.entityType, entity.entityId); if (!map.has(key)) map.set(key, []); map.get(key)!.push(entry.op); } diff --git a/src/app/op-log/persistence/schema-migration.service.ts b/src/app/op-log/persistence/schema-migration.service.ts index f3a1ebdcbd..4a5f37451f 100644 --- a/src/app/op-log/persistence/schema-migration.service.ts +++ b/src/app/op-log/persistence/schema-migration.service.ts @@ -146,6 +146,7 @@ export class SchemaMigrationService { entityType: op.entityType, entityId: op.entityId, entityIds: op.entityIds, + affectedEntities: op.affectedEntities, payload: op.payload, schemaVersion: opVersion, }; @@ -168,6 +169,8 @@ export class SchemaMigrationService { entityType: migratedOpLike.entityType as Operation['entityType'], entityId: migratedOpLike.entityId, entityIds: migratedOpLike.entityIds, + affectedEntities: + migratedOpLike.affectedEntities as Operation['affectedEntities'], payload: migratedOpLike.payload, schemaVersion: migratedOpLike.schemaVersion, })); @@ -179,6 +182,8 @@ export class SchemaMigrationService { opType: result.data.opType as Operation['opType'], entityType: result.data.entityType as Operation['entityType'], entityId: result.data.entityId, + entityIds: result.data.entityIds, + affectedEntities: result.data.affectedEntities as Operation['affectedEntities'], payload: result.data.payload, schemaVersion: result.data.schemaVersion, }; diff --git a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts index def763692c..0a20399cdd 100644 --- a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts +++ b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts @@ -24,6 +24,7 @@ import { OpType, EntityType, SyncImportReason, + toAppAffectedEntities, } from '../../core/operation.types'; import { FileBasedSyncData, @@ -981,6 +982,7 @@ export class FileBasedSyncAdapterService { entityType: op.entityType as EntityType, entityId: op.entityId, entityIds: op.entityIds, + affectedEntities: toAppAffectedEntities(op.affectedEntities), payload: op.payload, clientId: op.clientId, vectorClock: op.vectorClock, @@ -1006,6 +1008,7 @@ export class FileBasedSyncAdapterService { entityType: fullOp.entityType, entityId: fullOp.entityId, entityIds: fullOp.entityIds, + affectedEntities: fullOp.affectedEntities, payload: fullOp.payload, vectorClock: fullOp.vectorClock, timestamp: fullOp.timestamp, diff --git a/src/app/op-log/sync/conflict-resolution.service.spec.ts b/src/app/op-log/sync/conflict-resolution.service.spec.ts index e5c95a5455..3987c198d7 100644 --- a/src/app/op-log/sync/conflict-resolution.service.spec.ts +++ b/src/app/op-log/sync/conflict-resolution.service.spec.ts @@ -2565,6 +2565,10 @@ describe('ConflictResolutionService', () => { opType: OpType.Update, entityType: 'TASK', entityId: 'task-1', + affectedEntities: [ + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'TAG', entityId: 'TODAY' }, + ], payload: { task: { id: 'task-1', title: 'Archived Task' } }, vectorClock: { [TEST_CLIENT_ID]: 1 }, timestamp: 1000, @@ -2601,6 +2605,9 @@ describe('ConflictResolutionService', () => { // Local archive wins — a new op should be created expect(result.localWinOpsCreated).toBe(1); expect(mockOpLogStore.appendWithVectorClockUpdate).toHaveBeenCalled(); + const appendedOp = mockOpLogStore.appendWithVectorClockUpdate.calls.first() + .args[0] as Operation; + expect(appendedOp.affectedEntities).toEqual(localArchiveOp.affectedEntities); }); it('should resolve remote moveToArchive winning over local UPDATE with later timestamp', async () => { @@ -2880,6 +2887,34 @@ describe('ConflictResolutionService', () => { expect(result.conflict).not.toBeNull(); expect(result.conflict!.entityId).toBe('task-1'); }); + + it('should not skip a mixed-entity remote op when only one affected entity is covered', async () => { + const remoteOp: Operation = { + ...createMockOp('remote-1', 'clientA'), + actionType: ActionType.TASK_SHARED_COMPLETE_PROJECT, + opType: OpType.Batch, + entityType: 'PROJECT', + entityId: 'project-1', + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], + vectorClock: { clientA: 1 }, + }; + + const appliedFrontierByEntity = new Map(); + appliedFrontierByEntity.set('PROJECT:project-1', { clientA: 1 }); + + const result = await service.checkOpForConflicts( + remoteOp, + buildCtx({ appliedFrontierByEntity }), + ); + + expect(result.isSupersededOrDuplicate).toBe(false); + expect(result.conflict).not.toBeNull(); + expect(result.conflict!.entityType).toBe('PROJECT'); + expect(result.conflict!.remoteOps).toEqual([remoteOp]); + }); }); describe('_convertToLWWUpdatesIfNeeded', () => { diff --git a/src/app/op-log/sync/conflict-resolution.service.ts b/src/app/op-log/sync/conflict-resolution.service.ts index d0c5730e8a..916c7c39f3 100644 --- a/src/app/op-log/sync/conflict-resolution.service.ts +++ b/src/app/op-log/sync/conflict-resolution.service.ts @@ -34,7 +34,7 @@ import { OperationApplierService } from '../apply/operation-applier.service'; import { OperationLogStoreService } from '../persistence/operation-log-store.service'; import { OpLog } from '../../core/log'; import { toEntityKey } from '../util/entity-key.util'; -import { getOpEntityIds } from '../util/get-op-entity-ids.util'; +import { getOpAffectedEntities } from '../util/get-op-entity-ids.util'; import { firstValueFrom } from 'rxjs'; import { SnackService } from '../../core/snack/snack.service'; import { T } from '../../t.const'; @@ -676,6 +676,7 @@ export class ConflictResolutionService { entityType: archiveOp.entityType, entityId: archiveOp.entityId, entityIds: archiveOp.entityIds, + affectedEntities: archiveOp.affectedEntities, payload: archiveOp.payload, clientId, vectorClock: newClock, @@ -894,26 +895,55 @@ export class ConflictResolutionService { hasNoSnapshotClock: boolean; }, ): Promise<{ isSupersededOrDuplicate: boolean; conflict: EntityConflict | null }> { - const entityIdsToCheck = getOpEntityIds(remoteOp); + const entitiesToCheck = getOpAffectedEntities(remoteOp); + let hasCoveredEntity = false; + let hasEntityThatNeedsApply = false; + let mixedCoverageConflict: EntityConflict | null = null; - for (const entityId of entityIdsToCheck) { - const entityKey = toEntityKey(remoteOp.entityType, entityId); + for (const entity of entitiesToCheck) { + const entityKey = toEntityKey(entity.entityType, entity.entityId); const localOpsForEntity = ctx.localPendingOpsByEntity.get(entityKey) || []; - const result = await this._checkEntityForConflict(remoteOp, entityId, entityKey, { - localOpsForEntity, - appliedFrontier: ctx.appliedFrontierByEntity.get(entityKey), - snapshotVectorClock: ctx.snapshotVectorClock, - snapshotEntityKeys: ctx.snapshotEntityKeys, - hasNoSnapshotClock: ctx.hasNoSnapshotClock, - }); + const result = await this._checkEntityForConflict( + remoteOp, + entity.entityType, + entity.entityId, + entityKey, + { + localOpsForEntity, + appliedFrontier: ctx.appliedFrontierByEntity.get(entityKey), + snapshotVectorClock: ctx.snapshotVectorClock, + snapshotEntityKeys: ctx.snapshotEntityKeys, + hasNoSnapshotClock: ctx.hasNoSnapshotClock, + }, + ); if (result.isSupersededOrDuplicate) { - return { isSupersededOrDuplicate: true, conflict: null }; + hasCoveredEntity = true; + mixedCoverageConflict ??= { + entityType: entity.entityType, + entityId: entity.entityId, + localOps: localOpsForEntity, + remoteOps: [remoteOp], + suggestedResolution: 'manual', + }; + continue; } if (result.conflict) { return { isSupersededOrDuplicate: false, conflict: result.conflict }; } + hasEntityThatNeedsApply = true; + } + + if (hasCoveredEntity && hasEntityThatNeedsApply) { + return { + isSupersededOrDuplicate: false, + conflict: mixedCoverageConflict, + }; + } + + if (hasCoveredEntity) { + return { isSupersededOrDuplicate: true, conflict: null }; } return { isSupersededOrDuplicate: false, conflict: null }; @@ -924,6 +954,7 @@ export class ConflictResolutionService { */ private async _checkEntityForConflict( remoteOp: Operation, + entityType: EntityType, entityId: string, entityKey: string, ctx: { @@ -972,14 +1003,11 @@ export class ConflictResolutionService { if (vcComparison === VectorClockComparison.CONCURRENT) { // CONCURRENT + no pending ops = entity may have been archived/deleted // by an already-synced operation. Check current state. - const entityState = await this.getCurrentEntityState( - remoteOp.entityType, - entityId, - ); + const entityState = await this.getCurrentEntityState(entityType, entityId); if (entityState === undefined || entityState === null) { OpLog.normal( `ConflictResolutionService: Skipping CONCURRENT remote op ${remoteOp.id} ` + - `for ${remoteOp.entityType}:${entityId} - entity no longer in state ` + + `for ${entityType}:${entityId} - entity no longer in state ` + `(archive/delete wins over concurrent update)`, ); return { isSupersededOrDuplicate: true, conflict: null }; @@ -993,7 +1021,7 @@ export class ConflictResolutionService { return { isSupersededOrDuplicate: false, conflict: { - entityType: remoteOp.entityType, + entityType, entityId, localOps: ctx.localOpsForEntity, remoteOps: [remoteOp], diff --git a/src/app/op-log/sync/operation-log-upload.service.ts b/src/app/op-log/sync/operation-log-upload.service.ts index d96ca0eb22..e225800b13 100644 --- a/src/app/op-log/sync/operation-log-upload.service.ts +++ b/src/app/op-log/sync/operation-log-upload.service.ts @@ -387,6 +387,7 @@ export class OperationLogUploadService { entityType: entry.op.entityType, entityId: entry.op.entityId, entityIds: entry.op.entityIds, + affectedEntities: entry.op.affectedEntities, payload: entry.op.payload, vectorClock: entry.op.vectorClock, timestamp: entry.op.timestamp, diff --git a/src/app/op-log/sync/operation-sync.util.ts b/src/app/op-log/sync/operation-sync.util.ts index 7a9a600369..a3d152f271 100644 --- a/src/app/op-log/sync/operation-sync.util.ts +++ b/src/app/op-log/sync/operation-sync.util.ts @@ -1,4 +1,10 @@ -import { ActionType, OpType, Operation, SyncImportReason } from '../core/operation.types'; +import { + ActionType, + OpType, + Operation, + SyncImportReason, + toAppAffectedEntities, +} from '../core/operation.types'; import { SyncProviderBase, OperationSyncCapable, @@ -66,6 +72,7 @@ export const syncOpToOperation = (syncOp: SyncOperation): Operation => { entityType: syncOp.entityType as Operation['entityType'], entityId: syncOp.entityId, entityIds: syncOp.entityIds, + affectedEntities: toAppAffectedEntities(syncOp.affectedEntities), payload: syncOp.payload, vectorClock: syncOp.vectorClock, timestamp: syncOp.timestamp, diff --git a/src/app/op-log/sync/rejected-ops-handler.service.ts b/src/app/op-log/sync/rejected-ops-handler.service.ts index 2556b927c6..9bf21822f4 100644 --- a/src/app/op-log/sync/rejected-ops-handler.service.ts +++ b/src/app/op-log/sync/rejected-ops-handler.service.ts @@ -9,6 +9,7 @@ import { DownloadCallback, RejectedOpInfo } from '../core/types/sync-results.typ import { handleStorageQuotaError } from './sync-error-utils'; import { MAX_CONCURRENT_RESOLUTION_ATTEMPTS } from '../core/operation-log.const'; import { toEntityKey } from '../util/entity-key.util'; +import { getOpAffectedEntities } from '../util/get-op-entity-ids.util'; // Re-export for consumers that import from this service export type { @@ -435,8 +436,8 @@ export class RejectedOpsHandlerService { } private _getEntityKey(op: Operation): string { - const entityId = op.entityId || op.entityIds?.[0]; - if (!entityId) { + const [entity] = getOpAffectedEntities(op); + if (!entity) { OpLog.warn( '[RejectedOpsHandler] Operation has no entityId/entityIds, using wildcard key', op.actionType, @@ -444,6 +445,6 @@ export class RejectedOpsHandlerService { ); return toEntityKey(op.entityType, '*'); } - return toEntityKey(op.entityType, entityId); + return toEntityKey(entity.entityType, entity.entityId); } } diff --git a/src/app/op-log/sync/remote-ops-processing.service.ts b/src/app/op-log/sync/remote-ops-processing.service.ts index 7133c7161a..9481c5f752 100644 --- a/src/app/op-log/sync/remote-ops-processing.service.ts +++ b/src/app/op-log/sync/remote-ops-processing.service.ts @@ -27,6 +27,7 @@ import { OperationLogCompactionService } from '../persistence/operation-log-comp import { SyncImportFilterService } from './sync-import-filter.service'; import { OperationWriteFlushService } from './operation-write-flush.service'; import { processDeferredActionsAfterRemoteApply } from './process-deferred-actions-flush.util'; +import { getOpAffectedEntities } from '../util/get-op-entity-ids.util'; /** * Handles the core pipeline for processing remote operations. @@ -151,12 +152,9 @@ export class RemoteOpsProcessingService { const migrated = this.schemaMigrationService.migrateOperation(op); if (migrated === null) { // Track dropped entity IDs for dependency warning - if (op.entityId) { - droppedEntityIds.add(op.entityId); - } - if (op.entityIds) { - op.entityIds.forEach((id) => droppedEntityIds.add(id)); - } + getOpAffectedEntities(op).forEach((entity) => + droppedEntityIds.add(entity.entityId), + ); OpLog.verbose( `RemoteOpsProcessingService: Dropped op ${op.id} (migrated to null)`, ); diff --git a/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts b/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts index b5ae21e39a..ee54256e27 100644 --- a/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts +++ b/src/app/op-log/sync/superseded-operation-resolver.service.spec.ts @@ -585,6 +585,7 @@ describe('SupersededOperationResolverService', () => { entityType: 'TASK', entityId: entityIds[0], entityIds, + affectedEntities: entityIds.map((entityId) => ({ entityType: 'TASK', entityId })), payload: { actionPayload: { tasks: entityIds.map((eid) => ({ id: eid, title: `Task ${eid}` })), @@ -622,6 +623,10 @@ describe('SupersededOperationResolverService', () => { expect(appendedOp.actionType).toBe(ActionType.TASK_SHARED_MOVE_TO_ARCHIVE); expect(appendedOp.opType).toBe(OpType.Update); expect(appendedOp.entityType).toBe('TASK'); + expect(appendedOp.affectedEntities).toEqual([ + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'TASK', entityId: 'task-2' }, + ]); }); it('should preserve original payload exactly', async () => { @@ -643,6 +648,56 @@ describe('SupersededOperationResolverService', () => { expect(appendedOp.payload).toEqual(archiveOp.payload); }); + it('should re-create completeProject batch op instead of project-only LWW update', async () => { + const completeProjectOp: Operation = { + id: 'op-complete-project-1', + actionType: ActionType.TASK_SHARED_COMPLETE_PROJECT, + opType: OpType.Batch, + entityType: 'PROJECT', + entityId: 'project-1', + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'TASK', entityId: 'task-2' }, + ], + payload: { + actionPayload: { + id: 'project-1', + doneOn: 1000, + taskIdsToMarkDone: ['task-1', 'task-2'], + }, + entityChanges: [], + }, + clientId: 'original-client', + vectorClock: { clientA: 5 }, + timestamp: 1000, + schemaVersion: 1, + }; + + mockVectorClockService.getCurrentVectorClock.and.returnValue( + Promise.resolve({ clientB: 2 }), + ); + + await service.resolveSupersededLocalOps([ + { + opId: 'op-complete-project-1', + op: completeProjectOp, + existingClock: { clientC: 3 }, + }, + ]); + + expect( + mockConflictResolutionService.getCurrentEntityState, + ).not.toHaveBeenCalled(); + const appendedOp = mockOpLogStore.appendWithVectorClockUpdate.calls.first() + .args[0] as Operation; + expect(appendedOp.actionType).toBe(ActionType.TASK_SHARED_COMPLETE_PROJECT); + expect(appendedOp.opType).toBe(OpType.Batch); + expect(appendedOp.payload).toEqual(completeProjectOp.payload); + expect(appendedOp.affectedEntities).toEqual(completeProjectOp.affectedEntities); + expect(appendedOp.vectorClock['clientC']).toBeGreaterThanOrEqual(3); + }); + it('should preserve entityId and entityIds in new operation', async () => { const archiveOp = createMockMoveToArchiveOperation( 'op-archive-1', diff --git a/src/app/op-log/sync/superseded-operation-resolver.service.ts b/src/app/op-log/sync/superseded-operation-resolver.service.ts index f51fcb5715..e326b9c063 100644 --- a/src/app/op-log/sync/superseded-operation-resolver.service.ts +++ b/src/app/op-log/sync/superseded-operation-resolver.service.ts @@ -59,6 +59,7 @@ export class SupersededOperationResolverService { entityType: sourceOp.entityType, entityId: sourceOp.entityId, entityIds: sourceOp.entityIds, + affectedEntities: sourceOp.affectedEntities, payload: sourceOp.payload, clientId, vectorClock, @@ -121,24 +122,21 @@ export class SupersededOperationResolverService { const newOpsCreated: Operation[] = []; // Handle bulk semantic operations BEFORE entity-by-entity grouping. - // moveToArchive uses OpType.Update but its reducer removes entities from the NgRx store - // (via deleteTaskHelper). This is the ONLY action with this pattern — all other entity - // removals use OpType.Delete (handled below). The normal resolution path would call - // getCurrentEntityState() → undefined → discard, permanently losing the archive. - // Instead, re-create the operation with a merged clock preserving the original payload. - // NOTE: If a future action type also removes entities with OpType.Update, add it here. + // Some semantic multi-entity actions cannot be replaced by a single LWW + // update for their primary entity. Re-create them with a merged clock, + // preserving the original payload and affected entity metadata. const regularSupersededOps: Array<{ opId: string; op: Operation; existingClock?: VectorClock; }> = []; for (const item of supersededOps) { - if (item.op.actionType === ActionType.TASK_SHARED_MOVE_TO_ARCHIVE) { - // Re-create the archive operation with a merged vector clock. - // The original payload is preserved exactly (MultiEntityPayload format with - // actionPayload.tasks containing full task data for remote archive writes). + if ( + item.op.actionType === ActionType.TASK_SHARED_MOVE_TO_ARCHIVE || + item.op.actionType === ActionType.TASK_SHARED_COMPLETE_PROJECT + ) { const mergedClock = this.conflictResolutionService.mergeAndIncrementClocks( - [globalClock, item.op.vectorClock], + [globalClock, item.op.vectorClock, item.existingClock ?? {}], clientId, ); // Don't prune here — the server prunes AFTER conflict detection (before storage). @@ -154,8 +152,8 @@ export class SupersededOperationResolverService { newOpsCreated.push(newOp); opsToReject.push(item.opId); OpLog.normal( - `SupersededOperationResolverService: Created replacement moveToArchive op ${newOp.id} ` + - `with ${item.op.entityIds?.length ?? 0} tasks, replacing superseded op ${item.opId}`, + `SupersededOperationResolverService: Created replacement semantic op ${newOp.id} ` + + `for ${item.op.actionType}, replacing superseded op ${item.opId}`, ); } else { regularSupersededOps.push(item); diff --git a/src/app/op-log/sync/sync-import-conflict-gate.service.spec.ts b/src/app/op-log/sync/sync-import-conflict-gate.service.spec.ts index d833226618..295785e67b 100644 --- a/src/app/op-log/sync/sync-import-conflict-gate.service.spec.ts +++ b/src/app/op-log/sync/sync-import-conflict-gate.service.spec.ts @@ -223,6 +223,45 @@ describe('SyncImportConflictGateService', () => { }); }); + it('should treat pending user batch ops as meaningful', async () => { + const incomingSyncImport = createOperation(); + const pendingProjectCompletionEntry = createEntry( + createOperation({ + id: 'local-project-complete', + actionType: ActionType.TASK_SHARED_COMPLETE_PROJECT, + opType: OpType.Batch, + entityType: 'PROJECT', + entityId: 'project-1', + affectedEntities: [ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'TASK', entityId: 'task-1' }, + ], + payload: { + actionPayload: { + id: 'project-1', + doneOn: 1_800_000_000_000, + taskIdsToMarkDone: ['task-1'], + }, + entityChanges: [], + }, + clientId: 'client-A', + vectorClock: { clientA: 1 }, + }), + ); + opLogStoreSpy.getUnsynced.and.resolveTo([pendingProjectCompletionEntry]); + + const result = await service.checkIncomingFullStateConflict([incomingSyncImport]); + + expect(result.hasMeaningfulPending).toBeTrue(); + expect(result.dialogData).toEqual({ + filteredOpCount: 1, + localImportTimestamp: 123, + syncImportReason: undefined, + scenario: 'INCOMING_IMPORT', + isNeverSynced: true, + }); + }); + it('should mark dialogData.isNeverSynced=false for an already-synced client', async () => { opLogStoreSpy.hasSyncedOps.and.resolveTo(true); const incomingSyncImport = createOperation(); diff --git a/src/app/op-log/sync/sync-import-conflict-gate.service.ts b/src/app/op-log/sync/sync-import-conflict-gate.service.ts index b8013aee3a..450b64a5cf 100644 --- a/src/app/op-log/sync/sync-import-conflict-gate.service.ts +++ b/src/app/op-log/sync/sync-import-conflict-gate.service.ts @@ -52,7 +52,9 @@ export class SyncImportConflictGateService { USER_ENTITY_TYPES.has(entry.op.entityType) && (entry.op.opType === OpType.Create || entry.op.opType === OpType.Update || - entry.op.opType === OpType.Delete) + entry.op.opType === OpType.Delete || + entry.op.opType === OpType.Move || + entry.op.opType === OpType.Batch) ); }); } diff --git a/src/app/op-log/sync/vector-clock.service.ts b/src/app/op-log/sync/vector-clock.service.ts index 616ff1f7d8..62daa51d40 100644 --- a/src/app/op-log/sync/vector-clock.service.ts +++ b/src/app/op-log/sync/vector-clock.service.ts @@ -3,7 +3,7 @@ import { OperationLogStoreService } from '../persistence/operation-log-store.ser import { VectorClock, EntityType } from '../core/operation.types'; import { mergeVectorClocks } from '../../core/util/vector-clock'; import { toEntityKey } from '../util/entity-key.util'; -import { getOpEntityIds } from '../util/get-op-entity-ids.util'; +import { getOpAffectedEntities } from '../util/get-op-entity-ids.util'; /** * Service for managing vector clocks in the operation log system. @@ -175,13 +175,13 @@ export class VectorClockService { // Skip rejected ops - they shouldn't affect the frontier if (entry.rejectedAt) continue; - const ids = getOpEntityIds(entry.op); + const affectedEntities = getOpAffectedEntities(entry.op); - for (const id of ids) { - if (entityType && entry.op.entityType !== entityType) continue; - if (entityId && id !== entityId) continue; + for (const entity of affectedEntities) { + if (entityType && entity.entityType !== entityType) continue; + if (entityId && entity.entityId !== entityId) continue; - const key = toEntityKey(entry.op.entityType, id); + const key = toEntityKey(entity.entityType, entity.entityId); map.set(key, entry.op.vectorClock); } } diff --git a/src/app/op-log/testing/integration/helpers/simulated-client.helper.ts b/src/app/op-log/testing/integration/helpers/simulated-client.helper.ts index af5f594d4a..30a5cbd91a 100644 --- a/src/app/op-log/testing/integration/helpers/simulated-client.helper.ts +++ b/src/app/op-log/testing/integration/helpers/simulated-client.helper.ts @@ -4,6 +4,7 @@ import { Operation, OperationLogEntry, OpType, + toAppAffectedEntities, } from '../../../core/operation.types'; import { OperationLogStoreService } from '../../../persistence/operation-log-store.service'; import { @@ -148,6 +149,7 @@ export class SimulatedClient { entityType: entry.op.entityType, entityId: entry.op.entityId, entityIds: entry.op.entityIds, + affectedEntities: entry.op.affectedEntities, payload: entry.op.payload, vectorClock: entry.op.vectorClock, timestamp: entry.op.timestamp, @@ -314,6 +316,7 @@ export class SimulatedClient { entityType: serverOp.op.entityType as any, entityId: serverOp.op.entityId, entityIds: serverOp.op.entityIds, + affectedEntities: toAppAffectedEntities(serverOp.op.affectedEntities), payload: serverOp.op.payload, vectorClock: serverOp.op.vectorClock, timestamp: serverOp.op.timestamp, diff --git a/src/app/op-log/testing/integration/helpers/test-client.helper.ts b/src/app/op-log/testing/integration/helpers/test-client.helper.ts index e9d9d2057d..f1ec5967d1 100644 --- a/src/app/op-log/testing/integration/helpers/test-client.helper.ts +++ b/src/app/op-log/testing/integration/helpers/test-client.helper.ts @@ -4,6 +4,7 @@ import { EntityType, VectorClock, ActionType, + AffectedEntity, } from '../../../core/operation.types'; import { mergeVectorClocks } from '../../../../core/util/vector-clock'; import { CURRENT_SCHEMA_VERSION } from '../../../persistence/schema-migration.service'; @@ -61,6 +62,7 @@ export class TestClient { entityId: string; payload: unknown; entityIds?: string[]; + affectedEntities?: AffectedEntity[]; }): Operation { // Increment our clock component before creating the operation this.vectorClock[this.clientId] = (this.vectorClock[this.clientId] || 0) + 1; @@ -72,6 +74,7 @@ export class TestClient { entityType: params.entityType, entityId: params.entityId, entityIds: params.entityIds, + affectedEntities: params.affectedEntities, payload: params.payload, clientId: this.clientId, vectorClock: { ...this.vectorClock }, diff --git a/src/app/op-log/util/get-op-entity-ids.util.ts b/src/app/op-log/util/get-op-entity-ids.util.ts index cad7785328..0b0d7521bd 100644 --- a/src/app/op-log/util/get-op-entity-ids.util.ts +++ b/src/app/op-log/util/get-op-entity-ids.util.ts @@ -1,3 +1,5 @@ +import { AffectedEntity, EntityType } from '../core/operation.types'; + /** * Normalizes an operation's entity references to a flat id list. * @@ -9,3 +11,33 @@ export const getOpEntityIds = (op: { entityId?: string; entityIds?: string[]; }): string[] => (op.entityIds?.length ? op.entityIds : op.entityId ? [op.entityId] : []); + +export const getOpAffectedEntities = (op: { + entityType: EntityType; + entityId?: string; + entityIds?: string[]; + affectedEntities?: AffectedEntity[]; +}): AffectedEntity[] => { + const rawEntities: AffectedEntity[] = [ + ...(op.affectedEntities ?? []), + ...getOpEntityIds(op).map((entityId) => ({ + entityType: op.entityType, + entityId, + })), + ]; + + const seen = new Set(); + const result: AffectedEntity[] = []; + for (const entity of rawEntities) { + if (!entity.entityType || !entity.entityId) { + continue; + } + const key = `${entity.entityType}\u0000${entity.entityId}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + result.push(entity); + } + return result; +}; diff --git a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.spec.ts index b6c3089706..d61c1a1289 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.spec.ts @@ -449,6 +449,35 @@ describe('projectSharedMetaReducer', () => { expect(updatedState[TASK_FEATURE_NAME].lastCurrentTaskId).toBe('task1'); }); + it('should clear reminder timestamps when marking unfinished tasks done', () => { + const testState = createStateWithExistingTasks(['task1'], [], [], []); + testState[TASK_FEATURE_NAME].entities.task1 = createMockTask({ + id: 'task1', + projectId: 'project1', + isDone: false, + remindAt: 1_800_000_010_000, + deadlineRemindAt: 1_800_000_020_000, + }); + + const action = TaskSharedActions.completeProject({ + id: 'project1', + doneOn, + taskIdsToMarkDone: ['task1'], + }); + + metaReducer(testState, action); + const updatedState = mockReducer.calls.mostRecent().args[0]; + + expect(updatedState[TASK_FEATURE_NAME].entities.task1).toEqual( + jasmine.objectContaining({ + isDone: true, + doneOn, + remindAt: undefined, + deadlineRemindAt: undefined, + }), + ); + }); + it('should move unresolved top-level task trees to Inbox before completing', () => { const testState = createStateWithExistingTasks(['task1'], [], [], []); testState[PROJECT_FEATURE_NAME].ids = ['project1', INBOX_PROJECT.id]; diff --git a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts index 3019998643..b92bc00a19 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/project-shared.reducer.ts @@ -135,7 +135,14 @@ const updateTasksDoneState = ( const update: Update = { id: taskId, - changes: isDone ? { isDone: true, doneOn } : { isDone: false }, + changes: isDone + ? { + isDone: true, + doneOn, + remindAt: undefined, + deadlineRemindAt: undefined, + } + : { isDone: false }, }; taskState = updateTimeEstimateForTask(update, null, taskState); diff --git a/src/app/root-store/meta/task-shared.actions.spec.ts b/src/app/root-store/meta/task-shared.actions.spec.ts new file mode 100644 index 0000000000..556e899384 --- /dev/null +++ b/src/app/root-store/meta/task-shared.actions.spec.ts @@ -0,0 +1,40 @@ +import { INBOX_PROJECT } from '../../features/project/project.const'; +import { TODAY_TAG } from '../../features/tag/tag.const'; +import { TaskSharedActions } from './task-shared.actions'; + +describe('TaskSharedActions', () => { + describe('completeProject', () => { + it('declares every entity touched by project completion resolution', () => { + const action = TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: 1_800_000_000_000, + taskIdsToMarkDone: ['task-1', 'task-2'], + topLevelTaskIdsToMoveToInbox: ['task-2'], + taskIdsToMoveToInbox: ['task-2', 'sub-task-1'], + taskIdsToMarkUndone: ['task-2'], + }); + + expect(action.meta.affectedEntities).toEqual([ + { entityType: 'PROJECT', entityId: 'project-1' }, + { entityType: 'PROJECT', entityId: INBOX_PROJECT.id }, + { entityType: 'TASK', entityId: 'task-1' }, + { entityType: 'TASK', entityId: 'task-2' }, + { entityType: 'TASK', entityId: 'sub-task-1' }, + { entityType: 'TAG', entityId: TODAY_TAG.id }, + ]); + }); + + it('marks Inbox affected when only full moved task ids are provided', () => { + const action = TaskSharedActions.completeProject({ + id: 'project-1', + doneOn: 1_800_000_000_000, + taskIdsToMoveToInbox: ['task-1', 'sub-task-1'], + }); + + expect(action.meta.affectedEntities).toContain({ + entityType: 'PROJECT', + entityId: INBOX_PROJECT.id, + }); + }); + }); +}); diff --git a/src/app/root-store/meta/task-shared.actions.ts b/src/app/root-store/meta/task-shared.actions.ts index c045ebc73f..228e3d969c 100644 --- a/src/app/root-store/meta/task-shared.actions.ts +++ b/src/app/root-store/meta/task-shared.actions.ts @@ -5,7 +5,55 @@ import { IssueDataReduced } from '../../features/issue/issue.model'; import { WorkContextType } from '../../features/work-context/work-context.model'; import { BatchOperation } from '@super-productivity/plugin-api'; import { PersistentActionMeta } from '../../op-log/core/persistent-action.interface'; -import { OpType } from '../../op-log/core/operation.types'; +import { AffectedEntity, EntityType, OpType } from '../../op-log/core/operation.types'; +import { INBOX_PROJECT } from '../../features/project/project.const'; +import { TODAY_TAG } from '../../features/tag/tag.const'; + +interface CompleteProjectProps { + id: string; + doneOn: number; + taskIdsToMarkDone?: string[]; + topLevelTaskIdsToMoveToInbox?: string[]; + taskIdsToMoveToInbox?: string[]; + taskIdsToMarkUndone?: string[]; +} + +const buildCompleteProjectAffectedEntities = ( + projectProps: CompleteProjectProps, +): AffectedEntity[] => { + const seen = new Set(); + const affectedEntities: AffectedEntity[] = []; + const addEntity = (entityType: EntityType, entityId: string): void => { + const key = `${entityType}\u0000${entityId}`; + if (seen.has(key)) { + return; + } + seen.add(key); + affectedEntities.push({ entityType, entityId }); + }; + + addEntity('PROJECT', projectProps.id); + + if ( + projectProps.topLevelTaskIdsToMoveToInbox?.length || + projectProps.taskIdsToMoveToInbox?.length + ) { + addEntity('PROJECT', INBOX_PROJECT.id); + } + + [ + ...(projectProps.taskIdsToMarkDone ?? []), + ...(projectProps.topLevelTaskIdsToMoveToInbox ?? []), + ...(projectProps.taskIdsToMoveToInbox ?? []), + ...(projectProps.taskIdsToMarkUndone ?? []), + ].forEach((taskId) => addEntity('TASK', taskId)); + + if (projectProps.taskIdsToMarkDone?.length) { + addEntity('TAG', TODAY_TAG.id); + } + + return affectedEntities; +}; /** * Shared actions that affect multiple reducers (tasks, projects, tags) @@ -301,17 +349,13 @@ export const TaskSharedActions = createActionGroup({ } satisfies PersistentActionMeta, }), - completeProject: (projectProps: { - id: string; - doneOn: number; - taskIdsToMarkDone?: string[]; - topLevelTaskIdsToMoveToInbox?: string[]; - }) => ({ + completeProject: (projectProps: CompleteProjectProps) => ({ ...projectProps, meta: { isPersistent: true, entityType: 'PROJECT', entityId: projectProps.id, + affectedEntities: buildCompleteProjectAffectedEntities(projectProps), opType: OpType.Batch, } satisfies PersistentActionMeta, }),