Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@red-hat-developer-hub/backstage-plugin-intelligent-assistant': minor
'@red-hat-developer-hub/backstage-plugin-intelligent-assistant-backend': minor
---

Add inline rename for notebook resources with double-click or kebab menu, frontend conflict validation, and backend PATCH endpoint with rollback on failure
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ export class NotebookSurfacePage {
firstListedDocumentOverflowMenuToggle(): Locator {
return this.chatbotRegion()
.getByRole('button', {
name: this.t['notebook.document.delete'],
exact: true,
name: new RegExp(`^${this.t['aria.options.label']} `),
})
.first();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { mockServices } from '@backstage/backend-test-utils';
import { NotFoundError } from '@backstage/errors';
import { ConflictError, NotFoundError } from '@backstage/errors';

import { setupServer } from 'msw/node';

Expand Down Expand Up @@ -288,6 +288,64 @@ describe('DocumentService', () => {
});
});

describe('renameDocument', () => {
it('should rename a document successfully', async () => {
const fileId = await documentService.uploadFile(
'Content',
'Original Name',
);
await documentService.upsertDocument(
sessionId,
'Original Name',
'text',
fileId,
);

await documentService.renameDocument(
sessionId,
'Original Name',
'New Name',
);

const documents = await documentService.listDocuments(sessionId);
expect(documents).toHaveLength(1);
expect(documents[0].document_id).toBe('New Name');
});

it('should throw NotFoundError for non-existent document', async () => {
await expect(
documentService.renameDocument(sessionId, 'Non-existent', 'New Name'),
).rejects.toThrow(NotFoundError);
});

it('should throw ConflictError when new title already exists', async () => {
const fileId1 = await documentService.uploadFile('Content 1', 'Doc A');
await documentService.upsertDocument(sessionId, 'Doc A', 'text', fileId1);

const fileId2 = await documentService.uploadFile('Content 2', 'Doc B');
await documentService.upsertDocument(sessionId, 'Doc B', 'text', fileId2);

await expect(
documentService.renameDocument(sessionId, 'Doc A', 'Doc B'),
).rejects.toThrow(ConflictError);
});

it('should preserve other documents when renaming', async () => {
const fileId1 = await documentService.uploadFile('Content 1', 'Doc A');
await documentService.upsertDocument(sessionId, 'Doc A', 'text', fileId1);

const fileId2 = await documentService.uploadFile('Content 2', 'Doc B');
await documentService.upsertDocument(sessionId, 'Doc B', 'text', fileId2);

await documentService.renameDocument(sessionId, 'Doc A', 'Doc A Renamed');

const documents = await documentService.listDocuments(sessionId);
expect(documents).toHaveLength(2);
expect(documents.map(d => d.document_id)).toContain('Doc A Renamed');
expect(documents.map(d => d.document_id)).toContain('Doc B');
});
});

describe('deleteDocument', () => {
it('should delete a document successfully', async () => {
const fileId = await documentService.uploadFile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,74 @@
return documents;
}

/**
* Rename a document by updating its title attribute in the vector store.
* Deletes the old vector store file entry and re-creates it with the
* same underlying file but an updated title attribute.
* @param sessionId - Vector store ID
* @param currentTitle - Current document title
* @param newTitle - New document title
* @throws NotFoundError if document not found
* @throws ConflictError if newTitle conflicts with existing document
*/
async renameDocument(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There already exists a function upsertDocument that handles document updates, which follows the same logic as this function, please use it instead.

sessionId: string,
currentTitle: string,
newTitle: string,
): Promise<void> {
const existingFile = await this.findFileByTitle(sessionId, currentTitle);
if (!existingFile) {
throw new NotFoundError(`Document not found: ${currentTitle}`);
}

const conflicting = await this.findFileByTitle(sessionId, newTitle);
if (conflicting) {
throw new ConflictError(
`A document with the title "${newTitle}" already exists in this session`,
);
}

const attrs = existingFile.attributes || {};
const fileId = existingFile.id;

await this.client.vectorStores.files.delete(sessionId, fileId);

try {
await this.client.vectorStores.files.create(sessionId, {
file_id: fileId,
chunking_strategy: this.chunkingStrategy,
attributes: {
...attrs,
title: newTitle,
updated_at: new Date().toISOString(),
},
});
} catch (error) {
this.logger.error(
`Failed to re-create vector store entry after delete during rename of "${currentTitle}" to "${newTitle}". Attempting rollback.`,
);
try {
await this.client.vectorStores.files.create(sessionId, {
file_id: fileId,
chunking_strategy: this.chunkingStrategy,
attributes: attrs,
});
this.logger.info(
`Rollback succeeded: restored "${currentTitle}" in session ${sessionId}`,
);
} catch (rollbackError) {
this.logger.error(
`Rollback failed: document "${currentTitle}" (file ${fileId}) is orphaned in session ${sessionId}`,
);
}

Check warning on line 321 in workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/notebooks/documents/documentService.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Handle this exception or don't catch it at all.

See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-plugins&issues=AZ953r5sGC2MEnW6u522&open=AZ953r5sGC2MEnW6u522&pullRequest=3834
throw error;
}

this.logger.info(
`Renamed document "${currentTitle}" to "${newTitle}" in session ${sessionId}`,
);
}

/**
* Delete a document from the vector store and Files API
* @param sessionId - Vector store ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,87 @@ describe('Notebooks Router', () => {
});
});

describe('PATCH /v1/sessions/:sessionId/documents/:documentId', () => {
it('should rename a document', async () => {
await request(app)
.put(`/notebooks/v1/sessions/${sessionId}/documents`)
.field('title', 'Original Name')
.field('fileType', 'txt')
.attach('file', Buffer.from('Content'), 'test.txt');

const response = await request(app)
.patch(
`/notebooks/v1/sessions/${sessionId}/documents/${encodeURIComponent('Original Name')}`,
)
.send({ title: 'New Name' });

expect(response.status).toBe(200);
expect(response.body.document_id).toBe('New Name');
expect(response.body.message).toContain('renamed successfully');

const listResponse = await request(app).get(
`/notebooks/v1/sessions/${sessionId}/documents`,
);
expect(
listResponse.body.documents.map((d: any) => d.document_id),
).toContain('New Name');
});

it('should return 400 if title is missing', async () => {
const response = await request(app)
.patch(
`/notebooks/v1/sessions/${sessionId}/documents/${encodeURIComponent('Some Doc')}`,
)
.send({});

expect(response.status).toBe(400);
expect(response.body.error).toBe('title is required');
});

it('should return 400 if title is empty string', async () => {
const response = await request(app)
.patch(
`/notebooks/v1/sessions/${sessionId}/documents/${encodeURIComponent('Some Doc')}`,
)
.send({ title: ' ' });

expect(response.status).toBe(400);
expect(response.body.error).toBe('title is required');
});

it('should return 404 for non-existent document', async () => {
const response = await request(app)
.patch(
`/notebooks/v1/sessions/${sessionId}/documents/${encodeURIComponent('Non-existent')}`,
)
.send({ title: 'New Name' });

expect(response.status).toBe(404);
});

it('should return 409 when new title conflicts', async () => {
await request(app)
.put(`/notebooks/v1/sessions/${sessionId}/documents`)
.field('title', 'Doc A')
.field('fileType', 'txt')
.attach('file', Buffer.from('Content A'), 'a.txt');

await request(app)
.put(`/notebooks/v1/sessions/${sessionId}/documents`)
.field('title', 'Doc B')
.field('fileType', 'txt')
.attach('file', Buffer.from('Content B'), 'b.txt');

const response = await request(app)
.patch(
`/notebooks/v1/sessions/${sessionId}/documents/${encodeURIComponent('Doc A')}`,
)
.send({ title: 'Doc B' });

expect(response.status).toBe(409);
});
});

describe('DELETE /v1/sessions/:sessionId/documents/:documentId', () => {
it('should delete document', async () => {
await request(app)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,31 @@ export async function createNotebooksRouter(
}),
);

notebooksRouter.patch(
'/v1/sessions/:sessionId/documents/:documentId',
generalRateLimiter,
requireNotebooksPermission,
requireSessionOwnership(),
withAuth(async (req, res) => {
const { sessionId, documentId } = req.params;
const { title } = req.body;

if (!title || typeof title !== 'string' || !title.trim()) {
handleError(logger, res, 'title is required');
return;
}

await documentService.renameDocument(sessionId, documentId, title.trim());
res.json(
createDocumentResponse(
title.trim(),
sessionId,
'Document renamed successfully',
),
);
}),
);

notebooksRouter.delete(
'/v1/sessions/:sessionId/documents/:documentId',
generalRateLimiter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ export const intelligentAssistantTranslationRef: TranslationRef<
readonly 'notebook.overwrite.modal.title': string;
readonly 'notebook.overwrite.modal.description': string;
readonly 'notebook.overwrite.modal.action': string;
readonly 'notebook.document.rename': string;
readonly 'notebook.document.rename.tooltip': string;
readonly 'notebook.document.rename.success': string;
readonly 'notebook.document.rename.error': string;
readonly 'notebook.document.rename.conflict': string;
readonly 'notebook.document.delete': string;
readonly 'notebook.document.delete.title': string;
readonly 'notebook.document.delete.description': string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,21 @@ export class NotebooksApiClient implements NotebooksAPI {
}));
}

async renameDocument(
sessionId: string,
documentId: string,
newTitle: string,
) {
const baseUrl = await this.getBaseUrl();
await this.fetchJson(
`${baseUrl}/v1/sessions/${encodeURIComponent(sessionId)}/documents/${encodeURIComponent(documentId)}`,
{
method: 'PATCH',
body: JSON.stringify({ title: newTitle }),
},
);
}

async deleteDocument(sessionId: string, documentId: string) {
const baseUrl = await this.getBaseUrl();
await this.fetchJson(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export type NotebooksAPI = {
newTitle?: string,
) => Promise<UploadDocumentResponse>;
listDocuments: (sessionId: string) => Promise<SessionDocument[]>;
renameDocument: (
sessionId: string,
documentId: string,
newTitle: string,
) => Promise<void>;
deleteDocument: (sessionId: string, documentId: string) => Promise<void>;
getDocumentStatus: (
sessionId: string,
Expand Down
Loading
Loading