From 2ddef80222cdc0cd35201e89f1dcade39b58a3b4 Mon Sep 17 00:00:00 2001 From: WSHAPER <42714629+WSHAPER@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:03:00 +0200 Subject: [PATCH 1/3] feat(chat): add mass deletion of conversations Add a bulk delete endpoint that takes a list of session IDs and deletes the sessions and their messages in a single transaction, restricted to sessions owned by the current user. Non-existent or foreign session IDs are ignored, like the single deletion endpoint ignores unknown IDs. In the conversation list, add a deletion mode with a "Delete multiple conversations" entry button matching the "New conversation" button style, per-conversation selection, a select-all/deselect-all toggle and a confirmation dialog. Single deletion reuses the bulk endpoint and dialog. Destructive buttons use the error variant like the confirmation dialogs of the server apps, and the deletion mode is left with Escape. The dialog is bound with v-model on a writable computed and remounted via a key on every open: closing it with Escape leaves a 300ms delayed internal teardown in NcDialog which, with a one-way open binding, kept the open prop stuck true when the dialog was quickly reopened, making the delete button unresponsive until a page reload. Fixes #639 Assisted-by: opencode:glm-5.3 Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com> --- appinfo/routes.php | 2 + lib/Controller/ChattyLLMController.php | 30 ++ lib/Db/ChattyLLM/MessageMapper.php | 14 + lib/Db/ChattyLLM/SessionMapper.php | 31 ++ lib/Service/ChatService.php | 43 +++ openapi.json | 266 ++++++++++++++++++ .../ChattyLLM/ChattyLLMInputForm.vue | 207 ++++++++++++-- tests/unit/Service/ChatServiceTest.php | 132 +++++++++ 8 files changed, 695 insertions(+), 30 deletions(-) create mode 100644 tests/unit/Service/ChatServiceTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index e06389476..3dc4df1d0 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -45,10 +45,12 @@ ['name' => 'chattyLLM#newSession', 'url' => '/chat/sessions', 'verb' => 'POST', 'postfix' => 'restful'], ['name' => 'chattyLLM#updateChatSession', 'url' => '/chat/sessions/{sessionId}', 'verb' => 'PUT', 'postfix' => 'restful'], ['name' => 'chattyLLM#deleteSession', 'url' => '/chat/sessions/{sessionId}', 'verb' => 'DELETE', 'postfix' => 'restful'], + ['name' => 'chattyLLM#deleteSessions', 'url' => '/chat/sessions', 'verb' => 'DELETE', 'postfix' => 'restful'], ['name' => 'chattyLLM#newSession', 'url' => '/chat/new_session', 'verb' => 'PUT'], ['name' => 'chattyLLM#updateSessionTitle', 'url' => '/chat/update_session', 'verb' => 'PATCH'], ['name' => 'chattyLLM#deleteSession', 'url' => '/chat/delete_session', 'verb' => 'DELETE'], + ['name' => 'chattyLLM#deleteSessions', 'url' => '/chat/delete_sessions', 'verb' => 'DELETE'], ['name' => 'chattyLLM#getSessions', 'url' => '/chat/sessions', 'verb' => 'GET'], ['name' => 'chattyLLM#newMessage', 'url' => '/chat/new_message', 'verb' => 'PUT'], ['name' => 'chattyLLM#deleteMessage', 'url' => '/chat/delete_message', 'verb' => 'DELETE'], diff --git a/lib/Controller/ChattyLLMController.php b/lib/Controller/ChattyLLMController.php index 0772d325c..cc750a605 100644 --- a/lib/Controller/ChattyLLMController.php +++ b/lib/Controller/ChattyLLMController.php @@ -306,6 +306,36 @@ public function deleteSession(int $sessionId): JSONResponse { } } + /** + * Delete chat sessions + * + * Delete several chat sessions by ID + * + * @param list $sessionIds The session IDs + * @return JSONResponse|JSONResponse + * + * 200: The sessions have been deleted successfully + * 400: The list of session IDs is empty or invalid + * 401: Not logged in + */ + #[NoAdminRequired] + #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT, tags: ['chat_api'])] + public function deleteSessions(array $sessionIds): JSONResponse { + if ($sessionIds === []) { + return new JSONResponse(['error' => $this->l10n->t('Invalid session IDs')], Http::STATUS_BAD_REQUEST); + } + try { + // we don't delete the tasks + $this->chatService->deleteSessions($this->userId, $sessionIds); + return new JSONResponse(); + } catch (InternalException $e) { + $this->logger->warning('Failed to delete the chat sessions', ['exception' => $e]); + return new JSONResponse(['error' => $this->l10n->t('Failed to delete the chat sessions')], Http::STATUS_INTERNAL_SERVER_ERROR); + } catch (\OCA\Assistant\Service\UnauthorizedException $e) { + return new JSONResponse(['error' => $this->l10n->t('User not logged in')], Http::STATUS_UNAUTHORIZED); + } + } + /** * Get chat sessions * diff --git a/lib/Db/ChattyLLM/MessageMapper.php b/lib/Db/ChattyLLM/MessageMapper.php index 86204fc27..e635ca7bd 100644 --- a/lib/Db/ChattyLLM/MessageMapper.php +++ b/lib/Db/ChattyLLM/MessageMapper.php @@ -177,6 +177,20 @@ public function deleteMessagesBySession(int $sessionId): void { $qb->executeStatement(); } + /** + * @param list $sessionIds + * @throws \OCP\DB\Exception + * @throws \RuntimeException + * @return void + */ + public function deleteMessagesBySessions(array $sessionIds): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where($qb->expr()->in('session_id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); + + $qb->executeStatement(); + } + /** * @param int $sessionId * @param integer $messageId diff --git a/lib/Db/ChattyLLM/SessionMapper.php b/lib/Db/ChattyLLM/SessionMapper.php index c8359173a..4a67e9747 100644 --- a/lib/Db/ChattyLLM/SessionMapper.php +++ b/lib/Db/ChattyLLM/SessionMapper.php @@ -218,6 +218,37 @@ public function deleteSession(string $userId, int $sessionId) { $qb->executeStatement(); } + /** + * @param string $userId + * @param list $sessionIds + * @return list + * @throws \OCP\DB\Exception + */ + public function getUserSessionsByIds(string $userId, array $sessionIds): array { + $qb = $this->db->getQueryBuilder(); + $qb->select(Session::$columns) + ->from($this->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createPositionalParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); + + return $this->findEntities($qb); + } + + /** + * @param string $userId + * @param list $sessionIds + * @throws \OCP\DB\Exception + * @throws \RuntimeException + */ + public function deleteSessionsByUser(string $userId, array $sessionIds) { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createPositionalParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); + + $qb->executeStatement(); + } + /** * @throws \OCP\DB\Exception */ diff --git a/lib/Service/ChatService.php b/lib/Service/ChatService.php index 18adfc6c3..1f27de531 100644 --- a/lib/Service/ChatService.php +++ b/lib/Service/ChatService.php @@ -18,6 +18,7 @@ use OCP\DB\Exception; use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; +use OCP\IDBConnection; use OCP\IL10N; use OCP\IUserManager; use OCP\TaskProcessing\Exception\PreConditionNotMetException; @@ -36,6 +37,7 @@ public function __construct( private readonly IL10N $l10n, private readonly SessionMapper $sessionMapper, private readonly MessageMapper $messageMapper, + private readonly IDBConnection $db, private readonly SessionSummaryService $sessionSummaryService, private readonly IManager $taskProcessingManager, private readonly LoggerInterface $logger, @@ -152,6 +154,47 @@ public function deleteSession(?string $userId, int $sessionId): void { } } + /** + * @param string|null $userId + * @param list $sessionIds + * @throws InternalException + * @throws UnauthorizedException + */ + public function deleteSessions(?string $userId, array $sessionIds): void { + if ($userId === null) { + throw new UnauthorizedException($this->l10n->t('Unauthorized')); + } + + $sessionIds = array_values(array_unique(array_map(static function ($sessionId) { + return (int)$sessionId; + }, $sessionIds))); + + if ($sessionIds === []) { + return; + } + + try { + $ownedSessions = $this->sessionMapper->getUserSessionsByIds($userId, $sessionIds); + $ownedSessionIds = array_map(static function (Session $session) { + return $session->getId(); + }, $ownedSessions); + + if ($ownedSessionIds === []) { + return; + } + + $this->db->beginTransaction(); + $this->sessionMapper->deleteSessionsByUser($userId, $ownedSessionIds); + $this->messageMapper->deleteMessagesBySessions($ownedSessionIds); + $this->db->commit(); + } catch (Exception|\RuntimeException $e) { + if ($this->db->inTransaction()) { + $this->db->rollBack(); + } + throw new InternalException(previous: $e); + } + } + /** * @throws InternalException */ diff --git a/openapi.json b/openapi.json index ca07990e5..ec9569407 100644 --- a/openapi.json +++ b/openapi.json @@ -3818,6 +3818,138 @@ } } }, + "delete": { + "operationId": "chattyllm-delete-sessions-restful", + "summary": "Delete chat sessions", + "description": "Delete several chat sessions by ID", + "tags": [ + "chat_api" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "sessionIds[]", + "in": "query", + "description": "The session IDs", + "required": true, + "schema": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + } + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "The sessions have been deleted successfully", + "content": { + "application/json": { + "schema": { + "type": "object" + } + } + } + }, + "500": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + }, + "401": { + "description": "Not logged in", + "content": { + "application/json": { + "schema": { + "anyOf": [ + { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + }, + { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + ] + } + } + } + }, + "400": { + "description": "The list of session IDs is empty or invalid", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + }, "get": { "operationId": "chattyllm-get-sessions", "summary": "Get chat sessions", @@ -4598,6 +4730,140 @@ } } }, + "/ocs/v2.php/apps/assistant/chat/delete_sessions": { + "delete": { + "operationId": "chattyllm-delete-sessions", + "summary": "Delete chat sessions", + "description": "Delete several chat sessions by ID", + "tags": [ + "chat_api" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "sessionIds[]", + "in": "query", + "description": "The session IDs", + "required": true, + "schema": { + "type": "array", + "items": { + "type": "integer", + "format": "int64" + } + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "The sessions have been deleted successfully", + "content": { + "application/json": { + "schema": { + "type": "object" + } + } + } + }, + "500": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + }, + "401": { + "description": "Not logged in", + "content": { + "application/json": { + "schema": { + "anyOf": [ + { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + }, + { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + ] + } + } + } + }, + "400": { + "description": "The list of session IDs is empty or invalid", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/assistant/chat/new_message": { "put": { "operationId": "chattyllm-new-message", diff --git a/src/components/ChattyLLM/ChattyLLMInputForm.vue b/src/components/ChattyLLM/ChattyLLMInputForm.vue index 51da553de..558b4bffa 100644 --- a/src/components/ChattyLLM/ChattyLLMInputForm.vue +++ b/src/components/ChattyLLM/ChattyLLMInputForm.vue @@ -6,7 +6,7 @@
- @@ -14,6 +14,40 @@ + + + {{ t('assistant', 'Delete multiple conversations') }} + +
+
+ + {{ selectAllLabel }} + + + {{ t('assistant', 'Cancel') }} + +
+ + + {{ deleteSelectedLabel }} + +
{{ isAssignment ? t('assistant', 'Loading scheduled tasks…') : t('assistant', 'Loading conversations…') }} @@ -25,21 +59,28 @@ v-for="session in sessions" v-else :key="'conversation' + session.id" - :active="session.id === active?.id" + :active="deletionMode ? selectedSessionIds.includes(session.id) : session.id === active?.id" :name="getSessionTitle(session)" :title="getSessionTitle(session)" :aria-description="getSessionTitle(session)" :editable="false" - :inline-actions="1" - @click="onSessionSelect(session)"> + :inline-actions="deletionMode ? 0 : 1" + @click="deletionMode ? toggleSessionSelection(session.id) : onSessionSelect(session)"> @@ -198,22 +239,21 @@ @submit="handleSubmit" @submit-audio="handleSubmitAudio" /> - + @closing="sessionIdsToDelete = null"> @@ -226,6 +266,7 @@ import AutoFixIcon from 'vue-material-design-icons/AutoFix.vue' import PencilOutlineIcon from 'vue-material-design-icons/PencilOutline.vue' import PlusIcon from 'vue-material-design-icons/Plus.vue' import TrashCanOutlineIcon from 'vue-material-design-icons/TrashCanOutline.vue' +import DeleteSweepOutlineIcon from 'vue-material-design-icons/DeleteSweepOutline.vue' import MemoryIcon from 'vue-material-design-icons/Memory.vue' import TimerOutlineIcon from 'vue-material-design-icons/TimerOutline.vue' @@ -282,6 +323,7 @@ export default { AgencyConfirmation, AutoFixIcon, TrashCanOutlineIcon, + DeleteSweepOutlineIcon, PencilOutlineIcon, PlusIcon, MemoryIcon, @@ -324,7 +366,10 @@ export default { return { // { id: number, title: string, user_id: string, timestamp: number } active: null, - sessionIdToDelete: null, + sessionIdsToDelete: null, + deletionDialogKey: 0, + deletionMode: false, + selectedSessionIds: [], chatContent: '', sessions: null, assignmentDetails: null, @@ -349,7 +394,7 @@ export default { newHumanMessage: false, newSession: false, messageDelete: false, - sessionDelete: false, + sessionsDelete: false, taskPosition: null, }, msgCursor: 0, @@ -413,13 +458,37 @@ export default { }, computed: { + deletionDialogOpen: { + get() { + return this.sessionIdsToDelete !== null + }, + set(value) { + if (!value) { + this.sessionIdsToDelete = null + } + }, + }, deletionConfirmationMessage() { - if (this.sessions === null || this.sessionIdToDelete === null) { + if (this.sessions === null || this.sessionIdsToDelete === null || this.sessionIdsToDelete.length === 0) { return '' } - const session = this.sessions.find(s => s.id === this.sessionIdToDelete) - const sessionTitle = this.getSessionTitle(session)?.trim() - return t('assistant', 'Are you sure you want to delete "{sessionTitle}"?', { sessionTitle }) + if (this.sessionIdsToDelete.length === 1) { + const session = this.sessions.find(s => s.id === this.sessionIdsToDelete[0]) + const sessionTitle = this.getSessionTitle(session)?.trim() + return t('assistant', 'Are you sure you want to delete "{sessionTitle}"?', { sessionTitle }) + } + return n('assistant', 'Are you sure you want to delete %n conversation?', 'Are you sure you want to delete %n conversations?', this.sessionIdsToDelete.length) + }, + deleteSelectedLabel() { + return n('assistant', 'Delete %n conversation', 'Delete %n conversations', this.selectedSessionIds.length) + }, + selectAllLabel() { + return this.allSessionsSelected ? t('assistant', 'Deselect all') : t('assistant', 'Select all') + }, + allSessionsSelected() { + return this.sessions !== null + && this.sessions.length > 0 + && this.selectedSessionIds.length === this.sessions.length }, rrule() { const raw = this.assignmentDetails?.recurrence ?? '' @@ -484,11 +553,19 @@ export default { this.active = null clearTimeout(this.pollCheckSessionTimeout) }, + deletionMode(enabled) { + if (enabled) { + window.addEventListener('keydown', this.onKeydownEscape, true) + } else { + window.removeEventListener('keydown', this.onKeydownEscape, true) + } + }, }, beforeUnmount() { this.pollMessageGenerationCancel?.() cancelTaskPositionPolling() + window.removeEventListener('keydown', this.onKeydownEscape, true) if (this.pollMessageGenerationTimerId) { clearInterval(this.pollMessageGenerationTimerId) } @@ -781,22 +858,67 @@ export default { } }, - async deleteSession(sessionId) { + enterDeletionMode(preselectSessionId = null) { + this.selectedSessionIds = preselectSessionId !== null ? [preselectSessionId] : [] + this.deletionMode = true + }, + + exitDeletionMode() { + this.deletionMode = false + this.selectedSessionIds = [] + }, + + openDeletionDialog(sessionIds) { + this.deletionDialogKey++ + this.sessionIdsToDelete = sessionIds + }, + + onKeydownEscape(event) { + if (event.key !== 'Escape' || this.sessionIdsToDelete !== null) { + return + } + this.exitDeletionMode() + }, + + toggleSessionSelection(sessionId) { + if (this.selectedSessionIds.includes(sessionId)) { + this.selectedSessionIds = this.selectedSessionIds.filter(id => id !== sessionId) + } else { + this.selectedSessionIds.push(sessionId) + } + }, + + toggleSelectAll() { + if (this.allSessionsSelected) { + this.selectedSessionIds = [] + } else { + this.selectedSessionIds = this.sessions.map(session => session.id) + } + }, + + async deleteSessions(sessionIds) { + if (!Array.isArray(sessionIds) || sessionIds.length === 0) { + this.sessionIdsToDelete = null + return + } try { - this.loading.sessionDelete = true - await axios.delete(getChatURL('/delete_session'), { - params: { sessionId }, + this.loading.sessionsDelete = true + await axios.delete(getChatURL('/delete_sessions'), { + params: { sessionIds }, }) - this.sessions = this.sessions.filter((session) => session.id !== sessionId) - if (this.active?.id === sessionId) { + this.sessions = this.sessions.filter((session) => !sessionIds.includes(session.id)) + if (this.active !== null && sessionIds.includes(this.active.id)) { this.active = null } + if (this.deletionMode) { + this.exitDeletionMode() + } } catch (error) { - console.error('deleteSession error:', error) - showError(error?.response?.data?.error ?? t('assistant', 'Error deleting conversation')) + console.error('deleteSessions error:', error) + showError(error?.response?.data?.error ?? t('assistant', 'Error deleting conversations')) } finally { - this.loading.sessionDelete = false - this.sessionIdToDelete = null + this.loading.sessionsDelete = false + this.sessionIdsToDelete = null } }, @@ -1318,6 +1440,31 @@ export default { height: 100%; } + .deletion-mode-button { + width: 100%; + } + + .deletion-mode-bar { + display: flex; + flex-direction: column; + gap: var(--default-grid-baseline); + padding: 0; + + &__first-line { + display: flex; + align-items: center; + gap: var(--default-grid-baseline); + + :deep(button) { + flex: 1 1 0; + } + } + + &__delete { + width: 100%; + } + } + :deep(.app-navigation) { --app-navigation-max-width: calc(100vw - (var(--app-navigation-padding) + 24px + var(--default-grid-baseline))); background-color: var(--color-primary-element-light); diff --git a/tests/unit/Service/ChatServiceTest.php b/tests/unit/Service/ChatServiceTest.php new file mode 100644 index 000000000..8aeadeaa9 --- /dev/null +++ b/tests/unit/Service/ChatServiceTest.php @@ -0,0 +1,132 @@ +userExists($uid)) { + $userManager->createUser($uid, $uid . '_password123'); + } + } + + $this->service = Server::get(ChatService::class); + $this->sessionMapper = Server::get(SessionMapper::class); + $this->messageMapper = Server::get(MessageMapper::class); + } + + protected function tearDown(): void { + $userManager = Server::get(IUserManager::class); + foreach ([self::TEST_USER, self::OTHER_USER] as $uid) { + if ($userManager->userExists($uid)) { + $this->service->deleteAllUserChatData($uid); + $userManager->get($uid)->delete(); + } + } + parent::tearDown(); + } + + private function createSession(string $userId): Session { + $session = new Session(); + $session->setUserId($userId); + $session->setTimestamp(time()); + return $this->sessionMapper->insert($session); + } + + private function createMessage(int $sessionId): Message { + $message = new Message(); + $message->setSessionId($sessionId); + $message->setRole(Message::ROLE_HUMAN); + $message->setAttachments('[]'); + $message->setContent('test message'); + $message->setSources('[]'); + $message->setTimestamp(time()); + return $this->messageMapper->insert($message); + } + + public function testDeleteSessions(): void { + $session1 = $this->createSession(self::TEST_USER); + $session2 = $this->createSession(self::TEST_USER); + $session3 = $this->createSession(self::TEST_USER); + $this->createMessage($session1->getId()); + $this->createMessage($session2->getId()); + $this->createMessage($session3->getId()); + + $this->service->deleteSessions(self::TEST_USER, [$session1->getId(), $session2->getId(), $session3->getId()]); + + $this->assertCount(0, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $this->assertCount(0, $this->messageMapper->getMessages($session1->getId(), 0, 100)); + $this->assertCount(0, $this->messageMapper->getMessages($session2->getId(), 0, 100)); + $this->assertCount(0, $this->messageMapper->getMessages($session3->getId(), 0, 100)); + } + + public function testDeleteSessionsLeavesOtherUsersDataAlone(): void { + $ownSession = $this->createSession(self::TEST_USER); + $otherSession = $this->createSession(self::OTHER_USER); + $this->createMessage($ownSession->getId()); + $otherMessage = $this->createMessage($otherSession->getId()); + + // the other user's session ID is passed but must not be deleted, + // unknown IDs are ignored, duplicates are harmless + $this->service->deleteSessions(self::TEST_USER, [$ownSession->getId(), $otherSession->getId(), $otherSession->getId(), 999999999]); + + $this->assertCount(0, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $this->assertCount(0, $this->messageMapper->getMessages($ownSession->getId(), 0, 100)); + + $otherUserSessions = $this->sessionMapper->getUserSessions(self::OTHER_USER, false); + $this->assertCount(1, $otherUserSessions); + $this->assertEquals($otherSession->getId(), $otherUserSessions[0]->getId()); + $this->assertCount(1, $this->messageMapper->getMessages($otherSession->getId(), 0, 100)); + $this->assertEquals($otherMessage->getId(), $this->messageMapper->getMessageById($otherSession->getId(), $otherMessage->getId())->getId()); + } + + public function testDeleteSessionsWithEmptyList(): void { + $session = $this->createSession(self::TEST_USER); + + $this->service->deleteSessions(self::TEST_USER, []); + + $this->assertCount(1, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + } + + public function testDeleteSessionsWithoutUser(): void { + $this->expectException(UnauthorizedException::class); + $this->service->deleteSessions(null, [1, 2]); + } +} From 69c41e872493ebd74ad8406f7847901413c71b1e Mon Sep 17 00:00:00 2001 From: WSHAPER <42714629+WSHAPER@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:16:14 +0200 Subject: [PATCH 2/3] fix(chat): drop the per-item multi-delete action, keep single delete inline Address review feedback: having one action inline and one in the action menu looked inconsistent, and NcAppNavigationItem's action menu does not open when the assistant is used inside the viewer because its popover container is hardcoded to "#app-navigation-vue", of which a second instance exists beneath the viewer. The global "Delete multiple conversations" button at the top of the list is enough to enter the deletion mode; the single-delete action is now the only per-item action and is always inline, so no action menu is rendered at all. Assisted-by: opencode:glm-5.3 Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com> --- src/components/ChattyLLM/ChattyLLMInputForm.vue | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/components/ChattyLLM/ChattyLLMInputForm.vue b/src/components/ChattyLLM/ChattyLLMInputForm.vue index 558b4bffa..c21d87a10 100644 --- a/src/components/ChattyLLM/ChattyLLMInputForm.vue +++ b/src/components/ChattyLLM/ChattyLLMInputForm.vue @@ -64,7 +64,7 @@ :title="getSessionTitle(session)" :aria-description="getSessionTitle(session)" :editable="false" - :inline-actions="deletionMode ? 0 : 1" + :inline-actions="1" @click="deletionMode ? toggleSessionSelection(session.id) : onSessionSelect(session)"> {{ t('assistant', 'Delete') }} - - - {{ t('assistant', 'Delete multiple conversations') }} - @@ -858,8 +851,8 @@ export default { } }, - enterDeletionMode(preselectSessionId = null) { - this.selectedSessionIds = preselectSessionId !== null ? [preselectSessionId] : [] + enterDeletionMode() { + this.selectedSessionIds = [] this.deletionMode = true }, From 2df8538220ee62e4c5ac274c186dc5cda588968b Mon Sep 17 00:00:00 2001 From: WSHAPER <42714629+WSHAPER@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:16:08 +0200 Subject: [PATCH 3/3] fix(chat): address review feedback on bulk deletion Chunk the IN queries of the bulk session operations with IQueryBuilder::MAX_IN_PARAMETERS so large ID lists stay within the database bind limits, guard the confirmation message and title getter against missing sessions, and make the ChatServiceTest only remove the users and sessions it created so tests don't alter instance state in CI. Assisted-by: opencode:glm-5.3 Signed-off-by: WSHAPER <42714629+WSHAPER@users.noreply.github.com> --- lib/Db/ChattyLLM/MessageMapper.php | 7 ++- lib/Db/ChattyLLM/SessionMapper.php | 28 ++++++----- .../ChattyLLM/ChattyLLMInputForm.vue | 8 ++-- tests/unit/Service/ChatServiceTest.php | 47 +++++++++++++++---- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/lib/Db/ChattyLLM/MessageMapper.php b/lib/Db/ChattyLLM/MessageMapper.php index e635ca7bd..8b44208e5 100644 --- a/lib/Db/ChattyLLM/MessageMapper.php +++ b/lib/Db/ChattyLLM/MessageMapper.php @@ -186,9 +186,12 @@ public function deleteMessagesBySession(int $sessionId): void { public function deleteMessagesBySessions(array $sessionIds): void { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) - ->where($qb->expr()->in('session_id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); + ->where($qb->expr()->in('session_id', $qb->createParameter('ids'))); - $qb->executeStatement(); + foreach (array_chunk($sessionIds, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY); + $qb->executeStatement(); + } } /** diff --git a/lib/Db/ChattyLLM/SessionMapper.php b/lib/Db/ChattyLLM/SessionMapper.php index 4a67e9747..5c089c9bd 100644 --- a/lib/Db/ChattyLLM/SessionMapper.php +++ b/lib/Db/ChattyLLM/SessionMapper.php @@ -225,13 +225,16 @@ public function deleteSession(string $userId, int $sessionId) { * @throws \OCP\DB\Exception */ public function getUserSessionsByIds(string $userId, array $sessionIds): array { - $qb = $this->db->getQueryBuilder(); - $qb->select(Session::$columns) - ->from($this->getTableName()) - ->where($qb->expr()->eq('user_id', $qb->createPositionalParameter($userId, IQueryBuilder::PARAM_STR))) - ->andWhere($qb->expr()->in('id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); - - return $this->findEntities($qb); + $sessions = []; + foreach (array_chunk($sessionIds, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select(Session::$columns) + ->from($this->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createPositionalParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('id', $qb->createPositionalParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $sessions = array_merge($sessions, $this->findEntities($qb)); + } + return $sessions; } /** @@ -240,13 +243,16 @@ public function getUserSessionsByIds(string $userId, array $sessionIds): array { * @throws \OCP\DB\Exception * @throws \RuntimeException */ - public function deleteSessionsByUser(string $userId, array $sessionIds) { + public function deleteSessionsByUser(string $userId, array $sessionIds): void { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) - ->where($qb->expr()->eq('user_id', $qb->createPositionalParameter($userId, IQueryBuilder::PARAM_STR))) - ->andWhere($qb->expr()->in('id', $qb->createPositionalParameter($sessionIds, IQueryBuilder::PARAM_INT_ARRAY))); + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('id', $qb->createParameter('ids'))); - $qb->executeStatement(); + foreach (array_chunk($sessionIds, IQueryBuilder::MAX_IN_PARAMETERS) as $chunk) { + $qb->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY); + $qb->executeStatement(); + } } /** diff --git a/src/components/ChattyLLM/ChattyLLMInputForm.vue b/src/components/ChattyLLM/ChattyLLMInputForm.vue index c21d87a10..401345e99 100644 --- a/src/components/ChattyLLM/ChattyLLMInputForm.vue +++ b/src/components/ChattyLLM/ChattyLLMInputForm.vue @@ -467,8 +467,10 @@ export default { } if (this.sessionIdsToDelete.length === 1) { const session = this.sessions.find(s => s.id === this.sessionIdsToDelete[0]) - const sessionTitle = this.getSessionTitle(session)?.trim() - return t('assistant', 'Are you sure you want to delete "{sessionTitle}"?', { sessionTitle }) + if (session) { + const sessionTitle = this.getSessionTitle(session)?.trim() + return t('assistant', 'Are you sure you want to delete "{sessionTitle}"?', { sessionTitle }) + } } return n('assistant', 'Are you sure you want to delete %n conversation?', 'Are you sure you want to delete %n conversations?', this.sessionIdsToDelete.length) }, @@ -754,7 +756,7 @@ export default { * @param {{ id: number, title: string, user_id: string, timestamp: number }} session Chat session */ getSessionTitle(session) { - if (session === null) { + if (session === null || session === undefined) { return '' } diff --git a/tests/unit/Service/ChatServiceTest.php b/tests/unit/Service/ChatServiceTest.php index 8aeadeaa9..eb784c6c9 100644 --- a/tests/unit/Service/ChatServiceTest.php +++ b/tests/unit/Service/ChatServiceTest.php @@ -35,6 +35,10 @@ class ChatServiceTest extends TestCase { private ChatService $service; private SessionMapper $sessionMapper; private MessageMapper $messageMapper; + /** @var list user IDs created by this test, deleted again in tearDown */ + private array $createdUsers = []; + /** @var array> session IDs created by this test, per user ID */ + private array $createdSessionIds = []; protected function setUp(): void { parent::setUp(); @@ -44,6 +48,7 @@ protected function setUp(): void { foreach ([self::TEST_USER, self::OTHER_USER] as $uid) { if (!$userManager->userExists($uid)) { $userManager->createUser($uid, $uid . '_password123'); + $this->createdUsers[] = $uid; } } @@ -53,8 +58,15 @@ protected function setUp(): void { } protected function tearDown(): void { + // only remove what this test created, the instance is not always in a clean state in CI + foreach ($this->createdSessionIds as $uid => $sessionIds) { + foreach ($sessionIds as $sessionId) { + $this->sessionMapper->deleteSession($uid, $sessionId); + $this->messageMapper->deleteMessagesBySession($sessionId); + } + } $userManager = Server::get(IUserManager::class); - foreach ([self::TEST_USER, self::OTHER_USER] as $uid) { + foreach ($this->createdUsers as $uid) { if ($userManager->userExists($uid)) { $this->service->deleteAllUserChatData($uid); $userManager->get($uid)->delete(); @@ -67,7 +79,9 @@ private function createSession(string $userId): Session { $session = new Session(); $session->setUserId($userId); $session->setTimestamp(time()); - return $this->sessionMapper->insert($session); + $session = $this->sessionMapper->insert($session); + $this->createdSessionIds[$userId][] = $session->getId(); + return $session; } private function createMessage(int $sessionId): Message { @@ -91,7 +105,12 @@ public function testDeleteSessions(): void { $this->service->deleteSessions(self::TEST_USER, [$session1->getId(), $session2->getId(), $session3->getId()]); - $this->assertCount(0, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $remainingIds = array_map(static function (Session $session) { + return $session->getId(); + }, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $this->assertNotContains($session1->getId(), $remainingIds); + $this->assertNotContains($session2->getId(), $remainingIds); + $this->assertNotContains($session3->getId(), $remainingIds); $this->assertCount(0, $this->messageMapper->getMessages($session1->getId(), 0, 100)); $this->assertCount(0, $this->messageMapper->getMessages($session2->getId(), 0, 100)); $this->assertCount(0, $this->messageMapper->getMessages($session3->getId(), 0, 100)); @@ -107,14 +126,19 @@ public function testDeleteSessionsLeavesOtherUsersDataAlone(): void { // unknown IDs are ignored, duplicates are harmless $this->service->deleteSessions(self::TEST_USER, [$ownSession->getId(), $otherSession->getId(), $otherSession->getId(), 999999999]); - $this->assertCount(0, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $remainingOwnIds = array_map(static function (Session $session) { + return $session->getId(); + }, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $this->assertNotContains($ownSession->getId(), $remainingOwnIds); $this->assertCount(0, $this->messageMapper->getMessages($ownSession->getId(), 0, 100)); - $otherUserSessions = $this->sessionMapper->getUserSessions(self::OTHER_USER, false); - $this->assertCount(1, $otherUserSessions); - $this->assertEquals($otherSession->getId(), $otherUserSessions[0]->getId()); - $this->assertCount(1, $this->messageMapper->getMessages($otherSession->getId(), 0, 100)); - $this->assertEquals($otherMessage->getId(), $this->messageMapper->getMessageById($otherSession->getId(), $otherMessage->getId())->getId()); + $remainingOtherIds = array_map(static function (Session $session) { + return $session->getId(); + }, $this->sessionMapper->getUserSessions(self::OTHER_USER, false)); + $this->assertContains($otherSession->getId(), $remainingOtherIds); + $this->assertContains($otherMessage->getContent(), array_map(static function (Message $message) { + return $message->getContent(); + }, $this->messageMapper->getMessages($otherSession->getId(), 0, 100))); } public function testDeleteSessionsWithEmptyList(): void { @@ -122,7 +146,10 @@ public function testDeleteSessionsWithEmptyList(): void { $this->service->deleteSessions(self::TEST_USER, []); - $this->assertCount(1, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $remainingIds = array_map(static function (Session $session) { + return $session->getId(); + }, $this->sessionMapper->getUserSessions(self::TEST_USER, false)); + $this->assertContains($session->getId(), $remainingIds); } public function testDeleteSessionsWithoutUser(): void {