diff --git a/api/src/core/GoalPermissionsRepository.ts b/api/src/core/GoalPermissionsRepository.ts index 3f6e084..678de4d 100644 --- a/api/src/core/GoalPermissionsRepository.ts +++ b/api/src/core/GoalPermissionsRepository.ts @@ -9,7 +9,7 @@ import { PermissionsSchema, } from 'taskview-db-schemas'; import { Database } from '../modules/db'; -import type { FetchGoalIdsWithAnyPermissionParams, GoalPermissionItemsFromDb } from '../types/auth.types'; +import type { FetchGoalIdsWithAnyPermissionParams, FetchPermissionsForGoalByUserParams, GoalPermissionItemsFromDb } from '../types/auth.types'; import type { GoalItemInDb } from '../types/goal.type'; import type { ListItemInDb } from '../types/lists.types'; import type { TaskItemInDb } from '../types/tasks.types'; @@ -33,7 +33,18 @@ export class GoalPermissionsRepository { } async fetchPermissionsForGoal(goalId: number, user: AppUser): Promise { - const goalInfo = await this.db.query('select * from tasks.goals where id = $1', [goalId]); + const userData = user.getUserData(); + if (!userData) return []; + return this.fetchPermissionsForGoalByUser({ goalId, userId: userData.id, email: userData.email }); + } + + /** + * Permission set of an arbitrary user for a goal, without an AppUser/request + * context — used by background workers (e.g. deadline notifications) that + * must gate content per recipient. + */ + async fetchPermissionsForGoalByUser(params: FetchPermissionsForGoalByUserParams): Promise { + const goalInfo = await this.db.query('select * from tasks.goals where id = $1', [params.goalId]); if (goalInfo.rows.length === 0) { return []; @@ -42,7 +53,7 @@ export class GoalPermissionsRepository { let query = ''; let args: any = []; - if (goalInfo.rows[0].owner === user.getUserData()?.id) { + if (goalInfo.rows[0].owner === params.userId) { query = `select name as "permissionName", id as "permissionId" from tv_auth.permissions;`; } else { query = `select p.name as "permissionName", p.id as "permissionId" @@ -54,7 +65,7 @@ export class GoalPermissionsRepository { left join collaboration.permissions_to_role ptr on rol.id = ptr.role_id left join tv_auth.permissions p on ptr.permission_id = p.id where email = $1 and tg.id = $2 and p.name is not null and p.id is not null;`; - args = [user.getUserData()?.email, goalId]; + args = [params.email, params.goalId]; } const result = await this.db.query(query, args); diff --git a/api/src/migrations/taskview/migrate.json b/api/src/migrations/taskview/migrate.json index 26cb972..b92cefc 100644 --- a/api/src/migrations/taskview/migrate.json +++ b/api/src/migrations/taskview/migrate.json @@ -618,5 +618,27 @@ "Extended tasks.tasks with recurrence_rule_id (FK SET NULL — instances survive rule deletion) and recurrence_instance_date. Partial unique index on (recurrence_rule_id, recurrence_instance_date) makes instance materialization idempotent under concurrent triggers/reconciliation.", "Exactly one open instance per series exists at any time: completing it materializes the next occurrence (event-driven, O(1)); a nightly pg-boss reconcile job re-creates the open instance only for series stalled by a crash. No new permissions — recurrence editing is gated by existing task_can_edit_deadline." ] + }, + "48": { + "version": "1.54.1", + "name": "Release 1.54.1", + "releaseDate": "20260613", + "scripts": [ + "/1.54.1/0.add-recurrence-template-task-unique.sql" + ], + "description": [ + "Partial unique index on tasks.recurrence_rules(template_task_id) WHERE state != 'ended' — one live series per origin task. DB-level backstop for the createRule race where two concurrent POSTs both pass the recurrenceRuleId == null check and create two rules for the same task (the orphaned rule would materialize a duplicate card via the nightly reconcile job)." + ] + }, + "49": { + "version": "1.54.2", + "name": "Release 1.54.2", + "releaseDate": "20260613", + "scripts": [ + "/1.54.2/0.add-recurrence-has-time.sql" + ], + "description": [ + "Added tasks.recurrence_rules.has_time — explicit flag for whether a series is anchored to a wall-clock time or is date-only. Previously the code inferred 'no time' from a midnight dtstart, which silently collapsed an explicit 00:00 series into date-only. Backfill (has_time = dtstart::time <> '00:00:00') reproduces the old inference so existing series keep their behavior; new series carry the flag through from the origin task's start_time (null = date-only, set = timed, including midnight)." + ] } } \ No newline at end of file diff --git a/api/src/migrations/taskview/sql/1.54.1/0.add-recurrence-template-task-unique.sql b/api/src/migrations/taskview/sql/1.54.1/0.add-recurrence-template-task-unique.sql new file mode 100644 index 0000000..4adc6c6 --- /dev/null +++ b/api/src/migrations/taskview/sql/1.54.1/0.add-recurrence-template-task-unique.sql @@ -0,0 +1,7 @@ +-- One live series per origin task: DB-level backstop for the createRule race +-- where two concurrent POSTs both pass the recurrenceRuleId == null check and +-- insert two rules for the same task. Ended series keep their row but release +-- the slot (the origin task itself stays attached to the ended rule anyway). +CREATE UNIQUE INDEX IF NOT EXISTS uniq_recurrence_rules_template_task + ON tasks.recurrence_rules(template_task_id) + WHERE state != 'ended'; diff --git a/api/src/migrations/taskview/sql/1.54.2/0.add-recurrence-has-time.sql b/api/src/migrations/taskview/sql/1.54.2/0.add-recurrence-has-time.sql new file mode 100644 index 0000000..5dbe2fc --- /dev/null +++ b/api/src/migrations/taskview/sql/1.54.2/0.add-recurrence-has-time.sql @@ -0,0 +1,13 @@ +-- A series anchored to a wall-clock time (incl. exactly 00:00) vs a date-only +-- series ("every day", no time) used to be told apart by inspecting dtstart: +-- midnight meant "no time". That collapses an explicit midnight into date-only. +-- Store the distinction explicitly instead. +ALTER TABLE tasks.recurrence_rules + ADD COLUMN IF NOT EXISTS has_time BOOLEAN NOT NULL DEFAULT FALSE; + +-- Backfill reproduces the old inference exactly: any series whose dtstart +-- carries a non-midnight wall-clock time was a timed series. Existing +-- midnight/date-only series keep has_time = FALSE — no behavior change. +UPDATE tasks.recurrence_rules + SET has_time = TRUE + WHERE dtstart::time <> '00:00:00'; diff --git a/api/src/tv-modules/kanban/KanbanRepository.ts b/api/src/tv-modules/kanban/KanbanRepository.ts index 86e1848..839bf33 100644 --- a/api/src/tv-modules/kanban/KanbanRepository.ts +++ b/api/src/tv-modules/kanban/KanbanRepository.ts @@ -3,7 +3,7 @@ import { Database } from '../../modules/db'; import type { GoalItemInDb } from '../../types/goal.type'; import { logError } from '../../utils/api'; import { updateQuery } from '../../utils/db-helper'; -import type { KanbanStatusItemInDb } from './types'; +import type { KanbanStatusItemInDb, StatusBelongsToGoalArgs } from './types'; import { callWithCatch } from '../../utils/helpers'; import { and, asc, eq, gt, gte, isNull, lt, lte, sql } from 'drizzle-orm'; import type { TaskItemInDb } from '../../types/tasks.types'; @@ -218,4 +218,14 @@ export class KanbanRepository { return result?.[0]?.columnVersion ?? null; } + + /** Validates payload references to a kanban column from other modules (recurrence templates, etc.). */ + async statusBelongsToGoal(args: StatusBelongsToGoalArgs): Promise { + const result = await callWithCatch(() => this.db.dbDrizzle + .select({ id: TasksStatusesSchema.id }) + .from(TasksStatusesSchema) + .where(and(eq(TasksStatusesSchema.id, args.statusId), eq(TasksStatusesSchema.goalId, args.goalId))) + .limit(1)); + return !!result?.[0]; + } } diff --git a/api/src/tv-modules/kanban/types.ts b/api/src/tv-modules/kanban/types.ts index 45058df..e169f6a 100644 --- a/api/src/tv-modules/kanban/types.ts +++ b/api/src/tv-modules/kanban/types.ts @@ -33,6 +33,8 @@ export type KanbanArgUpdateStatus = typeof KanbanArkTypeStatusUpdate.infer; export type KanbanArgFetchAllStatuses = typeof KanbanArkTypeFetchAllStatuses.infer; +export type StatusBelongsToGoalArgs = { statusId: number; goalId: number }; + export type KanbanStatusInDb = { id: number; goal_id: number; diff --git a/api/src/tv-modules/lists/GoalListsRepository.ts b/api/src/tv-modules/lists/GoalListsRepository.ts index 7938d50..8bd0c08 100644 --- a/api/src/tv-modules/lists/GoalListsRepository.ts +++ b/api/src/tv-modules/lists/GoalListsRepository.ts @@ -1,4 +1,4 @@ -import { desc, eq } from 'drizzle-orm'; +import { and, desc, eq } from 'drizzle-orm'; import type { GoalsListSchemaTypeForSelect } from 'taskview-db-schemas'; import { GoalsListSchema } from 'taskview-db-schemas'; import type { AppUser } from '../../core/AppUser'; @@ -7,7 +7,7 @@ import type { GoalListInDb } from '../../types/goal-list.types'; import { logError } from '../../utils/api'; import { updateQuery } from '../../utils/db-helper'; import { callWithCatch } from '../../utils/helpers'; -import type { GoalListArgAdd, GoalListArgDelete, GoalListArgFetch, GoalListArgUpdate } from './list.types'; +import type { GoalListArgAdd, GoalListArgDelete, GoalListArgFetch, GoalListArgUpdate, ListBelongsToGoalArgs } from './list.types'; export class GoalListsRepository { private readonly db: Database; @@ -145,4 +145,16 @@ export class GoalListsRepository { return !!(result.rowCount && result.rowCount > 0); } + + /** Validates payload references to a list from other modules (recurrence templates, etc.). */ + async listBelongsToGoal(args: ListBelongsToGoalArgs): Promise { + const result = await callWithCatch(() => + this.db.dbDrizzle + .select({ id: GoalsListSchema.id }) + .from(GoalsListSchema) + .where(and(eq(GoalsListSchema.id, args.listId), eq(GoalsListSchema.goalId, args.goalId))) + .limit(1) + ); + return !!result?.[0]; + } } diff --git a/api/src/tv-modules/lists/list.types.ts b/api/src/tv-modules/lists/list.types.ts index 4f69c6d..f5fe63c 100644 --- a/api/src/tv-modules/lists/list.types.ts +++ b/api/src/tv-modules/lists/list.types.ts @@ -27,3 +27,5 @@ export const GoalListArkTypeFetch = type({ }); export type GoalListArgFetch = typeof GoalListArkTypeFetch.infer; + +export type ListBelongsToGoalArgs = { listId: number; goalId: number }; diff --git a/api/src/tv-modules/notifications/NotificationMessages.ts b/api/src/tv-modules/notifications/NotificationMessages.ts index 359921c..219bd5b 100644 --- a/api/src/tv-modules/notifications/NotificationMessages.ts +++ b/api/src/tv-modules/notifications/NotificationMessages.ts @@ -3,7 +3,8 @@ import { parseUtcTime } from './utils'; export class NotificationMessages { static deadline(description: string | null, endDate: string, endTime: string | null, timezone: string): NotificationMessage { - const title = `Task: ${description || 'Task'}`; + // description is null for recipients without COMPONENT_CAN_WATCH_CONTENT + const title = description ? `Task: ${description}` : 'Task deadline'; if (endTime) { const deadline = parseUtcTime(endDate, endTime); diff --git a/api/src/tv-modules/notifications/schedulers/DeadlineScheduler.ts b/api/src/tv-modules/notifications/schedulers/DeadlineScheduler.ts index 2d37b1a..fee0519 100644 --- a/api/src/tv-modules/notifications/schedulers/DeadlineScheduler.ts +++ b/api/src/tv-modules/notifications/schedulers/DeadlineScheduler.ts @@ -1,13 +1,16 @@ -import { eq, and, or, isNull } from 'drizzle-orm'; +import { eq, and, or, isNull, inArray } from 'drizzle-orm'; import { alias } from 'drizzle-orm/pg-core'; import { TasksSchema, TasksAssigneeSchema, GoalsSchema, CollaborationUsersSchema, UsersSchema } from 'taskview-db-schemas'; import { getJobQueue, cancelJobBySingletonKey } from '../../../core/JobQueue'; +import { GoalPermissionsChecker } from '../../../core/GoalPermissionsChecker'; +import { GoalPermissionsRepository } from '../../../core/GoalPermissionsRepository'; import { Database } from '../../../modules/db'; import { $logger } from '../../../modules/logget'; +import { GoalPermissions } from '../../../types/auth.types'; import { getNotificationService } from '../NotificationService'; import { NotificationMessages } from '../NotificationMessages'; import { DeviceTokensRepository } from '../repositories/DeviceTokensRepository'; -import { NotificationType, type DeadlineJobData, type TaskWithDeadline } from '../types'; +import { NotificationType, type DeadlineJobData, type DeadlineRecipient, type TaskWithDeadline } from '../types'; import { parseUtcTime, localHourToUtc } from '../utils'; const DEADLINE_JOB = 'deadline-notification'; @@ -86,31 +89,36 @@ export class DeadlineScheduler { return; } - const recipientIds = await this.resolveRecipients(db, taskId, goalId, task[0].owner); - if (!recipientIds || recipientIds.length === 0) { + let recipients = await this.resolveRecipients(db, taskId, goalId, task[0].owner); + if (!recipients || recipients.length === 0) { $logger.info(`[DeadlineScheduler] Task ${taskId}: no recipients`); return; } if (immediate && initiatorId) { - const idx = recipientIds.indexOf(initiatorId); - if (idx !== -1) recipientIds.splice(idx, 1); + recipients = recipients.filter((r) => r.userId !== initiatorId); } - if (recipientIds.length === 0) return; + if (recipients.length === 0) return; - $logger.info(`[DeadlineScheduler] Task ${taskId}: sending to [${recipientIds.join(',')}]`); + // Description rides in the notification title, but it is gated by + // COMPONENT_CAN_WATCH_CONTENT — split recipients so those without + // the permission get a generic title (no leak over push). + const { canWatch, cannotWatch } = await this.splitByContentPermission(goalId, recipients); + + $logger.info(`[DeadlineScheduler] Task ${taskId}: sending to content=[${canWatch.join(',')}] generic=[${cannotWatch.join(',')}]`); const tz = task[0].owner ? await this.deviceTokensRepo.getTimezoneByUserId(task[0].owner) : 'UTC'; - const message = NotificationMessages.deadline(description, endDate, endTime, tz); + const meta = { goalId, goalListId, organizationId: organizationId ?? null }; - await getNotificationService().notifyMany( - recipientIds, - NotificationType.DEADLINE, - message, - { goalId, goalListId, organizationId: organizationId ?? null }, - taskId, - ); + if (canWatch.length > 0) { + const message = NotificationMessages.deadline(description, endDate, endTime, tz); + await getNotificationService().notifyMany(canWatch, NotificationType.DEADLINE, message, meta, taskId); + } + if (cannotWatch.length > 0) { + const message = NotificationMessages.deadline(null, endDate, endTime, tz); + await getNotificationService().notifyMany(cannotWatch, NotificationType.DEADLINE, message, meta, taskId); + } }); } @@ -127,7 +135,7 @@ export class DeadlineScheduler { return result[0]?.organizationId ?? null; } - private async resolveRecipients(db: Database, taskId: number, goalId: number, taskOwner: number | null): Promise { + private async resolveRecipients(db: Database, taskId: number, goalId: number, taskOwner: number | null): Promise { const authUsers = alias(UsersSchema, 'auth_users'); try { @@ -146,13 +154,46 @@ export class DeadlineScheduler { const ids = new Set(); if (taskOwner) ids.add(taskOwner); - assignees.forEach((r) => ids.add(r.userId)); + for (const r of assignees) ids.add(r.userId); if (goal[0]) ids.add(goal[0].owner); + if (ids.size === 0) return []; - return [...ids]; + // Emails are needed to resolve per-recipient goal permissions (role join keys on email). + return await db.dbDrizzle + .select({ userId: UsersSchema.id, email: UsersSchema.email }) + .from(UsersSchema) + .where(inArray(UsersSchema.id, [...ids])); } catch (err) { $logger.error(err, '[DeadlineScheduler] Failed to resolve recipients'); return null; } } + + /** Partition recipients into those allowed to see task content and those who are not. */ + private async splitByContentPermission(goalId: number,recipients: DeadlineRecipient[]): Promise<{ canWatch: number[]; cannotWatch: number[] }> { + const permissionsRepo = new GoalPermissionsRepository(); + const canWatch: number[] = []; + const cannotWatch: number[] = []; + + await Promise.all( + recipients.map(async (recipient) => { + // Fail closed per-recipient: a permission-fetch error drops this + // recipient to the generic message, never aborts the whole job. + const permissions = await permissionsRepo + .fetchPermissionsForGoalByUser({ goalId, userId: recipient.userId, email: recipient.email }) + .catch((err) => { + $logger.error(err, `[DeadlineScheduler] permission check failed for user=${recipient.userId}`); + return []; + }); + const checker = new GoalPermissionsChecker(permissions); + if (checker.hasPermissions(GoalPermissions.COMPONENT_CAN_WATCH_CONTENT)) { + canWatch.push(recipient.userId); + } else { + cannotWatch.push(recipient.userId); + } + }), + ); + + return { canWatch, cannotWatch }; + } } diff --git a/api/src/tv-modules/notifications/types.ts b/api/src/tv-modules/notifications/types.ts index 54cede3..8115a03 100644 --- a/api/src/tv-modules/notifications/types.ts +++ b/api/src/tv-modules/notifications/types.ts @@ -121,4 +121,9 @@ export interface TaskWithDeadline { owner: number | null; endDate: string | null; endTime: string | null; +} + +export interface DeadlineRecipient { + userId: number; + email: string; } \ No newline at end of file diff --git a/api/src/tv-modules/recurrence/RecurrenceDispatcher.ts b/api/src/tv-modules/recurrence/RecurrenceDispatcher.ts index 8eda1f0..71e31a8 100644 --- a/api/src/tv-modules/recurrence/RecurrenceDispatcher.ts +++ b/api/src/tv-modules/recurrence/RecurrenceDispatcher.ts @@ -90,16 +90,17 @@ export class RecurrenceDispatcher implements Dispatcher { const memberIds = await this.resolveGoalMemberIds(data.task.goalId); if (memberIds.length === 0) return; const centrifugo = getCentrifugoClient(); - // The broadcast goes to every goal member, while notes are gated by a - // separate per-role permission — never put them on the wire here. - // Clients with note access get it when they open the task. - const task = { ...data.task, note: null }; + // Task fields are gated per role (TaskFieldPermissionsForWatching), + // and recipients have different roles — so the broadcast carries ids + // only, never content (same thin-event convention as goals.changed). + // Each client fetches the task through REST, where fields are + // cleaned for that user (fail closed). await Promise.all( memberIds.map((userId) => centrifugo.publishToUser(userId, RECURRENCE_RT_EVENT, { goalId: data.task.goalId, ruleId: data.task.recurrenceRuleId, - task, + taskId: data.task.id, }) ) ); diff --git a/api/src/tv-modules/recurrence/RecurrenceGenerator.ts b/api/src/tv-modules/recurrence/RecurrenceGenerator.ts index 0925c3f..50aea2b 100644 --- a/api/src/tv-modules/recurrence/RecurrenceGenerator.ts +++ b/api/src/tv-modules/recurrence/RecurrenceGenerator.ts @@ -178,6 +178,7 @@ export class RecurrenceGenerator { const window = RecurrenceParser.instanceWindowUtc({ occurrenceDate: instanceDate, dtstart: rule.dtstart, + hasTime: rule.hasTime, timezone: rule.timezone, durationMinutes: rule.templateDurationMinutes, }); diff --git a/api/src/tv-modules/recurrence/RecurrenceManager.ts b/api/src/tv-modules/recurrence/RecurrenceManager.ts index a330cc6..064e4ac 100644 --- a/api/src/tv-modules/recurrence/RecurrenceManager.ts +++ b/api/src/tv-modules/recurrence/RecurrenceManager.ts @@ -1,7 +1,9 @@ import type { RecurrenceRulesSchemaTypeForSelect } from 'taskview-db-schemas'; import type { AppUser } from '../../core/AppUser'; import { eventBus } from '../../core/EventBus'; -import { GoalPermissions } from '../../types/auth.types'; +import { KanbanRepository } from '../kanban/KanbanRepository'; +import { GoalListsRepository } from '../lists/GoalListsRepository'; +import { TaskFieldPermissionsForWatching } from '../tasks/tasks.server.types'; import { TasksRepository } from '../tasks/TasksRepository'; import { RecurrenceGenerator } from './RecurrenceGenerator'; import { RecurrenceParser } from './RecurrenceParser'; @@ -17,17 +19,31 @@ import type { const ok = (data: T): RecurrenceResult => ({ ok: true, data }); const fail = (code: RecurrenceErrorCode, message?: string): RecurrenceResult => ({ ok: false, code, message }); +// Template fields mirror task fields, so the task-field permission map stays +// the single source of truth for which permission gates which field. +const RuleTemplateFieldPermissionsForWatching = { + templateDescription: TaskFieldPermissionsForWatching.description, + templateNote: TaskFieldPermissionsForWatching.note, + templatePriorityId: TaskFieldPermissionsForWatching.priorityId, + templateStatusId: TaskFieldPermissionsForWatching.statusId, + templateGoalListId: TaskFieldPermissionsForWatching.goalListId, +} as const; + export class RecurrenceManager { private readonly user: AppUser; public readonly repository: RecurrenceRepository; private readonly generator: RecurrenceGenerator; private readonly tasksRepository: TasksRepository; + private readonly kanbanRepository: KanbanRepository; + private readonly goalListsRepository: GoalListsRepository; constructor(user: AppUser) { this.user = user; this.repository = new RecurrenceRepository(); this.generator = new RecurrenceGenerator(); this.tasksRepository = new TasksRepository(); + this.kanbanRepository = new KanbanRepository(); + this.goalListsRepository = new GoalListsRepository(); } private get initiatorId(): number { @@ -46,9 +62,10 @@ export class RecurrenceManager { } let dtstart: Date; + let hasTime: boolean; try { RecurrenceParser.validateRuleString(args.rrule); - dtstart = RecurrenceParser.parseDtstart(args.dtstart); + ({ date: dtstart, hasTime } = RecurrenceParser.parseDtstart(args.dtstart)); } catch (err) { return fail('invalid_rule', (err as Error).message); } @@ -56,55 +73,71 @@ export class RecurrenceManager { const originInstanceDate = RecurrenceParser.firstOccurrenceDate({ rrule: args.rrule, dtstart }); if (!originInstanceDate) return fail('invalid_rule', 'rule produces no occurrences'); - const rule = await this.repository.create({ - goalId: task.goalId, - templateTaskId: task.id, - templateDescription: task.description ?? '', - templateNote: task.note, - templatePriorityId: task.priorityId, - templateStatusId: task.statusId, - templateGoalListId: task.goalListId, - templateDurationMinutes: this.durationFromTask({ - startDate: task.startDate ?? originInstanceDate, - startTime: task.startTime, - endDate: task.endDate, - endTime: task.endTime, - }), - rrule: args.rrule, - dtstart, - timezone: args.timezone, - lastInstanceDate: originInstanceDate, - notifyOnOccurrence: args.notifyOnOccurrence ?? false, - creatorId: this.initiatorId, + const templateDurationMinutes = this.durationFromTask({ + startDate: task.startDate ?? originInstanceDate, + startTime: task.startTime, + endDate: task.endDate, + endTime: task.endTime, }); - if (!rule) return fail('invalid_state', 'could not create rule'); - // The origin task becomes the first (and only open) instance of the series. - await this.repository.attachTaskToRule({ taskId: task.id, ruleId: rule.id, instanceDate: originInstanceDate }); - // Normalize its window into the same UTC frame future instances will use — + // One transaction: rule + origin attachment + snapshot. The origin task + // becomes the first (and only open) instance of the series, its window + // normalized into the same UTC frame future instances will use — // otherwise a series created through a non-browser client (MCP, raw API) // could leave the origin and its successors in different time frames. - await this.repository.applyInstanceWindow({ - taskId: task.id, + const outcome = await this.repository.createWithOriginTask({ + rule: { + goalId: task.goalId, + templateTaskId: task.id, + templateDescription: task.description ?? '', + templateNote: task.note, + templatePriorityId: task.priorityId, + templateStatusId: task.statusId, + templateGoalListId: task.goalListId, + templateDurationMinutes, + rrule: args.rrule, + dtstart, + hasTime, + timezone: args.timezone, + lastInstanceDate: originInstanceDate, + notifyOnOccurrence: args.notifyOnOccurrence ?? false, + creatorId: this.initiatorId, + }, + originTaskId: task.id, + originInstanceDate, window: RecurrenceParser.instanceWindowUtc({ occurrenceDate: originInstanceDate, dtstart, + hasTime, timezone: args.timezone, - durationMinutes: rule.templateDurationMinutes, + durationMinutes: templateDurationMinutes, }), }); + if ('error' in outcome) { + if (outcome.error === 'not_found') return fail('not_found', 'task not found'); + if (outcome.error === 'conflict') return fail('conflict', 'task is already part of a series'); + return fail('invalid_state', 'could not create rule'); + } - const [assignees, tags] = await Promise.all([ - this.repository.getTaskAssignees(task.id), - this.repository.getTaskTags(task.id), - ]); - await Promise.all([ - this.repository.setTemplateAssignees({ ruleId: rule.id, collabUserIds: assignees }), - this.repository.setTemplateTags({ ruleId: rule.id, tagIds: tags }), - ]); + eventBus.emit('recurrence.created', { rule: outcome.rule, initiatorId: this.initiatorId }); + return ok(await this.cleanRuleFieldsRegardPermissions(outcome.rule)); + } - eventBus.emit('recurrence.created', { rule, initiatorId: this.initiatorId }); - return ok(rule); + /** + * Task fields are permission-gated for reading (TaskFieldPermissionsForWatching, + * applied by cleanTaskFieldsRegardPermissions in the task API) — no series + * endpoint may become a side door to them, so every response carrying a rule + * strips the template fields the caller is not allowed to watch. + */ + private async cleanRuleFieldsRegardPermissions(rule: RecurrenceRulesSchemaTypeForSelect): Promise { + const checker = await this.user.permissionsFetcher.getCheckerForGoal(rule.goalId).catch(() => null); + const cleaned = { ...rule }; + (Object.keys(RuleTemplateFieldPermissionsForWatching) as (keyof typeof RuleTemplateFieldPermissionsForWatching)[]).forEach((field) => { + if (!checker?.hasPermissions(RuleTemplateFieldPermissionsForWatching[field])) { + cleaned[field] = null; + } + }); + return cleaned; } async getDetails(ruleId: number): Promise> { @@ -115,16 +148,17 @@ export class RecurrenceManager { this.repository.findOpenInstance(ruleId), ]); - // Notes are gated by a separate permission in the task API - // (cleanTaskFieldsRegardPermissions) — the series endpoints must not - // become a side door to them. - const checker = await this.user.permissionsFetcher.getCheckerForGoal(rule.goalId).catch(() => null); - if (!checker?.hasPermissions(GoalPermissions.TASK_CAN_WATCH_NOTE)) { - rule.templateNote = null; - if (openInstance) openInstance.note = null; - } + // The open instance is an ordinary task — gate its fields exactly like + // the task API does. Fail closed: better no instance than a leak. + const cleanedInstance = openInstance + ? await this.user.tasksManager.cleanTaskFieldsRegardPermissions(openInstance).catch(() => null) + : null; - return ok({ rule, skipDates, openInstance }); + return ok({ + rule: await this.cleanRuleFieldsRegardPermissions(rule), + skipDates, + openInstance: cleanedInstance, + }); } async getDetailsForTask(taskId: number): Promise> { @@ -149,7 +183,9 @@ export class RecurrenceManager { } if (args.dtstart !== undefined) { try { - patch.dtstart = RecurrenceParser.parseDtstart(args.dtstart); + const parsed = RecurrenceParser.parseDtstart(args.dtstart); + patch.dtstart = parsed.date; + patch.hasTime = parsed.hasTime; } catch (err) { return fail('invalid_rule', (err as Error).message); } @@ -160,9 +196,28 @@ export class RecurrenceManager { } patch.timezone = args.timezone; } + if (patch.rrule !== undefined || patch.dtstart !== undefined) { + const nextDate = RecurrenceParser.nextOccurrenceDate({ + rrule: patch.rrule ?? rule.rrule, + dtstart: patch.dtstart ?? rule.dtstart, + afterDate: RecurrenceParser.todayInTimezone(patch.timezone ?? rule.timezone), + skipDates: new Set(), + }); + if (!nextDate) return fail('invalid_rule', 'rule produces no occurrences'); + } if (args.notifyOnOccurrence !== undefined) patch.notifyOnOccurrence = args.notifyOnOccurrence; if (args.templateOverrides) { const o = args.templateOverrides; + // Foreign/dead ids must fail here, not get stored and silently + // fall back at materialization time (null clears the override). + if (o.statusId !== undefined && o.statusId !== null) { + const belongs = await this.kanbanRepository.statusBelongsToGoal({ statusId: o.statusId, goalId: rule.goalId }); + if (!belongs) return fail('invalid_rule', 'status does not belong to the goal'); + } + if (o.goalListId !== undefined && o.goalListId !== null) { + const belongs = await this.goalListsRepository.listBelongsToGoal({ listId: o.goalListId, goalId: rule.goalId }); + if (!belongs) return fail('invalid_rule', 'list does not belong to the goal'); + } if (o.description !== undefined) patch.templateDescription = o.description; if (o.note !== undefined) patch.templateNote = o.note; if (o.priorityId !== undefined) patch.templatePriorityId = o.priorityId; @@ -170,13 +225,13 @@ export class RecurrenceManager { if (o.goalListId !== undefined) patch.templateGoalListId = o.goalListId; if (o.durationMinutes !== undefined) patch.templateDurationMinutes = o.durationMinutes; } - if (Object.keys(patch).length === 0) return ok(rule); + if (Object.keys(patch).length === 0) return ok(await this.cleanRuleFieldsRegardPermissions(rule)); const updated = await this.repository.patch({ ruleId: args.ruleId, patch }); if (!updated) return fail('invalid_state'); eventBus.emit('recurrence.updated', { rule: updated, changes: patch, initiatorId: this.initiatorId }); - return ok(updated); + return ok(await this.cleanRuleFieldsRegardPermissions(updated)); } async pauseRule(ruleId: number): Promise> { @@ -187,7 +242,7 @@ export class RecurrenceManager { const updated = await this.repository.patch({ ruleId, patch: { state: 'paused' } }); if (!updated) return fail('invalid_state'); eventBus.emit('recurrence.paused', { ruleId, goalId: rule.goalId, initiatorId: this.initiatorId }); - return ok(updated); + return ok(await this.cleanRuleFieldsRegardPermissions(updated)); } async resumeRule(ruleId: number): Promise> { @@ -206,7 +261,7 @@ export class RecurrenceManager { } eventBus.emit('recurrence.resumed', { ruleId, goalId: rule.goalId, initiatorId: this.initiatorId }); - return ok(updated); + return ok(await this.cleanRuleFieldsRegardPermissions(updated)); } /** Skip the current occurrence: the open instance is removed and the card "jumps" to the next date. */ diff --git a/api/src/tv-modules/recurrence/RecurrenceParser.ts b/api/src/tv-modules/recurrence/RecurrenceParser.ts index 923dc48..23501d2 100644 --- a/api/src/tv-modules/recurrence/RecurrenceParser.ts +++ b/api/src/tv-modules/recurrence/RecurrenceParser.ts @@ -81,13 +81,16 @@ export class RecurrenceParser { return today.isValid ? (today.toISODate() as string) : (DateTime.utc().toISODate() as string); } - /** 'HH:mm:ss' wall-clock time of day of the series, or null when the series carries no time (midnight dtstart). */ - static timeOfDay(dtstart: Date): string | null { - const hours = dtstart.getUTCHours(); - const minutes = dtstart.getUTCMinutes(); - if (hours === 0 && minutes === 0) return null; + /** + * 'HH:mm:ss' wall-clock time of day of the series, or null for a date-only + * series. The distinction is carried explicitly by `hasTime` (rule column) + * — never inferred from a midnight dtstart, otherwise an explicit 00:00 + * series would be indistinguishable from "no time". + */ + static timeOfDay(args: { dtstart: Date; hasTime: boolean }): string | null { + if (!args.hasTime) return null; const pad = (n: number) => String(n).padStart(2, '0'); - return `${pad(hours)}:${pad(minutes)}:00`; + return `${pad(args.dtstart.getUTCHours())}:${pad(args.dtstart.getUTCMinutes())}:00`; } static isValidTimezone(timezone: string): boolean { @@ -104,7 +107,7 @@ export class RecurrenceParser { * days are pushed forward by luxon to the nearest valid time. */ static instanceWindowUtc(args: InstanceWindowArgs): InstanceWindow { - const wallTime = RecurrenceParser.timeOfDay(args.dtstart); + const wallTime = RecurrenceParser.timeOfDay({ dtstart: args.dtstart, hasTime: args.hasTime }); // Date-only series: calendar dates pass through untouched (no instant semantics). if (!wallTime) { @@ -142,11 +145,17 @@ export class RecurrenceParser { return window; } - /** Floating wall-clock 'YYYY-MM-DDTHH:mm:ss' string → Date with the same UTC components. */ - static parseDtstart(dtstart: string): Date { - const date = new Date(`${dtstart}Z`); + /** + * dtstart in RFC 5545 DATE or DATE-TIME shape → Date (+ whether a time was + * given). `YYYY-MM-DD` is date-only (`hasTime: false`); `YYYY-MM-DDTHH:mm:ss` + * carries a wall-clock time (`hasTime: true`), including an explicit + * `T00:00:00`. Either way the Date holds floating wall-clock UTC components. + */ + static parseDtstart(dtstart: string): { date: Date; hasTime: boolean } { + const hasTime = dtstart.includes('T'); + const date = new Date(`${hasTime ? dtstart : `${dtstart}T00:00:00`}Z`); if (Number.isNaN(date.getTime())) throw new Error('Invalid dtstart'); - return date; + return { date, hasTime }; } static toIsoDate(date: Date): string { diff --git a/api/src/tv-modules/recurrence/RecurrenceRepository.ts b/api/src/tv-modules/recurrence/RecurrenceRepository.ts index dcff68a..7c1572c 100644 --- a/api/src/tv-modules/recurrence/RecurrenceRepository.ts +++ b/api/src/tv-modules/recurrence/RecurrenceRepository.ts @@ -1,7 +1,6 @@ import { and, eq, inArray, isNotNull, ne, notExists, sql } from 'drizzle-orm'; import { RecurrenceRulesSchema, - type RecurrenceRulesSchemaTypeForInsert, type RecurrenceRulesSchemaTypeForSelect, RecurrenceSkipDatesSchema, RecurrenceTemplateAssigneesSchema, @@ -12,17 +11,18 @@ import { TasksToTagsSchema, } from 'taskview-db-schemas'; import { Database } from '../../modules/db'; +import { $logger } from '../../modules/logget'; import { callWithCatch } from '../../utils/helpers'; import type { AddSkipDateArgs, - ApplyInstanceWindowArgs, - AttachTaskToRuleArgs, + CreateRuleWithOriginArgs, + CreateRuleWithOriginResult, RecurrenceRulePatchArgs, RemoveTemplateAssigneeFromGoalArgs, - SetTemplateAssigneesArgs, - SetTemplateTagsArgs, } from './types'; +const PG_UNIQUE_VIOLATION = '23505'; + export class RecurrenceRepository { private readonly db: Database; @@ -30,11 +30,6 @@ export class RecurrenceRepository { this.db = Database.getInstance(); } - async create(data: RecurrenceRulesSchemaTypeForInsert): Promise { - const result = await callWithCatch(() => this.db.dbDrizzle.insert(RecurrenceRulesSchema).values(data).returning()); - return result?.[0] ?? null; - } - async getById(ruleId: number): Promise { const result = await callWithCatch(() => this.db.dbDrizzle.select().from(RecurrenceRulesSchema).where(eq(RecurrenceRulesSchema.id, ruleId)).limit(1) @@ -91,54 +86,6 @@ export class RecurrenceRepository { ); } - async getTemplateAssignees(ruleId: number): Promise { - const result = await callWithCatch(() => - this.db.dbDrizzle - .select({ collabUserId: RecurrenceTemplateAssigneesSchema.collabUserId }) - .from(RecurrenceTemplateAssigneesSchema) - .where(eq(RecurrenceTemplateAssigneesSchema.ruleId, ruleId)) - ); - return result?.map((r) => r.collabUserId) ?? []; - } - - async getTemplateTags(ruleId: number): Promise { - const result = await callWithCatch(() => - this.db.dbDrizzle - .select({ tagId: RecurrenceTemplateTagsSchema.tagId }) - .from(RecurrenceTemplateTagsSchema) - .where(eq(RecurrenceTemplateTagsSchema.ruleId, ruleId)) - ); - return result?.map((r) => r.tagId) ?? []; - } - - async setTemplateAssignees(args: SetTemplateAssigneesArgs): Promise { - await callWithCatch(() => - this.db.dbDrizzle.transaction(async (tx) => { - await tx.delete(RecurrenceTemplateAssigneesSchema).where(eq(RecurrenceTemplateAssigneesSchema.ruleId, args.ruleId)); - if (args.collabUserIds.length > 0) { - await tx - .insert(RecurrenceTemplateAssigneesSchema) - .values(args.collabUserIds.map((collabUserId) => ({ ruleId: args.ruleId, collabUserId }))) - .onConflictDoNothing(); - } - }) - ); - } - - async setTemplateTags(args: SetTemplateTagsArgs): Promise { - await callWithCatch(() => - this.db.dbDrizzle.transaction(async (tx) => { - await tx.delete(RecurrenceTemplateTagsSchema).where(eq(RecurrenceTemplateTagsSchema.ruleId, args.ruleId)); - if (args.tagIds.length > 0) { - await tx - .insert(RecurrenceTemplateTagsSchema) - .values(args.tagIds.map((tagId) => ({ ruleId: args.ruleId, tagId }))) - .onConflictDoNothing(); - } - }) - ); - } - /** Drops an ex-collaborator from the assignee snapshot of every rule in the goal. */ async removeTemplateAssigneeFromGoal(args: RemoveTemplateAssigneeFromGoalArgs): Promise { const goalRules = this.db.dbDrizzle @@ -157,26 +104,6 @@ export class RecurrenceRepository { ); } - async getTaskAssignees(taskId: number): Promise { - const result = await callWithCatch(() => - this.db.dbDrizzle - .select({ collabUserId: TasksAssigneeSchema.collabUserId }) - .from(TasksAssigneeSchema) - .where(eq(TasksAssigneeSchema.taskId, taskId)) - ); - return result?.map((r) => r.collabUserId) ?? []; - } - - async getTaskTags(taskId: number): Promise { - const result = await callWithCatch(() => - this.db.dbDrizzle - .select({ tagId: TasksToTagsSchema.tagId }) - .from(TasksToTagsSchema) - .where(eq(TasksToTagsSchema.taskId, taskId)) - ); - return result?.map((r) => r.tagId) ?? []; - } - async getTaskById(taskId: number): Promise { const result = await callWithCatch(() => this.db.dbDrizzle.select().from(TasksSchema).where(eq(TasksSchema.id, taskId)).limit(1) @@ -184,29 +111,75 @@ export class RecurrenceRepository { return result?.[0] ?? null; } - async attachTaskToRule(args: AttachTaskToRuleArgs): Promise { - const result = await callWithCatch(() => - this.db.dbDrizzle - .update(TasksSchema) - .set({ recurrenceRuleId: args.ruleId, recurrenceInstanceDate: args.instanceDate }) - .where(eq(TasksSchema.id, args.taskId)) - ); - return !!result?.rowCount; - } + /** + * Atomic series creation. The origin task row is locked FOR UPDATE, so + * concurrent creates on the same task serialize: the loser waits on the + * lock, then sees recurrenceRuleId already set and reports a conflict. + * The partial unique index uniq_recurrence_rules_template_task backs this + * up on the DB level. Everything — rule insert, origin attachment (with + * the window normalized to the series frame) and the assignee/tag + * snapshot — commits or rolls back together. + */ + async createWithOriginTask(args: CreateRuleWithOriginArgs): Promise { + try { + return await this.db.dbDrizzle.transaction(async (tx) => { + const taskRows = await tx + .select({ recurrenceRuleId: TasksSchema.recurrenceRuleId }) + .from(TasksSchema) + .where(eq(TasksSchema.id, args.originTaskId)) + .for('update') + .limit(1); + const task = taskRows[0]; + if (!task) return { error: 'not_found' as const }; + if (task.recurrenceRuleId) return { error: 'conflict' as const }; - /** Aligns a task's start/end window with the series frame (used to normalize the origin instance). */ - async applyInstanceWindow(args: ApplyInstanceWindowArgs): Promise { - await callWithCatch(() => - this.db.dbDrizzle - .update(TasksSchema) - .set({ - startDate: args.window.startDate, - startTime: args.window.startTime, - endDate: args.window.endDate, - endTime: args.window.endTime, - }) - .where(eq(TasksSchema.id, args.taskId)) - ); + const ruleRows = await tx.insert(RecurrenceRulesSchema).values(args.rule).returning(); + const rule = ruleRows[0]; + + await tx + .update(TasksSchema) + .set({ + recurrenceRuleId: rule.id, + recurrenceInstanceDate: args.originInstanceDate, + startDate: args.window.startDate, + startTime: args.window.startTime, + endDate: args.window.endDate, + endTime: args.window.endTime, + }) + .where(eq(TasksSchema.id, args.originTaskId)); + + const [assignees, tags] = await Promise.all([ + tx + .select({ collabUserId: TasksAssigneeSchema.collabUserId }) + .from(TasksAssigneeSchema) + .where(eq(TasksAssigneeSchema.taskId, args.originTaskId)), + tx + .select({ tagId: TasksToTagsSchema.tagId }) + .from(TasksToTagsSchema) + .where(eq(TasksToTagsSchema.taskId, args.originTaskId)), + ]); + if (assignees.length > 0) { + await tx + .insert(RecurrenceTemplateAssigneesSchema) + .values(assignees.map((a) => ({ ruleId: rule.id, collabUserId: a.collabUserId }))) + .onConflictDoNothing(); + } + if (tags.length > 0) { + await tx + .insert(RecurrenceTemplateTagsSchema) + .values(tags.map((t) => ({ ruleId: rule.id, tagId: t.tagId }))) + .onConflictDoNothing(); + } + + return { rule }; + }); + } catch (error) { + if ((error as { code?: string } | null)?.code === PG_UNIQUE_VIOLATION) { + return { error: 'conflict' }; + } + $logger.error(error, '[RecurrenceRepository] createWithOriginTask failed'); + return { error: 'failed' }; + } } /** The single not-completed instance of the rule (the lazy model keeps at most one). */ diff --git a/api/src/tv-modules/recurrence/RecurrenceRoutes.ts b/api/src/tv-modules/recurrence/RecurrenceRoutes.ts index 0b4792d..3063747 100644 --- a/api/src/tv-modules/recurrence/RecurrenceRoutes.ts +++ b/api/src/tv-modules/recurrence/RecurrenceRoutes.ts @@ -35,7 +35,10 @@ export default class RecurrenceRoutes implements Routable { this.router.patch('/:ruleId', [IsLoggedIn, requireRecurrencePermission(GoalPermissions.TASKS_CAN_EDIT_DEADLINE, goalIdFromRuleParam)], this.controller.update); this.router.post('/:ruleId/pause', [IsLoggedIn, requireRecurrencePermission(GoalPermissions.TASKS_CAN_EDIT_DEADLINE, goalIdFromRuleParam)], this.controller.pause); this.router.post('/:ruleId/resume', [IsLoggedIn, requireRecurrencePermission(GoalPermissions.TASKS_CAN_EDIT_DEADLINE, goalIdFromRuleParam)], this.controller.resume); - this.router.post('/:ruleId/skip', [IsLoggedIn, requireRecurrencePermission(GoalPermissions.TASKS_CAN_EDIT_DEADLINE, goalIdFromRuleParam)], this.controller.skip); + // Skip is the one schedule operation that physically deletes the open + // instance (with its subtasks and tracked time) — so on top of the + // schedule permission it requires the same right as DELETE /tasks. + this.router.post('/:ruleId/skip', [IsLoggedIn, requireRecurrencePermission([GoalPermissions.TASKS_CAN_EDIT_DEADLINE, GoalPermissions.TASKS_CAN_DELETE], goalIdFromRuleParam)], this.controller.skip); this.router.delete('/:ruleId', [IsLoggedIn, requireRecurrencePermission(GoalPermissions.TASKS_CAN_EDIT_DEADLINE, goalIdFromRuleParam)], this.controller.remove); } } diff --git a/api/src/tv-modules/recurrence/middlewares/require-recurrence-permission.ts b/api/src/tv-modules/recurrence/middlewares/require-recurrence-permission.ts index 62b9fe4..2299f0c 100644 --- a/api/src/tv-modules/recurrence/middlewares/require-recurrence-permission.ts +++ b/api/src/tv-modules/recurrence/middlewares/require-recurrence-permission.ts @@ -31,7 +31,9 @@ export const goalIdFromTaskParam: GoalIdResolver = async (req) => { return task?.goalId ?? null; }; -export function requireRecurrencePermission(permission: GoalPermissionType, resolveGoalId: GoalIdResolver) { +/** A single permission or a list — the caller must hold ALL of them. */ +export function requireRecurrencePermission(permission: GoalPermissionType | GoalPermissionType[], resolveGoalId: GoalIdResolver) { + const required = Array.isArray(permission) ? permission : [permission]; return async (req: Request, res: Response, next: NextFunction) => { const goalId = await resolveGoalId(req); if (!goalId) return res.status(404).end(); @@ -45,7 +47,7 @@ export function requireRecurrencePermission(permission: GoalPermissionType, reso return res.status(500).end(); } - if (permissions.hasPermissions(permission)) return next(); + if (required.every((p) => permissions.hasPermissions(p))) return next(); return res.status(403).end(); }; } diff --git a/api/src/tv-modules/recurrence/types.ts b/api/src/tv-modules/recurrence/types.ts index a6e2a5d..3c95b30 100644 --- a/api/src/tv-modules/recurrence/types.ts +++ b/api/src/tv-modules/recurrence/types.ts @@ -1,5 +1,9 @@ import { type } from 'arktype'; -import type { RecurrenceRulesSchemaTypeForSelect, TasksSchemaTypeForSelect } from 'taskview-db-schemas'; +import type { + RecurrenceRulesSchemaTypeForInsert, + RecurrenceRulesSchemaTypeForSelect, + TasksSchemaTypeForSelect, +} from 'taskview-db-schemas'; /** Request validators (ArkType) */ @@ -63,6 +67,8 @@ export type InstanceWindowArgs = { /** 'YYYY-MM-DD' wall-clock occurrence date in the rule's timezone. */ occurrenceDate: string; dtstart: Date; + /** False → date-only occurrence (no start/end time). */ + hasTime: boolean; timezone: string; durationMinutes: number | null; }; @@ -80,6 +86,7 @@ export type RecurrenceRulePatchArgs = { patch: Partial<{ rrule: string; dtstart: Date; + hasTime: boolean; timezone: string; state: 'active' | 'paused' | 'ended'; lastInstanceDate: string; @@ -95,12 +102,24 @@ export type RecurrenceRulePatchArgs = { }>; }; -export type AttachTaskToRuleArgs = { taskId: number; ruleId: number; instanceDate: string }; export type AddSkipDateArgs = { ruleId: number; skipDate: string }; -export type SetTemplateAssigneesArgs = { ruleId: number; collabUserIds: number[] }; -export type SetTemplateTagsArgs = { ruleId: number; tagIds: number[] }; export type RemoveTemplateAssigneeFromGoalArgs = { goalId: number; collabUserId: number }; -export type ApplyInstanceWindowArgs = { taskId: number; window: InstanceWindow }; + +/** + * Atomic series creation: rule insert + origin task attachment (with its + * window normalized to the series frame) + assignee/tag snapshot — one + * transaction with the origin task row locked FOR UPDATE, so concurrent + * creates on the same task serialize instead of producing two rules. + */ +export type CreateRuleWithOriginArgs = { + rule: RecurrenceRulesSchemaTypeForInsert; + originTaskId: number; + originInstanceDate: string; + window: InstanceWindow; +}; +export type CreateRuleWithOriginResult = + | { rule: RecurrenceRulesSchemaTypeForSelect } + | { error: 'not_found' | 'conflict' | 'failed' }; /** Detail shape returned by GET endpoints */ diff --git a/api/src/tv-modules/tasks/TasksManager.ts b/api/src/tv-modules/tasks/TasksManager.ts index 4ef7beb..2506d4e 100644 --- a/api/src/tv-modules/tasks/TasksManager.ts +++ b/api/src/tv-modules/tasks/TasksManager.ts @@ -307,7 +307,8 @@ export class TasksManager { return extendedTask[0] ?? null; } - private async cleanTaskFieldsRegardPermissions(task: TasksSchemaTypeForSelect): Promise { + /** Public: other modules returning task rows (e.g. recurrence) gate fields through the same cleaner. */ + async cleanTaskFieldsRegardPermissions(task: TasksSchemaTypeForSelect): Promise { const permissions = await this.user.permissionsFetcher.getCheckerForGoal(task.goalId); (Object.keys(TaskFieldPermissionsForWatching) as TaskFieldPermissionKey[]).forEach((key) => { if (!permissions.hasPermissions(TaskFieldPermissionsForWatching[key])) { diff --git a/api/src/types/auth.types.ts b/api/src/types/auth.types.ts index e1a65ed..79ac09b 100644 --- a/api/src/types/auth.types.ts +++ b/api/src/types/auth.types.ts @@ -150,3 +150,9 @@ export type FetchGoalIdsWithAnyPermissionParams = { organizationId: number; permissionNames: string[]; }; + +export type FetchPermissionsForGoalByUserParams = { + goalId: number; + userId: number; + email: string; +}; diff --git a/taskview-packages/taskview-api/src/api/__tests__/recurrence.test.ts b/taskview-packages/taskview-api/src/api/__tests__/recurrence.test.ts index a3fdb0d..2743e69 100644 --- a/taskview-packages/taskview-api/src/api/__tests__/recurrence.test.ts +++ b/taskview-packages/taskview-api/src/api/__tests__/recurrence.test.ts @@ -9,6 +9,7 @@ import { import axios, { type AxiosInstance } from 'axios' import { initApi, API_URL, DEFAULT_USER, DEFAULT_USER_2, DEFAULT_PASSWORD } from './init-api' import { ymd } from './test-helpers' +import { TvPermissions } from '@/api/permissions' import type { RecurrenceRuleDetails } from '@/api/recurrence.types' /** @@ -23,15 +24,18 @@ import type { RecurrenceRuleDetails } from '@/api/recurrence.types' */ describe('Recurrence', () => { let $api: TvApi + let $apiUser2: TvApi let raw: AxiosInstance let goalId: number const MSK_TIME = 'T10:45:00' const UTC_TIME = '07:45:00' + const USER2_EMAIL = 'user2@test.com' beforeAll(async () => { - const { $tvApi } = await initApi() + const { $tvApi, $tvApiForSecondUser } = await initApi() $api = $tvApi + $apiUser2 = $tvApiForSecondUser // Raw axios with validateStatus:true to assert error statuses directly. const auth = await axios.post(`${API_URL}/module/auth/login`, { @@ -375,14 +379,15 @@ describe('Recurrence', () => { expect(next.openInstance?.recurrenceInstanceDate).toBe(secondEnd) }) - it('a date-only series gets deadline = occurrence date (shows up in Today/Upcoming)', async () => { + it('a date-only series (date-only dtstart) gets deadline = occurrence date (shows up in Today/Upcoming)', async () => { const task = await $api.tasks.createTask({ goalId, description: 'No-end daily', startDate: ymd(3) }) const rule = await $api.recurrence.create({ taskId: task!.id, rrule: 'FREQ=DAILY', - dtstart: `${ymd(3)}T00:00:00`, + dtstart: ymd(3), // date-only: no wall-clock time timezone: 'Europe/Moscow', }) + expect(rule!.hasTime).toBe(false) // the origin window is normalized the same way const origin = await $api.tasks.fetchTaskById(task!.id) @@ -397,6 +402,23 @@ describe('Recurrence', () => { expect(details.openInstance?.endTime).toBeNull() }) + it('an explicit midnight series (T00:00:00 dtstart) is timed, not date-only', async () => { + const task = await $api.tasks.createTask({ goalId, description: 'Midnight daily', startDate: ymd(3) }) + const rule = await $api.recurrence.create({ + taskId: task!.id, + rrule: 'FREQ=DAILY', + dtstart: `${ymd(3)}T00:00:00`, // explicit midnight wall-clock in MSK + timezone: 'Europe/Moscow', + }) + expect(rule!.hasTime).toBe(true) + + await $api.tasks.updateTask({ id: task!.id, complete: true }) + const details = await waitFor(rule.id, (d) => !!d.openInstance && d.openInstance.id !== task!.id) + // 00:00 MSK = 21:00 UTC the previous calendar day — a real time, not null. + expect(details.openInstance?.startTime).toBe('21:00:00') + expect(details.openInstance?.startDate).toBe(ymd(3)) + }) + it('a timed series without duration is due at the occurrence moment', async () => { const task = await $api.tasks.createTask({ goalId, @@ -524,4 +546,148 @@ describe('Recurrence', () => { expect(intact?.rule.state).toBe('active') }) }) + + describe('has_time flag', () => { + it('a rule created from a timed dtstart carries has_time=true', async () => { + const task = await createTask('Timed origin') + const rule = await createRule(task.id, 'FREQ=DAILY') // dtstart includes MSK_TIME + expect(rule.hasTime).toBe(true) + }) + + it('updating dtstart toggles has_time both ways', async () => { + const task = await $api.tasks.createTask({ goalId, description: 'Toggle time', startDate: ymd(3) }) + const rule = await $api.recurrence.create({ + taskId: task!.id, rrule: 'FREQ=DAILY', dtstart: ymd(3), timezone: 'Europe/Moscow', + }) + expect(rule.hasTime).toBe(false) + + const timed = await $api.recurrence.update({ ruleId: rule.id, dtstart: `${ymd(3)}T09:00:00` }) + expect(timed.hasTime).toBe(true) + + const back = await $api.recurrence.update({ ruleId: rule.id, dtstart: ymd(3) }) + expect(back.hasTime).toBe(false) + }) + }) + + describe('rule edit validation', () => { + let ruleId: number + beforeAll(async () => { + const task = await createTask('Edit validation target') + ruleId = (await createRule(task.id, 'FREQ=DAILY')).id + }) + + it('rejects a templateOverrides.statusId not belonging to the goal', async () => { + const res = await raw.patch(`/module/recurrence/${ruleId}`, { templateOverrides: { statusId: 99999999 } }) + expect(res.status).toBe(422) + }) + + it('rejects a templateOverrides.goalListId not belonging to the goal', async () => { + const res = await raw.patch(`/module/recurrence/${ruleId}`, { templateOverrides: { goalListId: 99999999 } }) + expect(res.status).toBe(422) + }) + + it('allows clearing overrides with null', async () => { + const res = await raw.patch(`/module/recurrence/${ruleId}`, { templateOverrides: { statusId: null, goalListId: null } }) + expect(res.status).toBe(200) + }) + + it('rejects an impossible rule on update (no occurrences — UNTIL in the past)', async () => { + const res = await raw.patch(`/module/recurrence/${ruleId}`, { rrule: 'FREQ=DAILY;UNTIL=20000101T000000Z' }) + expect(res.status).toBe(422) + }) + }) + + describe('concurrent creation (race)', () => { + it('two parallel creates on the same task yield exactly one rule', async () => { + const task = await createTask('Race target') + const body = { taskId: task.id, rrule: 'FREQ=DAILY', dtstart: `${ymd(3)}${MSK_TIME}`, timezone: 'Europe/Moscow' } + const [a, b] = await Promise.all([ + raw.post('/module/recurrence', body), + raw.post('/module/recurrence', body), + ]) + // exactly one wins (200), the other is rejected as a conflict (409) — + // guaranteed by the FOR UPDATE transaction + partial unique index. + expect([a.status, b.status].sort()).toEqual([200, 409]) + }) + }) + + describe('permission gating', () => { + let gateGoalId: number + let user2Raw: AxiosInstance + let collabId: number + let roleSeq = 0 + + beforeAll(async () => { + const goal = await $api.goals.createGoal({ name: `Gating project-${Date.now()}` }) + gateGoalId = goal!.id! + await $api.collaboration.inviteUserToGoal({ goalId: gateGoalId, email: USER2_EMAIL }) + const users = await $api.collaboration.fetchUsersForGoal(gateGoalId) + collabId = users!.find((u) => u.email === USER2_EMAIL)!.id + + const auth = await axios.post(`${API_URL}/module/auth/login`, { login: DEFAULT_USER_2, password: DEFAULT_PASSWORD }) + user2Raw = axios.create({ + baseURL: API_URL, + headers: { Authorization: `Bearer ${auth.data.access}` }, + validateStatus: () => true, + }) + }) + + afterAll(async () => { + await $api.goals.deleteGoal(gateGoalId).catch(() => {}) + }) + + /** Replace user2's role set with a single fresh role holding exactly `permissionNames`. */ + async function grantUser2(permissionNames: string[]) { + const role = await $api.collaboration.createRoleForGoal({ goalId: gateGoalId, roleName: `r-${Date.now()}-${roleSeq++}` }) + const allPerms = await $api.collaboration.fetchAllPermissions() + for (const name of permissionNames) { + const perm = allPerms!.find((p) => p.name === name)! + await $api.collaboration.toggleRolePermission({ roleId: role!.id, permissionId: perm.id }) + } + await $api.collaboration.toggleUserRoles({ userId: collabId, goalId: gateGoalId, roles: [role!.id] }) + } + + async function createGateRule(description: string, note: string) { + const task = await $api.tasks.createTask({ goalId: gateGoalId, description, note, startDate: ymd(3), startTime: UTC_TIME }) + const rule = await $api.recurrence.create({ taskId: task!.id, rrule: 'FREQ=DAILY', dtstart: `${ymd(3)}${MSK_TIME}`, timezone: 'Europe/Moscow' }) + return { taskId: task!.id, rule } + } + + it('owner sees templateNote; a member without note permission gets it stripped', async () => { + const { rule } = await createGateRule('Secret standup', 'confidential note') + + const ownerView = await $api.recurrence.getById(rule.id) + expect(ownerView?.rule.templateNote).toBe('confidential note') + + // content-watch + deadline-edit, but NOT note-watch + await grantUser2([TvPermissions.COMPONENT_CAN_WATCH_CONTENT, TvPermissions.TASK_CAN_EDIT_DEADLINE]) + const memberView = await $apiUser2.recurrence.getById(rule.id) + expect(memberView?.rule.templateNote).toBeNull() + if (memberView?.openInstance) expect(memberView.openInstance.note).toBeNull() + }) + + it('the note is stripped from mutation responses too (pause)', async () => { + const { rule } = await createGateRule('Secret pausable', 'hidden note') + await grantUser2([TvPermissions.COMPONENT_CAN_WATCH_CONTENT, TvPermissions.TASK_CAN_EDIT_DEADLINE]) + const paused = await $apiUser2.recurrence.pause(rule.id) + expect(paused.templateNote).toBeNull() + }) + + it('skip requires task-delete permission on top of deadline-edit', async () => { + const { rule } = await createGateRule('Skippable gated', 'note') + + // deadline-edit only: pause is allowed, but skip (which deletes the instance) is forbidden + await grantUser2([TvPermissions.COMPONENT_CAN_WATCH_CONTENT, TvPermissions.TASK_CAN_EDIT_DEADLINE]) + const skipForbidden = await user2Raw.post(`/module/recurrence/${rule.id}/skip`, {}) + expect(skipForbidden.status).toBe(403) + const pauseOk = await user2Raw.post(`/module/recurrence/${rule.id}/pause`, {}) + expect(pauseOk.status).toBe(200) + await user2Raw.post(`/module/recurrence/${rule.id}/resume`, {}) // restore active state + + // grant delete: skip now allowed + await grantUser2([TvPermissions.COMPONENT_CAN_WATCH_CONTENT, TvPermissions.TASK_CAN_EDIT_DEADLINE, TvPermissions.TASK_CAN_DELETE]) + const skipOk = await user2Raw.post(`/module/recurrence/${rule.id}/skip`, {}) + expect(skipOk.status).toBe(200) + }) + }) }) diff --git a/taskview-packages/taskview-api/src/api/recurrence.types.ts b/taskview-packages/taskview-api/src/api/recurrence.types.ts index 10f8cd4..e43614b 100644 --- a/taskview-packages/taskview-api/src/api/recurrence.types.ts +++ b/taskview-packages/taskview-api/src/api/recurrence.types.ts @@ -16,6 +16,8 @@ export type RecurrenceRule = { rrule: string; /** Floating wall-clock 'YYYY-MM-DDTHH:mm:ss' anchor of the series. */ dtstart: string; + /** Whether the series is anchored to a wall-clock time (incl. 00:00) or is date-only. */ + hasTime: boolean; /** IANA timezone name, e.g. 'Europe/Moscow'. */ timezone: string; state: RecurrenceState; @@ -36,6 +38,7 @@ export type RecurrenceRuleDetails = { export type RecurrenceCreateArgs = { taskId: number; rrule: string; + /** 'YYYY-MM-DD' for a date-only series, 'YYYY-MM-DDTHH:mm:ss' for a timed one (incl. 00:00). */ dtstart: string; timezone: string; notifyOnOccurrence?: boolean; diff --git a/taskview-packages/taskview-db-schemas/src/schemas/recurrence-rules.schema.ts b/taskview-packages/taskview-db-schemas/src/schemas/recurrence-rules.schema.ts index da22f6a..27382a8 100644 --- a/taskview-packages/taskview-db-schemas/src/schemas/recurrence-rules.schema.ts +++ b/taskview-packages/taskview-db-schemas/src/schemas/recurrence-rules.schema.ts @@ -17,6 +17,7 @@ export const RecurrenceRulesSchema = pgSchema('tasks').table('recurrence_rules', templateDurationMinutes: integer('template_duration_minutes'), rrule: text().notNull(), dtstart: timestamp().notNull(), + hasTime: boolean('has_time').notNull().default(false), timezone: varchar({ length: 50 }).notNull(), state: varchar({ length: 20 }).$type().notNull().default('active'), lastInstanceDate: date('last_instance_date').notNull(), diff --git a/web/src/components/features/tasks/parts/TaskRecurrence.vue b/web/src/components/features/tasks/parts/TaskRecurrence.vue index a7b9cd6..0abd24f 100644 --- a/web/src/components/features/tasks/parts/TaskRecurrence.vue +++ b/web/src/components/features/tasks/parts/TaskRecurrence.vue @@ -76,6 +76,7 @@ const summary = computed(() => { rrule: rule.rrule, dtstart: new Date(`${rule.dtstart.replace(' ', 'T')}Z`), notifyOnOccurrence: rule.notifyOnOccurrence, + hasTime: rule.hasTime, }) const unit = t(`recurrence.units.${form.frequency}`) diff --git a/web/src/components/features/tasks/parts/TaskRecurrenceDialog.vue b/web/src/components/features/tasks/parts/TaskRecurrenceDialog.vue index 1159b35..29339cb 100644 --- a/web/src/components/features/tasks/parts/TaskRecurrenceDialog.vue +++ b/web/src/components/features/tasks/parts/TaskRecurrenceDialog.vue @@ -28,6 +28,7 @@