feat(plainspace): auto-remove local task when it leaves my Plainspace list

When a Plainspace task is no longer mine — deleted on the server, or reassigned
away from me — and the local task has no local content (time tracking, notes,
sub-tasks, attachments, repeat config, or a done state), the regular issue poll
removes the orphaned local task.

Detection is a single list-diff: getMyTasks$ already returns every task assigned
to me (including done ones), so a local task whose issueId is missing from that
list is no longer mine. Done tasks stay in the list, so completing one never
removes it. Plainspace has no deleteIssue adapter, so removing a reassigned task
never deletes the still-living remote item; a single orphan keeps the deleteTask
UNDO snackbar, while a bulk vanish collapses to one deleteTasks op (sync rule #3).
The delete op is idempotent across synced devices.

Safety gate against fleet-wide data loss: skip when NONE of my polled tasks are
still in the fetched list. getMyTasks$ fails soft to [] (offline / bad token /
wholesale 404), the server returns [] when membership scope is empty (removed
from the space), and a wrong/garbage list shares no ids with mine — treating any
of those as "everything removed" would wipe every task on every synced device.
(Trusts GET /tasks to be complete; it is unpaginated today.)

Adds optional getRemovedRemoteTasks to IssueServiceInterface; the host applies
the local-content guard and the removal.
This commit is contained in:
Johannes Millan 2026-06-26 14:36:42 +02:00
parent 932e2f8ec9
commit 314e352fe9
5 changed files with 311 additions and 5 deletions

View file

@ -41,6 +41,16 @@ export interface IssueServiceInterface {
// OPTIONAL
// --------
/**
* Returns the subset of the given imported tasks that are no longer present
* for this user on the provider (deleted, or reassigned away). The caller
* decides whether and how to remove them locally. Implementations MUST NOT act
* on an inconclusive fetch an outage or lost access must never look like a
* mass removal.
*/
getRemovedRemoteTasks?(tasks: Task[]): Promise<Task[]>;
getMappedAttachments?(issueData: IssueData): TaskAttachment[];
getNewIssuesToAddToBacklog?(

View file

@ -76,6 +76,9 @@ describe('IssueService', () => {
'addAndSchedule',
'addSubTaskTo',
'restoreTask',
'update',
'remove',
'removeMultipleTasks',
]);
snackServiceSpy = jasmine.createSpyObj('SnackService', ['open']);
workContextServiceSpy = jasmine.createSpyObj('WorkContextService', [], {
@ -725,4 +728,121 @@ describe('IssueService', () => {
);
});
});
describe('refreshIssueTasks - orphaned task auto-removal', () => {
const provider = { id: 'p1', issueProviderKey: 'PLAINSPACE' } as any;
const mkTask = (overrides: Partial<Task> = {}): Task =>
({
id: `task-${overrides.issueId ?? 'x'}`,
title: 'Imported',
issueId: 'i1',
issueType: 'PLAINSPACE',
issueProviderId: 'p1',
timeSpent: 0,
timeSpentOnDay: {},
subTaskIds: [],
attachments: [],
tagIds: [],
projectId: 'project-1',
...overrides,
}) as Task;
let plainspaceMock: any;
beforeEach(() => {
plainspaceMock = service.ISSUE_SERVICE_MAP['PLAINSPACE'];
plainspaceMock.getFreshDataForIssueTasks = jasmine
.createSpy('getFreshDataForIssueTasks')
.and.resolveTo([]);
});
it('removes a deleted task that has no local content', async () => {
const gone = mkTask({ issueId: 'gone', id: 'task-gone' });
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.resolveTo([gone]);
await service.refreshIssueTasks([gone], provider);
expect(taskServiceSpy.remove).toHaveBeenCalledTimes(1);
const removed = taskServiceSpy.remove.calls.mostRecent().args[0];
expect(removed.id).toBe('task-gone');
expect(removed.subTasks).toEqual([]);
});
it('keeps a deleted task that has tracked time', async () => {
const tracked = mkTask({
issueId: 'tracked',
id: 'task-tracked',
timeSpent: 60000,
});
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.resolveTo([tracked]);
await service.refreshIssueTasks([tracked], provider);
expect(taskServiceSpy.remove).not.toHaveBeenCalled();
});
(
[
['notes', { notes: 'a thought' }],
['sub-tasks', { subTaskIds: ['sub-1'] }],
['attachments', { attachments: [{ id: 'a' } as any] }],
['a repeat config', { repeatCfgId: 'repeat-1' }],
['a done state', { isDone: true }],
] as [string, Partial<Task>][]
).forEach(([label, extra]) => {
it(`keeps a deleted task that has local ${label}`, async () => {
const task = mkTask({ issueId: 'kept', id: 'task-kept', ...extra });
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.resolveTo([task]);
await service.refreshIssueTasks([task], provider);
expect(taskServiceSpy.remove).not.toHaveBeenCalled();
});
});
it('does not remove anything when the provider reports no deletions', async () => {
const alive = mkTask({ issueId: 'alive', id: 'task-alive' });
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.resolveTo([]);
await service.refreshIssueTasks([alive], provider);
expect(taskServiceSpy.remove).not.toHaveBeenCalled();
});
it('does not throw if detection fails (poll stays alive)', async () => {
const gone = mkTask({ issueId: 'gone', id: 'task-gone' });
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.rejectWith(new Error('network down'));
await expectAsync(service.refreshIssueTasks([gone], provider)).toBeResolved();
expect(taskServiceSpy.remove).not.toHaveBeenCalled();
});
it('removes several orphans in ONE bulk op (not N deleteTask dispatches)', async () => {
const a = mkTask({ issueId: 'a', id: 'task-a' });
const b = mkTask({ issueId: 'b', id: 'task-b' });
plainspaceMock.getRemovedRemoteTasks = jasmine
.createSpy('getRemovedRemoteTasks')
.and.resolveTo([a, b]);
await service.refreshIssueTasks([a, b], provider);
expect(taskServiceSpy.remove).not.toHaveBeenCalled();
expect(taskServiceSpy.removeMultipleTasks).toHaveBeenCalledTimes(1);
expect(taskServiceSpy.removeMultipleTasks).toHaveBeenCalledWith([
'task-a',
'task-b',
]);
});
});
});

View file

@ -28,7 +28,7 @@ import {
PLAINSPACE_TYPE,
} from './issue.const';
import { TaskService } from '../tasks/task.service';
import { IssueTask, Task, TaskCopy } from '../tasks/task.model';
import { IssueTask, Task, TaskCopy, TaskWithSubTasks } from '../tasks/task.model';
import { IssueServiceInterface } from './issue-service-interface';
import { JiraCommonInterfacesService } from './providers/jira/jira-common-interfaces.service';
// Trello is now a plugin — no built-in service needed
@ -396,16 +396,18 @@ export class IssueService {
labelParams: pollingLabelParams,
});
const service = this._getService(providerKey);
if (!service) {
this._globalProgressBarService.countDown();
continue;
}
let updates: {
task: Task;
taskChanges: Partial<Task>;
issue: IssueData;
}[] = [];
try {
const service = this._getService(providerKey);
if (!service) {
continue;
}
updates = await service.getFreshDataForIssueTasks(
tasksIssueIdsByIssueProviderKey[providerKey],
);
@ -449,6 +451,13 @@ export class IssueService {
});
}
}
if (service.getRemovedRemoteTasks) {
await this._removeOrphanedRemoteTasks(
service,
tasksIssueIdsByIssueProviderKey[providerKey],
);
}
}
for (const taskWithoutIssueId of tasksWithoutIssueId) {
@ -456,6 +465,69 @@ export class IssueService {
}
}
/**
* Removes local tasks that are gone from the provider (deleted or reassigned
* away), but only when nothing has been invested in them locally. Failures are
* logged and swallowed so a detection hiccup never aborts the rest of the poll.
*/
private async _removeOrphanedRemoteTasks(
service: IssueServiceInterface,
tasks: Task[],
): Promise<void> {
try {
const orphaned = await service.getRemovedRemoteTasks!(tasks);
const removable = orphaned.filter((task) => !this._hasLocalContent(task));
if (removable.length === 0) {
return;
}
// The deleteTask effect that mirrors deletions back to the provider no-ops
// here (Plainspace has no deleteIssue adapter) — which is also why removing
// a reassigned task never deletes the still-living remote item. Each device
// polls independently and the delete op is idempotent, so a concurrent
// same-task delete on another device is harmless. Tasks are childless by
// the _hasLocalContent guard, so an empty subTasks list is accurate.
if (removable.length === 1) {
// Single-task path keeps the built-in deleteTask UNDO snackbar.
this._taskService.remove({
...removable[0],
subTasks: [],
} as TaskWithSubTasks);
} else {
// A bulk vanish (e.g. many reassigned at once) collapses to ONE deleteTasks
// op instead of N rapid deleteTask dispatches (sync rule #3: one
// reconciliation = one op). The bulk path has no per-task undo snackbar.
this._taskService.removeMultipleTasks(removable.map((task) => task.id));
}
} catch (e) {
// Never log the raw error object — it may be an HttpErrorResponse whose
// url/body carry the issue id or task content, and the log is exportable.
IssueLog.err(
'Failed to remove orphaned issue tasks',
e instanceof Error ? e : String(e),
);
}
}
/**
* Whether the task holds local work or a completion record that removal would
* lose. Time tracking is the headline case; notes, sub-tasks, attachments and a
* local repeat config all count. `isDone` is kept too: a finished task is a
* record worth preserving, it covers "reassigned away after I completed it",
* and it decouples removal from the server keeping done items in its list.
* Deliberately excluded: dueDay/dueWithTime and tagIds are seeded by the import
* itself, so every imported task has them.
*/
private _hasLocalContent(task: Task): boolean {
return (
task.timeSpent > 0 ||
(task.subTaskIds?.length ?? 0) > 0 ||
!!task.notes ||
(task.attachments?.length ?? 0) > 0 ||
!!task.repeatCfgId ||
task.isDone
);
}
async addTaskFromIssue({
issueDataReduced,
issueProviderId,

View file

@ -182,4 +182,58 @@ describe('PlainspaceCommonInterfacesService', () => {
expect(updates[0]?.taskChanges.isDone).toBe(true);
});
});
describe('getRemovedRemoteTasks (orphan detection by list-diff)', () => {
const setMyTasks = (ids: string[]): void => {
spyOn(
service as unknown as { _getCfgOnce$: (id: string) => unknown },
'_getCfgOnce$',
).and.returnValue(of({}));
(api as unknown as { getMyTasks$: unknown }).getMyTasks$ = () =>
of(ids.map((id) => ({ ...issue(null), id })));
};
it('returns tasks missing from my task list (deleted or reassigned away)', async () => {
setMyTasks(['kept']);
const tasks = [
{ issueProviderId: 'p1', issueId: 'kept', issueLastUpdated: 0 },
{ issueProviderId: 'p1', issueId: 'gone', issueLastUpdated: 0 },
] as Task[];
const removed = await service.getRemovedRemoteTasks(tasks);
expect(removed.map((t) => t.issueId)).toEqual(['gone']);
});
it('keeps tasks still in my list (done tasks stay in the list)', async () => {
setMyTasks(['a', 'b']);
const tasks = [
{ issueProviderId: 'p1', issueId: 'a', issueLastUpdated: 0 },
{ issueProviderId: 'p1', issueId: 'b', issueLastUpdated: 0 },
] as Task[];
expect(await service.getRemovedRemoteTasks(tasks)).toEqual([]);
});
// Critical data-loss guard: getMyTasks$ returns [] both on a fail-soft error
// (offline / bad token / wholesale 404) AND when the caller's membership
// scope is empty (removed from the space). An empty list must NOT be read as
// "everything removed" — else losing access wipes every task fleet-wide.
it('never removes anything when my task list is empty', async () => {
setMyTasks([]);
const tasks = [
{ issueProviderId: 'p1', issueId: 'a', issueLastUpdated: 0 },
{ issueProviderId: 'p1', issueId: 'b', issueLastUpdated: 0 },
] as Task[];
expect(await service.getRemovedRemoteTasks(tasks)).toEqual([]);
});
// Generalised gate: a NON-empty list that contains none of my tasks (wrong
// space / garbage / total churn) is just as untrustworthy as an empty one.
it('never removes when the list shares no ids with my tasks', async () => {
setMyTasks(['someone-elses-task']);
const tasks = [
{ issueProviderId: 'p1', issueId: 'a', issueLastUpdated: 0 },
{ issueProviderId: 'p1', issueId: 'b', issueLastUpdated: 0 },
] as Task[];
expect(await service.getRemovedRemoteTasks(tasks)).toEqual([]);
});
});
});

View file

@ -132,6 +132,56 @@ export class PlainspaceCommonInterfacesService extends BaseIssueProviderService<
return updates;
}
/**
* Detect imported tasks that are no longer mine on Plainspace, for the safe
* auto-removal of orphans. `getMyTasks$` returns every task assigned to me
* including done ones so a task missing from that list was either deleted or
* reassigned away from me; both mean "no longer my task" and are removed the
* same way. Done tasks stay in the list, so completing a task never removes it.
*
* Safety gate against fleet-wide data loss: skip when NONE of my polled tasks
* are still in the fetched list. `getMyTasks$` fails soft to `[]` (offline /
* bad token / wholesale 404), the server returns `[]` when my membership scope
* is empty (removed from the space), and a wrong/garbage list shares no ids
* with mine reading any of those as "everything was removed" would wipe every
* task on every synced device. A list that still contains some of my tasks
* proves it is live and authorized, so the rest can be trusted as removed.
* Trade-off: if every one of my tasks disappears in one window we skip, leaving
* the orphans until the next appears the safe direction for a destructive
* action. NB: this trusts the list to be COMPLETE; GET /tasks is unpaginated
* today, so if it ever paginates/truncates the missing entries would be misread
* as removals here.
*/
async getRemovedRemoteTasks(tasks: Task[]): Promise<Task[]> {
const tasksByProviderId = new Map<string, Task[]>();
for (const task of tasks) {
if (!task.issueProviderId || !task.issueId) {
continue;
}
const group = tasksByProviderId.get(task.issueProviderId) ?? [];
group.push(task);
tasksByProviderId.set(task.issueProviderId, group);
}
const removed: Task[] = [];
for (const [providerId, providerTasks] of tasksByProviderId) {
const cfg = await firstValueFrom(this._getCfgOnce$(providerId));
const myTaskIds = new Set(
(await firstValueFrom(this._plainspaceApiService.getMyTasks$(cfg))).map(
(issue) => issue.id,
),
);
const gone = providerTasks.filter((task) => !myTaskIds.has(task.issueId!));
// All of my polled tasks gone at once → distrust the list, skip (see gate
// above). This subsumes the empty-list case.
if (gone.length === providerTasks.length) {
continue;
}
removed.push(...gone);
}
return removed;
}
private _toFreshData(
task: Task,
issue: PlainspaceIssue | null,