diff --git a/frontend/src/i18n/lang/en.json b/frontend/src/i18n/lang/en.json index 3f7ee2588..1025f6526 100644 --- a/frontend/src/i18n/lang/en.json +++ b/frontend/src/i18n/lang/en.json @@ -955,6 +955,7 @@ "task": { "new": "Create a task", "createSuccess": "The task was successfully created.", + "bulkCreateUnexpectedResponse": "The server returned an unexpected number of created tasks.", "addReminder": "Add a reminder…", "removeReminder": "Remove this reminder", "doneSuccess": "The task was successfully marked as done.", diff --git a/frontend/src/message/index.ts b/frontend/src/message/index.ts index 7990dac9c..b14a61fce 100644 --- a/frontend/src/message/index.ts +++ b/frontend/src/message/index.ts @@ -28,6 +28,10 @@ export function getErrorText(r): string { return message } +export function translatedError(key: string): Error { + return new Error(i18n.global.t(key)) +} + export interface Action { title: string, callback: () => void, diff --git a/frontend/src/services/task.test.ts b/frontend/src/services/task.test.ts new file mode 100644 index 000000000..8ab5a46cf --- /dev/null +++ b/frontend/src/services/task.test.ts @@ -0,0 +1,173 @@ +import {describe, it, expect, vi, beforeEach} from 'vitest' + +import TaskService from './task' +import TaskModel from '@/models/task' +import type {ITask} from '@/modelTypes/ITask' + +interface BulkPayload { + tasks: {title: string}[], +} + +interface BulkResponse { + data: {tasks: {id: number, title: string}[]}, +} + +const post = vi.hoisted(() => vi.fn<(url: string, payload: BulkPayload) => Promise>()) + +vi.mock('@/helpers/fetcher', () => ({ + getApiBaseUrl: () => '/api/v1/', + apiV2Url: (path: string) => `/api/v2/${path}`, + HTTPFactory: () => ({post, interceptors: {request: {use: vi.fn()}, response: {use: vi.fn()}}}), + AuthenticatedHTTPFactory: () => ({post, interceptors: {request: {use: vi.fn()}, response: {use: vi.fn()}}}), +})) + +let nextId = 1 + +function echoResponse(payload: BulkPayload): BulkResponse { + return {data: {tasks: payload.tasks.map(t => ({id: nextId++, title: t.title}))}} +} + +function buildTasks(titles: string[], projectId: number): ITask[] { + return titles.map(title => new TaskModel({title, projectId})) +} + +function titlesOfCall(call: [string, BulkPayload]): string[] { + return call[1].tasks.map(t => t.title) +} + +describe('TaskService.bulkCreate', () => { + beforeEach(() => { + nextId = 1 + post.mockReset() + post.mockImplementation(async (_url, payload) => echoResponse(payload)) + }) + + it('creates all tasks of one project in a single request', async () => { + const titles = ['first', 'second', 'third'] + const {tasks, error} = await new TaskService().bulkCreate(buildTasks(titles, 42)) + + expect(post).toHaveBeenCalledTimes(1) + expect(post.mock.calls[0][0]).toBe('/api/v2/projects/42/tasks/bulk') + expect(error).toBeNull() + expect(tasks.map(t => t?.title)).toEqual(titles) + }) + + it('sends only the fields the v2 task schema allows', async () => { + await new TaskService().bulkCreate([new TaskModel({title: 'first', projectId: 42})]) + + const payloadTask = post.mock.calls[0][1].tasks[0] + expect(Object.keys(payloadTask).sort()).toEqual([ + 'assignees', + 'bucket_id', + 'description', + 'done', + 'due_date', + 'end_date', + 'hex_color', + 'is_favorite', + 'percent_done', + 'priority', + 'reminders', + 'repeat_after', + 'repeat_mode', + 'start_date', + 'title', + ]) + }) + + it('sends one request per project and keeps the input order', async () => { + const input = [ + new TaskModel({title: 'a', projectId: 1}), + new TaskModel({title: 'b', projectId: 2}), + new TaskModel({title: 'c', projectId: 1}), + new TaskModel({title: 'd', projectId: 2}), + ] + + const {tasks, error} = await new TaskService().bulkCreate(input) + + expect(post).toHaveBeenCalledTimes(2) + expect(post.mock.calls.map(c => c[0])).toEqual([ + '/api/v2/projects/1/tasks/bulk', + '/api/v2/projects/2/tasks/bulk', + ]) + expect(titlesOfCall(post.mock.calls[0])).toEqual(['a', 'c']) + expect(titlesOfCall(post.mock.calls[1])).toEqual(['b', 'd']) + expect(error).toBeNull() + expect(tasks.map(t => t?.title)).toEqual(['a', 'b', 'c', 'd']) + }) + + it('chunks at 100 tasks and submits the last chunk first', async () => { + const titles = Array.from({length: 205}, (_, i) => `task ${i}`) + + const {tasks, error} = await new TaskService().bulkCreate(buildTasks(titles, 7)) + + expect(post).toHaveBeenCalledTimes(3) + expect(post.mock.calls.map(c => c[1].tasks.length)).toEqual([5, 100, 100]) + expect(titlesOfCall(post.mock.calls[0])).toEqual(titles.slice(200)) + expect(titlesOfCall(post.mock.calls[1])).toEqual(titles.slice(100, 200)) + expect(titlesOfCall(post.mock.calls[2])).toEqual(titles.slice(0, 100)) + expect(error).toBeNull() + expect(tasks.map(t => t?.title)).toEqual(titles) + }) + + it('does not send an empty trailing batch', async () => { + const titles = Array.from({length: 200}, (_, i) => `task ${i}`) + + const {tasks} = await new TaskService().bulkCreate(buildTasks(titles, 7)) + + expect(post.mock.calls.map(c => c[1].tasks.length)).toEqual([100, 100]) + expect(tasks.map(t => t?.title)).toEqual(titles) + }) + + it('stops after a failed batch and keeps the batches sent before it', async () => { + const failure = new Error('boom') + let calls = 0 + post.mockImplementation(async (_url, payload) => { + calls++ + if (calls === 2) { + throw failure + } + return echoResponse(payload) + }) + + const titles = Array.from({length: 205}, (_, i) => `task ${i}`) + const {tasks, error} = await new TaskService().bulkCreate(buildTasks(titles, 7)) + + expect(post).toHaveBeenCalledTimes(2) + expect(error).toBe(failure) + expect(tasks.slice(200).map(t => t?.title)).toEqual(titles.slice(200)) + expect(tasks.slice(0, 200).every(t => t === null)).toBe(true) + }) + + it('fails the batch when the response does not match the payload', async () => { + post.mockImplementation(async () => ({data: {tasks: []}})) + + const {tasks, error} = await new TaskService().bulkCreate(buildTasks(['a', 'b'], 7)) + + expect(error).toBeInstanceOf(Error) + expect(tasks).toEqual([null, null]) + }) + + it('keeps tasks created by other projects when a batch fails', async () => { + const failure = new Error('boom') + post.mockImplementation(async (url, payload) => { + if (url === '/api/v2/projects/1/tasks/bulk') { + throw failure + } + return echoResponse(payload) + }) + + const input = [ + new TaskModel({title: 'a', projectId: 1}), + new TaskModel({title: 'b', projectId: 2}), + new TaskModel({title: 'c', projectId: 1}), + ] + + const {tasks, error} = await new TaskService().bulkCreate(input) + + expect(error).toBe(failure) + expect(tasks[0]).toBeNull() + expect(tasks[2]).toBeNull() + expect(tasks[1]?.title).toBe('b') + }) +}) diff --git a/frontend/src/services/task.ts b/frontend/src/services/task.ts index 73f2c5858..0022db1ed 100644 --- a/frontend/src/services/task.ts +++ b/frontend/src/services/task.ts @@ -7,7 +7,11 @@ import LabelService from './label' import {colorFromHex} from '@/helpers/color/colorFromHex' import {SECONDS_A_DAY, SECONDS_A_HOUR, SECONDS_A_WEEK} from '@/constants/date' import {objectToSnakeCase} from '@/helpers/case' -import {AuthenticatedHTTPFactory} from '@/helpers/fetcher' +import {apiV2Url, AuthenticatedHTTPFactory} from '@/helpers/fetcher' +import {translatedError} from '@/message' + +// Mirrors models.MaxTasksPerBulkCreation on the backend. +const MAX_TASKS_PER_BULK_CREATION = 100 const parseDate = date => { if (date) { @@ -122,6 +126,102 @@ export default class TaskService extends AbstractService { return transformed as ITask } + // The v2 endpoint validates strictly against the task schema and rejects the + // frontend-only properties (max_permission, reminder_dates, …) processModel + // adds, hence the allowlist. + private toBulkCreatePayload(task: ITask) { + // processModel lies about its return type — it returns the snake_cased + // wire format, not an ITask. + const processed = this.processModel(task) as unknown as { + assignees: {id: number, username: string}[], + reminders: {reminder: string | null, relative_period: number, relative_to: string | null}[], + } & Record + return { + title: processed.title, + description: processed.description, + done: processed.done, + due_date: processed.due_date, + start_date: processed.start_date, + end_date: processed.end_date, + priority: processed.priority, + hex_color: processed.hex_color, + percent_done: processed.percent_done, + repeat_after: processed.repeat_after, + repeat_mode: processed.repeat_mode, + is_favorite: processed.is_favorite, + bucket_id: processed.bucket_id, + assignees: processed.assignees.map(a => ({ + id: a.id, + username: a.username, + })), + reminders: processed.reminders.map(r => ({ + reminder: r.reminder, + relative_period: r.relative_period, + relative_to: r.relative_to, + })), + } + } + + // Returns tasks aligned 1:1 with the input (null = not created). Grouped per + // project because the endpoint takes the project from the URL. + async bulkCreate(tasks: ITask[]): Promise<{tasks: (ITask | null)[], error: unknown | null}> { + const cancel = this.setLoading() + + try { + const groups = new Map() + tasks.forEach((task, index) => { + const group = groups.get(task.projectId) + if (group) { + group.push(index) + } else { + groups.set(task.projectId, [index]) + } + }) + + const created: (ITask | null)[] = new Array(tasks.length).fill(null) + let error: unknown | null = null + // Sequential throughout: the server assigns task indexes at insert time, + // and concurrent bulk writes fail under write contention (SQLite). + for (const [projectId, indexes] of groups) { + const batches: number[][] = [] + for (let i = 0; i < indexes.length; i += MAX_TASKS_PER_BULK_CREATION) { + batches.push(indexes.slice(i, i + MAX_TASKS_PER_BULK_CREATION)) + } + + // Last chunk first: the server puts each batch on top in every view, so + // posting in reverse leaves the earliest input lines topmost. Tradeoff: + // per-project index numbers then run backwards across batches. + for (const batch of batches.reverse()) { + try { + // Fresh http instance: the shared one's interceptors would run + // processModel on the {tasks} wrapper. + const {data} = await AuthenticatedHTTPFactory().post( + apiV2Url(`projects/${Number(projectId)}/tasks/bulk`), + {tasks: batch.map(index => this.toBulkCreatePayload(tasks[index]))}, + ) + if (!Array.isArray(data?.tasks) || data.tasks.length !== batch.length) { + throw translatedError('task.bulkCreateUnexpectedResponse') + } + // The response is in payload order. Don't match by title — quick + // add magic cleans titles and duplicates would collide. + data.tasks.forEach((t: Partial, batchIndex: number) => { + created[batch[batchIndex]] = this.modelCreateFactory(t) + }) + } catch (e) { + // Keep what other batches created so the caller can retry only + // the missing tasks instead of duplicating everything. + error ??= e + break + } + } + } + + return {tasks: created, error} + } finally { + cancel() + } + } + async markTaskAsRead(taskId: ITask['id']): Promise { const cancel = this.setLoading()