diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index 06f6982195..0c49b05fb5 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -65,6 +65,7 @@ module.exports = { getConvo: jest.fn(), deleteConvos: jest.fn(), saveConvo: jest.fn(), + setConvoPinned: jest.fn(), deleteAllSharedLinks: jest.fn(), deleteConvoSharedLink: jest.fn(), deleteToolCalls: jest.fn(), diff --git a/api/server/routes/__tests__/convos.spec.js b/api/server/routes/__tests__/convos.spec.js index 51f93153e0..ec39d8e947 100644 --- a/api/server/routes/__tests__/convos.spec.js +++ b/api/server/routes/__tests__/convos.spec.js @@ -659,49 +659,45 @@ describe('Convos Routes', () => { describe('POST /convos/pin', () => { const mockConversationId = 'conv-123'; + const { setConvoPinned } = require('~/models'); it('should pin a conversation', async () => { const mockPinnedConvo = { conversationId: mockConversationId, pinned: true }; - saveConvo.mockResolvedValue(mockPinnedConvo); + setConvoPinned.mockResolvedValue(mockPinnedConvo); const response = await request(app).post('/api/convos/pin').send({ arg: mockPinnedConvo }); expect(response.status).toBe(200); expect(response.body).toEqual(mockPinnedConvo); - expect(saveConvo).toHaveBeenCalledWith( - { userId: 'test-user-123' }, - { conversationId: mockConversationId, pinned: true }, - { - context: `POST /api/convos/pin ${mockConversationId}`, - preserveUpdatedAt: true, - noUpsert: true, - }, - ); + expect(setConvoPinned).toHaveBeenCalledWith('test-user-123', mockConversationId, true); + }); + + /** A pin is one boolean: it must not drag in `saveConvo`'s message-id refresh + * and project-stats recompute, which cost an extra read and a large write. */ + it('does not route a pin through the full conversation save', async () => { + setConvoPinned.mockResolvedValue({ conversationId: mockConversationId, pinned: true }); + + await request(app) + .post('/api/convos/pin') + .send({ arg: { conversationId: mockConversationId, pinned: true } }); + + expect(setConvoPinned).toHaveBeenCalledTimes(1); + expect(saveConvo).not.toHaveBeenCalled(); }); it('should unpin a conversation', async () => { const mockUnpinnedConvo = { conversationId: mockConversationId, pinned: false }; - saveConvo.mockResolvedValue(mockUnpinnedConvo); + setConvoPinned.mockResolvedValue(mockUnpinnedConvo); const response = await request(app).post('/api/convos/pin').send({ arg: mockUnpinnedConvo }); expect(response.status).toBe(200); expect(response.body).toEqual(mockUnpinnedConvo); - expect(saveConvo).toHaveBeenCalledWith( - { userId: 'test-user-123' }, - { conversationId: mockConversationId, pinned: false }, - { - context: `POST /api/convos/pin ${mockConversationId}`, - preserveUpdatedAt: true, - noUpsert: true, - }, - ); + expect(setConvoPinned).toHaveBeenCalledWith('test-user-123', mockConversationId, false); }); - /** With timestamps suppressed, an upsert would insert a conversation carrying - * neither createdAt nor updatedAt, so an unknown id has to be rejected instead. */ it('should return 404 when the conversation does not exist', async () => { - saveConvo.mockResolvedValue(null); + setConvoPinned.mockResolvedValue(null); const response = await request(app) .post('/api/convos/pin') @@ -718,7 +714,7 @@ describe('Convos Routes', () => { expect(response.status).toBe(400); expect(response.body).toEqual({ error: 'conversationId is required' }); - expect(saveConvo).not.toHaveBeenCalled(); + expect(setConvoPinned).not.toHaveBeenCalled(); }); it('should return 400 when pinned is not a boolean', async () => { @@ -728,7 +724,7 @@ describe('Convos Routes', () => { expect(response.status).toBe(400); expect(response.body).toEqual({ error: 'pinned must be a boolean' }); - expect(saveConvo).not.toHaveBeenCalled(); + expect(setConvoPinned).not.toHaveBeenCalled(); }); it('should return 400 when pinned is missing', async () => { @@ -738,11 +734,11 @@ describe('Convos Routes', () => { expect(response.status).toBe(400); expect(response.body).toEqual({ error: 'pinned is required' }); - expect(saveConvo).not.toHaveBeenCalled(); + expect(setConvoPinned).not.toHaveBeenCalled(); }); - it('should return 500 when saveConvo fails', async () => { - saveConvo.mockRejectedValue(new Error('Database error')); + it('should return 500 when the pin update fails', async () => { + setConvoPinned.mockRejectedValue(new Error('Database error')); const response = await request(app) .post('/api/convos/pin') diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index e56622b618..c52e4de47c 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -245,17 +245,7 @@ router.post('/pin', validateConvoAccess, async (req, res) => { } try { - const dbResponse = await db.saveConvo( - { userId: req.user.id }, - { conversationId, pinned }, - { - context: `POST /api/convos/pin ${conversationId}`, - /** Pinning is a bookmark, not activity, so the sidebar's `updatedAt` order stands. */ - preserveUpdatedAt: true, - /** Without timestamps, an upsert would insert a conversation that has none. */ - noUpsert: true, - }, - ); + const dbResponse = await db.setConvoPinned(req.user.id, conversationId, pinned); if (!dbResponse) { return res.status(404).json({ error: 'Conversation not found' }); diff --git a/packages/data-schemas/src/methods/conversation.spec.ts b/packages/data-schemas/src/methods/conversation.spec.ts index 7dc0ee5fb5..1ddf9a0bbd 100644 --- a/packages/data-schemas/src/methods/conversation.spec.ts +++ b/packages/data-schemas/src/methods/conversation.spec.ts @@ -70,6 +70,8 @@ afterAll(async () => { const saveConvo = (...args: Parameters) => methods.saveConvo(...args) as Promise; +const setConvoPinned = (...args: Parameters) => + methods.setConvoPinned(...args); const getConvo = (...args: Parameters) => methods.getConvo(...args); const getConvoRetention = (...args: Parameters) => @@ -408,29 +410,6 @@ describe('Conversation Operations', () => { return conversationId; }; - /** The sidebar orders by `updatedAt`, so a pin that bumped it would hoist an - * untouched chat into Today and drop it back out of place on unpin. */ - it('leaves updatedAt untouched across a pin and unpin round trip', async () => { - const conversationId = await seedAgedConvo(); - - const pinned = await saveConvo( - { userId: 'user123' }, - { conversationId, pinned: true }, - { preserveUpdatedAt: true, noUpsert: true }, - ); - const unpinned = await saveConvo( - { userId: 'user123' }, - { conversationId, pinned: false }, - { preserveUpdatedAt: true, noUpsert: true }, - ); - - expect(pinned?.pinned).toBe(true); - expect(unpinned?.pinned).toBe(false); - expect(new Date(pinned?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString()); - expect(new Date(unpinned?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString()); - expect(new Date(pinned?.createdAt ?? 0).toISOString()).toBe(anchor.toISOString()); - }); - /** Archiving files a chat away rather than touching it, and unarchiving has to * restore it to its real place in the date groups. */ it('leaves updatedAt untouched across an archive and unarchive round trip', async () => { @@ -537,13 +516,96 @@ describe('Conversation Operations', () => { it('does not insert a timestampless conversation for an unknown id', async () => { const result = await saveConvo( { userId: 'user123' }, - { conversationId: uuidv4(), pinned: true }, + { conversationId: uuidv4(), isArchived: true }, { preserveUpdatedAt: true, noUpsert: true }, ); expect(result).toBeNull(); }); }); + + describe('setConvoPinned', () => { + const anchor = new Date('2026-03-05T22:17:14.997Z'); + + const seedAgedConvo = async (user = 'user123') => { + const conversationId = uuidv4(); + await Conversation.collection.insertOne({ + conversationId, + user, + title: 'Living Room Light And Temperature', + endpoint: EModelEndpoint.openAI, + expiredAt: null, + isArchived: false, + messages: [], + createdAt: anchor, + updatedAt: anchor, + }); + return conversationId; + }; + + it('pins a conversation without disturbing its timestamps', async () => { + const conversationId = await seedAgedConvo(); + + const result = await setConvoPinned('user123', conversationId, true); + + expect(result?.pinned).toBe(true); + expect(result?.title).toBe('Living Room Light And Temperature'); + expect(new Date(result?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString()); + expect(new Date(result?.createdAt ?? 0).toISOString()).toBe(anchor.toISOString()); + }); + + it('unpins a conversation without disturbing its timestamps', async () => { + const conversationId = await seedAgedConvo(); + await setConvoPinned('user123', conversationId, true); + + const result = await setConvoPinned('user123', conversationId, false); + + expect(result?.pinned).toBe(false); + expect(new Date(result?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString()); + }); + + /** The whole point of the dedicated method: a one-field toggle should not pay for + * `saveConvo`'s message-id read, nor rewrite the array it returns. */ + it('does not read the conversation messages', async () => { + const conversationId = await seedAgedConvo(); + + await setConvoPinned('user123', conversationId, true); + + expect(getMessages).not.toHaveBeenCalled(); + }); + + it('leaves the stored message list alone', async () => { + const conversationId = await seedAgedConvo(); + await Conversation.collection.updateOne( + { conversationId }, + { $set: { messages: ['message-a', 'message-b'] } }, + ); + + await setConvoPinned('user123', conversationId, true); + + const stored = await Conversation.findOne({ conversationId }).lean(); + expect(stored?.messages).toEqual(['message-a', 'message-b']); + }); + + it('returns null for an unknown id rather than inserting one', async () => { + const conversationId = uuidv4(); + + const result = await setConvoPinned('user123', conversationId, true); + + expect(result).toBeNull(); + expect(await Conversation.countDocuments({ conversationId })).toBe(0); + }); + + it('cannot pin another user’s conversation', async () => { + const conversationId = await seedAgedConvo('other-user'); + + const result = await setConvoPinned('user123', conversationId, true); + + expect(result).toBeNull(); + const stored = await Conversation.findOne({ conversationId }).lean(); + expect(stored?.pinned).toBeUndefined(); + }); + }); }); describe('isTemporary conversation handling', () => { diff --git a/packages/data-schemas/src/methods/conversation.ts b/packages/data-schemas/src/methods/conversation.ts index 938e862827..0c1a209be9 100644 --- a/packages/data-schemas/src/methods/conversation.ts +++ b/packages/data-schemas/src/methods/conversation.ts @@ -36,6 +36,11 @@ export interface ConversationMethods { preserveUpdatedAt?: boolean; }, ): Promise; + setConvoPinned( + user: string, + conversationId: string, + pinned: boolean, + ): Promise; bulkSaveConvos(conversations: Array>): Promise; getConvosByCursor( user: string, @@ -416,6 +421,31 @@ export function createConversationMethods( } } + /** + * Flips the pinned flag on its own rather than routing through `saveConvo`. + * + * Pinning is pure metadata: it moves no chat between projects, changes nothing the + * project workspace hides, and opens no retention window, so none of `saveConvo`'s + * tail work applies. Going direct drops the `getMessages` round trip and the rewrite + * of the whole message-id array that a one-field toggle would otherwise pay for, and + * `timestamps: false` keeps the sidebar ordering by real activity. + * + * Returns null when no conversation matched, so a pin can never insert one. + */ + async function setConvoPinned(user: string, conversationId: string, pinned: boolean) { + try { + const Conversation = mongoose.models.Conversation as Model; + return await Conversation.findOneAndUpdate( + { conversationId, user }, + { $set: { pinned } }, + { new: true, timestamps: false }, + ).lean(); + } catch (error) { + logger.error('[setConvoPinned] Error updating pinned state', error); + throw new Error('Error updating pinned state'); + } + } + /** * Saves multiple conversations in bulk. */ @@ -945,6 +975,7 @@ export function createConversationMethods( searchConversation, deleteNullOrEmptyConversations, saveConvo, + setConvoPinned, bulkSaveConvos, getConvosByCursor, getConvosQueried,