mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
refactor(op-log): merge LWWOperationFactory into ConflictResolutionService
- Consolidate LWWOperationFactory methods (createLWWUpdateOp, mergeAndIncrementClocks) into ConflictResolutionService to reduce service count - Remove redundant caching in SuperSyncProvider (SyncCredentialStore already caches) - Remove _onBlurWhenNotTracking$ sync trigger from SyncTriggerService - Add documentation for dueDay/dueWithTime mutual exclusivity pattern - Change docker-compose postgres port to 55432 to avoid conflicts
This commit is contained in:
parent
6fbda9d25f
commit
bfdac6d19a
16 changed files with 861 additions and 425 deletions
141
ARCHITECTURE-DECISIONS.md
Normal file
141
ARCHITECTURE-DECISIONS.md
Normal file
|
|
@ -0,0 +1,141 @@
|
|||
# Architecture Decision Records
|
||||
|
||||
This document tracks significant architectural decisions and patterns in the Super Productivity codebase. When making changes that affect these patterns, reference this document and update it if needed.
|
||||
|
||||
## Active Patterns & Decisions
|
||||
|
||||
### 1. dueDay/dueWithTime Mutual Exclusivity Pattern
|
||||
|
||||
**Status**: ✅ Active (since commit `400ca8c1`, 2026-01-29)
|
||||
|
||||
**Decision**: The `task.dueDay` and `task.dueWithTime` fields are mutually exclusive in new data. When setting `dueWithTime`, `dueDay` must be cleared (set to `undefined`). When reading, `dueWithTime` takes priority over `dueDay`.
|
||||
|
||||
**Rationale**:
|
||||
|
||||
- Prevents state inconsistency bugs where both fields had conflicting values
|
||||
- Single source of truth for task scheduling
|
||||
- Simpler state management
|
||||
|
||||
**Implementation**:
|
||||
|
||||
- **Writing**: Clear `dueDay` when setting `dueWithTime` (in meta-reducers)
|
||||
- **Reading**: Check `dueWithTime` first; only check `dueDay` if `dueWithTime` is not set (in selectors)
|
||||
- **Legacy Data**: Old data with both fields works via priority pattern (no migration needed)
|
||||
|
||||
**Documentation**: [`docs/ai/dueDay-dueWithTime-mutual-exclusivity.md`](docs/ai/dueDay-dueWithTime-mutual-exclusivity.md)
|
||||
|
||||
**Key Files**:
|
||||
|
||||
- [`task.model.ts`](src/app/features/tasks/task.model.ts) - Field definitions with JSDoc
|
||||
- [`task-shared-scheduling.reducer.ts`](src/app/root-store/meta/task-shared-meta-reducers/task-shared-scheduling.reducer.ts) - Write implementation
|
||||
- [`work-context.selectors.ts`](src/app/features/work-context/store/work-context.selectors.ts) - Read pattern
|
||||
- [`planner.selectors.ts`](src/app/features/planner/store/planner.selectors.ts) - Read pattern
|
||||
- [`task.selectors.ts`](src/app/features/tasks/store/task.selectors.ts) - Read pattern
|
||||
|
||||
**When to Update This Pattern**:
|
||||
|
||||
- Adding new date/time scheduling fields
|
||||
- Modifying task scheduling logic
|
||||
- Working with task selectors that check due dates
|
||||
|
||||
---
|
||||
|
||||
### 2. TODAY_TAG Virtual Tag Pattern
|
||||
|
||||
**Status**: ✅ Active (established pattern)
|
||||
|
||||
**Decision**: `TODAY_TAG` (ID: `'TODAY'`) is a **virtual tag** whose membership is determined by `task.dueWithTime` or `task.dueDay`, not by `task.tagIds`. The tag's `taskIds` field stores only the ordering of tasks, not membership.
|
||||
|
||||
**Key Invariant**: `TODAY_TAG.id` must NEVER be added to `task.tagIds`
|
||||
|
||||
**Rationale**:
|
||||
|
||||
- Uniform move operations across all tags (virtual and regular)
|
||||
- Single source of truth for "today" membership (date fields, not tagIds)
|
||||
- Self-healing ordering (stale entries automatically filtered)
|
||||
- Natural integration with planner (which uses date fields)
|
||||
|
||||
**Documentation**: [`docs/ai/today-tag-architecture.md`](docs/ai/today-tag-architecture.md)
|
||||
|
||||
**Related**: Uses the dueDay/dueWithTime mutual exclusivity pattern (Decision #1)
|
||||
|
||||
**Key Files**:
|
||||
|
||||
- [`tag.const.ts`](src/app/features/tag/tag.const.ts) - TODAY_TAG definition
|
||||
- [`work-context.selectors.ts`](src/app/features/work-context/store/work-context.selectors.ts) - Membership computation
|
||||
- [`task-shared-helpers.ts`](src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts) - Invariant enforcement
|
||||
|
||||
**When to Update This Pattern**:
|
||||
|
||||
- Adding new virtual tags
|
||||
- Modifying tag membership logic
|
||||
- Working with today's task list
|
||||
|
||||
---
|
||||
|
||||
## How to Use This Document
|
||||
|
||||
### When Making Architectural Changes
|
||||
|
||||
1. **Before implementing**: Check if your change affects any active pattern
|
||||
2. **During implementation**: Follow the documented patterns
|
||||
3. **After implementation**: Update this document if you've:
|
||||
- Changed an existing pattern
|
||||
- Added a new architectural pattern
|
||||
- Made a decision that affects future development
|
||||
|
||||
### When to Add a New Decision
|
||||
|
||||
Add a new decision record when:
|
||||
|
||||
- The decision affects multiple files/modules
|
||||
- Future developers need to understand "why" not just "what"
|
||||
- The pattern needs to be followed consistently across the codebase
|
||||
- The decision prevents a specific class of bugs
|
||||
|
||||
### Decision Record Template
|
||||
|
||||
```markdown
|
||||
### N. [Pattern/Decision Name]
|
||||
|
||||
**Status**: ✅ Active | 🚧 Draft | ⚠️ Deprecated | ❌ Superseded
|
||||
|
||||
**Decision**: [One-sentence summary of the decision]
|
||||
|
||||
**Rationale**:
|
||||
|
||||
- [Why was this decision made?]
|
||||
- [What problems does it solve?]
|
||||
|
||||
**Implementation**:
|
||||
|
||||
- [How is it implemented?]
|
||||
- [Key techniques or patterns used]
|
||||
|
||||
**Documentation**: [Link to detailed docs]
|
||||
|
||||
**Key Files**: [List of primary files implementing this pattern]
|
||||
|
||||
**When to Update This Pattern**: [Scenarios when someone should review/update this]
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- [`docs/ai/`](docs/ai/) - AI/Developer architecture docs (detailed patterns)
|
||||
- [`docs/sync-and-op-log/`](docs/sync-and-op-log/) - Operation log architecture
|
||||
- [`docs/long-term-plans/`](docs/long-term-plans/) - Future architectural plans
|
||||
|
||||
---
|
||||
|
||||
## Commit Reference
|
||||
|
||||
When committing changes related to these patterns, reference this document and the specific decision:
|
||||
|
||||
```
|
||||
feat(tasks): implement feature X
|
||||
|
||||
Uses dueDay/dueWithTime mutual exclusivity pattern (ARCHITECTURE-DECISIONS.md #1)
|
||||
See: docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
```
|
||||
|
|
@ -8,7 +8,7 @@ services:
|
|||
POSTGRES_PASSWORD: superpassword
|
||||
POSTGRES_DB: supersync_db
|
||||
ports:
|
||||
- '5432:5432'
|
||||
- '55432:5432'
|
||||
volumes:
|
||||
- db_data:/var/lib/postgresql/data
|
||||
healthcheck:
|
||||
|
|
|
|||
78
docs/ai/README.md
Normal file
78
docs/ai/README.md
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
# AI/Developer Architecture Documentation
|
||||
|
||||
This directory contains architectural documentation, design patterns, and guides specifically written for AI assistants and developers working on the Super Productivity codebase.
|
||||
|
||||
## Core Patterns & Architecture
|
||||
|
||||
### Task Scheduling & Date Management
|
||||
|
||||
- **[dueDay/dueWithTime Mutual Exclusivity Pattern](dueDay-dueWithTime-mutual-exclusivity.md)** ⭐ CRITICAL
|
||||
- Explains how `dueDay` and `dueWithTime` fields interact on tasks
|
||||
- **Why it matters**: These fields are mutually exclusive; setting one clears the other
|
||||
- **When to read**: Before working with task scheduling, planner, or date selectors
|
||||
- **Related commit**: `400ca8c1` (2026-01-29)
|
||||
|
||||
- **[TODAY_TAG Architecture](today-tag-architecture.md)** ⭐ CRITICAL
|
||||
- Explains the virtual tag pattern for the TODAY_TAG
|
||||
- **Why it matters**: TODAY_TAG behaves fundamentally differently from regular tags
|
||||
- **When to read**: Before working with today's task list, planner, or tag operations
|
||||
- **Related**: Uses the dueDay/dueWithTime mutual exclusivity pattern
|
||||
|
||||
## Entity Management
|
||||
|
||||
- **[Adding New Entity Type Checklist](adding-new-entity-type-checklist.md)**
|
||||
- Step-by-step guide for adding new entity types to the app
|
||||
- **When to use**: When adding a new feature that requires persistent state
|
||||
|
||||
## Sync & Operation Log
|
||||
|
||||
- **[File-Based OpLog Sync Implementation Plan](file-based-oplog-sync-implementation-plan.md)**
|
||||
- Technical plan for file-based sync implementation
|
||||
- **Related**: See also `docs/sync-and-op-log/` for comprehensive sync documentation
|
||||
|
||||
## Plugin System
|
||||
|
||||
- **[Issue Providers to Plugins Evaluation](issue-providers-to-plugins-evaluation.md)**
|
||||
- Analysis of migrating issue providers to plugin architecture
|
||||
|
||||
- **[Plugin UI Consistency Plan](plugin-ui-consistency-plan.md)**
|
||||
- Design plan for consistent plugin UI/UX
|
||||
|
||||
## Documentation Conventions
|
||||
|
||||
### Document Types
|
||||
|
||||
1. **Architecture Docs** - Explain fundamental patterns (e.g., virtual tags, mutual exclusivity)
|
||||
2. **Implementation Plans** - Detailed technical plans for features
|
||||
3. **Checklists** - Step-by-step guides for common tasks
|
||||
4. **Evaluations** - Analysis of technical decisions
|
||||
|
||||
### Criticality Markers
|
||||
|
||||
- ⭐ **CRITICAL**: Must read before working in related areas
|
||||
- 📋 **REFERENCE**: Useful reference material
|
||||
- 📝 **DRAFT**: Work in progress, may be incomplete
|
||||
|
||||
### When to Update
|
||||
|
||||
- **After architectural changes**: Document new patterns immediately
|
||||
- **When patterns emerge**: If you notice repeated code patterns, document them
|
||||
- **When questions arise**: If developers ask the same question twice, document the answer
|
||||
- **After major refactors**: Especially when behavior changes (like `400ca8c1`)
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- [`docs/sync-and-op-log/`](../sync-and-op-log/) - Comprehensive operation log and sync documentation
|
||||
- [`docs/wiki/`](../wiki/) - User-facing wiki documentation
|
||||
- [`docs/long-term-plans/`](../long-term-plans/) - Future technical plans and proposals
|
||||
|
||||
## Contributing
|
||||
|
||||
When adding new documentation to this directory:
|
||||
|
||||
1. **Use clear titles** that describe the pattern/feature
|
||||
2. **Add a summary section** explaining what, why, and when to read
|
||||
3. **Include code examples** showing correct and incorrect usage
|
||||
4. **Link related files** using relative paths
|
||||
5. **Update this README** with your new document
|
||||
6. **Reference related commits** when documenting a specific change
|
||||
412
docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
Normal file
412
docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
Normal file
|
|
@ -0,0 +1,412 @@
|
|||
# dueDay/dueWithTime Mutual Exclusivity Pattern
|
||||
|
||||
## Overview
|
||||
|
||||
As of commit `400ca8c1` (2026-01-29), the `dueDay` and `dueWithTime` fields on [`Task`](../../src/app/features/tasks/task.model.ts) follow a **mutual exclusivity pattern**. These fields must NOT both be set simultaneously in new data. This document explains the pattern, its rationale, and implementation details.
|
||||
|
||||
## The Pattern
|
||||
|
||||
### Rule
|
||||
|
||||
**When `dueWithTime` is set, `dueDay` MUST be `undefined` (or `null`).**
|
||||
|
||||
```typescript
|
||||
// CORRECT - Task scheduled with specific time
|
||||
task.dueWithTime = 1706537400000; // Tomorrow at 9:00 AM
|
||||
task.dueDay = undefined; // Cleared
|
||||
|
||||
// CORRECT - Task scheduled without specific time (all-day)
|
||||
task.dueDay = '2026-01-30';
|
||||
task.dueWithTime = undefined;
|
||||
|
||||
// WRONG - Both fields set (legacy data only)
|
||||
task.dueDay = '2026-01-30';
|
||||
task.dueWithTime = 1706537400000; // DO NOT create new data like this
|
||||
```
|
||||
|
||||
### Priority/Precedence
|
||||
|
||||
When reading task data, **`dueWithTime` takes priority over `dueDay`**:
|
||||
|
||||
```typescript
|
||||
// Determining if a task is "due today"
|
||||
let isDueToday = false;
|
||||
if (task.dueWithTime) {
|
||||
// Check dueWithTime first (takes priority)
|
||||
isDueToday = isToday(task.dueWithTime);
|
||||
// DO NOT check dueDay if dueWithTime is set
|
||||
} else if (task.dueDay === todayStr) {
|
||||
// Only check dueDay if dueWithTime is not set
|
||||
isDueToday = true;
|
||||
}
|
||||
```
|
||||
|
||||
## Rationale
|
||||
|
||||
### The Problem This Solves
|
||||
|
||||
**Prior to this change**, both fields could coexist with conflicting values, causing bugs:
|
||||
|
||||
1. **Bug Scenario**: User moves task from "today" to "tomorrow 9am"
|
||||
- Action: `scheduleTask` sets `dueWithTime = tomorrow 9am`
|
||||
- **Old behavior**: Also set `dueDay = 'tomorrow'`
|
||||
- **Problem**: If selector checked `dueDay` first, stale values could cause incorrect categorization
|
||||
|
||||
2. **State Inconsistency**: Task could have:
|
||||
```typescript
|
||||
dueDay = 'today'
|
||||
dueWithTime = tomorrow 9am
|
||||
```
|
||||
Different selectors checking different fields would disagree on the task's due date.
|
||||
|
||||
### Why Mutual Exclusivity?
|
||||
|
||||
1. **Single Source of Truth**: No ambiguity about when a task is due
|
||||
2. **Simpler State Management**: No need to keep two fields in sync
|
||||
3. **Bug Prevention**: Eliminates entire class of state inconsistency bugs
|
||||
4. **Clear Semantics**:
|
||||
- `dueWithTime` = scheduled for specific time
|
||||
- `dueDay` = scheduled for all-day (no specific time)
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### Writing: Setting the Fields
|
||||
|
||||
**File**: [`task-shared-scheduling.reducer.ts`](../../src/app/root-store/meta/task-shared-meta-reducers/task-shared-scheduling.reducer.ts)
|
||||
|
||||
When scheduling a task with a specific time, `dueDay` is explicitly cleared:
|
||||
|
||||
```typescript
|
||||
const handleScheduleTaskWithTime = (
|
||||
state: AppDataComplete,
|
||||
taskId: string,
|
||||
dueWithTime: number,
|
||||
// ...
|
||||
): Update<Task> => {
|
||||
return {
|
||||
id: taskId,
|
||||
changes: {
|
||||
dueWithTime,
|
||||
dueDay: undefined, // ← Mutual exclusivity: dueWithTime clears dueDay
|
||||
remindAt,
|
||||
},
|
||||
};
|
||||
};
|
||||
```
|
||||
|
||||
### Reading: Checking the Fields
|
||||
|
||||
All selectors follow this pattern - **check `dueWithTime` first, only fall back to `dueDay` if `dueWithTime` is not set**:
|
||||
|
||||
#### Example 1: Planner Selector
|
||||
|
||||
**File**: [`planner.selectors.ts`](../../src/app/features/planner/store/planner.selectors.ts)
|
||||
|
||||
```typescript
|
||||
export const selectAllTasksDueToday = createSelector(
|
||||
/*...*/ (taskState, todayStr) => {
|
||||
// ...
|
||||
for (const id of taskState.ids) {
|
||||
const task = taskState.entities[id];
|
||||
if (!task) continue;
|
||||
|
||||
// Check if task is due today
|
||||
// Priority: dueWithTime takes precedence over dueDay (mutual exclusivity pattern)
|
||||
let isDueToday = false;
|
||||
if (task.dueWithTime) {
|
||||
isDueToday = isToday(task.dueWithTime);
|
||||
} else if (task.dueDay === todayStr) {
|
||||
isDueToday = true;
|
||||
}
|
||||
|
||||
if (isDueToday) {
|
||||
allDue.push(task);
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
```
|
||||
|
||||
#### Example 2: Work Context Selector
|
||||
|
||||
**File**: [`work-context.selectors.ts`](../../src/app/features/work-context/store/work-context.selectors.ts)
|
||||
|
||||
```typescript
|
||||
const computeOrderedTaskIdsForToday = (todayTag, taskEntities, todayStr) => {
|
||||
const tasksForToday: string[] = [];
|
||||
for (const taskId of Object.keys(taskEntities)) {
|
||||
const task = taskEntities[taskId];
|
||||
if (task) {
|
||||
// Check dueWithTime first (takes priority - mutual exclusivity)
|
||||
if (task.dueWithTime) {
|
||||
if (isToday(task.dueWithTime)) {
|
||||
tasksForToday.push(taskId);
|
||||
}
|
||||
// If dueWithTime is set but not for today, skip (don't check dueDay)
|
||||
}
|
||||
// Fallback: check dueDay only if dueWithTime is not set
|
||||
else if (task.dueDay === todayStr) {
|
||||
tasksForToday.push(taskId);
|
||||
}
|
||||
}
|
||||
}
|
||||
// ...
|
||||
};
|
||||
```
|
||||
|
||||
#### Example 3: Task Selector Helper
|
||||
|
||||
**File**: [`task.selectors.ts`](../../src/app/features/tasks/store/task.selectors.ts)
|
||||
|
||||
```typescript
|
||||
// Helper to check if task is "in TODAY" via virtual tag pattern
|
||||
// Priority: dueWithTime takes precedence over dueDay (mutual exclusivity)
|
||||
const isInToday = (task: Task): boolean => {
|
||||
if (task.dueWithTime) {
|
||||
return isToday(task.dueWithTime);
|
||||
}
|
||||
return task.dueDay === todayStr;
|
||||
};
|
||||
```
|
||||
|
||||
## Legacy Data Handling
|
||||
|
||||
### The Challenge
|
||||
|
||||
**Existing data may have both fields set** from before this pattern was introduced. This is unavoidable due to:
|
||||
|
||||
1. Persisted local data
|
||||
2. Synced data from other clients
|
||||
3. Data from backups/archives
|
||||
|
||||
### The Solution
|
||||
|
||||
**All selectors implement the priority pattern** (`dueWithTime` first), ensuring correct behavior even with legacy data:
|
||||
|
||||
- If **both fields are set**: `dueWithTime` determines the task's due date
|
||||
- If **only `dueDay` is set**: Use `dueDay` (legacy all-day tasks)
|
||||
- If **only `dueWithTime` is set**: Use `dueWithTime` (new data)
|
||||
|
||||
### Migration Strategy
|
||||
|
||||
**No data migration is performed**. Instead:
|
||||
|
||||
1. **Graceful degradation**: Old data continues to work via priority pattern
|
||||
2. **Natural migration**: As users interact with tasks, new operations will clear `dueDay` when setting `dueWithTime`
|
||||
3. **No breaking changes**: Both fields remain in the model as optional
|
||||
|
||||
## Testing
|
||||
|
||||
The mutual exclusivity pattern is extensively tested:
|
||||
|
||||
### Test File: [`task-shared-scheduling.reducer.spec.ts`](../../src/app/root-store/meta/task-shared-meta-reducers/task-shared-scheduling.reducer.spec.ts)
|
||||
|
||||
```typescript
|
||||
it('should set dueDay to undefined when scheduling for today (mutual exclusivity)', () => {
|
||||
const now = Date.now();
|
||||
const testState = createStateWithExistingTasks(['task1'], [], [], []);
|
||||
const action = createScheduleAction({}, now);
|
||||
|
||||
metaReducer(testState, action);
|
||||
expectStateUpdate(
|
||||
expectTaskUpdate('task1', { dueWithTime: now, dueDay: undefined }),
|
||||
action,
|
||||
mockReducer,
|
||||
testState,
|
||||
);
|
||||
});
|
||||
|
||||
it('should set dueDay to undefined when scheduling for a different day (mutual exclusivity)', () => {
|
||||
const testState = createStateWithExistingTasks(['task1'], [], [], ['task1']);
|
||||
const tomorrowTimestamp = Date.now() + 24 * 60 * 60 * 1000;
|
||||
const action = createScheduleAction({}, tomorrowTimestamp);
|
||||
|
||||
metaReducer(testState, action);
|
||||
expectStateUpdate(
|
||||
expectTaskUpdate('task1', {
|
||||
dueWithTime: tomorrowTimestamp,
|
||||
dueDay: undefined, // ← Cleared
|
||||
}),
|
||||
action,
|
||||
mockReducer,
|
||||
testState,
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
### Test File: [`work-context.selectors.spec.ts`](../../src/app/features/work-context/store/work-context.selectors.spec.ts)
|
||||
|
||||
```typescript
|
||||
it('should INCLUDE task with dueWithTime for today even when dueDay is set to different date (dueWithTime takes priority)', () => {
|
||||
const today = new Date();
|
||||
today.setHours(8, 0, 0, 0);
|
||||
const todayTimestamp = today.getTime();
|
||||
|
||||
// This is a legacy data scenario - with mutual exclusivity, this shouldn't happen in new data
|
||||
const taskWithBothFields = {
|
||||
id: 'task1',
|
||||
tagIds: [],
|
||||
dueDay: '2000-01-01', // Legacy dueDay set to different date
|
||||
dueWithTime: todayTimestamp, // dueWithTime for today takes priority
|
||||
subTaskIds: [],
|
||||
} as TaskCopy;
|
||||
|
||||
const result = selectTodayTaskIds.projector(tagState, taskState);
|
||||
expect(result).toEqual(['task1']); // dueWithTime takes priority - task IS in today
|
||||
});
|
||||
|
||||
it('task scheduled for tomorrow via dialog should NOT appear in today (mutual exclusivity)', () => {
|
||||
// This tests the mutual exclusivity pattern:
|
||||
// 1. Task starts in "today" list (dueDay = today)
|
||||
// 2. User schedules it for tomorrow via schedule dialog
|
||||
// 3. After scheduling: dueWithTime is set, dueDay is cleared (undefined)
|
||||
// 4. Task should NOT appear in today's list (dueWithTime is for tomorrow)
|
||||
|
||||
const tomorrow = new Date();
|
||||
tomorrow.setDate(tomorrow.getDate() + 1);
|
||||
tomorrow.setHours(9, 0, 0, 0);
|
||||
const tomorrowTimestamp = tomorrow.getTime();
|
||||
|
||||
const taskScheduledForTomorrow = {
|
||||
id: 'task1',
|
||||
tagIds: [],
|
||||
dueDay: undefined, // Mutual exclusivity: dueDay cleared when dueWithTime is set
|
||||
dueWithTime: tomorrowTimestamp, // Scheduled for 9am tomorrow
|
||||
subTaskIds: [],
|
||||
} as TaskCopy;
|
||||
|
||||
const result = selectTodayTaskIds.projector(tagState, taskState);
|
||||
expect(result).toEqual([]); // Task should NOT appear in today
|
||||
});
|
||||
```
|
||||
|
||||
## Code Patterns to Follow
|
||||
|
||||
### ✅ DO: Check dueWithTime first
|
||||
|
||||
```typescript
|
||||
if (task.dueWithTime) {
|
||||
// Handle time-specific scheduling
|
||||
isDueToday = isToday(task.dueWithTime);
|
||||
} else if (task.dueDay === todayStr) {
|
||||
// Only check dueDay if dueWithTime is not set
|
||||
isDueToday = true;
|
||||
}
|
||||
```
|
||||
|
||||
### ✅ DO: Clear dueDay when setting dueWithTime
|
||||
|
||||
```typescript
|
||||
updateTask({
|
||||
id: taskId,
|
||||
changes: {
|
||||
dueWithTime: timestamp,
|
||||
dueDay: undefined, // Always clear when setting dueWithTime
|
||||
},
|
||||
});
|
||||
```
|
||||
|
||||
### ❌ DON'T: Check dueDay first
|
||||
|
||||
```typescript
|
||||
// WRONG - Don't do this
|
||||
if (task.dueDay === todayStr) {
|
||||
isDueToday = true;
|
||||
} else if (task.dueWithTime && isToday(task.dueWithTime)) {
|
||||
isDueToday = true;
|
||||
}
|
||||
```
|
||||
|
||||
### ❌ DON'T: Set both fields
|
||||
|
||||
```typescript
|
||||
// WRONG - Never set both fields in new code
|
||||
updateTask({
|
||||
id: taskId,
|
||||
changes: {
|
||||
dueWithTime: timestamp,
|
||||
dueDay: getDbDateStr(timestamp), // DON'T DO THIS
|
||||
},
|
||||
});
|
||||
```
|
||||
|
||||
### ❌ DON'T: Check both fields with OR
|
||||
|
||||
```typescript
|
||||
// WRONG - This violates priority pattern
|
||||
const isDueByDay = task.dueDay === todayStr;
|
||||
const isDueByTime = task.dueWithTime && isToday(task.dueWithTime);
|
||||
if (isDueByDay || isDueByTime) {
|
||||
// This can give wrong results with legacy data
|
||||
}
|
||||
```
|
||||
|
||||
## Related Patterns
|
||||
|
||||
### TODAY_TAG Virtual Tag
|
||||
|
||||
The mutual exclusivity pattern complements the **TODAY_TAG virtual tag pattern** where:
|
||||
|
||||
- `TODAY_TAG.id` must NEVER be in `task.tagIds`
|
||||
- Membership in "today" is determined by `dueDay` OR `dueWithTime` (via priority pattern)
|
||||
|
||||
See: [`today-tag-architecture.md`](today-tag-architecture.md)
|
||||
|
||||
### When to Use Each Field
|
||||
|
||||
| Scenario | Use `dueWithTime` | Use `dueDay` |
|
||||
| ------------------------------------ | ------------------- | ---------------------------- |
|
||||
| Task scheduled for specific time | ✅ | ❌ (undefined) |
|
||||
| Task scheduled for all-day (no time) | ❌ (undefined) | ✅ |
|
||||
| Task not scheduled at all | ❌ (undefined) | ❌ (undefined) |
|
||||
| Reading legacy data with both fields | ✅ (takes priority) | ignore if dueWithTime exists |
|
||||
|
||||
## Key Files Modified
|
||||
|
||||
The following files were changed as part of implementing this pattern in commit `400ca8c1`:
|
||||
|
||||
| File | Change |
|
||||
| ------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------- |
|
||||
| [`task-shared-scheduling.reducer.ts`](../../src/app/root-store/meta/task-shared-meta-reducers/task-shared-scheduling.reducer.ts) | Clear `dueDay` when setting `dueWithTime` |
|
||||
| [`planner.selectors.ts`](../../src/app/features/planner/store/planner.selectors.ts) | Check `dueWithTime` first, then `dueDay` |
|
||||
| [`task.selectors.ts`](../../src/app/features/tasks/store/task.selectors.ts) | Check `dueWithTime` first, then `dueDay` |
|
||||
| [`work-context.selectors.ts`](../../src/app/features/work-context/store/work-context.selectors.ts) | Check `dueWithTime` first, then `dueDay` |
|
||||
| [`task-shared-scheduling.reducer.spec.ts`](../../src/app/root-store/meta/task-shared-meta-reducers/task-shared-scheduling.reducer.spec.ts) | Test mutual exclusivity on write |
|
||||
| [`work-context.selectors.spec.ts`](../../src/app/features/work-context/store/work-context.selectors.spec.ts) | Test priority pattern on read |
|
||||
|
||||
## Future Considerations
|
||||
|
||||
### Potential Migration
|
||||
|
||||
If needed in the future, a **data migration** could be implemented to clean up legacy data:
|
||||
|
||||
```typescript
|
||||
// Potential migration (not currently implemented)
|
||||
function migrateDueDateFields(task: Task): Task {
|
||||
if (task.dueWithTime && task.dueDay) {
|
||||
// Clear dueDay if dueWithTime is set
|
||||
return { ...task, dueDay: undefined };
|
||||
}
|
||||
return task;
|
||||
}
|
||||
```
|
||||
|
||||
However, this is **not currently necessary** because the priority pattern handles legacy data correctly.
|
||||
|
||||
### Adding New Date/Time Fields
|
||||
|
||||
If new scheduling fields are added in the future:
|
||||
|
||||
1. **Consider mutual exclusivity** with existing fields
|
||||
2. **Document priority order** if multiple fields can express similar concepts
|
||||
3. **Update all selectors** consistently with the priority pattern
|
||||
4. **Add comprehensive tests** for legacy data scenarios
|
||||
|
||||
## Summary
|
||||
|
||||
- **Pattern**: `dueWithTime` and `dueDay` are mutually exclusive in new data
|
||||
- **Priority**: `dueWithTime` takes precedence when reading data
|
||||
- **Writing**: Clear `dueDay` when setting `dueWithTime`
|
||||
- **Reading**: Check `dueWithTime` first, only check `dueDay` if `dueWithTime` is not set
|
||||
- **Legacy**: Old data with both fields works via priority pattern
|
||||
- **Testing**: Extensively tested in reducers and selectors
|
||||
|
|
@ -8,25 +8,25 @@ TODAY_TAG is a **virtual tag** that behaves differently from regular tags. Under
|
|||
|
||||
**TODAY_TAG (ID: `'TODAY'`) must NEVER be added to `task.tagIds`.**
|
||||
|
||||
Membership in TODAY_TAG is determined by `task.dueDay`, not by `task.tagIds`. The `TODAY_TAG.taskIds` field stores only the **ordering** of today's tasks, not membership.
|
||||
Membership in TODAY_TAG is determined by **`task.dueWithTime` OR `task.dueDay`** (via [mutual exclusivity pattern](dueDay-dueWithTime-mutual-exclusivity.md)), not by `task.tagIds`. The `TODAY_TAG.taskIds` field stores only the **ordering** of today's tasks, not membership.
|
||||
|
||||
## Virtual Tag vs. Board-Style Pattern
|
||||
|
||||
| Aspect | TODAY_TAG (Virtual) | Regular Tags (Board-Style) |
|
||||
| ----------------------- | ----------------------- | ----------------------------- |
|
||||
| **Membership source** | `task.dueDay === today` | `task.tagIds.includes(tagId)` |
|
||||
| **Stored on task** | `task.dueDay` | `task.tagIds` |
|
||||
| **In task.tagIds?** | NO (invariant) | YES (required) |
|
||||
| **tag.taskIds purpose** | Ordering only | Ordering only |
|
||||
| **Use case** | Time-based grouping | Category/label grouping |
|
||||
| Aspect | TODAY_TAG (Virtual) | Regular Tags (Board-Style) |
|
||||
| ----------------------- | -------------------------------------------------------------------------------------------------------- | ----------------------------- |
|
||||
| **Membership source** | `task.dueWithTime` OR `task.dueDay` (see [mutual exclusivity](dueDay-dueWithTime-mutual-exclusivity.md)) | `task.tagIds.includes(tagId)` |
|
||||
| **Stored on task** | `task.dueWithTime` or `task.dueDay` | `task.tagIds` |
|
||||
| **In task.tagIds?** | NO (invariant) | YES (required) |
|
||||
| **tag.taskIds purpose** | Ordering only | Ordering only |
|
||||
| **Use case** | Time-based grouping | Category/label grouping |
|
||||
|
||||
## Why This Pattern?
|
||||
|
||||
1. **Uniform move operations**: Drag/drop and keyboard shortcuts (Ctrl+↑/↓) work identically for TODAY_TAG and regular tags because all tags store ordering in `tag.taskIds`.
|
||||
|
||||
2. **Single source of truth**: `task.dueDay` is the canonical field for "is this task scheduled for today?" - no dual bookkeeping between `dueDay` and a hypothetical `tagIds` entry.
|
||||
2. **Single source of truth**: `task.dueWithTime` OR `task.dueDay` (via [mutual exclusivity pattern](dueDay-dueWithTime-mutual-exclusivity.md)) is the canonical field for "is this task scheduled for today?" - no dual bookkeeping between date fields and a hypothetical `tagIds` entry.
|
||||
|
||||
3. **Planner integration**: The planner view uses `task.dueDay` to organize tasks by day. TODAY_TAG naturally aligns with this.
|
||||
3. **Planner integration**: The planner view uses `task.dueWithTime` and `task.dueDay` to organize tasks by day. TODAY_TAG naturally aligns with this.
|
||||
|
||||
4. **Self-healing**: Stale ordering entries are gracefully filtered out by the selector. No manual cleanup needed when a task's `dueDay` changes.
|
||||
|
||||
|
|
@ -51,7 +51,7 @@ export const TODAY_TAG: Tag = {
|
|||
|
||||
The `computeOrderedTaskIdsForToday()` function:
|
||||
|
||||
1. Finds all tasks where `dueDay === today` (membership)
|
||||
1. Finds all tasks where `dueWithTime` is today OR `dueDay === today` (membership via [mutual exclusivity pattern](dueDay-dueWithTime-mutual-exclusivity.md))
|
||||
2. Orders them according to `TODAY_TAG.taskIds` (ordering)
|
||||
3. Appends any unordered tasks at the end (self-healing)
|
||||
4. Filters out stale entries from `TODAY_TAG.taskIds` (self-healing)
|
||||
|
|
@ -60,10 +60,24 @@ The `computeOrderedTaskIdsForToday()` function:
|
|||
const computeOrderedTaskIdsForToday = (todayTag, taskEntities) => {
|
||||
const todayStr = getDbDateStr();
|
||||
|
||||
// Membership: tasks where dueDay === today
|
||||
const tasksForToday = Object.values(taskEntities)
|
||||
.filter((t) => t && !t.parentId && t.dueDay === todayStr)
|
||||
.map((t) => t.id);
|
||||
// Membership: tasks where dueWithTime is today OR dueDay === today
|
||||
// See: dueDay-dueWithTime-mutual-exclusivity.md for priority pattern
|
||||
const tasksForToday: string[] = [];
|
||||
for (const taskId of Object.keys(taskEntities)) {
|
||||
const task = taskEntities[taskId];
|
||||
if (task) {
|
||||
// Check dueWithTime first (takes priority - mutual exclusivity)
|
||||
if (task.dueWithTime) {
|
||||
if (isToday(task.dueWithTime)) {
|
||||
tasksForToday.push(taskId);
|
||||
}
|
||||
}
|
||||
// Fallback: check dueDay only if dueWithTime is not set
|
||||
else if (task.dueDay === todayStr) {
|
||||
tasksForToday.push(taskId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Ordering: use TODAY_TAG.taskIds, filter stale, append missing
|
||||
const storedOrder = todayTag?.taskIds || [];
|
||||
|
|
@ -144,11 +158,16 @@ This handles state divergence from per-entity conflict resolution during sync.
|
|||
task.tagIds = [...task.tagIds, TODAY_TAG.id];
|
||||
```
|
||||
|
||||
### Correct: Set task.dueDay
|
||||
### Correct: Set task.dueDay or task.dueWithTime
|
||||
|
||||
```typescript
|
||||
// CORRECT - Set dueDay to add to today
|
||||
// CORRECT - Set dueDay to add to today (all-day, no specific time)
|
||||
task.dueDay = getDbDateStr(); // Today's date string
|
||||
task.dueWithTime = undefined; // See: dueDay-dueWithTime-mutual-exclusivity.md
|
||||
|
||||
// ALSO CORRECT - Set dueWithTime for specific time today
|
||||
task.dueWithTime = Date.now(); // Specific time today
|
||||
task.dueDay = undefined; // See: dueDay-dueWithTime-mutual-exclusivity.md
|
||||
```
|
||||
|
||||
### Wrong: Checking tagIds for TODAY membership
|
||||
|
|
@ -158,23 +177,33 @@ task.dueDay = getDbDateStr(); // Today's date string
|
|||
const isToday = task.tagIds.includes(TODAY_TAG.id);
|
||||
```
|
||||
|
||||
### Correct: Check dueDay
|
||||
### Correct: Check dueWithTime and dueDay (with priority)
|
||||
|
||||
```typescript
|
||||
// CORRECT - Check dueDay for TODAY membership
|
||||
const isToday = task.dueDay === getDbDateStr();
|
||||
// CORRECT - Check dueWithTime first, then dueDay (mutual exclusivity pattern)
|
||||
// See: dueDay-dueWithTime-mutual-exclusivity.md
|
||||
let isInToday = false;
|
||||
if (task.dueWithTime) {
|
||||
isInToday = isToday(task.dueWithTime);
|
||||
} else if (task.dueDay === getDbDateStr()) {
|
||||
isInToday = true;
|
||||
}
|
||||
```
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- **[dueDay/dueWithTime Mutual Exclusivity Pattern](dueDay-dueWithTime-mutual-exclusivity.md)** - Understanding how `dueDay` and `dueWithTime` interact (critical for TODAY_TAG membership)
|
||||
|
||||
## Key Files Reference
|
||||
|
||||
| File | Purpose |
|
||||
| ----------------------------------------------------------------------------- | --------------------------------------------- |
|
||||
| `src/app/features/tag/tag.const.ts` | TODAY_TAG definition |
|
||||
| `src/app/features/work-context/store/work-context.selectors.ts` | Membership computation, repair selector |
|
||||
| `src/app/features/tag/store/tag.reducer.ts` | Move operations (uniform for all tags) |
|
||||
| `src/app/root-store/meta/task-shared-meta-reducers/planner-shared.reducer.ts` | Multi-entity updates for planner actions |
|
||||
| `src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts` | `filterOutTodayTag()`, `hasInvalidTodayTag()` |
|
||||
| `src/app/features/tag/store/tag.effects.ts` | `repairTodayTagConsistency$` effect |
|
||||
| File | Purpose |
|
||||
| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------- |
|
||||
| `src/app/features/tag/tag.const.ts` | TODAY_TAG definition |
|
||||
| `src/app/features/work-context/store/work-context.selectors.ts` | Membership computation (uses mutual exclusivity pattern), repair selector |
|
||||
| `src/app/features/tag/store/tag.reducer.ts` | Move operations (uniform for all tags) |
|
||||
| `src/app/root-store/meta/task-shared-meta-reducers/planner-shared.reducer.ts` | Multi-entity updates for planner actions |
|
||||
| `src/app/root-store/meta/task-shared-meta-reducers/task-shared-helpers.ts` | `filterOutTodayTag()`, `hasInvalidTodayTag()` |
|
||||
| `src/app/features/tag/store/tag.effects.ts` | `repairTodayTagConsistency$` effect |
|
||||
|
||||
## Testing
|
||||
|
||||
|
|
|
|||
|
|
@ -88,7 +88,29 @@ export interface TaskCopy
|
|||
timeSpentOnDay: TimeSpentOnDay;
|
||||
|
||||
// Additional app-specific fields
|
||||
|
||||
/**
|
||||
* Scheduled time as Unix timestamp (ms). For tasks scheduled with a specific time.
|
||||
*
|
||||
* IMPORTANT: dueWithTime and dueDay follow a mutual exclusivity pattern:
|
||||
* - When dueWithTime is set, dueDay MUST be undefined/null (not both set)
|
||||
* - When reading, check dueWithTime FIRST (it takes priority over dueDay)
|
||||
*
|
||||
* @see docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
*/
|
||||
dueWithTime?: number | null;
|
||||
|
||||
/**
|
||||
* Scheduled date as ISO date string (YYYY-MM-DD). For tasks scheduled for all-day (no specific time).
|
||||
*
|
||||
* IMPORTANT: dueDay and dueWithTime follow a mutual exclusivity pattern:
|
||||
* - When dueDay is set, dueWithTime should be undefined/null (for new data)
|
||||
* - When dueWithTime is set, dueDay MUST be undefined/null (not both set)
|
||||
* - When reading, check dueWithTime FIRST (it takes priority over dueDay)
|
||||
* - Legacy data may have both fields set; handle via priority pattern
|
||||
*
|
||||
* @see docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
*/
|
||||
dueDay?: string | null;
|
||||
hasPlannedTime?: boolean;
|
||||
attachments: TaskAttachment[];
|
||||
|
|
|
|||
|
|
@ -49,8 +49,12 @@ const computeOrderedTaskIdsForToday = (
|
|||
const todayStr = getDbDateStr();
|
||||
const storedOrder = todayTag?.taskIds || [];
|
||||
|
||||
// Find all tasks where dueWithTime is for today OR dueDay === today
|
||||
// Priority: dueWithTime takes precedence over dueDay (mutual exclusivity pattern)
|
||||
// IMPORTANT: Implements dueDay/dueWithTime mutual exclusivity pattern
|
||||
// - Check dueWithTime FIRST (it takes priority over dueDay)
|
||||
// - Only check dueDay if dueWithTime is not set
|
||||
// - If dueWithTime is set, do NOT check dueDay (even for legacy data with both fields)
|
||||
// - This ensures correct behavior with both new data (only one field set) and legacy data (both fields set)
|
||||
// See: docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
const tasksForToday: string[] = [];
|
||||
for (const taskId of Object.keys(taskEntities)) {
|
||||
const task = taskEntities[taskId];
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ import {
|
|||
take,
|
||||
tap,
|
||||
throttleTime,
|
||||
withLatestFrom,
|
||||
} from 'rxjs/operators';
|
||||
import { GlobalConfigService } from '../../features/config/global-config.service';
|
||||
import { isOnline$ } from '../../util/is-online';
|
||||
|
|
@ -33,16 +32,14 @@ import { androidInterface } from '../../features/android/android-interface';
|
|||
import { ipcResume$, ipcSuspend$ } from '../../core/ipc-events';
|
||||
import { IS_TOUCH_PRIMARY } from '../../util/is-mouse-primary';
|
||||
import { DataInitStateService } from '../../core/data-init/data-init-state.service';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { selectCurrentTaskId } from '../../features/tasks/store/task.selectors';
|
||||
import { SyncLog } from '../../core/log';
|
||||
import { SyncWrapperService } from './sync-wrapper.service';
|
||||
import { SyncProviderId } from '../../op-log/sync-exports';
|
||||
|
||||
const MAX_WAIT_FOR_INITIAL_SYNC = 25000;
|
||||
const USER_INTERACTION_SYNC_CHECK_THROTTLE_TIME = 15 * 60 * 10000;
|
||||
/** 15 minutes in milliseconds - throttle time for user activity sync checks */
|
||||
const USER_ACTIVITY_SYNC_THROTTLE_TIME = 15 * 60 * 1000;
|
||||
|
||||
// TODO naming
|
||||
@Injectable({
|
||||
providedIn: 'root',
|
||||
})
|
||||
|
|
@ -50,7 +47,6 @@ export class SyncTriggerService {
|
|||
private readonly _globalConfigService = inject(GlobalConfigService);
|
||||
private readonly _dataInitStateService = inject(DataInitStateService);
|
||||
private readonly _idleService = inject(IdleService);
|
||||
private readonly _store = inject(Store);
|
||||
private readonly _syncWrapperService = inject(SyncWrapperService);
|
||||
|
||||
// Note: This was previously connected to PFAPI's onLocalMetaUpdate$, which was a no-op.
|
||||
|
|
@ -87,11 +83,11 @@ export class SyncTriggerService {
|
|||
fromEvent(document, 'visibilitychange'),
|
||||
).pipe(
|
||||
mapTo('I_MOUSE_TOUCH_MOVE_OR_VISIBILITYCHANGE'),
|
||||
throttleTime(USER_INTERACTION_SYNC_CHECK_THROTTLE_TIME),
|
||||
throttleTime(USER_ACTIVITY_SYNC_THROTTLE_TIME),
|
||||
)
|
||||
: fromEvent(window, 'focus').pipe(
|
||||
mapTo('I_FOCUS_THROTTLED'),
|
||||
throttleTime(USER_INTERACTION_SYNC_CHECK_THROTTLE_TIME),
|
||||
throttleTime(USER_ACTIVITY_SYNC_THROTTLE_TIME),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
|
@ -120,17 +116,6 @@ export class SyncTriggerService {
|
|||
)
|
||||
: EMPTY;
|
||||
|
||||
private _onBlurWhenNotTracking$: Observable<string | never> = fromEvent(
|
||||
window,
|
||||
'blur',
|
||||
).pipe(
|
||||
withLatestFrom(this._store.select(selectCurrentTaskId)),
|
||||
filter(([, currentTaskId]) => !currentTaskId),
|
||||
mapTo('I_BLUR_WHILE_NOT_TRACKING'),
|
||||
// we throttle this to prevent lots of updates
|
||||
throttleTime(10 * 60 * 1000),
|
||||
);
|
||||
|
||||
private _isOnlineTrigger$: Observable<string> = isOnline$.pipe(
|
||||
// skip initial online which always fires on page load
|
||||
skip(1),
|
||||
|
|
@ -216,7 +201,6 @@ export class SyncTriggerService {
|
|||
this._isOnlineTrigger$,
|
||||
this._onIdleTrigger$,
|
||||
this._onElectronResumeTrigger$,
|
||||
this._onBlurWhenNotTracking$,
|
||||
);
|
||||
return merge(
|
||||
// once immediately
|
||||
|
|
|
|||
|
|
@ -189,8 +189,11 @@ describe('SuperSyncProvider', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('config caching', () => {
|
||||
it('should cache config and only call privateCfg.load once for multiple operations', async () => {
|
||||
describe('config loading', () => {
|
||||
it('should call privateCfg.load for each operation (relies on SyncCredentialStore caching)', async () => {
|
||||
// Note: We removed redundant caching in SuperSyncProvider since SyncCredentialStore
|
||||
// already has its own in-memory caching. Each call to _cfgOrError now calls load(),
|
||||
// but SyncCredentialStore returns cached value after first load.
|
||||
mockPrivateCfgStore.load.and.returnValue(Promise.resolve(testConfig));
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
|
|
@ -204,11 +207,13 @@ describe('SuperSyncProvider', () => {
|
|||
await provider.downloadOps(1);
|
||||
await provider.downloadOps(2);
|
||||
|
||||
// privateCfg.load should only be called once due to caching
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(1);
|
||||
// privateCfg.load is called for each operation, but SyncCredentialStore caches internally
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('should cache server seq key and only call privateCfg.load once for multiple seq operations', async () => {
|
||||
it('should call privateCfg.load for each server seq operation (relies on SyncCredentialStore caching)', async () => {
|
||||
// Note: We removed redundant caching in SuperSyncProvider since SyncCredentialStore
|
||||
// already has its own in-memory caching.
|
||||
mockPrivateCfgStore.load.and.returnValue(Promise.resolve(testConfig));
|
||||
localStorageSpy.getItem.and.returnValue('10');
|
||||
|
||||
|
|
@ -217,8 +222,8 @@ describe('SuperSyncProvider', () => {
|
|||
await provider.getLastServerSeq();
|
||||
await provider.getLastServerSeq();
|
||||
|
||||
// privateCfg.load should only be called once due to caching
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(1);
|
||||
// privateCfg.load is called for each operation, but SyncCredentialStore caches internally
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -605,8 +610,9 @@ describe('SuperSyncProvider', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should clear cached config on auth failure', async () => {
|
||||
// First call succeeds - config gets cached
|
||||
it('should throw AuthFailSPError and allow next operation to proceed', async () => {
|
||||
// Note: We removed the local caching in SuperSyncProvider since SyncCredentialStore
|
||||
// already has its own in-memory caching. Each operation calls load() directly.
|
||||
mockPrivateCfgStore.load.and.returnValue(Promise.resolve(testConfig));
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
|
|
@ -634,7 +640,7 @@ describe('SuperSyncProvider', () => {
|
|||
expect(e).toBeInstanceOf(AuthFailSPError);
|
||||
}
|
||||
|
||||
// Third call should reload config from store (cache was cleared)
|
||||
// Third call should succeed - config reloaded from SyncCredentialStore
|
||||
fetchSpy.and.returnValue(
|
||||
Promise.resolve({
|
||||
ok: true,
|
||||
|
|
@ -642,7 +648,8 @@ describe('SuperSyncProvider', () => {
|
|||
} as Response),
|
||||
);
|
||||
await provider.downloadOps(0);
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(2);
|
||||
// Each operation calls load() directly (SyncCredentialStore handles caching)
|
||||
expect(mockPrivateCfgStore.load).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('should throw AuthFailSPError for uploadOps on 401', async () => {
|
||||
|
|
|
|||
|
|
@ -57,10 +57,6 @@ export class SuperSyncProvider
|
|||
|
||||
public privateCfg: SyncCredentialStore<SyncProviderId.SuperSync>;
|
||||
|
||||
// Caches to reduce repeated async loads during sync operations
|
||||
private _cachedCfg?: SuperSyncPrivateCfg;
|
||||
private _cachedServerSeqKey?: string;
|
||||
|
||||
constructor(_basePath?: string) {
|
||||
// basePath is ignored - SuperSync uses operation-based sync only
|
||||
this.privateCfg = new SyncCredentialStore(SyncProviderId.SuperSync);
|
||||
|
|
@ -82,9 +78,6 @@ export class SuperSyncProvider
|
|||
}
|
||||
|
||||
async setPrivateCfg(cfg: SuperSyncPrivateCfg): Promise<void> {
|
||||
// Invalidate caches when config changes
|
||||
this._cachedCfg = undefined;
|
||||
this._cachedServerSeqKey = undefined;
|
||||
await this.privateCfg.setComplete(cfg);
|
||||
}
|
||||
|
||||
|
|
@ -359,14 +352,11 @@ export class SuperSyncProvider
|
|||
// === Private Helper Methods ===
|
||||
|
||||
private async _cfgOrError(): Promise<SuperSyncPrivateCfg> {
|
||||
if (this._cachedCfg) {
|
||||
return this._cachedCfg;
|
||||
}
|
||||
// Note: SyncCredentialStore.load() has its own in-memory caching
|
||||
const cfg = await this.privateCfg.load();
|
||||
if (!cfg) {
|
||||
throw new MissingCredentialsSPError();
|
||||
}
|
||||
this._cachedCfg = cfg;
|
||||
return cfg;
|
||||
}
|
||||
|
||||
|
|
@ -375,9 +365,7 @@ export class SuperSyncProvider
|
|||
* when switching between different accounts or servers.
|
||||
*/
|
||||
private async _getServerSeqKey(): Promise<string> {
|
||||
if (this._cachedServerSeqKey) {
|
||||
return this._cachedServerSeqKey;
|
||||
}
|
||||
// Note: SyncCredentialStore.load() has its own in-memory caching
|
||||
const cfg = await this.privateCfg.load();
|
||||
const baseUrl = cfg?.baseUrl ?? 'default';
|
||||
// Include accessToken in the hash so different users on the same server
|
||||
|
|
@ -389,20 +377,14 @@ export class SuperSyncProvider
|
|||
.split('')
|
||||
.reduce((acc, char) => ((acc << 5) - acc + char.charCodeAt(0)) | 0, 0)
|
||||
.toString(16);
|
||||
this._cachedServerSeqKey = `${LAST_SERVER_SEQ_KEY_PREFIX}${hash}`;
|
||||
return this._cachedServerSeqKey;
|
||||
return `${LAST_SERVER_SEQ_KEY_PREFIX}${hash}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check HTTP response status and throw AuthFailSPError for auth failures.
|
||||
* Clears cached config so next operation will reload from store.
|
||||
*/
|
||||
private _checkHttpStatus(status: number, body?: string): void {
|
||||
if (status === 401 || status === 403) {
|
||||
// Clear cached config so next operation will reload from store
|
||||
// (allowing user to re-configure after auth failure)
|
||||
this._cachedCfg = undefined;
|
||||
this._cachedServerSeqKey = undefined;
|
||||
throw new AuthFailSPError(`Authentication failed (HTTP ${status})`, body);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import { MAX_CONFLICT_RETRY_ATTEMPTS } from '../core/operation-log.const';
|
|||
import { DUPLICATE_OPERATION_ERROR_PATTERN } from '../persistence/op-log-errors.const';
|
||||
import {
|
||||
compareVectorClocks,
|
||||
incrementVectorClock,
|
||||
mergeVectorClocks,
|
||||
VectorClockComparison,
|
||||
} from '../../core/util/vector-clock';
|
||||
|
|
@ -33,7 +34,8 @@ import {
|
|||
isArrayEntity,
|
||||
} from '../core/entity-registry';
|
||||
import { selectIssueProviderById } from '../../features/issue/store/issue-provider.selectors';
|
||||
import { LWWOperationFactory } from './lww-operation-factory.service';
|
||||
import { uuidv7 } from '../../util/uuid-v7';
|
||||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
|
||||
/**
|
||||
* Represents the result of LWW (Last-Write-Wins) conflict resolution.
|
||||
|
|
@ -81,7 +83,72 @@ export class ConflictResolutionService {
|
|||
private snackService = inject(SnackService);
|
||||
private validateStateService = inject(ValidateStateService);
|
||||
private clientIdProvider = inject(CLIENT_ID_PROVIDER);
|
||||
private lwwOperationFactory = inject(LWWOperationFactory);
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// LWW OPERATION FACTORY METHODS
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
/**
|
||||
* Creates a new LWW Update operation for syncing local state.
|
||||
*
|
||||
* LWW Update operations are synthetic operations created during conflict resolution
|
||||
* to carry the winning local state to remote clients. They are created when:
|
||||
* 1. Local state wins LWW conflict resolution
|
||||
* 2. Stale local operations need to be re-uploaded with merged clocks
|
||||
*
|
||||
* These operations use dynamically constructed action types (e.g., '[TASK] LWW Update')
|
||||
* that are matched by regex in lwwUpdateMetaReducer.
|
||||
*
|
||||
* @param entityType - Type of the entity being updated
|
||||
* @param entityId - ID of the entity being updated
|
||||
* @param entityState - Current state of the entity to sync
|
||||
* @param clientId - Client creating this operation
|
||||
* @param vectorClock - Merged vector clock (should dominate all conflicting ops)
|
||||
* @param timestamp - Preserved timestamp for correct LWW semantics
|
||||
* @returns New UPDATE operation ready for upload
|
||||
*/
|
||||
createLWWUpdateOp(
|
||||
entityType: EntityType,
|
||||
entityId: string,
|
||||
entityState: unknown,
|
||||
clientId: string,
|
||||
vectorClock: VectorClock,
|
||||
timestamp: number,
|
||||
): Operation {
|
||||
// NOTE: LWW Update action types (e.g., '[TASK] LWW Update') are intentionally
|
||||
// NOT in the ActionType enum. They are dynamically constructed here and matched
|
||||
// by regex in lwwUpdateMetaReducer. This is by design - LWW ops are synthetic,
|
||||
// created during conflict resolution to carry the winning local state to remote clients.
|
||||
return {
|
||||
id: uuidv7(),
|
||||
actionType: `[${entityType}] LWW Update` as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType,
|
||||
entityId,
|
||||
payload: entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges multiple vector clocks and increments for the given client.
|
||||
* Used when creating LWW Update operations that need to dominate
|
||||
* all previously known clocks.
|
||||
*
|
||||
* @param clocks - Array of vector clocks to merge
|
||||
* @param clientId - Client ID to increment in the final clock
|
||||
* @returns Merged and incremented vector clock
|
||||
*/
|
||||
mergeAndIncrementClocks(clocks: VectorClock[], clientId: string): VectorClock {
|
||||
let mergedClock: VectorClock = {};
|
||||
for (const clock of clocks) {
|
||||
mergedClock = mergeVectorClocks(mergedClock, clock);
|
||||
}
|
||||
return incrementVectorClock(mergedClock, clientId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the current state after conflict resolution and repairs if necessary.
|
||||
|
|
@ -591,10 +658,7 @@ export class ConflictResolutionService {
|
|||
...conflict.localOps.map((op) => op.vectorClock),
|
||||
...conflict.remoteOps.map((op) => op.vectorClock),
|
||||
];
|
||||
const newClock = this.lwwOperationFactory.mergeAndIncrementClocks(
|
||||
allClocks,
|
||||
clientId,
|
||||
);
|
||||
const newClock = this.mergeAndIncrementClocks(allClocks, clientId);
|
||||
|
||||
// Preserve the maximum timestamp from local ops.
|
||||
// This is critical for LWW semantics: we're creating a new op to carry the
|
||||
|
|
@ -602,7 +666,7 @@ export class ConflictResolutionService {
|
|||
// it to win. Using Date.now() would give it an unfair advantage in future conflicts.
|
||||
const preservedTimestamp = Math.max(...conflict.localOps.map((op) => op.timestamp));
|
||||
|
||||
return this.lwwOperationFactory.createLWWUpdateOp(
|
||||
return this.createLWWUpdateOp(
|
||||
conflict.entityType,
|
||||
conflict.entityId,
|
||||
entityState,
|
||||
|
|
|
|||
|
|
@ -1,249 +0,0 @@
|
|||
import { TestBed } from '@angular/core/testing';
|
||||
import { LWWOperationFactory } from './lww-operation-factory.service';
|
||||
import { EntityType, OpType, VectorClock } from '../core/operation.types';
|
||||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
|
||||
describe('LWWOperationFactory', () => {
|
||||
let service: LWWOperationFactory;
|
||||
|
||||
beforeEach(() => {
|
||||
TestBed.configureTestingModule({
|
||||
providers: [LWWOperationFactory],
|
||||
});
|
||||
service = TestBed.inject(LWWOperationFactory);
|
||||
});
|
||||
|
||||
describe('createLWWUpdateOp', () => {
|
||||
const entityType: EntityType = 'TASK';
|
||||
const entityId = 'task-123';
|
||||
const entityState = { id: entityId, title: 'Test Task', done: false };
|
||||
const clientId = 'client_abc';
|
||||
const vectorClock: VectorClock = { client_abc: 5, client_xyz: 3 };
|
||||
const timestamp = 1700000000000;
|
||||
|
||||
it('should create operation with correct action type format [ENTITY] LWW Update', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.actionType).toBe('[TASK] LWW Update');
|
||||
});
|
||||
|
||||
it('should create action type for different entity types', () => {
|
||||
const projectOp = service.createLWWUpdateOp(
|
||||
'PROJECT',
|
||||
'proj-1',
|
||||
{},
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
expect(projectOp.actionType).toBe('[PROJECT] LWW Update');
|
||||
|
||||
const tagOp = service.createLWWUpdateOp(
|
||||
'TAG',
|
||||
'tag-1',
|
||||
{},
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
expect(tagOp.actionType).toBe('[TAG] LWW Update');
|
||||
});
|
||||
|
||||
it('should assign correct opType (Update)', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.opType).toBe(OpType.Update);
|
||||
});
|
||||
|
||||
it('should use provided entityType and entityId', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.entityType).toBe(entityType);
|
||||
expect(op.entityId).toBe(entityId);
|
||||
});
|
||||
|
||||
it('should include entityState as payload', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.payload).toEqual(entityState);
|
||||
});
|
||||
|
||||
it('should generate UUIDv7 ID', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
// UUIDv7 format: 8-4-4-4-12 hex characters
|
||||
expect(op.id).toMatch(
|
||||
/^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i,
|
||||
);
|
||||
});
|
||||
|
||||
it('should generate unique IDs for each call', () => {
|
||||
const op1 = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
const op2 = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op1.id).not.toBe(op2.id);
|
||||
});
|
||||
|
||||
it('should include CURRENT_SCHEMA_VERSION', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.schemaVersion).toBe(CURRENT_SCHEMA_VERSION);
|
||||
});
|
||||
|
||||
it('should preserve provided vectorClock', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.vectorClock).toEqual(vectorClock);
|
||||
});
|
||||
|
||||
it('should preserve provided timestamp', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.timestamp).toBe(timestamp);
|
||||
});
|
||||
|
||||
it('should preserve provided clientId', () => {
|
||||
const op = service.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
);
|
||||
|
||||
expect(op.clientId).toBe(clientId);
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergeAndIncrementClocks', () => {
|
||||
it('should return incremented clock for single input clock', () => {
|
||||
const clock: VectorClock = { clientA: 5 };
|
||||
const result = service.mergeAndIncrementClocks([clock], 'clientA');
|
||||
|
||||
expect(result).toEqual({ clientA: 6 });
|
||||
});
|
||||
|
||||
it('should merge two clocks taking max of each component', () => {
|
||||
const clock1: VectorClock = { clientA: 5, clientB: 3 };
|
||||
const clock2: VectorClock = { clientA: 3, clientB: 7 };
|
||||
const result = service.mergeAndIncrementClocks([clock1, clock2], 'clientA');
|
||||
|
||||
expect(result['clientA']).toBe(6); // max(5,3) + 1
|
||||
expect(result['clientB']).toBe(7); // max(3,7)
|
||||
});
|
||||
|
||||
it('should merge three+ clocks correctly', () => {
|
||||
const clock1: VectorClock = { clientA: 1 };
|
||||
const clock2: VectorClock = { clientA: 5, clientB: 2 };
|
||||
const clock3: VectorClock = { clientB: 8, clientC: 3 };
|
||||
const result = service.mergeAndIncrementClocks([clock1, clock2, clock3], 'clientA');
|
||||
|
||||
expect(result['clientA']).toBe(6); // max(1,5,0) + 1
|
||||
expect(result['clientB']).toBe(8); // max(0,2,8)
|
||||
expect(result['clientC']).toBe(3); // max(0,0,3)
|
||||
});
|
||||
|
||||
it('should handle empty clocks array', () => {
|
||||
const result = service.mergeAndIncrementClocks([], 'clientA');
|
||||
|
||||
expect(result).toEqual({ clientA: 1 });
|
||||
});
|
||||
|
||||
it('should increment merged clock for given clientId', () => {
|
||||
const clock: VectorClock = { clientA: 10, clientB: 5 };
|
||||
const result = service.mergeAndIncrementClocks([clock], 'clientB');
|
||||
|
||||
expect(result['clientA']).toBe(10); // unchanged
|
||||
expect(result['clientB']).toBe(6); // incremented
|
||||
});
|
||||
|
||||
it('should add new clientId if not in any input clocks', () => {
|
||||
const clock: VectorClock = { clientA: 5 };
|
||||
const result = service.mergeAndIncrementClocks([clock], 'clientNew');
|
||||
|
||||
expect(result['clientA']).toBe(5); // preserved
|
||||
expect(result['clientNew']).toBe(1); // new entry, starts at 1
|
||||
});
|
||||
|
||||
it('should handle clocks with non-overlapping clients', () => {
|
||||
const clock1: VectorClock = { clientA: 3 };
|
||||
const clock2: VectorClock = { clientB: 7 };
|
||||
const result = service.mergeAndIncrementClocks([clock1, clock2], 'clientC');
|
||||
|
||||
expect(result['clientA']).toBe(3);
|
||||
expect(result['clientB']).toBe(7);
|
||||
expect(result['clientC']).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,81 +0,0 @@
|
|||
import { Injectable } from '@angular/core';
|
||||
import {
|
||||
ActionType,
|
||||
EntityType,
|
||||
Operation,
|
||||
OpType,
|
||||
VectorClock,
|
||||
} from '../core/operation.types';
|
||||
import { uuidv7 } from '../../util/uuid-v7';
|
||||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
import { incrementVectorClock, mergeVectorClocks } from '../../core/util/vector-clock';
|
||||
|
||||
/**
|
||||
* Factory for creating LWW (Last-Write-Wins) Update operations.
|
||||
*
|
||||
* LWW Update operations are synthetic operations created during conflict resolution
|
||||
* to carry the winning local state to remote clients. They are created when:
|
||||
* 1. Local state wins LWW conflict resolution (ConflictResolutionService)
|
||||
* 2. Stale local operations need to be re-uploaded with merged clocks (StaleOperationResolverService)
|
||||
*
|
||||
* These operations use dynamically constructed action types (e.g., '[TASK] LWW Update')
|
||||
* that are matched by regex in lwwUpdateMetaReducer.
|
||||
*/
|
||||
@Injectable({
|
||||
providedIn: 'root',
|
||||
})
|
||||
export class LWWOperationFactory {
|
||||
/**
|
||||
* Creates a new LWW Update operation for syncing local state.
|
||||
*
|
||||
* @param entityType - Type of the entity being updated
|
||||
* @param entityId - ID of the entity being updated
|
||||
* @param entityState - Current state of the entity to sync
|
||||
* @param clientId - Client creating this operation
|
||||
* @param vectorClock - Merged vector clock (should dominate all conflicting ops)
|
||||
* @param timestamp - Preserved timestamp for correct LWW semantics
|
||||
* @returns New UPDATE operation ready for upload
|
||||
*/
|
||||
createLWWUpdateOp(
|
||||
entityType: EntityType,
|
||||
entityId: string,
|
||||
entityState: unknown,
|
||||
clientId: string,
|
||||
vectorClock: VectorClock,
|
||||
timestamp: number,
|
||||
): Operation {
|
||||
// NOTE: LWW Update action types (e.g., '[TASK] LWW Update') are intentionally
|
||||
// NOT in the ActionType enum. They are dynamically constructed here and matched
|
||||
// by regex in lwwUpdateMetaReducer. This is by design - LWW ops are synthetic,
|
||||
// created during conflict resolution to carry the winning local state to remote clients.
|
||||
return {
|
||||
id: uuidv7(),
|
||||
actionType: `[${entityType}] LWW Update` as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType,
|
||||
entityId,
|
||||
payload: entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges multiple vector clocks and increments for the given client.
|
||||
* Used when creating LWW Update operations that need to dominate
|
||||
* all previously known clocks.
|
||||
*
|
||||
* @param clocks - Array of vector clocks to merge
|
||||
* @param clientId - Client ID to increment in the final clock
|
||||
* @returns Merged and incremented vector clock
|
||||
*/
|
||||
mergeAndIncrementClocks(clocks: VectorClock[], clientId: string): VectorClock {
|
||||
let mergedClock: VectorClock = {};
|
||||
for (const clock of clocks) {
|
||||
mergedClock = mergeVectorClocks(mergedClock, clock);
|
||||
}
|
||||
return incrementVectorClock(mergedClock, clientId);
|
||||
}
|
||||
}
|
||||
|
|
@ -49,6 +49,8 @@ describe('StaleOperationResolverService', () => {
|
|||
]);
|
||||
mockConflictResolutionService = jasmine.createSpyObj('ConflictResolutionService', [
|
||||
'getCurrentEntityState',
|
||||
'mergeAndIncrementClocks',
|
||||
'createLWWUpdateOp',
|
||||
]);
|
||||
mockLockService = jasmine.createSpyObj('LockService', ['request']);
|
||||
mockSnackService = jasmine.createSpyObj('SnackService', ['open']);
|
||||
|
|
@ -66,6 +68,40 @@ describe('StaleOperationResolverService', () => {
|
|||
mockLockService.request.and.callFake(
|
||||
(_lockName: string, callback: () => Promise<any>) => callback(),
|
||||
);
|
||||
// Mock merged clock methods - merged from LWWOperationFactory
|
||||
mockConflictResolutionService.mergeAndIncrementClocks.and.callFake(
|
||||
(clocks: VectorClock[], clientId: string) => {
|
||||
const merged: VectorClock = {};
|
||||
for (const clock of clocks) {
|
||||
for (const [k, v] of Object.entries(clock)) {
|
||||
merged[k] = Math.max(merged[k] || 0, v);
|
||||
}
|
||||
}
|
||||
merged[clientId] = (merged[clientId] || 0) + 1;
|
||||
return merged;
|
||||
},
|
||||
);
|
||||
mockConflictResolutionService.createLWWUpdateOp.and.callFake(
|
||||
(
|
||||
entityType: EntityType,
|
||||
entityId: string,
|
||||
entityState: unknown,
|
||||
clientId: string,
|
||||
vectorClock: VectorClock,
|
||||
timestamp: number,
|
||||
) => ({
|
||||
id: 'generated-id-' + Math.random().toString(36).substring(7),
|
||||
actionType: `[${entityType}] LWW Update` as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType,
|
||||
entityId,
|
||||
payload: entityState,
|
||||
clientId,
|
||||
vectorClock,
|
||||
timestamp,
|
||||
schemaVersion: 1,
|
||||
}),
|
||||
);
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ import { LOCK_NAMES } from '../core/operation-log.const';
|
|||
import { SnackService } from '../../core/snack/snack.service';
|
||||
import { T } from '../../t.const';
|
||||
import { CLIENT_ID_PROVIDER } from '../util/client-id.provider';
|
||||
import { LWWOperationFactory } from './lww-operation-factory.service';
|
||||
import { uuidv7 } from '../../util/uuid-v7';
|
||||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
|
||||
|
|
@ -42,7 +41,6 @@ export class StaleOperationResolverService {
|
|||
private lockService = inject(LockService);
|
||||
private snackService = inject(SnackService);
|
||||
private clientIdProvider = inject(CLIENT_ID_PROVIDER);
|
||||
private lwwOperationFactory = inject(LWWOperationFactory);
|
||||
|
||||
/**
|
||||
* Resolves stale local operations by creating new LWW Update operations.
|
||||
|
|
@ -123,7 +121,7 @@ export class StaleOperationResolverService {
|
|||
|
||||
// Start with the global clock, merge in local pending ops' clocks, and increment
|
||||
const allClocks = [globalClock, ...entityOps.map(({ op }) => op.vectorClock)];
|
||||
const newClock = this.lwwOperationFactory.mergeAndIncrementClocks(
|
||||
const newClock = this.conflictResolutionService.mergeAndIncrementClocks(
|
||||
allClocks,
|
||||
clientId,
|
||||
);
|
||||
|
|
@ -183,7 +181,7 @@ export class StaleOperationResolverService {
|
|||
const preservedTimestamp = Math.max(...entityOps.map((e) => e.op.timestamp));
|
||||
|
||||
// Create new UPDATE op with current state and merged clock
|
||||
const newOp = this.lwwOperationFactory.createLWWUpdateOp(
|
||||
const newOp = this.conflictResolutionService.createLWWUpdateOp(
|
||||
entityType,
|
||||
entityId,
|
||||
entityState,
|
||||
|
|
|
|||
|
|
@ -26,6 +26,12 @@ import { filterOutId } from '../../../util/filter-out-id';
|
|||
// =============================================================================
|
||||
// ACTION HANDLERS
|
||||
// =============================================================================
|
||||
//
|
||||
// IMPORTANT: These handlers implement the dueDay/dueWithTime mutual exclusivity pattern.
|
||||
// When setting dueWithTime, dueDay is cleared (set to undefined).
|
||||
// See: docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
//
|
||||
// =============================================================================
|
||||
|
||||
const handleScheduleTaskWithTime = (
|
||||
state: RootState,
|
||||
|
|
@ -62,7 +68,10 @@ const handleScheduleTaskWithTime = (
|
|||
id: task.id,
|
||||
changes: {
|
||||
dueWithTime,
|
||||
dueDay: undefined, // Mutual exclusivity: dueWithTime clears dueDay
|
||||
// CRITICAL: Mutual exclusivity pattern - setting dueWithTime clears dueDay
|
||||
// This prevents state inconsistency where both fields are set with conflicting dates
|
||||
// See: docs/ai/dueDay-dueWithTime-mutual-exclusivity.md
|
||||
dueDay: undefined,
|
||||
remindAt,
|
||||
},
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue