From 3d0e7a6c15b06e5bf06b1ff8ae7d1d997a71c75e Mon Sep 17 00:00:00 2001 From: fullex <0xfullex@gmail.com> Date: Mon, 29 Dec 2025 13:42:05 +0800 Subject: [PATCH] feat(api): enhance message deletion functionality with activeNodeId management - Introduced `ActiveNodeStrategy` type to define strategies for updating `activeNodeId` when a message is deleted. - Updated `DeleteMessageResponse` to include `newActiveNodeId` for tracking changes to the active node after deletion. - Modified the `DELETE` endpoint to accept `activeNodeStrategy` as a query parameter, allowing for flexible handling of active node updates. - Enhanced the `delete` method in `MessageService` to implement the new strategies, ensuring consistent behavior during message deletions. --- packages/shared/data/api/schemas/messages.ts | 20 +++- src/main/data/api/handlers/messages.ts | 12 +- src/main/data/db/schemas/topic.ts | 3 - src/main/data/services/MessageService.ts | 116 +++++++++++++++---- 4 files changed, 123 insertions(+), 28 deletions(-) diff --git a/packages/shared/data/api/schemas/messages.ts b/packages/shared/data/api/schemas/messages.ts index f659491f93..b952156da5 100644 --- a/packages/shared/data/api/schemas/messages.ts +++ b/packages/shared/data/api/schemas/messages.ts @@ -64,6 +64,11 @@ export interface UpdateMessageDto { status?: MessageStatus } +/** + * Strategy for updating activeNodeId when the active message is deleted + */ +export type ActiveNodeStrategy = 'parent' | 'clear' + /** * Response for delete operation */ @@ -72,6 +77,8 @@ export interface DeleteMessageResponse { deletedIds: string[] /** IDs of reparented children (only when cascade=false) */ reparentedIds?: string[] + /** New activeNodeId for the topic (only if activeNodeId was affected by deletion) */ + newActiveNodeId?: string | null } // ============================================================================ @@ -168,10 +175,19 @@ export interface MessageSchemas { body: UpdateMessageDto response: Message } - /** Delete a message (cascade=true deletes descendants, cascade=false reparents children) */ + /** + * Delete a message + * - cascade=true: deletes message and all descendants + * - cascade=false: reparents children to grandparent + * - activeNodeStrategy='parent' (default): sets activeNodeId to parent if affected + * - activeNodeStrategy='clear': sets activeNodeId to null if affected + */ DELETE: { params: { id: string } - query?: { cascade?: boolean } + query?: { + cascade?: boolean + activeNodeStrategy?: ActiveNodeStrategy + } response: DeleteMessageResponse } } diff --git a/src/main/data/api/handlers/messages.ts b/src/main/data/api/handlers/messages.ts index 0dde772331..2f7faaded3 100644 --- a/src/main/data/api/handlers/messages.ts +++ b/src/main/data/api/handlers/messages.ts @@ -9,7 +9,12 @@ import { messageService } from '@data/services/MessageService' import type { ApiHandler, ApiMethods } from '@shared/data/api/apiTypes' -import type { BranchMessagesQueryParams, MessageSchemas, TreeQueryParams } from '@shared/data/api/schemas/messages' +import type { + ActiveNodeStrategy, + BranchMessagesQueryParams, + MessageSchemas, + TreeQueryParams +} from '@shared/data/api/schemas/messages' /** * Handler type for a specific message endpoint @@ -61,9 +66,10 @@ export const messageHandlers: { }, DELETE: async ({ params, query }) => { - const q = (query || {}) as { cascade?: boolean } + const q = (query || {}) as { cascade?: boolean; activeNodeStrategy?: ActiveNodeStrategy } const cascade = q.cascade ?? false - return await messageService.delete(params.id, cascade) + const activeNodeStrategy = q.activeNodeStrategy ?? 'parent' + return await messageService.delete(params.id, cascade, activeNodeStrategy) } } } diff --git a/src/main/data/db/schemas/topic.ts b/src/main/data/db/schemas/topic.ts index 3f19ae7a7f..68078d8f86 100644 --- a/src/main/data/db/schemas/topic.ts +++ b/src/main/data/db/schemas/topic.ts @@ -3,7 +3,6 @@ import { index, integer, sqliteTable, text } from 'drizzle-orm/sqlite-core' import { createUpdateDeleteTimestamps, uuidPrimaryKey } from './columnHelpers' import { groupTable } from './group' -// import { messageTable } from './message' /** * Topic table - stores conversation topics/threads @@ -25,9 +24,7 @@ export const topicTable = sqliteTable( // Topic-specific prompt override prompt: text(), // Active node ID in the message tree - // SET NULL: reset to null when the referenced message is deleted activeNodeId: text(), - // .references(() => messageTable.id, { onDelete: 'set null' }), // FK to group table for organization // SET NULL: preserve topic when group is deleted diff --git a/src/main/data/services/MessageService.ts b/src/main/data/services/MessageService.ts index 4093aad802..9956983b6a 100644 --- a/src/main/data/services/MessageService.ts +++ b/src/main/data/services/MessageService.ts @@ -13,7 +13,12 @@ import { messageTable } from '@data/db/schemas/message' import { topicTable } from '@data/db/schemas/topic' import { loggerService } from '@logger' import { DataApiErrorFactory } from '@shared/data/api' -import type { CreateMessageDto, UpdateMessageDto } from '@shared/data/api/schemas/messages' +import type { + ActiveNodeStrategy, + CreateMessageDto, + DeleteMessageResponse, + UpdateMessageDto +} from '@shared/data/api/schemas/messages' import type { BranchMessage, BranchMessagesResponse, @@ -431,13 +436,42 @@ export class MessageService { /** * Delete a message (hard delete) + * + * Supports two modes: + * - cascade=true: Delete the message and all its descendants + * - cascade=false: Delete only this message, reparent children to grandparent + * + * When the deleted message(s) include the topic's activeNodeId, it will be + * automatically updated based on activeNodeStrategy: + * - 'parent' (default): Sets activeNodeId to the deleted message's parent + * - 'clear': Sets activeNodeId to null + * + * All operations are performed within a transaction for consistency. + * + * @param id - Message ID to delete + * @param cascade - If true, delete descendants; if false, reparent children (default: false) + * @param activeNodeStrategy - Strategy for updating activeNodeId if affected (default: 'parent') + * @returns Deletion result including deletedIds, reparentedIds, and newActiveNodeId + * @throws NOT_FOUND if message doesn't exist + * @throws INVALID_OPERATION if deleting root without cascade=true */ - async delete(id: string, cascade: boolean = false): Promise<{ deletedIds: string[]; reparentedIds?: string[] }> { + async delete( + id: string, + cascade: boolean = false, + activeNodeStrategy: ActiveNodeStrategy = 'parent' + ): Promise { const db = dbService.getDb() // Get the message const message = await this.getById(id) + // Get topic to check activeNodeId + const [topic] = await db.select().from(topicTable).where(eq(topicTable.id, message.topicId)).limit(1) + + if (!topic) { + throw DataApiErrorFactory.notFound('Topic', message.topicId) + } + // Check if it's a root message const isRoot = message.parentId === null @@ -445,34 +479,76 @@ export class MessageService { throw DataApiErrorFactory.invalidOperation('delete root message', 'cascade=true required') } + // Get all descendant IDs before transaction (for cascade delete) + let descendantIds: string[] = [] if (cascade) { - // Get all descendants - const descendantIds = await this.getDescendantIds(id) - const allIds = [id, ...descendantIds] + descendantIds = await this.getDescendantIds(id) + } - // Hard delete all - await db.delete(messageTable).where(inArray(messageTable.id, allIds)) + // Use transaction for atomic delete + activeNodeId update + return await db.transaction(async (tx) => { + let deletedIds: string[] + let reparentedIds: string[] | undefined + let newActiveNodeId: string | null | undefined - logger.info('Cascade deleted messages', { rootId: id, count: allIds.length }) + if (cascade) { + deletedIds = [id, ...descendantIds] - return { deletedIds: allIds } - } else { - // Reparent children to this message's parent - const children = await db.select({ id: messageTable.id }).from(messageTable).where(eq(messageTable.parentId, id)) + // Check if activeNodeId is affected + if (topic.activeNodeId && deletedIds.includes(topic.activeNodeId)) { + newActiveNodeId = activeNodeStrategy === 'clear' ? null : message.parentId + } - const childIds = children.map((c) => c.id) + // Hard delete all + await tx.delete(messageTable).where(inArray(messageTable.id, deletedIds)) - if (childIds.length > 0) { - await db.update(messageTable).set({ parentId: message.parentId }).where(inArray(messageTable.id, childIds)) + logger.info('Cascade deleted messages', { rootId: id, count: deletedIds.length }) + } else { + // Reparent children to this message's parent + const children = await tx + .select({ id: messageTable.id }) + .from(messageTable) + .where(eq(messageTable.parentId, id)) + + reparentedIds = children.map((c) => c.id) + + if (reparentedIds.length > 0) { + await tx + .update(messageTable) + .set({ parentId: message.parentId }) + .where(inArray(messageTable.id, reparentedIds)) + } + + deletedIds = [id] + + // Check if activeNodeId is affected + if (topic.activeNodeId === id) { + newActiveNodeId = activeNodeStrategy === 'clear' ? null : message.parentId + } + + // Hard delete this message + await tx.delete(messageTable).where(eq(messageTable.id, id)) + + logger.info('Deleted message with reparenting', { id, reparentedCount: reparentedIds.length }) } - // Hard delete this message - await db.delete(messageTable).where(eq(messageTable.id, id)) + // Update topic.activeNodeId if needed + if (newActiveNodeId !== undefined) { + await tx.update(topicTable).set({ activeNodeId: newActiveNodeId }).where(eq(topicTable.id, message.topicId)) - logger.info('Deleted message with reparenting', { id, reparentedCount: childIds.length }) + logger.info('Updated topic activeNodeId after message deletion', { + topicId: message.topicId, + oldActiveNodeId: topic.activeNodeId, + newActiveNodeId + }) + } - return { deletedIds: [id], reparentedIds: childIds } - } + return { + deletedIds, + reparentedIds: reparentedIds?.length ? reparentedIds : undefined, + newActiveNodeId + } + }) } /**