mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
[codex] Fix sync import conflict from startup example tasks (#7976)
* fix(sync): ignore startup example tasks during import
* fix(sync): defer startup example tasks until initial sync completes
Switch ExampleTasksService to afterInitialSyncDoneStrict$ so example tasks
are not created before the first remote import lands. On a fresh synced
client the imported tasks are then already in the store and the length===0
guard skips creation entirely — closing the same conflict for file-based
providers (Dropbox/WebDAV), which the op-log marker gate does not cover.
The isExampleTask marker + gate exclusion stay as a safety net for the
narrow case where example tasks are created on a still-empty server and an
import arrives before they are uploaded.
Also dedupe the two markRejected call sites into _discardExampleTaskOps and
document the local-only read (a remote isExampleTask flag can never bypass
the conflict dialog) and the intentional mixed-case behavior.
* test(sync): cover mixed-pending and empty-discard import gate cases
Add a SyncImportConflictGateService test for the mixed case (real pending
work + startup example tasks): the conflict dialog is still shown, but the
example-task id is reported in discardablePendingOpIds and intentionally
left for the caller to keep on USE_LOCAL — locking the documented invariant.
Also assert markRejected is not called on the config-only silent-accept path,
pinning the empty-array guard in _discardExampleTaskOps.
* test(sync): integration coverage for example-task import gate
Run the real OperationLogStoreService + SyncImportConflictGateService against
IndexedDB (no mocks) to cover the seam the unit specs stub:
- example-task creates persisted with their real multi-entity payload are
recognized as non-meaningful and reported as discardable
- after markRejected they are actually excluded from getUnsynced() (never
uploaded), while a pending config op is preserved
- real user work alongside example tasks still triggers the dialog
- a remote-sourced example-task op is never discardable (gate reads local
pending only)
Verified load-bearing: removing the gate exclusion turns the first case red.
* fix(sync): mark onboarding done when example tasks are skipped
EXAMPLE_TASKS_CREATED was only written after creating example tasks, so a
synced client that skipped creation because real tasks already existed never
set the flag — and could recreate onboarding tasks on a later startup when
the task list happened to be empty.
Set the flag whenever tasks already exist at the initial-sync gate, so
onboarding runs at most once per client.
* docs(sync): note known limits of the example-task import gate
Document two accepted, narrow residuals of syncing onboarding example tasks:
- upload→piggyback path: example ops accepted in the same upload round are
already synced and not in the discard list; state stays correct because the
SYNC_IMPORT filter drops them as CONCURRENT on receivers.
- file-based snapshot path: hasMeaningfulStoreData() counts example tasks (no
in-state marker), so a fresh Dropbox/WebDAV client that made example tasks
while sync was disabled can still see the conflict dialog.
* test(sync): cover fresh-client example-task import gate (e2e)
Add a SuperSync e2e proving a fresh client with first-run example tasks accepts
an incoming SYNC_IMPORT without a conflict dialog, plus a backward-compatible
{ allowExampleTasks } opt-in to createSimulatedClient (the harness otherwise
pre-sets SUP_EXAMPLE_TASKS_CREATED).
Uses waitForInitialSync:false + a race between the conflict dialog and sync
completion so the test actually fails when the dialog appears (pre-fix), and
asserts all four onboarding titles are absent. Guards the op-log isExampleTask
marker/gate path (not the afterInitialSyncDoneStrict$ timing, which a fresh
e2e client cannot exercise since sync is disabled at boot).
* docs(sync): link example-task import-gate limitations to #7985
This commit is contained in:
parent
fcc4216391
commit
dc0378edd6
10 changed files with 533 additions and 9 deletions
116
e2e/tests/sync/supersync-example-task-fresh-client.spec.ts
Normal file
116
e2e/tests/sync/supersync-example-task-fresh-client.spec.ts
Normal file
|
|
@ -0,0 +1,116 @@
|
|||
import { test, expect } from '../../fixtures/supersync.fixture';
|
||||
import {
|
||||
createTestUser,
|
||||
getSuperSyncConfig,
|
||||
createSimulatedClient,
|
||||
closeClient,
|
||||
waitForTask,
|
||||
getTaskTitles,
|
||||
type SimulatedE2EClient,
|
||||
} from '../../utils/supersync-helpers';
|
||||
|
||||
/**
|
||||
* Regression test for #7976 — a fresh client's first-run example tasks must not trip the
|
||||
* SYNC_IMPORT conflict gate when pulling an account that already has data.
|
||||
*
|
||||
* THE BUG: ExampleTasksService creates onboarding task-create ops on first run. When a
|
||||
* fresh client then syncs an account that already has remote data, those pending ops used
|
||||
* to make the conflict gate treat the client as having meaningful local work, so it showed
|
||||
* a `dialog-sync-import-conflict` instead of silently accepting the import.
|
||||
*
|
||||
* WHAT THIS GUARDS: the op-log `isExampleTask` marker + gate exclusion. It does NOT
|
||||
* exercise the afterInitialSyncDoneStrict$ timing — on a fresh e2e client sync is disabled
|
||||
* at boot, so example tasks are created before sync is configured regardless of the strict
|
||||
* gate; the marker is what suppresses the dialog here. Covering the strict-wait timing
|
||||
* needs a client with sync pre-enabled at boot (a separate test).
|
||||
*
|
||||
* METHOD: configure sync with `waitForInitialSync: false` so setupSuperSync does NOT
|
||||
* auto-resolve the conflict dialog (it otherwise clicks "Use Server Data"), then race
|
||||
* "conflict dialog visible" against "sync complete". The fix means sync completes WITHOUT
|
||||
* the dialog; pre-fix the dialog wins the race.
|
||||
*
|
||||
* REPRODUCE-FIRST: expected to FAIL on `bb4b625645^` (race resolves to 'dialog') and PASS
|
||||
* on the PR head. Run on both before trusting it.
|
||||
*
|
||||
* Run: npm run e2e:supersync:file e2e/tests/sync/supersync-example-task-fresh-client.spec.ts -- --retries=0
|
||||
*/
|
||||
const EXAMPLE_TASK_TITLES = [
|
||||
'Create your first project',
|
||||
'Set up Sync',
|
||||
'Learn the keyboard shortcuts',
|
||||
'Go further',
|
||||
];
|
||||
|
||||
test.describe('@supersync Fresh-client example tasks vs incoming import (#7976)', () => {
|
||||
test('import is accepted without an example-task conflict dialog', async ({
|
||||
browser,
|
||||
baseURL,
|
||||
testRunId,
|
||||
}) => {
|
||||
const appUrl = baseURL || 'http://localhost:4242';
|
||||
const uniqueId = Date.now();
|
||||
let seeder: SimulatedE2EClient | null = null;
|
||||
let freshClient: SimulatedE2EClient | null = null;
|
||||
|
||||
try {
|
||||
const user = await createTestUser(testRunId);
|
||||
const syncConfig = getSuperSyncConfig(user);
|
||||
|
||||
// Seed the account with real data (example tasks suppressed for the seeder).
|
||||
seeder = await createSimulatedClient(browser, appUrl, 'Seeder', testRunId);
|
||||
await seeder.sync.setupSuperSync(syncConfig);
|
||||
const realTask = `Real-Task-${uniqueId}`;
|
||||
await seeder.workView.addTask(realTask);
|
||||
await seeder.sync.syncAndWait();
|
||||
|
||||
// Fresh client WITH onboarding example tasks (created at boot, before sync config).
|
||||
freshClient = await createSimulatedClient(browser, appUrl, 'Fresh', testRunId, {
|
||||
allowExampleTasks: true,
|
||||
});
|
||||
|
||||
// Configure sync but do NOT let setup auto-resolve the conflict dialog
|
||||
// (waitForInitialSync:true would click "Use Server Data" and hide the bug).
|
||||
await freshClient.sync.setupSuperSync({
|
||||
...syncConfig,
|
||||
waitForInitialSync: false,
|
||||
});
|
||||
|
||||
// Race: does the example-task conflict dialog appear, or does the initial sync
|
||||
// complete cleanly? (Pattern from supersync-import-clean-slate.spec.ts.)
|
||||
const syncResult = await Promise.race([
|
||||
freshClient.sync.syncImportConflictDialog
|
||||
.waitFor({ state: 'visible', timeout: 30000 })
|
||||
.then(() => 'dialog' as const),
|
||||
freshClient.sync.syncCheckIcon
|
||||
.waitFor({ state: 'visible', timeout: 30000 })
|
||||
.then(() => 'complete' as const),
|
||||
]);
|
||||
|
||||
// CORE REGRESSION: the import is accepted silently — no example-task conflict dialog.
|
||||
// Pre-fix this resolves to 'dialog'.
|
||||
expect(syncResult).toBe('complete');
|
||||
|
||||
// The import replaced local state: the real remote task is present and NONE of the
|
||||
// onboarding example tasks survive. Example tasks live in the INBOX project.
|
||||
// (If the seeder's task lands elsewhere on your setup, adjust this navigation.)
|
||||
await freshClient.page.goto('/#/project/INBOX_PROJECT/tasks', {
|
||||
waitUntil: 'domcontentloaded',
|
||||
timeout: 30000,
|
||||
});
|
||||
await freshClient.page.waitForLoadState('networkidle');
|
||||
await waitForTask(freshClient.page, realTask);
|
||||
|
||||
const titles = await getTaskTitles(freshClient);
|
||||
for (const exampleTitle of EXAMPLE_TASK_TITLES) {
|
||||
expect(titles).not.toContain(exampleTitle);
|
||||
}
|
||||
} finally {
|
||||
if (freshClient) {
|
||||
await closeClient(freshClient);
|
||||
}
|
||||
if (seeder) {
|
||||
await closeClient(seeder);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -211,7 +211,9 @@ export const createSimulatedClient = async (
|
|||
baseURL: string,
|
||||
clientName: string,
|
||||
testPrefix: string,
|
||||
options: { allowExampleTasks?: boolean } = {},
|
||||
): Promise<SimulatedE2EClient> => {
|
||||
const { allowExampleTasks = false } = options;
|
||||
// Use provided baseURL or fall back to localhost:4242 (Playwright fixture may be undefined)
|
||||
const effectiveBaseURL = baseURL || 'http://localhost:4242';
|
||||
|
||||
|
|
@ -228,12 +230,16 @@ export const createSimulatedClient = async (
|
|||
|
||||
// Skip onboarding, hints, and example tasks before the app boots.
|
||||
// This runs before any page JavaScript, so Angular sees the flags immediately.
|
||||
await page.addInitScript(() => {
|
||||
// Tests of the example-task sync gate opt back in via { allowExampleTasks: true }
|
||||
// so first-run onboarding tasks are actually created.
|
||||
await page.addInitScript((allowExamples) => {
|
||||
localStorage.setItem('SUP_ONBOARDING_PRESET_DONE', 'true');
|
||||
localStorage.setItem('SUP_ONBOARDING_HINTS_DONE', 'true');
|
||||
localStorage.setItem('SUP_IS_SHOW_TOUR', 'true');
|
||||
localStorage.setItem('SUP_EXAMPLE_TASKS_CREATED', 'true');
|
||||
});
|
||||
if (!allowExamples) {
|
||||
localStorage.setItem('SUP_EXAMPLE_TASKS_CREATED', 'true');
|
||||
}
|
||||
}, allowExampleTasks);
|
||||
|
||||
page.on('console', (msg) => {
|
||||
if (msg.type() === 'error') {
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ describe('ExampleTasksService', () => {
|
|||
{
|
||||
provide: SyncTriggerService,
|
||||
useValue: {
|
||||
afterInitialSyncDoneAndDataLoadedInitially$: syncReady$,
|
||||
afterInitialSyncDoneStrict$: syncReady$,
|
||||
},
|
||||
},
|
||||
{ provide: TaskService, useValue: taskService },
|
||||
|
|
@ -93,11 +93,12 @@ describe('ExampleTasksService', () => {
|
|||
expect(action.workContextType).toBe(WorkContextType.PROJECT);
|
||||
expect(action.isAddToBacklog).toBe(false);
|
||||
expect(action.isAddToBottom).toBe(true);
|
||||
expect(action.isExampleTask).toBe(true);
|
||||
}
|
||||
expect(localStorage.getItem(LS.EXAMPLE_TASKS_CREATED)).toBe('true');
|
||||
});
|
||||
|
||||
it('should NOT create example tasks when tasks already exist', () => {
|
||||
it('should NOT create example tasks when tasks already exist, but mark onboarding done', () => {
|
||||
store.overrideSelector(selectAllTasks, [
|
||||
{ id: 'existing', title: 'Existing' } as Task,
|
||||
]);
|
||||
|
|
@ -107,7 +108,8 @@ describe('ExampleTasksService', () => {
|
|||
|
||||
expect(taskService.createNewTaskWithDefaults).not.toHaveBeenCalled();
|
||||
expect(dispatchSpy).not.toHaveBeenCalled();
|
||||
expect(localStorage.getItem(LS.EXAMPLE_TASKS_CREATED)).toBeNull();
|
||||
// Flag is set so a future empty-task startup does not recreate example tasks (#7976).
|
||||
expect(localStorage.getItem(LS.EXAMPLE_TASKS_CREATED)).toBe('true');
|
||||
});
|
||||
|
||||
it('should NOT create example tasks when localStorage flag is already set', () => {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { TranslateService } from '@ngx-translate/core';
|
||||
import { filter, first, switchMap } from 'rxjs/operators';
|
||||
import { filter, first, switchMap, tap } from 'rxjs/operators';
|
||||
import { selectAllTasks } from '../../features/tasks/store/task.selectors';
|
||||
import { TaskService } from '../../features/tasks/task.service';
|
||||
import { T } from '../../t.const';
|
||||
|
|
@ -54,10 +54,26 @@ export class ExampleTasksService {
|
|||
return;
|
||||
}
|
||||
|
||||
this._syncTriggerService.afterInitialSyncDoneAndDataLoadedInitially$
|
||||
// Wait for the STRICT initial-sync signal. For SuperSync the non-strict signal
|
||||
// resolves immediately (before the first download completes), so example tasks
|
||||
// would be created before an incoming SYNC_IMPORT lands and then collide with it.
|
||||
// Waiting for the actual initial sync means any imported tasks are already in the
|
||||
// store, so the `length === 0` guard below short-circuits and no example tasks are
|
||||
// created on a fresh synced client (this also covers file-based providers, which
|
||||
// the op-log conflict gate cannot). The `isExampleTask` marker on the dispatched
|
||||
// action below stays as a safety net for the narrow case where example tasks are
|
||||
// created on a still-empty server and an import arrives before they are uploaded.
|
||||
this._syncTriggerService.afterInitialSyncDoneStrict$
|
||||
.pipe(
|
||||
first(),
|
||||
switchMap(() => this._store.select(selectAllTasks).pipe(first())),
|
||||
// Tasks already exist (e.g. synced from another device): mark onboarding done
|
||||
// so a future empty-task startup does not recreate example tasks. (#7976)
|
||||
tap((tasks) => {
|
||||
if (tasks.length > 0) {
|
||||
localStorage.setItem(LS.EXAMPLE_TASKS_CREATED, 'true');
|
||||
}
|
||||
}),
|
||||
filter((tasks) => tasks.length === 0),
|
||||
switchMap(() => {
|
||||
const keys = EXAMPLE_TASK_DEFS.flatMap((def) => [def.titleKey, def.notesKey]);
|
||||
|
|
@ -77,7 +93,13 @@ export class ExampleTasksService {
|
|||
additional: { notes: translations[def.notesKey] },
|
||||
...TASK_CONTEXT,
|
||||
});
|
||||
this._store.dispatch(TaskSharedActions.addTask({ task, ...TASK_CONTEXT }));
|
||||
this._store.dispatch(
|
||||
TaskSharedActions.addTask({
|
||||
task,
|
||||
...TASK_CONTEXT,
|
||||
isExampleTask: true,
|
||||
}),
|
||||
);
|
||||
}
|
||||
localStorage.setItem(LS.EXAMPLE_TASKS_CREATED, 'true');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2522,6 +2522,89 @@ describe('OperationLogSyncService', () => {
|
|||
expect(
|
||||
syncImportConflictDialogServiceSpy.showConflictDialog,
|
||||
).not.toHaveBeenCalled();
|
||||
// No example-task ops pending -> nothing is rejected (empty-array guard).
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
incomingSyncImport,
|
||||
]);
|
||||
expect(mockProvider.setLastServerSeq).toHaveBeenCalledWith(42);
|
||||
expect(result.kind).toBe('ops_processed');
|
||||
});
|
||||
|
||||
it('should process incoming SYNC_IMPORT when pending ops are only config and startup example tasks', async () => {
|
||||
const incomingSyncImport = createIncomingSyncImport();
|
||||
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [incomingSyncImport],
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 42,
|
||||
});
|
||||
|
||||
const pendingConfigEntry: OperationLogEntry = {
|
||||
seq: 1,
|
||||
op: {
|
||||
id: 'local-config-op-1',
|
||||
clientId: 'client-A',
|
||||
actionType: ActionType.GLOBAL_CONFIG_UPDATE_SECTION,
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
payload: { sectionKey: 'sync' },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
};
|
||||
const pendingExampleTaskEntries: OperationLogEntry[] = [1, 2, 3, 4].map(
|
||||
(counter) => ({
|
||||
seq: counter + 1,
|
||||
op: {
|
||||
id: `local-example-task-op-${counter}`,
|
||||
clientId: 'client-A',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: `example-task-${counter}`,
|
||||
payload: {
|
||||
actionPayload: {
|
||||
task: { id: `example-task-${counter}` },
|
||||
isExampleTask: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
vectorClock: { clientA: counter + 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
}),
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
pendingConfigEntry,
|
||||
...pendingExampleTaskEntries,
|
||||
]);
|
||||
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(
|
||||
syncImportConflictDialogServiceSpy.showConflictDialog,
|
||||
).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'local-example-task-op-1',
|
||||
'local-example-task-op-2',
|
||||
'local-example-task-op-3',
|
||||
'local-example-task-op-4',
|
||||
]);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
incomingSyncImport,
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -250,6 +250,14 @@ export class OperationLogSyncService {
|
|||
rejectedOps: [],
|
||||
};
|
||||
} else {
|
||||
// Known limitation (#7985, upload→piggyback path): example-task ops accepted earlier in
|
||||
// THIS same upload round were already marked synced, so they have left
|
||||
// getUnsynced() and are absent from discardablePendingOpIds here — they remain on
|
||||
// the server. State stays correct because receivers drop them as CONCURRENT
|
||||
// against the import (SyncImportFilterService). Only reachable in the narrow window
|
||||
// where example tasks are created on a still-empty server and uploaded just as a
|
||||
// remote import arrives; afterInitialSyncDoneStrict$ shrinks it further.
|
||||
await this._discardExampleTaskOps(piggybackedConflict.discardablePendingOpIds);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Accepting piggybacked ${fullStateOp.opType} from client ` +
|
||||
`${fullStateOp.clientId} without conflict dialog; ` +
|
||||
|
|
@ -441,6 +449,12 @@ export class OperationLogSyncService {
|
|||
// The store check catches provider-switch scenarios: user switches from
|
||||
// SuperSync→Dropbox, only has a config-change op (not "meaningful"), but the
|
||||
// store is full of real data that would be overwritten by old Dropbox state.
|
||||
//
|
||||
// Known limitation (#7985): hasMeaningfulStoreData() counts ANY task, including onboarding
|
||||
// example tasks (they carry the isExampleTask marker only on their op-log ops, not
|
||||
// in NgRx state). So a fresh file-based client (Dropbox/WebDAV) that created example
|
||||
// tasks while sync was disabled — or before a slow first sync completed — can still
|
||||
// hit this dialog. afterInitialSyncDoneStrict$ shrinks but does not close that window.
|
||||
const hasMeaningfulUserData =
|
||||
this.syncImportConflictGateService.hasMeaningfulPendingOps(unsyncedOps) ||
|
||||
this.syncLocalStateService.hasMeaningfulStoreData();
|
||||
|
|
@ -678,6 +692,7 @@ export class OperationLogSyncService {
|
|||
// the session-validation latch — wrapper reads it. (#7330)
|
||||
return { kind: 'no_new_ops' };
|
||||
} else {
|
||||
await this._discardExampleTaskOps(incomingConflict.discardablePendingOpIds);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Accepting incoming ${fullStateOp.opType} from client ` +
|
||||
`${fullStateOp.clientId} without conflict dialog; ` +
|
||||
|
|
@ -766,6 +781,22 @@ export class OperationLogSyncService {
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Rejects the auto-generated startup example-task ops so they are NOT uploaded
|
||||
* after a SYNC_IMPORT is accepted silently. They were already excluded from the
|
||||
* conflict gate's "meaningful work" check (see SyncImportConflictGateService); the
|
||||
* import replaces local state, so rejecting them keeps the op-log consistent with
|
||||
* the just-applied remote data instead of re-uploading throwaway onboarding tasks.
|
||||
*
|
||||
* These ids always come from getUnsynced() (local pending ops, never remote ops),
|
||||
* so a remote `isExampleTask` flag can never reach this path.
|
||||
*/
|
||||
private async _discardExampleTaskOps(opIds: string[]): Promise<void> {
|
||||
if (opIds.length > 0) {
|
||||
await this.opLogStore.markRejected(opIds);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shows the SYNC_IMPORT conflict dialog and executes the user's chosen action.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -112,6 +112,85 @@ describe('SyncImportConflictGateService', () => {
|
|||
expect(result.dialogData).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should not produce dialog data when pending task creates are startup example tasks', async () => {
|
||||
const incomingSyncImport = createOperation();
|
||||
const pendingExampleTaskEntry = createEntry(
|
||||
createOperation({
|
||||
id: 'local-example-task-create',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: 'example-task-1',
|
||||
payload: {
|
||||
actionPayload: {
|
||||
task: { id: 'example-task-1' },
|
||||
isExampleTask: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 1 },
|
||||
}),
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([pendingExampleTaskEntry]);
|
||||
|
||||
const result = await service.checkIncomingFullStateConflict([incomingSyncImport]);
|
||||
|
||||
expect(result.fullStateOp).toBe(incomingSyncImport);
|
||||
expect(result.pendingOps).toEqual([pendingExampleTaskEntry]);
|
||||
expect(result.hasMeaningfulPending).toBeFalse();
|
||||
expect(result.discardablePendingOpIds).toEqual(['local-example-task-create']);
|
||||
expect(result.dialogData).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reports example-task ids as discardable but still shows the dialog when real work is also pending', async () => {
|
||||
const incomingSyncImport = createOperation();
|
||||
const pendingRealTaskEntry = createEntry(
|
||||
createOperation({
|
||||
id: 'local-task-update',
|
||||
actionType: 'test' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
payload: { title: 'Local title' },
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 1 },
|
||||
}),
|
||||
);
|
||||
const pendingExampleTaskEntry = createEntry(
|
||||
createOperation({
|
||||
id: 'local-example-task-create',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: 'example-task-1',
|
||||
payload: {
|
||||
actionPayload: {
|
||||
task: { id: 'example-task-1' },
|
||||
isExampleTask: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 2 },
|
||||
}),
|
||||
{ seq: 2 },
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
pendingRealTaskEntry,
|
||||
pendingExampleTaskEntry,
|
||||
]);
|
||||
|
||||
const result = await service.checkIncomingFullStateConflict([incomingSyncImport]);
|
||||
|
||||
// Real pending work blocks silent acceptance -> the conflict dialog is shown...
|
||||
expect(result.hasMeaningfulPending).toBeTrue();
|
||||
expect(result.dialogData).toBeDefined();
|
||||
// ...but the example-task id is still reported. The caller intentionally leaves
|
||||
// these untouched in the dialog path, so they ride along if the user keeps local.
|
||||
expect(result.discardablePendingOpIds).toEqual(['local-example-task-create']);
|
||||
});
|
||||
|
||||
it('should treat pending full-state ops as meaningful', async () => {
|
||||
const incomingSyncImport = createOperation({
|
||||
id: 'incoming-sync-import',
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { OperationLogStoreService } from '../persistence/operation-log-store.service';
|
||||
import {
|
||||
ActionType,
|
||||
extractActionPayload,
|
||||
FULL_STATE_OP_TYPES,
|
||||
Operation,
|
||||
OperationLogEntry,
|
||||
|
|
@ -11,10 +13,32 @@ import { SyncImportConflictData } from './dialog-sync-import-conflict/dialog-syn
|
|||
|
||||
const USER_ENTITY_TYPES = new Set(['TASK', 'PROJECT', 'TAG', 'NOTE']);
|
||||
|
||||
/**
|
||||
* Startup example/onboarding tasks are generated locally on first run (see
|
||||
* ExampleTasksService) and must not count as "meaningful local work" that would block
|
||||
* an incoming SYNC_IMPORT. This only ever runs against local pending ops from
|
||||
* getUnsynced() — never against incoming remote ops — so a remote-supplied
|
||||
* `isExampleTask` flag cannot be used to bypass the conflict dialog.
|
||||
*/
|
||||
const isExampleTaskCreateOp = (entry: OperationLogEntry): boolean => {
|
||||
const { op } = entry;
|
||||
if (
|
||||
op.actionType !== ActionType.TASK_SHARED_ADD ||
|
||||
op.opType !== OpType.Create ||
|
||||
op.entityType !== 'TASK'
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const actionPayload = extractActionPayload(op.payload);
|
||||
return actionPayload['isExampleTask'] === true;
|
||||
};
|
||||
|
||||
export interface IncomingFullStateConflictGateResult {
|
||||
fullStateOp?: Operation;
|
||||
pendingOps: OperationLogEntry[];
|
||||
hasMeaningfulPending: boolean;
|
||||
discardablePendingOpIds: string[];
|
||||
dialogData?: SyncImportConflictData;
|
||||
}
|
||||
|
||||
|
|
@ -43,6 +67,9 @@ export class SyncImportConflictGateService {
|
|||
if (FULL_STATE_OP_TYPES.has(entry.op.opType as OpType)) {
|
||||
return true;
|
||||
}
|
||||
if (isExampleTaskCreateOp(entry)) {
|
||||
return false;
|
||||
}
|
||||
return (
|
||||
USER_ENTITY_TYPES.has(entry.op.entityType) &&
|
||||
(entry.op.opType === OpType.Create ||
|
||||
|
|
@ -62,6 +89,7 @@ export class SyncImportConflictGateService {
|
|||
return {
|
||||
pendingOps: [],
|
||||
hasMeaningfulPending: false,
|
||||
discardablePendingOpIds: [],
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -74,11 +102,19 @@ export class SyncImportConflictGateService {
|
|||
|
||||
const pendingOps = await this.opLogStore.getUnsynced();
|
||||
const hasMeaningfulPending = this.hasMeaningfulPendingOps(pendingOps);
|
||||
// Example-task ops that the caller may reject when it accepts the import silently.
|
||||
// When `hasMeaningfulPending` is true (real work pending alongside example tasks),
|
||||
// the conflict dialog is shown instead and these are intentionally left untouched:
|
||||
// if the user keeps local state, their example tasks ride along with the rest.
|
||||
const discardablePendingOpIds = pendingOps
|
||||
.filter(isExampleTaskCreateOp)
|
||||
.map((entry) => entry.op.id);
|
||||
|
||||
const result = {
|
||||
fullStateOp,
|
||||
pendingOps,
|
||||
hasMeaningfulPending,
|
||||
discardablePendingOpIds,
|
||||
};
|
||||
|
||||
if (!hasMeaningfulPending) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,148 @@
|
|||
/* eslint-disable @typescript-eslint/naming-convention */
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { OperationLogStoreService } from '../../persistence/operation-log-store.service';
|
||||
import { ActionType, OpType, Operation } from '../../core/operation.types';
|
||||
import { SyncImportConflictGateService } from '../../sync/sync-import-conflict-gate.service';
|
||||
import { OperationWriteFlushService } from '../../sync/operation-write-flush.service';
|
||||
import { resetTestUuidCounter, TestClient } from './helpers/test-client.helper';
|
||||
|
||||
/**
|
||||
* Integration tests for the startup-example-task path of the SYNC_IMPORT conflict gate.
|
||||
*
|
||||
* Unlike the gate/sync unit specs (which mock OperationLogStoreService.getUnsynced and
|
||||
* markRejected), these run the REAL store against IndexedDB. They therefore verify the
|
||||
* seam the unit tests stub: that example-task ops persisted with their real
|
||||
* multi-entity payload shape are (a) recognized by the gate as non-meaningful, and
|
||||
* (b) actually excluded from getUnsynced() after markRejected — i.e. never uploaded.
|
||||
*
|
||||
* NOTE: This covers the op-log marker/gate/discard layer. It does NOT exercise the
|
||||
* ExampleTasksService -> afterInitialSyncDoneStrict$ timing — that is e2e-only.
|
||||
*/
|
||||
describe('Example-task SYNC_IMPORT gate (integration)', () => {
|
||||
let storeService: OperationLogStoreService;
|
||||
let gate: SyncImportConflictGateService;
|
||||
|
||||
const local = new TestClient('client-local');
|
||||
const remote = new TestClient('client-remote');
|
||||
|
||||
const exampleTaskOp = (id: string): Operation =>
|
||||
local.createOperation({
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: id,
|
||||
payload: {
|
||||
actionPayload: { task: { id }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
});
|
||||
|
||||
const configOp = (): Operation =>
|
||||
local.createOperation({
|
||||
actionType: ActionType.GLOBAL_CONFIG_UPDATE_SECTION,
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
payload: { sectionKey: 'sync' },
|
||||
});
|
||||
|
||||
const incomingSyncImport = (): Operation =>
|
||||
remote.createOperation({
|
||||
actionType: '[SP_ALL] Load(import) all data' as ActionType,
|
||||
opType: OpType.SyncImport,
|
||||
entityType: 'ALL',
|
||||
entityId: 'incoming-import',
|
||||
payload: { appDataComplete: { task: { ids: [], entities: {} } } },
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
OperationLogStoreService,
|
||||
SyncImportConflictGateService,
|
||||
{
|
||||
provide: OperationWriteFlushService,
|
||||
useValue: { flushPendingWrites: () => Promise.resolve() },
|
||||
},
|
||||
],
|
||||
});
|
||||
storeService = TestBed.inject(OperationLogStoreService);
|
||||
gate = TestBed.inject(SyncImportConflictGateService);
|
||||
|
||||
await storeService.init();
|
||||
await storeService._clearAllDataForTesting();
|
||||
resetTestUuidCounter();
|
||||
});
|
||||
|
||||
it('treats pending example-task creates + config as non-meaningful and reports them as discardable', async () => {
|
||||
const example1 = exampleTaskOp('example-task-1');
|
||||
const example2 = exampleTaskOp('example-task-2');
|
||||
await storeService.append(configOp(), 'local');
|
||||
await storeService.append(example1, 'local');
|
||||
await storeService.append(example2, 'local');
|
||||
|
||||
const result = await gate.checkIncomingFullStateConflict([incomingSyncImport()]);
|
||||
|
||||
expect(result.fullStateOp).toBeDefined();
|
||||
expect(result.hasMeaningfulPending).toBeFalse();
|
||||
expect(result.dialogData).toBeUndefined();
|
||||
expect(result.discardablePendingOpIds.sort()).toEqual(
|
||||
[example1.id, example2.id].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('actually excludes example-task ops from getUnsynced after markRejected (so they are not uploaded)', async () => {
|
||||
const config = configOp();
|
||||
const example = exampleTaskOp('example-task-1');
|
||||
await storeService.append(config, 'local');
|
||||
await storeService.append(example, 'local');
|
||||
|
||||
const result = await gate.checkIncomingFullStateConflict([incomingSyncImport()]);
|
||||
await storeService.markRejected(result.discardablePendingOpIds);
|
||||
|
||||
const remaining = (await storeService.getUnsynced()).map((e) => e.op.id);
|
||||
expect(remaining).toContain(config.id);
|
||||
expect(remaining).not.toContain(example.id);
|
||||
});
|
||||
|
||||
it('shows the dialog (and still lists the example id) when real user work is also pending', async () => {
|
||||
const example = exampleTaskOp('example-task-1');
|
||||
const realTaskUpdate = local.createOperation({
|
||||
actionType: '[Task] Update Task' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: 'real-task-1',
|
||||
payload: { actionPayload: { task: { id: 'real-task-1' } }, entityChanges: [] },
|
||||
});
|
||||
await storeService.append(example, 'local');
|
||||
await storeService.append(realTaskUpdate, 'local');
|
||||
|
||||
const result = await gate.checkIncomingFullStateConflict([incomingSyncImport()]);
|
||||
|
||||
expect(result.hasMeaningfulPending).toBeTrue();
|
||||
expect(result.dialogData).toBeDefined();
|
||||
// The example id is still reported; the sync service leaves it untouched on the
|
||||
// dialog path, so it rides along if the user keeps local state.
|
||||
expect(result.discardablePendingOpIds).toEqual([example.id]);
|
||||
});
|
||||
|
||||
it('does not treat a remote example-task op as discardable (gate reads local pending only)', async () => {
|
||||
// A remote op carrying the same flag is stored with source='remote' (syncedAt set),
|
||||
// so getUnsynced() never returns it and it cannot bypass the dialog.
|
||||
const remoteExample = remote.createOperation({
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: 'remote-example-1',
|
||||
payload: {
|
||||
actionPayload: { task: { id: 'remote-example-1' }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
});
|
||||
await storeService.append(remoteExample, 'remote');
|
||||
|
||||
const result = await gate.checkIncomingFullStateConflict([incomingSyncImport()]);
|
||||
|
||||
expect(result.discardablePendingOpIds).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
@ -25,6 +25,7 @@ export const TaskSharedActions = createActionGroup({
|
|||
isIgnoreShortSyntax?: boolean;
|
||||
autoPlanToday?: string;
|
||||
autoPlanStartOfNextDayDiffMs?: number;
|
||||
isExampleTask?: boolean;
|
||||
}) => ({
|
||||
...taskProps,
|
||||
meta: {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue