From e89306a630c6884ef948a8527671b00f8d4f267d Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Wed, 1 Jul 2026 19:36:52 +0200 Subject: [PATCH] feat(plainspace): create tasks directly in a Plainspace-backed project (#8676) * feat(plainspace): create tasks directly in a Plainspace-backed project Adding a top-level task to an SP project that has a bound PLAINSPACE issue provider now creates the task in the Plainspace space so the team sees it, symmetric with the existing auto-import. Wires into the generic autoCreateIssueOnTaskAdd$ pipeline: - PlainspaceApiService.createTask$ -> POST /api/integration/tasks { spaceId, title }; errors propagate so a failed add surfaces a snack. - PlainspaceSyncAdapterService.createIssue links the returned SPTask id and seeds the two-way-sync baseline (no issueNumber, so no '#123' title prefix). - _hasAutoCreateEnabled recognises the native PLAINSPACE key (no opt-in flag; the bound provider is the opt-in). Requires a new PAT-authed server route POST /api/integration/tasks (documented in docs/plainspace-api-extension-plan.md 4c); inert until that ships. * fix(plainspace): only auto-create tasks once the provider is configured Multi-review follow-ups to the create-task feature: - _hasAutoCreateEnabled now requires a bound Plainspace provider (spaceId + token), not just isEnabled. selectEnabledIssueProviders filters on the flag only, so a mid-connect provider (spaceId/token still null) previously POSTed an invalid create and error-snacked on every task add; now it skips silently until configured. Adds a covering spec. - Drop the unnecessary 'as unknown as' double cast in createIssue. - Drop the unnecessary 'as any' on the valid 'PLAINSPACE' key in the effect spec. --- docs/plainspace-api-extension-plan.md | 54 +++++++- .../plainspace/plainspace-api.service.spec.ts | 21 ++++ .../plainspace/plainspace-api.service.ts | 18 +++ .../plainspace-sync-adapter.service.spec.ts | 34 ++++- .../plainspace-sync-adapter.service.ts | 18 +++ .../issue-two-way-sync.effects.spec.ts | 118 ++++++++++++++++++ .../issue-two-way-sync.effects.ts | 10 ++ 7 files changed, 270 insertions(+), 3 deletions(-) diff --git a/docs/plainspace-api-extension-plan.md b/docs/plainspace-api-extension-plan.md index 3f02a03b09..31c09b9dc1 100644 --- a/docs/plainspace-api-extension-plan.md +++ b/docs/plainspace-api-extension-plan.md @@ -311,6 +311,53 @@ day-only task would fabricate one. --- +## 4c. New endpoint 4 — Create a task (add directly to Plainspace) + +``` +POST /api/integration/tasks +Authorization: Bearer pat_… +body: { spaceId: string, title: string } // title 1–500 chars +→ 201 { task: SPTask } +``` + +The symmetric twin of `claimTask$`: it lets a task added to a Plainspace-backed +SP project appear for the team. When SP adds a task to a project that has a bound +`PLAINSPACE` provider, it POSTs here and links the returned `SPTask.id` back to +the local task (then the existing done/title/`scheduledAt` write-back keeps it in +sync). `spaceId` accepts the project **UUID or slug** (same as everywhere SP +holds `cfg.spaceId`); resolve it against `scope.projectIds` so a token can't +create tasks in a foreign Space. + +This is the PAT-authed integration-API equivalent of the existing member-token +route `POST /api/projects/:slug/items` — reuse that route's item-creation logic +(append at end of the project's primary/hero list, `recordActivity`, broadcast +`item.created` over SSE), just behind `apiTokenMiddleware` + `loadIntegrationScope` +and returning the `SPTask` DTO instead of the internal activity entry. The +member-token route is **not** reusable from SP directly: the SP client only ever +holds a PAT (`pat_…`), never a per-session member token. + +```ts +const { spaceId, title } = c.req.valid('json'); // CreateTaskViaTokenSchema +const scope = await loadIntegrationScope(emailLookup); +const projectId = resolveScopedProjectId(spaceId, scope); // UUID or slug → UUID, or 404 +// then the same insert path as POST /api/projects/:slug/items (primary list, +// position = max+GAP), serializeSPTask(row) → c.json({ task }, 201) +``` + +- **Status codes:** `201` created · `422` validation · `404` unknown/foreign + `spaceId` · `401`/`428` auth (same as the other integration routes). +- No new table. `title` maps to the item `text` column (`MAX_ITEM_TEXT_LENGTH`). +- SP lets create errors propagate (unlike its fail-soft reads) so a failed add + surfaces a snack instead of silently dropping the task. + +**Client side is already implemented** (this repo): `PlainspaceApiService.createTask$`, +`PlainspaceSyncAdapterService.createIssue`, and `_hasAutoCreateEnabled` recognising +the native `PLAINSPACE` key (no opt-in flag — the bound provider is the opt-in). +It wires into the generic `autoCreateIssueOnTaskAdd$` effect and is inert until +this endpoint ships. + +--- + ## 5. Onboarding caveat (important for the SP "Share" UX) A PAT can only be minted from **inside an existing Space** @@ -423,6 +470,7 @@ All isolated in SP's `PlainspaceApiService` (one file) — see | scheduled-time sync | `PATCH /tasks/:id { scheduledAt }` + read | `dueWithTime ↔ scheduledAt` (§4b) | | `getUnclaimedTasks$` | `GET /claimable-tasks?projectId=cfg.spaceId` | claim pool feed | | `claimTask$` | `POST /tasks/:id/claim` | then `addTaskFromIssue` imports it | +| `createTask$` | `POST /tasks { spaceId, title }` | add task in a bound project → link `SPTask.id` | | `createSpace$` | `POST /spaces` | bind provider `spaceId = project.id` | Two **client-side** fixes SP must make when going real (server unaffected, noting @@ -437,8 +485,10 @@ link is `itemUrl` = `{origin}/{slug}/item/{id}`). - **Device-code auth** (`/api/integration/device-code` + `/device-token`) — the real onboarding fix; see §5 and the SP brainstorm doc. -- **SP → Plainspace promotion** (assign an SP task to someone → seed a Space + - invite) — the dominant flow in the product vision, larger than this PR's needs. +- **SP → Plainspace promotion** (assign an SP task to someone → seed a _new_ + Space + invite) — the dominant flow in the product vision, larger than this + PR's needs. Note: adding a task to an _already-bound_ Space is now in scope — + see §4c (`POST /api/integration/tasks`). - **Assignee/“waiting-on” surfacing**, presence, comments, attachments. - Per-occurrence reminders / repeat rules over the integration channel. diff --git a/src/app/features/issue/providers/plainspace/plainspace-api.service.spec.ts b/src/app/features/issue/providers/plainspace/plainspace-api.service.spec.ts index 14ffb7963c..4e8fcc75a7 100644 --- a/src/app/features/issue/providers/plainspace/plainspace-api.service.spec.ts +++ b/src/app/features/issue/providers/plainspace/plainspace-api.service.spec.ts @@ -154,6 +154,27 @@ describe('PlainspaceApiService', () => { expect((await p).id).toBe('proj-new'); }); + it('createTask$ POSTs { spaceId, title } and maps the created task', async () => { + const p = firstValueFrom(service.createTask$('Buy milk', cfg)); + const req = httpMock.expectOne(`${BASE}/tasks`); + expect(req.request.method).toBe('POST'); + expect(req.request.headers.get('Authorization')).toBe('Bearer pat_test'); + expect(req.request.body).toEqual({ spaceId: 'space-1', title: 'Buy milk' }); + req.flush({ task: { ...spTask('new-1', 'space-1'), title: 'Buy milk' } }); + const issue = await p; + expect(issue.id).toBe('new-1'); + expect(issue.title).toBe('Buy milk'); + expect(issue.isDone).toBe(false); + }); + + it('createTask$ lets errors propagate (so the auto-create effect can report)', async () => { + const p = firstValueFrom(service.createTask$('x', cfg)); + httpMock + .expectOne(`${BASE}/tasks`) + .flush('boom', { status: 500, statusText: 'Server Error' }); + await expectAsync(p).toBeRejected(); + }); + it('searchIssues$ filters my tasks by title', async () => { const p = firstValueFrom(service.searchIssues$('task a', cfg)); httpMock diff --git a/src/app/features/issue/providers/plainspace/plainspace-api.service.ts b/src/app/features/issue/providers/plainspace/plainspace-api.service.ts index fcaac0bb57..2f37cbbede 100644 --- a/src/app/features/issue/providers/plainspace/plainspace-api.service.ts +++ b/src/app/features/issue/providers/plainspace/plainspace-api.service.ts @@ -131,6 +131,24 @@ export class PlainspaceApiService { ); } + /** + * Creates a new task in the bound space (`cfg.spaceId`) and returns it mapped + * to a `PlainspaceIssue`. The symmetric twin of `claimTask$`: it lets a task + * added to a Plainspace-backed project appear for the team. Used by the + * two-way-sync adapter's `createIssue`. Errors propagate (unlike the reads and + * like `createSpace$`) so the auto-create effect can surface a failure snack + * instead of silently dropping the task. + */ + createTask$(title: string, cfg: PlainspaceCfg): Observable { + return this._http + .post( + `${this._base(cfg)}/tasks`, + { spaceId: cfg.spaceId, title }, + { headers: this._headers(cfg) }, + ) + .pipe(map((res) => mapSPTaskToIssue(res.task))); + } + /** Creates a remote space and returns its id (used by the share flow). */ createSpace$(title: string, cfg: PlainspaceCfg): Observable<{ id: string }> { return this._http diff --git a/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.spec.ts b/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.spec.ts index 3d3e180d3f..1391783c85 100644 --- a/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.spec.ts +++ b/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.spec.ts @@ -17,7 +17,11 @@ describe('PlainspaceSyncAdapterService', () => { }; beforeEach(() => { - api = jasmine.createSpyObj('PlainspaceApiService', ['getById$', 'patchTask$']); + api = jasmine.createSpyObj('PlainspaceApiService', [ + 'getById$', + 'patchTask$', + 'createTask$', + ]); TestBed.configureTestingModule({ providers: [ PlainspaceSyncAdapterService, @@ -97,6 +101,34 @@ describe('PlainspaceSyncAdapterService', () => { expect(api.patchTask$).not.toHaveBeenCalled(); }); + it('createIssue creates the task and returns id + baseline-seeding issueData', async () => { + const created = { + id: 'new-1', + title: 'Buy milk', + isDone: false, + updatedAt: '2026-01-02T00:00:00.000Z', + url: 'https://plainspace.org/p/item/new-1', + projectId: 'space-1', + scheduledAt: null, + isRecurring: false, + }; + api.createTask$.and.returnValue(of(created)); + + const res = await adapter.createIssue('Buy milk', cfg); + + expect(api.createTask$).toHaveBeenCalledWith('Buy milk', cfg); + expect(res.issueId).toBe('new-1'); + // issueData must carry the fields extractSyncValues reads, so the effect can + // seed the two-way-sync baseline (else the first push is dropped). + expect(adapter.extractSyncValues(res.issueData)).toEqual({ + isDone: false, + title: 'Buy milk', + scheduledAt: null, + }); + // No numeric issue number -> the SP title keeps no '#123' prefix. + expect((res as { issueNumber?: number }).issueNumber).toBeUndefined(); + }); + it('fetchIssue returns the issue, or {} when it is missing', async () => { api.getById$.and.returnValue( of({ diff --git a/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.ts b/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.ts index 3bb3414b73..c3e35a6d6a 100644 --- a/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.ts +++ b/src/app/features/issue/providers/plainspace/plainspace-sync-adapter.service.ts @@ -67,6 +67,24 @@ export class PlainspaceSyncAdapterService implements IssueSyncAdapter }> { + const issue = await firstValueFrom(this._api.createTask$(title, cfg)); + return { + issueId: issue.id, + issueData: issue as Record, + }; + } + async fetchIssue( issueId: string, cfg: PlainspaceCfg, diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts index 7131e9e9f1..7eed7ca9f5 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts @@ -1404,6 +1404,124 @@ describe('IssueTwoWaySyncEffects', () => { adapterRegistry.unregister('TEST_PROVIDER'); })); + it('should create issue for a native PLAINSPACE provider without a pluginConfig flag (collaborative by default)', fakeAsync(() => { + // Contrast with the TEST_PROVIDER case above: a native provider with no + // `pluginConfig.isAutoCreateIssues` normally does NOT auto-create. A bound + // Plainspace provider does — the binding itself is the opt-in, so tasks + // added to a shared project reach the team. + const createIssueSpy = jasmine.createSpy('createIssue').and.resolveTo({ + issueId: 'ps-issue-1', + issueData: { isDone: false, title: 'New Task', scheduledAt: null }, + }); + const adapter = createMockAdapter({ + createIssue: createIssueSpy, + getFieldMappings: jasmine.createSpy('getFieldMappings').and.returnValue([]), + getSyncConfig: jasmine.createSpy('getSyncConfig').and.returnValue({}), + }); + // Override the Plainspace adapter the effects constructor registered. + adapterRegistry.register('PLAINSPACE', adapter); + + const provider = createMockIssueProvider({ + id: 'provider-1', + issueProviderKey: 'PLAINSPACE', + defaultProjectId: 'project-1', + // Configured (bound), but deliberately NO pluginConfig / isAutoCreateIssues. + spaceId: 'space-1', + token: 'pat_x', + }); + + store.overrideSelector(selectEnabledIssueProviders, [provider]); + store.refreshState(); + + const cfg = createMockIssueProvider({ + id: 'provider-1', + issueProviderKey: 'PLAINSPACE', + }); + issueProviderServiceSpy.getCfgOnce$.and.returnValue(of(cfg)); + + const task = createMockTask({ + id: 'task-new', + title: 'New Task', + projectId: 'project-1', + parentId: undefined, + issueId: undefined, + }); + taskServiceSpy.getByIdOnce$.and.returnValue(of(task)); + + effects.autoCreateIssueOnTaskAdd$.subscribe(); + + actions$.next( + TaskSharedActions.addTask({ + task, + workContextId: 'project-1', + workContextType: WorkContextType.PROJECT, + isAddToBacklog: false, + isAddToBottom: false, + }), + ); + + tick(); + + expect(createIssueSpy).toHaveBeenCalledWith('New Task', cfg); + expect(taskServiceSpy.update).toHaveBeenCalledWith( + 'task-new', + jasmine.objectContaining({ + issueId: 'ps-issue-1', + issueType: 'PLAINSPACE', + issueProviderId: 'provider-1', + }), + ); + })); + + it('should NOT create issue for a PLAINSPACE provider that is enabled but not yet configured (no spaceId/token)', fakeAsync(() => { + // selectEnabledIssueProviders filters on the isEnabled flag only, so a + // mid-connect provider (enabled, bound to the project, but spaceId/token + // still null) reaches the gate. It must NOT auto-create — otherwise every + // add POSTs an invalid create and error-snacks. + const createIssueSpy = jasmine.createSpy('createIssue').and.resolveTo({ + issueId: 'ps-issue-1', + issueData: {}, + }); + const adapter = createMockAdapter({ createIssue: createIssueSpy }); + adapterRegistry.register('PLAINSPACE', adapter); + + const provider = createMockIssueProvider({ + id: 'provider-1', + issueProviderKey: 'PLAINSPACE', + defaultProjectId: 'project-1', + spaceId: null, + token: null, + }); + + store.overrideSelector(selectEnabledIssueProviders, [provider]); + store.refreshState(); + + const task = createMockTask({ + id: 'task-new', + title: 'New Task', + projectId: 'project-1', + issueId: undefined, + }); + + effects.autoCreateIssueOnTaskAdd$.subscribe(); + + actions$.next( + TaskSharedActions.addTask({ + task, + workContextId: 'project-1', + workContextType: WorkContextType.PROJECT, + isAddToBacklog: false, + isAddToBottom: false, + }), + ); + + tick(); + + expect(createIssueSpy).not.toHaveBeenCalled(); + + adapterRegistry.unregister('PLAINSPACE'); + })); + it('should not create issue when task has no projectId', fakeAsync(() => { const createIssueSpy = jasmine.createSpy('createIssue').and.resolveTo({ issueId: 'new-issue-1', diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts index 395d8b9a6c..2dd80d697c 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts @@ -418,6 +418,16 @@ export class IssueTwoWaySyncEffects { if (this._pluginRegistry.getUseAgendaView(provider.issueProviderKey)) { return false; } + // Plainspace-backed projects are collaborative by definition: a task added + // there is pushed up so the team sees it (symmetric with auto-import). The + // bound provider is itself the opt-in, so there is no separate flag — but + // only once it is actually configured. An enabled-but-unbound provider + // (mid-connect, spaceId/token still null) would otherwise POST an invalid + // create and error-snack on every add; skip silently until it's ready. + if (provider.issueProviderKey === 'PLAINSPACE') { + const ps = provider as { spaceId?: string | null; token?: string | null }; + return !!ps.spaceId && !!ps.token; + } // Check for plugin providers (both plugin:* and migrated keys like GITHUB) const pluginCfg = (provider as { pluginConfig?: Record }) .pluginConfig;