mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 08:56:41 +00:00
fix(sync): backfill required fields when LWW Update recreates a task
When an LWW Update arrives for a task that does not exist locally, lwwUpdateMetaReducer recreates the entity by calling adapter.addOne with the action payload. Some upstream producers (notably the _convertToLWWUpdatesIfNeeded fallback in conflict-resolution.service) emit partial payloads that only carry the changed fields. The recreated task then has required fields (`title`, `timeSpentOnDay`, `tagIds`, `subTaskIds`) undefined, which fails Typia validation. dataRepair had no rule for those undefined fields, so the user got stuck on the "Repair attempted but failed to fully fix data issues" dialog with no recovery path (issue #7330). This patch: - Merges DEFAULT_TASK before addOne in the recreate branch so a TASK is always schema-valid, regardless of which producer fed the payload. - Logs a warn when a TASK LWW Update arrives partial, so the upstream producer can be identified from logs in any future report. - Adds defense-in-depth rules to autoFixTypiaErrors for task.entities[*].title, .timeSpentOnDay, .tagIds, .subTaskIds and .attachments so dataRepair can recover any state that was already corrupted on disk before this fix shipped. Closes #7330
This commit is contained in:
parent
18acb5bd6c
commit
2c07306788
4 changed files with 222 additions and 13 deletions
|
|
@ -178,4 +178,89 @@ describe('autoFixTypiaErrors', () => {
|
|||
"Fixed: simpleCounter.entities['BpYFLFtlIGGgTNfZB-t2-'].countOnDay['2025-06-16'] from null to 0 for simpleCounter",
|
||||
);
|
||||
});
|
||||
|
||||
// Issue #7330: a partial LWW Update payload can recreate a task with
|
||||
// required fields undefined. The meta-reducer is the primary fix; these
|
||||
// rules ensure dataRepair can still recover any state that already
|
||||
// contains such an entity (e.g. on disk from a prior corrupted session).
|
||||
describe('issue #7330 — partial task entities from LWW recreate', () => {
|
||||
it('should fix undefined task.title to ""', () => {
|
||||
const mockData = createAppDataCompleteMock();
|
||||
const errors = [
|
||||
{
|
||||
path: '$input.task.entities["rpt_partial_2026-04-29"].title',
|
||||
expected: 'string',
|
||||
value: undefined,
|
||||
},
|
||||
];
|
||||
(mockData as any).task = {
|
||||
ids: ['rpt_partial_2026-04-29'],
|
||||
entities: {
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
'rpt_partial_2026-04-29': { id: 'rpt_partial_2026-04-29' },
|
||||
},
|
||||
};
|
||||
|
||||
const result = autoFixTypiaErrors(mockData, errors as any);
|
||||
|
||||
expect((result as any).task.entities['rpt_partial_2026-04-29'].title).toBe('');
|
||||
});
|
||||
|
||||
it('should fix undefined task.timeSpentOnDay to {}', () => {
|
||||
const mockData = createAppDataCompleteMock();
|
||||
const errors = [
|
||||
{
|
||||
path: '$input.task.entities["rpt_partial_2026-04-29"].timeSpentOnDay',
|
||||
expected: 'Readonly<TimeSpentOnDayCopy>',
|
||||
value: undefined,
|
||||
},
|
||||
];
|
||||
(mockData as any).task = {
|
||||
ids: ['rpt_partial_2026-04-29'],
|
||||
entities: {
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
'rpt_partial_2026-04-29': { id: 'rpt_partial_2026-04-29' },
|
||||
},
|
||||
};
|
||||
|
||||
const result = autoFixTypiaErrors(mockData, errors as any);
|
||||
|
||||
expect(
|
||||
(result as any).task.entities['rpt_partial_2026-04-29'].timeSpentOnDay,
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
it('should fix undefined task array fields (tagIds, subTaskIds, attachments) to []', () => {
|
||||
const mockData = createAppDataCompleteMock();
|
||||
const errors = [
|
||||
{
|
||||
path: '$input.task.entities["t1"].tagIds',
|
||||
expected: 'Array<string>',
|
||||
value: undefined,
|
||||
},
|
||||
{
|
||||
path: '$input.task.entities["t1"].subTaskIds',
|
||||
expected: 'Array<string>',
|
||||
value: undefined,
|
||||
},
|
||||
{
|
||||
path: '$input.task.entities["t1"].attachments',
|
||||
expected: 'Array<TaskAttachmentCopy>',
|
||||
value: undefined,
|
||||
},
|
||||
];
|
||||
(mockData as any).task = {
|
||||
ids: ['t1'],
|
||||
entities: {
|
||||
t1: { id: 't1' },
|
||||
},
|
||||
};
|
||||
|
||||
const result = autoFixTypiaErrors(mockData, errors as any);
|
||||
|
||||
expect((result as any).task.entities['t1'].tagIds).toEqual([]);
|
||||
expect((result as any).task.entities['t1'].subTaskIds).toEqual([]);
|
||||
expect((result as any).task.entities['t1'].attachments).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -58,6 +58,39 @@ export const autoFixTypiaErrors = (
|
|||
setValueByPath(data, keys, 0);
|
||||
OpLog.err(`Fixed: ${path} to 0 (was ${value})`);
|
||||
}
|
||||
} else if (
|
||||
// Issue #7330: a TASK entity recreated by lwwUpdateMetaReducer from a
|
||||
// partial LWW Update payload can be missing required scalar fields.
|
||||
// The primary fix is in the meta-reducer; these branches are
|
||||
// defense-in-depth so any future producer of partial task entities
|
||||
// doesn't dead-end the user on the "Repair attempted but failed" dialog.
|
||||
keys[0] === 'task' &&
|
||||
keys[1] === 'entities' &&
|
||||
keys.length === 4 &&
|
||||
keys[3] === 'title' &&
|
||||
error.expected.includes('string') &&
|
||||
value === undefined
|
||||
) {
|
||||
setValueByPath(data, keys, '');
|
||||
OpLog.err(`Fixed: ${path} from undefined to "" for task.title`);
|
||||
} else if (
|
||||
keys[0] === 'task' &&
|
||||
keys[1] === 'entities' &&
|
||||
keys.length === 4 &&
|
||||
keys[3] === 'timeSpentOnDay' &&
|
||||
value === undefined
|
||||
) {
|
||||
setValueByPath(data, keys, {});
|
||||
OpLog.err(`Fixed: ${path} from undefined to {} for task.timeSpentOnDay`);
|
||||
} else if (
|
||||
keys[0] === 'task' &&
|
||||
keys[1] === 'entities' &&
|
||||
keys.length === 4 &&
|
||||
(keys[3] === 'tagIds' || keys[3] === 'subTaskIds' || keys[3] === 'attachments') &&
|
||||
value === undefined
|
||||
) {
|
||||
setValueByPath(data, keys, []);
|
||||
OpLog.err(`Fixed: ${path} from undefined to [] for task.${keys[3]}`);
|
||||
} else if (
|
||||
keys[0] === 'simpleCounter' &&
|
||||
keys[1] === 'entities' &&
|
||||
|
|
|
|||
|
|
@ -240,6 +240,71 @@ describe('lwwUpdateMetaReducer', () => {
|
|||
expect(updatedState[TASK_FEATURE_NAME]?.ids).toContain('new-task-from-lww');
|
||||
});
|
||||
|
||||
// Regression for issue #7330: a partial LWW Update payload (e.g. only the
|
||||
// changed fields from _convertToLWWUpdatesIfNeeded fallback) used to create
|
||||
// a task entity with `title`, `timeSpentOnDay`, `tagIds`, `subTaskIds`
|
||||
// undefined, which then failed Typia validation and dead-ended the user
|
||||
// on the "Repair attempted but failed" dialog.
|
||||
it('should backfill required fields when recreating from a partial TASK LWW Update (#7330)', () => {
|
||||
const state = createMockState();
|
||||
const action = {
|
||||
type: '[TASK] LWW Update',
|
||||
id: 'rpt_partial_2026-04-29',
|
||||
dueDay: '2026-04-29',
|
||||
meta: {
|
||||
isPersistent: true,
|
||||
entityType: 'TASK',
|
||||
entityId: 'rpt_partial_2026-04-29',
|
||||
},
|
||||
};
|
||||
|
||||
spyOn(OpLog, 'warn');
|
||||
reducer(state, action);
|
||||
|
||||
const updatedState = mockReducer.calls.mostRecent().args[0] as Partial<RootState>;
|
||||
const recreated = updatedState[TASK_FEATURE_NAME]?.entities[
|
||||
'rpt_partial_2026-04-29'
|
||||
] as Task;
|
||||
|
||||
expect(recreated).toBeDefined();
|
||||
// Schema-required fields are backfilled from DEFAULT_TASK
|
||||
expect(recreated.title).toBe('');
|
||||
expect(recreated.timeSpentOnDay).toEqual({});
|
||||
expect(recreated.tagIds).toEqual([]);
|
||||
expect(recreated.subTaskIds).toEqual([]);
|
||||
expect(recreated.timeSpent).toBe(0);
|
||||
expect(recreated.timeEstimate).toBe(0);
|
||||
expect(recreated.isDone).toBe(false);
|
||||
expect(recreated.attachments).toEqual([]);
|
||||
// Fields the LWW Update did carry are preserved
|
||||
expect(recreated.dueDay).toBe('2026-04-29');
|
||||
// And we logged the partial-payload warning so the upstream producer
|
||||
// can be identified from logs.
|
||||
expect(OpLog.warn).toHaveBeenCalledWith(
|
||||
jasmine.stringMatching(/missing required fields/),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not warn when a TASK LWW Update payload is complete', () => {
|
||||
const state = createMockState();
|
||||
const completeAction = {
|
||||
type: '[TASK] LWW Update',
|
||||
id: 'complete-task',
|
||||
title: 'Complete Task',
|
||||
timeSpentOnDay: {},
|
||||
tagIds: [],
|
||||
subTaskIds: [],
|
||||
meta: { isPersistent: true, entityType: 'TASK', entityId: 'complete-task' },
|
||||
};
|
||||
|
||||
spyOn(OpLog, 'warn');
|
||||
reducer(state, completeAction);
|
||||
|
||||
expect(OpLog.warn).not.toHaveBeenCalledWith(
|
||||
jasmine.stringMatching(/missing required fields/),
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip update if action has no id', () => {
|
||||
const state = createMockState();
|
||||
const action = {
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ import {
|
|||
TASK_FEATURE_NAME,
|
||||
taskAdapter,
|
||||
} from '../../../features/tasks/store/task.reducer';
|
||||
import { Task } from '../../../features/tasks/task.model';
|
||||
import { DEFAULT_TASK, Task } from '../../../features/tasks/task.model';
|
||||
import { OpLog } from '../../../core/log';
|
||||
import { appStateFeatureKey } from '../../app-state/app-state.reducer';
|
||||
import { getDbDateStr, isDBDateStr } from '../../../util/get-db-date-str';
|
||||
|
|
@ -529,7 +529,7 @@ export const lwwUpdateMetaReducer: MetaReducer = (
|
|||
}
|
||||
).entities?.[entityId];
|
||||
|
||||
let updatedFeatureState;
|
||||
let updatedFeatureState: unknown;
|
||||
|
||||
if (!existingEntity) {
|
||||
// Entity was deleted locally but UPDATE won via LWW.
|
||||
|
|
@ -538,18 +538,44 @@ export const lwwUpdateMetaReducer: MetaReducer = (
|
|||
OpLog.log(
|
||||
`lwwUpdateMetaReducer: Entity ${entityType}:${entityId} not found, recreating from LWW update`,
|
||||
);
|
||||
|
||||
// Issue #7330: An LWW Update payload from a partial-delta producer
|
||||
// (e.g. _convertToLWWUpdatesIfNeeded fallback) can lack required fields.
|
||||
// Calling adapter.addOne with such a payload creates an entity that fails
|
||||
// Typia validation (e.g. task with undefined `title` / `timeSpentOnDay`),
|
||||
// which dataRepair has no rule for, leaving the user stuck on the
|
||||
// "Repair attempted but failed" dialog. For TASK entities we merge with
|
||||
// DEFAULT_TASK so the recreated entity is always schema-valid.
|
||||
//
|
||||
// INTENTIONAL: We set modified to Date.now() (local time), not the original timestamp.
|
||||
// Rationale:
|
||||
// - Vector clocks are the authoritative conflict resolution mechanism, not `modified`
|
||||
// - The `modified` field is used for UI display ("last edited X minutes ago")
|
||||
// - Setting it to local time reflects when THIS client applied the winning state
|
||||
// - The original timestamp from the winning client is preserved in entityData but
|
||||
// gets overwritten here because local display should show local application time
|
||||
let entityToAdd: Record<string, unknown> = {
|
||||
...entityData,
|
||||
modified: Date.now(),
|
||||
};
|
||||
if (entityType === 'TASK') {
|
||||
const partialKeys: string[] = [];
|
||||
if (entityData['title'] === undefined) partialKeys.push('title');
|
||||
if (entityData['timeSpentOnDay'] === undefined)
|
||||
partialKeys.push('timeSpentOnDay');
|
||||
if (entityData['tagIds'] === undefined) partialKeys.push('tagIds');
|
||||
if (entityData['subTaskIds'] === undefined) partialKeys.push('subTaskIds');
|
||||
if (partialKeys.length > 0) {
|
||||
OpLog.warn(
|
||||
`lwwUpdateMetaReducer: TASK LWW Update payload is missing required fields ` +
|
||||
`[${partialKeys.join(', ')}] for ${entityId} — backfilling from DEFAULT_TASK. ` +
|
||||
`This indicates an upstream producer emitted a partial LWW Update.`,
|
||||
);
|
||||
}
|
||||
entityToAdd = { ...DEFAULT_TASK, ...entityToAdd };
|
||||
}
|
||||
updatedFeatureState = (adapter as EntityAdapter<any>).addOne(
|
||||
{
|
||||
...entityData,
|
||||
// INTENTIONAL: We set modified to Date.now() (local time), not the original timestamp.
|
||||
// Rationale:
|
||||
// - Vector clocks are the authoritative conflict resolution mechanism, not `modified`
|
||||
// - The `modified` field is used for UI display ("last edited X minutes ago")
|
||||
// - Setting it to local time reflects when THIS client applied the winning state
|
||||
// - The original timestamp from the winning client is preserved in entityData but
|
||||
// gets overwritten here because local display should show local application time
|
||||
modified: Date.now(),
|
||||
} as any,
|
||||
entityToAdd as any,
|
||||
featureState as any,
|
||||
);
|
||||
} else {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue