🗑️ fix: Delete Shared Links on Conversation Deletion (#10396)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled

*  feat: Enhance DELETE /all endpoint to remove shared links alongside conversations and tool calls

- Added functionality to delete all shared links for a user when clearing conversations.
- Introduced comprehensive tests to ensure correct behavior and error handling for the new deletion process.

*  feat: Implement deleteConvoSharedLink method and update conversation deletion logic to remove associated shared links
This commit is contained in:
Danny Avila 2025-11-06 11:44:28 -05:00 committed by GitHub
parent c6611d4e77
commit ba71375982
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 640 additions and 3 deletions

View file

@ -0,0 +1,502 @@
const express = require('express');
const request = require('supertest');
jest.mock('@librechat/agents', () => ({
sleep: jest.fn(),
}));
jest.mock('@librechat/api', () => ({
isEnabled: jest.fn(),
createAxiosInstance: jest.fn(() => ({
get: jest.fn(),
post: jest.fn(),
put: jest.fn(),
delete: jest.fn(),
})),
logAxiosError: jest.fn(),
}));
jest.mock('@librechat/data-schemas', () => ({
logger: {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
},
createModels: jest.fn(() => ({
User: {},
Conversation: {},
Message: {},
SharedLink: {},
})),
}));
jest.mock('~/models/Conversation', () => ({
getConvosByCursor: jest.fn(),
getConvo: jest.fn(),
deleteConvos: jest.fn(),
saveConvo: jest.fn(),
}));
jest.mock('~/models/ToolCall', () => ({
deleteToolCalls: jest.fn(),
}));
jest.mock('~/models', () => ({
deleteAllSharedLinks: jest.fn(),
deleteConvoSharedLink: jest.fn(),
}));
jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next());
jest.mock('~/server/middleware', () => ({
createImportLimiters: jest.fn(() => ({
importIpLimiter: (req, res, next) => next(),
importUserLimiter: (req, res, next) => next(),
})),
createForkLimiters: jest.fn(() => ({
forkIpLimiter: (req, res, next) => next(),
forkUserLimiter: (req, res, next) => next(),
})),
configMiddleware: (req, res, next) => next(),
}));
jest.mock('~/server/utils/import/fork', () => ({
forkConversation: jest.fn(),
duplicateConversation: jest.fn(),
}));
jest.mock('~/server/utils/import', () => ({
importConversations: jest.fn(),
}));
jest.mock('~/cache/getLogStores', () => jest.fn());
jest.mock('~/server/routes/files/multer', () => ({
storage: {},
importFileFilter: jest.fn(),
}));
jest.mock('multer', () => {
return jest.fn(() => ({
single: jest.fn(() => (req, res, next) => {
req.file = { path: '/tmp/test-file.json' };
next();
}),
}));
});
jest.mock('librechat-data-provider', () => ({
CacheKeys: {
GEN_TITLE: 'GEN_TITLE',
},
EModelEndpoint: {
azureAssistants: 'azureAssistants',
assistants: 'assistants',
},
}));
jest.mock('~/server/services/Endpoints/azureAssistants', () => ({
initializeClient: jest.fn(),
}));
jest.mock('~/server/services/Endpoints/assistants', () => ({
initializeClient: jest.fn(),
}));
describe('Convos Routes', () => {
let app;
let convosRouter;
const { deleteAllSharedLinks, deleteConvoSharedLink } = require('~/models');
const { deleteConvos } = require('~/models/Conversation');
const { deleteToolCalls } = require('~/models/ToolCall');
beforeAll(() => {
convosRouter = require('../convos');
app = express();
app.use(express.json());
/** Mock authenticated user */
app.use((req, res, next) => {
req.user = { id: 'test-user-123' };
next();
});
app.use('/api/convos', convosRouter);
});
beforeEach(() => {
jest.clearAllMocks();
});
describe('DELETE /all', () => {
it('should delete all conversations, tool calls, and shared links for a user', async () => {
const mockDbResponse = {
deletedCount: 5,
message: 'All conversations deleted successfully',
};
deleteConvos.mockResolvedValue(mockDbResponse);
deleteToolCalls.mockResolvedValue({ deletedCount: 10 });
deleteAllSharedLinks.mockResolvedValue({
message: 'All shared links deleted successfully',
deletedCount: 3,
});
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(201);
expect(response.body).toEqual(mockDbResponse);
/** Verify deleteConvos was called with correct userId */
expect(deleteConvos).toHaveBeenCalledWith('test-user-123', {});
expect(deleteConvos).toHaveBeenCalledTimes(1);
/** Verify deleteToolCalls was called with correct userId */
expect(deleteToolCalls).toHaveBeenCalledWith('test-user-123');
expect(deleteToolCalls).toHaveBeenCalledTimes(1);
/** Verify deleteAllSharedLinks was called with correct userId */
expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123');
expect(deleteAllSharedLinks).toHaveBeenCalledTimes(1);
});
it('should call deleteAllSharedLinks even when no conversations exist', async () => {
const mockDbResponse = {
deletedCount: 0,
message: 'No conversations to delete',
};
deleteConvos.mockResolvedValue(mockDbResponse);
deleteToolCalls.mockResolvedValue({ deletedCount: 0 });
deleteAllSharedLinks.mockResolvedValue({
message: 'All shared links deleted successfully',
deletedCount: 0,
});
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(201);
expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123');
});
it('should return 500 if deleteConvos fails', async () => {
const errorMessage = 'Database connection error';
deleteConvos.mockRejectedValue(new Error(errorMessage));
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(500);
expect(response.text).toBe('Error clearing conversations');
/** Verify error was logged */
const { logger } = require('@librechat/data-schemas');
expect(logger.error).toHaveBeenCalledWith('Error clearing conversations', expect.any(Error));
});
it('should return 500 if deleteToolCalls fails', async () => {
deleteConvos.mockResolvedValue({ deletedCount: 5 });
deleteToolCalls.mockRejectedValue(new Error('Tool calls deletion failed'));
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(500);
expect(response.text).toBe('Error clearing conversations');
});
it('should return 500 if deleteAllSharedLinks fails', async () => {
deleteConvos.mockResolvedValue({ deletedCount: 5 });
deleteToolCalls.mockResolvedValue({ deletedCount: 10 });
deleteAllSharedLinks.mockRejectedValue(new Error('Shared links deletion failed'));
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(500);
expect(response.text).toBe('Error clearing conversations');
});
it('should handle multiple users independently', async () => {
/** First user */
deleteConvos.mockResolvedValue({ deletedCount: 3 });
deleteToolCalls.mockResolvedValue({ deletedCount: 5 });
deleteAllSharedLinks.mockResolvedValue({ deletedCount: 2 });
let response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(201);
expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123');
jest.clearAllMocks();
/** Second user (simulate different user by modifying middleware) */
const app2 = express();
app2.use(express.json());
app2.use((req, res, next) => {
req.user = { id: 'test-user-456' };
next();
});
app2.use('/api/convos', require('../convos'));
deleteConvos.mockResolvedValue({ deletedCount: 7 });
deleteToolCalls.mockResolvedValue({ deletedCount: 12 });
deleteAllSharedLinks.mockResolvedValue({ deletedCount: 4 });
response = await request(app2).delete('/api/convos/all');
expect(response.status).toBe(201);
expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-456');
});
it('should execute deletions in correct sequence', async () => {
const executionOrder = [];
deleteConvos.mockImplementation(() => {
executionOrder.push('deleteConvos');
return Promise.resolve({ deletedCount: 5 });
});
deleteToolCalls.mockImplementation(() => {
executionOrder.push('deleteToolCalls');
return Promise.resolve({ deletedCount: 10 });
});
deleteAllSharedLinks.mockImplementation(() => {
executionOrder.push('deleteAllSharedLinks');
return Promise.resolve({ deletedCount: 3 });
});
await request(app).delete('/api/convos/all');
/** Verify all three functions were called */
expect(executionOrder).toEqual(['deleteConvos', 'deleteToolCalls', 'deleteAllSharedLinks']);
});
it('should maintain data integrity by cleaning up shared links when conversations are deleted', async () => {
/** This test ensures that orphaned shared links are prevented */
const mockConvosDeleted = { deletedCount: 10 };
const mockToolCallsDeleted = { deletedCount: 15 };
const mockSharedLinksDeleted = {
message: 'All shared links deleted successfully',
deletedCount: 8,
};
deleteConvos.mockResolvedValue(mockConvosDeleted);
deleteToolCalls.mockResolvedValue(mockToolCallsDeleted);
deleteAllSharedLinks.mockResolvedValue(mockSharedLinksDeleted);
const response = await request(app).delete('/api/convos/all');
expect(response.status).toBe(201);
/** Verify that shared links cleanup was called for the same user */
expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123');
/** Verify no shared links remain for deleted conversations */
expect(deleteAllSharedLinks).toHaveBeenCalledAfter(deleteConvos);
});
});
describe('DELETE /', () => {
it('should delete a single conversation, tool calls, and associated shared links', async () => {
const mockConversationId = 'conv-123';
const mockDbResponse = {
deletedCount: 1,
message: 'Conversation deleted successfully',
};
deleteConvos.mockResolvedValue(mockDbResponse);
deleteToolCalls.mockResolvedValue({ deletedCount: 3 });
deleteConvoSharedLink.mockResolvedValue({
message: 'Shared links deleted successfully',
deletedCount: 1,
});
const response = await request(app)
.delete('/api/convos')
.send({
arg: {
conversationId: mockConversationId,
},
});
expect(response.status).toBe(201);
expect(response.body).toEqual(mockDbResponse);
/** Verify deleteConvos was called with correct parameters */
expect(deleteConvos).toHaveBeenCalledWith('test-user-123', {
conversationId: mockConversationId,
});
/** Verify deleteToolCalls was called */
expect(deleteToolCalls).toHaveBeenCalledWith('test-user-123', mockConversationId);
/** Verify deleteConvoSharedLink was called */
expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId);
});
it('should not call deleteConvoSharedLink when no conversationId provided', async () => {
deleteConvos.mockResolvedValue({ deletedCount: 0 });
deleteToolCalls.mockResolvedValue({ deletedCount: 0 });
const response = await request(app)
.delete('/api/convos')
.send({
arg: {
source: 'button',
},
});
expect(response.status).toBe(200);
expect(deleteConvoSharedLink).not.toHaveBeenCalled();
});
it('should handle deletion of conversation without shared links', async () => {
const mockConversationId = 'conv-no-shares';
deleteConvos.mockResolvedValue({ deletedCount: 1 });
deleteToolCalls.mockResolvedValue({ deletedCount: 0 });
deleteConvoSharedLink.mockResolvedValue({
message: 'Shared links deleted successfully',
deletedCount: 0,
});
const response = await request(app)
.delete('/api/convos')
.send({
arg: {
conversationId: mockConversationId,
},
});
expect(response.status).toBe(201);
expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId);
});
it('should return 400 when no parameters provided', async () => {
const response = await request(app).delete('/api/convos').send({
arg: {},
});
expect(response.status).toBe(400);
expect(response.body).toEqual({ error: 'no parameters provided' });
expect(deleteConvos).not.toHaveBeenCalled();
expect(deleteConvoSharedLink).not.toHaveBeenCalled();
});
it('should return 500 if deleteConvoSharedLink fails', async () => {
const mockConversationId = 'conv-error';
deleteConvos.mockResolvedValue({ deletedCount: 1 });
deleteToolCalls.mockResolvedValue({ deletedCount: 2 });
deleteConvoSharedLink.mockRejectedValue(new Error('Failed to delete shared links'));
const response = await request(app)
.delete('/api/convos')
.send({
arg: {
conversationId: mockConversationId,
},
});
expect(response.status).toBe(500);
expect(response.text).toBe('Error clearing conversations');
});
it('should execute deletions in correct sequence for single conversation', async () => {
const mockConversationId = 'conv-sequence';
const executionOrder = [];
deleteConvos.mockImplementation(() => {
executionOrder.push('deleteConvos');
return Promise.resolve({ deletedCount: 1 });
});
deleteToolCalls.mockImplementation(() => {
executionOrder.push('deleteToolCalls');
return Promise.resolve({ deletedCount: 2 });
});
deleteConvoSharedLink.mockImplementation(() => {
executionOrder.push('deleteConvoSharedLink');
return Promise.resolve({ deletedCount: 1 });
});
await request(app)
.delete('/api/convos')
.send({
arg: {
conversationId: mockConversationId,
},
});
expect(executionOrder).toEqual(['deleteConvos', 'deleteToolCalls', 'deleteConvoSharedLink']);
});
it('should prevent orphaned shared links when deleting single conversation', async () => {
const mockConversationId = 'conv-with-shares';
deleteConvos.mockResolvedValue({ deletedCount: 1 });
deleteToolCalls.mockResolvedValue({ deletedCount: 4 });
deleteConvoSharedLink.mockResolvedValue({
message: 'Shared links deleted successfully',
deletedCount: 2,
});
const response = await request(app)
.delete('/api/convos')
.send({
arg: {
conversationId: mockConversationId,
},
});
expect(response.status).toBe(201);
/** Verify shared links were deleted for the specific conversation */
expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId);
/** Verify it was called after the conversation was deleted */
expect(deleteConvoSharedLink).toHaveBeenCalledAfter(deleteConvos);
});
});
});
/**
* Custom Jest matcher to verify function call order
*/
expect.extend({
toHaveBeenCalledAfter(received, other) {
const receivedCalls = received.mock.invocationCallOrder;
const otherCalls = other.mock.invocationCallOrder;
if (receivedCalls.length === 0) {
return {
pass: false,
message: () =>
`Expected ${received.getMockName()} to have been called after ${other.getMockName()}, but ${received.getMockName()} was never called`,
};
}
if (otherCalls.length === 0) {
return {
pass: false,
message: () =>
`Expected ${received.getMockName()} to have been called after ${other.getMockName()}, but ${other.getMockName()} was never called`,
};
}
const lastReceivedCall = receivedCalls[receivedCalls.length - 1];
const firstOtherCall = otherCalls[0];
const pass = lastReceivedCall > firstOtherCall;
return {
pass,
message: () =>
pass
? `Expected ${received.getMockName()} not to have been called after ${other.getMockName()}`
: `Expected ${received.getMockName()} to have been called after ${other.getMockName()}`,
};
},
});

View file

@ -12,6 +12,7 @@ const {
const { getConvosByCursor, deleteConvos, getConvo, saveConvo } = require('~/models/Conversation');
const { forkConversation, duplicateConversation } = require('~/server/utils/import/fork');
const { storage, importFileFilter } = require('~/server/routes/files/multer');
const { deleteAllSharedLinks, deleteConvoSharedLink } = require('~/models');
const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const { importConversations } = require('~/server/utils/import');
const { deleteToolCalls } = require('~/models/ToolCall');
@ -124,7 +125,10 @@ router.delete('/', async (req, res) => {
try {
const dbResponse = await deleteConvos(req.user.id, filter);
if (filter.conversationId) {
await deleteToolCalls(req.user.id, filter.conversationId);
await deleteConvoSharedLink(req.user.id, filter.conversationId);
}
res.status(201).json(dbResponse);
} catch (error) {
logger.error('Error clearing conversations', error);
@ -136,6 +140,7 @@ router.delete('/all', async (req, res) => {
try {
const dbResponse = await deleteConvos(req.user.id, {});
await deleteToolCalls(req.user.id);
await deleteAllSharedLinks(req.user.id);
res.status(201).json(dbResponse);
} catch (error) {
logger.error('Error clearing conversations', error);

View file

@ -936,6 +936,107 @@ describe('Share Methods', () => {
});
});
describe('deleteConvoSharedLink', () => {
test('should delete all shared links for a specific conversation', async () => {
const userId = new mongoose.Types.ObjectId().toString();
const conversationId1 = 'conv-to-delete';
const conversationId2 = 'conv-to-keep';
await SharedLink.create([
{ shareId: 'share1', conversationId: conversationId1, user: userId, isPublic: true },
{ shareId: 'share2', conversationId: conversationId1, user: userId, isPublic: false },
{ shareId: 'share3', conversationId: conversationId2, user: userId, isPublic: true },
]);
const result = await shareMethods.deleteConvoSharedLink(userId, conversationId1);
expect(result.deletedCount).toBe(2);
expect(result.message).toContain('successfully');
const remainingShares = await SharedLink.find({});
expect(remainingShares).toHaveLength(1);
expect(remainingShares[0].conversationId).toBe(conversationId2);
});
test('should only delete shares for the specified user and conversation', async () => {
const userId1 = new mongoose.Types.ObjectId().toString();
const userId2 = new mongoose.Types.ObjectId().toString();
const conversationId = 'shared-conv';
await SharedLink.create([
{ shareId: 'share1', conversationId, user: userId1, isPublic: true },
{ shareId: 'share2', conversationId, user: userId2, isPublic: true },
{ shareId: 'share3', conversationId: 'other-conv', user: userId1, isPublic: true },
]);
const result = await shareMethods.deleteConvoSharedLink(userId1, conversationId);
expect(result.deletedCount).toBe(1);
const remainingShares = await SharedLink.find({});
expect(remainingShares).toHaveLength(2);
expect(
remainingShares.some((s) => s.user === userId2 && s.conversationId === conversationId),
).toBe(true);
expect(
remainingShares.some((s) => s.user === userId1 && s.conversationId === 'other-conv'),
).toBe(true);
});
test('should handle when no shares exist for the conversation', async () => {
const userId = new mongoose.Types.ObjectId().toString();
const conversationId = 'nonexistent-conv';
const result = await shareMethods.deleteConvoSharedLink(userId, conversationId);
expect(result.deletedCount).toBe(0);
expect(result.message).toContain('successfully');
});
test('should throw error when userId is missing', async () => {
await expect(shareMethods.deleteConvoSharedLink('', 'conv123')).rejects.toThrow(
'Missing required parameters',
);
});
test('should throw error when conversationId is missing', async () => {
await expect(shareMethods.deleteConvoSharedLink('user123', '')).rejects.toThrow(
'Missing required parameters',
);
});
test('should delete multiple shared links for same conversation', async () => {
const userId = new mongoose.Types.ObjectId().toString();
const conversationId = 'conv-with-many-shares';
await SharedLink.create([
{ shareId: 'share1', conversationId, user: userId, isPublic: true },
{
shareId: 'share2',
conversationId,
user: userId,
isPublic: true,
targetMessageId: 'msg1',
},
{
shareId: 'share3',
conversationId,
user: userId,
isPublic: true,
targetMessageId: 'msg2',
},
{ shareId: 'share4', conversationId, user: userId, isPublic: false },
]);
const result = await shareMethods.deleteConvoSharedLink(userId, conversationId);
expect(result.deletedCount).toBe(4);
const remainingShares = await SharedLink.find({ conversationId, user: userId });
expect(remainingShares).toHaveLength(0);
});
});
describe('Edge Cases and Error Handling', () => {
test('should handle conversation with special characters in ID', async () => {
const userId = new mongoose.Types.ObjectId().toString();

View file

@ -173,8 +173,8 @@ export function createShareMethods(mongoose: typeof import('mongoose')) {
return null;
}
// Filter messages based on targetMessageId if present (branch-specific sharing)
let messagesToShare = share.messages;
/** Filtered messages based on targetMessageId if present (branch-specific sharing) */
let messagesToShare: t.IMessage[] = share.messages;
if (share.targetMessageId) {
messagesToShare = getMessagesUpToTarget(share.messages, share.targetMessageId);
}
@ -310,6 +310,34 @@ export function createShareMethods(mongoose: typeof import('mongoose')) {
}
}
/**
* Delete shared links by conversation ID
*/
async function deleteConvoSharedLink(
user: string,
conversationId: string,
): Promise<t.DeleteAllSharesResult> {
if (!user || !conversationId) {
throw new ShareServiceError('Missing required parameters', 'INVALID_PARAMS');
}
try {
const SharedLink = mongoose.models.SharedLink as Model<t.ISharedLink>;
const result = await SharedLink.deleteMany({ user, conversationId });
return {
message: 'Shared links deleted successfully',
deletedCount: result.deletedCount,
};
} catch (error) {
logger.error('[deleteConvoSharedLink] Error deleting shared links', {
error: error instanceof Error ? error.message : 'Unknown error',
user,
conversationId,
});
throw new ShareServiceError('Error deleting shared links', 'SHARE_DELETE_ERROR');
}
}
/**
* Create a new shared link for a conversation
*/
@ -528,6 +556,7 @@ export function createShareMethods(mongoose: typeof import('mongoose')) {
deleteSharedLink,
getSharedMessages,
deleteAllSharedLinks,
deleteConvoSharedLink,
};
}