diff --git a/api/models/Prompt.js b/api/models/Prompt.js index d4737b6a8..d96780a03 100644 --- a/api/models/Prompt.js +++ b/api/models/Prompt.js @@ -269,7 +269,7 @@ async function getListPromptGroupsByAccess({ const baseQuery = { ...otherParams, _id: { $in: accessibleIds } }; // Add cursor condition - if (after) { + if (after && typeof after === 'string' && after !== 'undefined' && after !== 'null') { try { const cursor = JSON.parse(Buffer.from(after, 'base64').toString('utf8')); const { updatedAt, _id } = cursor; diff --git a/api/server/routes/prompts.js b/api/server/routes/prompts.js index 46bdf697e..300072b4d 100644 --- a/api/server/routes/prompts.js +++ b/api/server/routes/prompts.js @@ -156,7 +156,7 @@ router.get('/all', async (req, res) => { router.get('/groups', async (req, res) => { try { const userId = req.user.id; - const { pageSize, pageNumber, limit, cursor, name, category, ...otherFilters } = req.query; + const { pageSize, limit, cursor, name, category, ...otherFilters } = req.query; const { filter, searchShared, searchSharedOnly } = buildPromptGroupFilter({ name, @@ -171,6 +171,13 @@ router.get('/groups', async (req, res) => { actualLimit = parseInt(pageSize, 10); } + if ( + actualCursor && + (actualCursor === 'undefined' || actualCursor === 'null' || actualCursor.length === 0) + ) { + actualCursor = null; + } + let accessibleIds = await findAccessibleResources({ userId, role: req.user.role, @@ -190,6 +197,7 @@ router.get('/groups', async (req, res) => { publicPromptGroupIds: publiclyAccessibleIds, }); + // Cursor-based pagination only const result = await getListPromptGroupsByAccess({ accessibleIds: filteredAccessibleIds, otherParams: filter, @@ -198,19 +206,21 @@ router.get('/groups', async (req, res) => { }); if (!result) { - const emptyResponse = createEmptyPromptGroupsResponse({ pageNumber, pageSize, actualLimit }); + const emptyResponse = createEmptyPromptGroupsResponse({ + pageNumber: '1', + pageSize: actualLimit, + actualLimit, + }); return res.status(200).send(emptyResponse); } const { data: promptGroups = [], has_more = false, after = null } = result; - const groupsWithPublicFlag = markPublicPromptGroups(promptGroups, publiclyAccessibleIds); const response = formatPromptGroupsResponse({ promptGroups: groupsWithPublicFlag, - pageNumber, - pageSize, - actualLimit, + pageNumber: '1', // Always 1 for cursor-based pagination + pageSize: actualLimit.toString(), hasMore: has_more, after, }); diff --git a/api/server/routes/prompts.test.js b/api/server/routes/prompts.test.js index 29b806f62..e23676b8d 100644 --- a/api/server/routes/prompts.test.js +++ b/api/server/routes/prompts.test.js @@ -33,22 +33,11 @@ let promptRoutes; let Prompt, PromptGroup, AclEntry, AccessRole, User; let testUsers, testRoles; let grantPermission; +let currentTestUser; // Track current user for middleware // Helper function to set user in middleware function setTestUser(app, user) { - app.use((req, res, next) => { - req.user = { - ...(user.toObject ? user.toObject() : user), - id: user.id || user._id.toString(), - _id: user._id, - name: user.name, - role: user.role, - }; - if (user.role === SystemRoles.ADMIN) { - console.log('Setting admin user with role:', req.user.role); - } - next(); - }); + currentTestUser = user; } beforeAll(async () => { @@ -75,14 +64,35 @@ beforeAll(async () => { app = express(); app.use(express.json()); - // Mock authentication middleware - default to owner - setTestUser(app, testUsers.owner); + // Add user middleware before routes + app.use((req, res, next) => { + if (currentTestUser) { + req.user = { + ...(currentTestUser.toObject ? currentTestUser.toObject() : currentTestUser), + id: currentTestUser._id.toString(), + _id: currentTestUser._id, + name: currentTestUser.name, + role: currentTestUser.role, + }; + } + next(); + }); - // Import routes after mocks are set up + // Set default user + currentTestUser = testUsers.owner; + + // Import routes after middleware is set up promptRoutes = require('./prompts'); app.use('/api/prompts', promptRoutes); }); +afterEach(() => { + // Always reset to owner user after each test for isolation + if (currentTestUser !== testUsers.owner) { + currentTestUser = testUsers.owner; + } +}); + afterAll(async () => { await mongoose.disconnect(); await mongoServer.stop(); @@ -116,36 +126,26 @@ async function setupTestData() { // Create test users testUsers = { owner: await User.create({ - id: new ObjectId().toString(), - _id: new ObjectId(), name: 'Prompt Owner', email: 'owner@example.com', role: SystemRoles.USER, }), viewer: await User.create({ - id: new ObjectId().toString(), - _id: new ObjectId(), name: 'Prompt Viewer', email: 'viewer@example.com', role: SystemRoles.USER, }), editor: await User.create({ - id: new ObjectId().toString(), - _id: new ObjectId(), name: 'Prompt Editor', email: 'editor@example.com', role: SystemRoles.USER, }), noAccess: await User.create({ - id: new ObjectId().toString(), - _id: new ObjectId(), name: 'No Access', email: 'noaccess@example.com', role: SystemRoles.USER, }), admin: await User.create({ - id: new ObjectId().toString(), - _id: new ObjectId(), name: 'Admin', email: 'admin@example.com', role: SystemRoles.ADMIN, @@ -181,8 +181,7 @@ describe('Prompt Routes - ACL Permissions', () => { it('should have routes loaded', async () => { // This should at least not crash const response = await request(app).get('/api/prompts/test-404'); - console.log('Test 404 response status:', response.status); - console.log('Test 404 response body:', response.body); + // We expect a 401 or 404, not 500 expect(response.status).not.toBe(500); }); @@ -207,12 +206,6 @@ describe('Prompt Routes - ACL Permissions', () => { const response = await request(app).post('/api/prompts').send(promptData); - if (response.status !== 200) { - console.log('POST /api/prompts error status:', response.status); - console.log('POST /api/prompts error body:', response.body); - console.log('Console errors:', consoleErrorSpy.mock.calls); - } - expect(response.status).toBe(200); expect(response.body.prompt).toBeDefined(); expect(response.body.prompt.prompt).toBe(promptData.prompt.prompt); @@ -318,29 +311,8 @@ describe('Prompt Routes - ACL Permissions', () => { }); it('should allow admin access without explicit permissions', async () => { - // First, reset the app to remove previous middleware - app = express(); - app.use(express.json()); - - // Set admin user BEFORE adding routes - app.use((req, res, next) => { - req.user = { - ...testUsers.admin.toObject(), - id: testUsers.admin._id.toString(), - _id: testUsers.admin._id, - name: testUsers.admin.name, - role: testUsers.admin.role, - }; - next(); - }); - - // Now add the routes - const promptRoutes = require('./prompts'); - app.use('/api/prompts', promptRoutes); - - console.log('Admin user:', testUsers.admin); - console.log('Admin role:', testUsers.admin.role); - console.log('SystemRoles.ADMIN:', SystemRoles.ADMIN); + // Set admin user + setTestUser(app, testUsers.admin); const response = await request(app).get(`/api/prompts/${testPrompt._id}`).expect(200); @@ -432,21 +404,8 @@ describe('Prompt Routes - ACL Permissions', () => { grantedBy: testUsers.editor._id, }); - // Recreate app with viewer user - app = express(); - app.use(express.json()); - app.use((req, res, next) => { - req.user = { - ...testUsers.viewer.toObject(), - id: testUsers.viewer._id.toString(), - _id: testUsers.viewer._id, - name: testUsers.viewer.name, - role: testUsers.viewer.role, - }; - next(); - }); - const promptRoutes = require('./prompts'); - app.use('/api/prompts', promptRoutes); + // Set viewer user + setTestUser(app, testUsers.viewer); await request(app) .delete(`/api/prompts/${authorPrompt._id}`) @@ -499,21 +458,8 @@ describe('Prompt Routes - ACL Permissions', () => { grantedBy: testUsers.owner._id, }); - // Recreate app to ensure fresh middleware - app = express(); - app.use(express.json()); - app.use((req, res, next) => { - req.user = { - ...testUsers.owner.toObject(), - id: testUsers.owner._id.toString(), - _id: testUsers.owner._id, - name: testUsers.owner.name, - role: testUsers.owner.role, - }; - next(); - }); - const promptRoutes = require('./prompts'); - app.use('/api/prompts', promptRoutes); + // Ensure owner user + setTestUser(app, testUsers.owner); const response = await request(app) .patch(`/api/prompts/${testPrompt._id}/tags/production`) @@ -537,21 +483,8 @@ describe('Prompt Routes - ACL Permissions', () => { grantedBy: testUsers.owner._id, }); - // Recreate app with viewer user - app = express(); - app.use(express.json()); - app.use((req, res, next) => { - req.user = { - ...testUsers.viewer.toObject(), - id: testUsers.viewer._id.toString(), - _id: testUsers.viewer._id, - name: testUsers.viewer.name, - role: testUsers.viewer.role, - }; - next(); - }); - const promptRoutes = require('./prompts'); - app.use('/api/prompts', promptRoutes); + // Set viewer user + setTestUser(app, testUsers.viewer); await request(app).patch(`/api/prompts/${testPrompt._id}/tags/production`).expect(403); @@ -610,4 +543,305 @@ describe('Prompt Routes - ACL Permissions', () => { expect(response.body._id).toBe(publicPrompt._id.toString()); }); }); + + describe('Pagination', () => { + beforeEach(async () => { + // Create multiple prompt groups for pagination testing + const groups = []; + for (let i = 0; i < 15; i++) { + const group = await PromptGroup.create({ + name: `Test Group ${i + 1}`, + category: 'pagination-test', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - i * 1000), // Stagger updatedAt for consistent ordering + }); + groups.push(group); + + // Grant owner permissions on each group + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + }); + + afterEach(async () => { + await PromptGroup.deleteMany({}); + await AclEntry.deleteMany({}); + }); + + it('should correctly indicate hasMore when there are more pages', async () => { + const response = await request(app) + .get('/api/prompts/groups') + .query({ limit: '10' }) + .expect(200); + + expect(response.body.promptGroups).toHaveLength(10); + expect(response.body.has_more).toBe(true); + expect(response.body.after).toBeTruthy(); + // Since has_more is true, pages should be a high number (9999 in our fix) + expect(parseInt(response.body.pages)).toBeGreaterThan(1); + }); + + it('should correctly indicate no more pages on the last page', async () => { + // First get the cursor for page 2 + const firstPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '10' }) + .expect(200); + + expect(firstPage.body.has_more).toBe(true); + expect(firstPage.body.after).toBeTruthy(); + + // Now fetch the second page using the cursor + const response = await request(app) + .get('/api/prompts/groups') + .query({ limit: '10', cursor: firstPage.body.after }) + .expect(200); + + expect(response.body.promptGroups).toHaveLength(5); // 15 total, 10 on page 1, 5 on page 2 + expect(response.body.has_more).toBe(false); + }); + + it('should support cursor-based pagination', async () => { + // First page + const firstPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5' }) + .expect(200); + + expect(firstPage.body.promptGroups).toHaveLength(5); + expect(firstPage.body.has_more).toBe(true); + expect(firstPage.body.after).toBeTruthy(); + + // Second page using cursor + const secondPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5', cursor: firstPage.body.after }) + .expect(200); + + expect(secondPage.body.promptGroups).toHaveLength(5); + expect(secondPage.body.has_more).toBe(true); + expect(secondPage.body.after).toBeTruthy(); + + // Verify different groups + const firstPageIds = firstPage.body.promptGroups.map((g) => g._id); + const secondPageIds = secondPage.body.promptGroups.map((g) => g._id); + expect(firstPageIds).not.toEqual(secondPageIds); + }); + + it('should paginate correctly with category filtering', async () => { + // Create groups with different categories + await PromptGroup.deleteMany({}); // Clear existing groups + await AclEntry.deleteMany({}); + + // Create 8 groups with category 'test-cat-1' + for (let i = 0; i < 8; i++) { + const group = await PromptGroup.create({ + name: `Category 1 Group ${i + 1}`, + category: 'test-cat-1', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - i * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Create 7 groups with category 'test-cat-2' + for (let i = 0; i < 7; i++) { + const group = await PromptGroup.create({ + name: `Category 2 Group ${i + 1}`, + category: 'test-cat-2', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - (i + 8) * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Test pagination with category filter + const firstPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5', category: 'test-cat-1' }) + .expect(200); + + expect(firstPage.body.promptGroups).toHaveLength(5); + expect(firstPage.body.promptGroups.every((g) => g.category === 'test-cat-1')).toBe(true); + expect(firstPage.body.has_more).toBe(true); + expect(firstPage.body.after).toBeTruthy(); + + const secondPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5', cursor: firstPage.body.after, category: 'test-cat-1' }) + .expect(200); + + expect(secondPage.body.promptGroups).toHaveLength(3); // 8 total, 5 on page 1, 3 on page 2 + expect(secondPage.body.promptGroups.every((g) => g.category === 'test-cat-1')).toBe(true); + expect(secondPage.body.has_more).toBe(false); + }); + + it('should paginate correctly with name/keyword filtering', async () => { + // Create groups with specific names + await PromptGroup.deleteMany({}); // Clear existing groups + await AclEntry.deleteMany({}); + + // Create 12 groups with 'Search' in the name + for (let i = 0; i < 12; i++) { + const group = await PromptGroup.create({ + name: `Search Test Group ${i + 1}`, + category: 'search-test', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - i * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Create 5 groups without 'Search' in the name + for (let i = 0; i < 5; i++) { + const group = await PromptGroup.create({ + name: `Other Group ${i + 1}`, + category: 'other-test', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - (i + 12) * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Test pagination with name filter + const firstPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '10', name: 'Search' }) + .expect(200); + + expect(firstPage.body.promptGroups).toHaveLength(10); + expect(firstPage.body.promptGroups.every((g) => g.name.includes('Search'))).toBe(true); + expect(firstPage.body.has_more).toBe(true); + expect(firstPage.body.after).toBeTruthy(); + + const secondPage = await request(app) + .get('/api/prompts/groups') + .query({ limit: '10', cursor: firstPage.body.after, name: 'Search' }) + .expect(200); + + expect(secondPage.body.promptGroups).toHaveLength(2); // 12 total, 10 on page 1, 2 on page 2 + expect(secondPage.body.promptGroups.every((g) => g.name.includes('Search'))).toBe(true); + expect(secondPage.body.has_more).toBe(false); + }); + + it('should paginate correctly with combined filters', async () => { + // Create groups with various combinations + await PromptGroup.deleteMany({}); // Clear existing groups + await AclEntry.deleteMany({}); + + // Create 6 groups matching both category and name filters + for (let i = 0; i < 6; i++) { + const group = await PromptGroup.create({ + name: `API Test Group ${i + 1}`, + category: 'api-category', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - i * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Create groups that only match one filter + for (let i = 0; i < 4; i++) { + const group = await PromptGroup.create({ + name: `API Other Group ${i + 1}`, + category: 'other-category', + author: testUsers.owner._id, + authorName: testUsers.owner.name, + productionId: new ObjectId(), + updatedAt: new Date(Date.now() - (i + 6) * 1000), + }); + + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + resourceType: ResourceType.PROMPTGROUP, + resourceId: group._id, + accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER, + grantedBy: testUsers.owner._id, + }); + } + + // Test pagination with both filters + const response = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5', name: 'API', category: 'api-category' }) + .expect(200); + + expect(response.body.promptGroups).toHaveLength(5); + expect( + response.body.promptGroups.every( + (g) => g.name.includes('API') && g.category === 'api-category', + ), + ).toBe(true); + expect(response.body.has_more).toBe(true); + expect(response.body.after).toBeTruthy(); + + // Page 2 + const page2 = await request(app) + .get('/api/prompts/groups') + .query({ limit: '5', cursor: response.body.after, name: 'API', category: 'api-category' }) + .expect(200); + + expect(page2.body.promptGroups).toHaveLength(1); // 6 total, 5 on page 1, 1 on page 2 + expect(page2.body.has_more).toBe(false); + }); + }); }); diff --git a/client/src/components/Prompts/Groups/FilterPrompts.tsx b/client/src/components/Prompts/Groups/FilterPrompts.tsx index 35080693d..5ed3d51fa 100644 --- a/client/src/components/Prompts/Groups/FilterPrompts.tsx +++ b/client/src/components/Prompts/Groups/FilterPrompts.tsx @@ -11,9 +11,9 @@ import store from '~/store'; export default function FilterPrompts({ className = '' }: { className?: string }) { const localize = useLocalize(); - const { setName } = usePromptGroupsContext(); + const { name, setName } = usePromptGroupsContext(); const { categories } = useCategories('h-4 w-4'); - const [displayName, setDisplayName] = useState(''); + const [displayName, setDisplayName] = useState(name || ''); const [isSearching, setIsSearching] = useState(false); const [categoryFilter, setCategory] = useRecoilState(store.promptsCategory); @@ -60,13 +60,26 @@ export default function FilterPrompts({ className = '' }: { className?: string } [setCategory], ); + // Sync displayName with name prop when it changes externally useEffect(() => { + setDisplayName(name || ''); + }, [name]); + + useEffect(() => { + if (displayName === '') { + // Clear immediately when empty + setName(''); + setIsSearching(false); + return; + } + setIsSearching(true); const timeout = setTimeout(() => { setIsSearching(false); + setName(displayName); // Debounced setName call }, 500); return () => clearTimeout(timeout); - }, [displayName]); + }, [displayName, setName]); return (