From f86e9dd04c05377097d9d2e4b12cf2fdba4da848 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 22 Aug 2024 17:09:05 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=96=20feat:=20Enhance=20Bookmarks=20UX?= =?UTF-8?q?,=20add=20RBAC,=20toggle=20via=20`librechat.yaml`=20(#3747)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: update package version to 0.7.416 * chore: Update Role.js imports order * refactor: move updateTagsInConvo to tags route, add RBAC for tags * refactor: add updateTagsInConvoOptions * fix: loading state for bookmark form * refactor: update primaryText class in TitleButton component * refactor: remove duplicate bookmarks and theming * refactor: update EditIcon component to use React.forwardRef * refactor: add _id field to tConversationTagSchema * refactor: remove promises * refactor: move mutation logic from BookmarkForm -> BookmarkEditDialog * refactor: update button class in BookmarkForm component * fix: conversation mutations and add better logging to useConversationTagMutation * refactor: update logger message in BookmarkEditDialog component * refactor: improve UI consistency in BookmarkNav and NewChat components * refactor: update logger message in BookmarkEditDialog component * refactor: Add tags prop to BookmarkForm component * refactor: Update BookmarkForm to avoid tag mutation if the tag already exists; also close dialog on submission programmatically * refactor: general role helper function to support updating access permissions for different permission types * refactor: Update getLatestText function to handle undefined values in message.content * refactor: Update useHasAccess hook to handle null role values for authenticated users * feat: toggle bookmarks access * refactor: Update PromptsCommand to handle access permissions for prompts * feat: updateConversationSelector * refactor: rename `vars` to `tagToDelete` for clarity * fix: prevent recreation of deleted tags in BookmarkMenu on Item Click * ci: mock updateBookmarksAccess function * ci: mock updateBookmarksAccess function --- api/models/Role.js | 51 ++++--- api/models/schema/roleSchema.js | 6 + api/server/routes/convos.js | 15 -- api/server/routes/tags.js | 38 ++++- api/server/services/AppService.spec.js | 1 + api/server/services/start/interface.js | 8 +- api/server/services/start/interface.spec.js | 17 ++- .../Bookmarks/BookmarkEditDialog.tsx | 70 ++++++--- .../src/components/Bookmarks/BookmarkForm.tsx | 96 ++++++------ .../src/components/Bookmarks/BookmarkItem.tsx | 8 +- .../components/Bookmarks/BookmarkItems.tsx | 6 +- .../Bookmarks/EditBookmarkButton.tsx | 7 +- client/src/components/Chat/Header.tsx | 15 +- .../components/Chat/Input/PromptsCommand.tsx | 14 +- .../components/Chat/Menus/BookmarkMenu.tsx | 72 +++++---- .../Menus/Bookmarks/BookmarkMenuItems.tsx | 4 +- .../components/Chat/Menus/UI/TitleButton.tsx | 2 +- .../components/Nav/Bookmarks/BookmarkNav.tsx | 16 +- .../Nav/Bookmarks/BookmarkNavItems.tsx | 13 +- client/src/components/Nav/Nav.tsx | 19 ++- client/src/components/Nav/NewChat.tsx | 2 +- .../SidePanel/Bookmarks/BookmarkPanel.tsx | 2 +- .../SidePanel/Bookmarks/BookmarkTable.tsx | 33 ++-- .../SidePanel/Bookmarks/BookmarkTableRow.tsx | 4 +- client/src/components/svg/EditIcon.tsx | 57 ++++--- client/src/data-provider/mutations.ts | 142 ++++++++++++------ .../hooks/Conversations/useBookmarkSuccess.ts | 13 +- client/src/hooks/Nav/useSideNavLinks.ts | 20 ++- client/src/hooks/Roles/useHasAccess.ts | 2 +- client/src/localization/languages/Eng.ts | 1 + client/src/store/families.ts | 27 ++++ client/src/utils/messages.ts | 17 ++- package-lock.json | 2 +- packages/data-provider/package.json | 2 +- packages/data-provider/src/api-endpoints.ts | 2 +- packages/data-provider/src/config.ts | 1 + packages/data-provider/src/roles.ts | 16 ++ packages/data-provider/src/schemas.ts | 1 + packages/data-provider/src/types/mutations.ts | 6 + 39 files changed, 530 insertions(+), 298 deletions(-) diff --git a/api/models/Role.js b/api/models/Role.js index fed72ce50..f4015a70e 100644 --- a/api/models/Role.js +++ b/api/models/Role.js @@ -1,10 +1,11 @@ const { - SystemRoles, CacheKeys, + SystemRoles, roleDefaults, PermissionTypes, - Permissions, + removeNullishValues, promptPermissionsSchema, + bookmarkPermissionsSchema, } = require('librechat-data-provider'); const getLogStores = require('~/cache/getLogStores'); const Role = require('~/models/schema/roleSchema'); @@ -69,37 +70,52 @@ const updateRoleByName = async function (roleName, updates) { } }; +const permissionSchemas = { + [PermissionTypes.PROMPTS]: promptPermissionsSchema, + [PermissionTypes.BOOKMARKS]: bookmarkPermissionsSchema, +}; + /** - * Updates the Prompt access for a specific role. - * @param {SystemRoles} roleName - The role to update the prompt access for. - * @param {boolean | undefined} [value] - The new value for the prompt access. + * Updates access permissions for a specific role and permission type. + * @param {SystemRoles} roleName - The role to update. + * @param {PermissionTypes} permissionType - The type of permission to update. + * @param {Object.} permissions - Permissions to update and their values. */ -async function updatePromptsAccess(roleName, value) { - if (typeof value === 'undefined') { +async function updateAccessPermissions(roleName, permissionType, _permissions) { + const permissions = removeNullishValues(_permissions); + if (Object.keys(permissions).length === 0) { return; } try { - const parsedUpdates = promptPermissionsSchema.partial().parse({ [Permissions.USE]: value }); const role = await getRoleByName(roleName); - if (!role) { + if (!role || !permissionSchemas[permissionType]) { return; } - const mergedUpdates = { - [PermissionTypes.PROMPTS]: { - ...role[PermissionTypes.PROMPTS], - ...parsedUpdates, + await updateRoleByName(roleName, { + [permissionType]: { + ...role[permissionType], + ...permissionSchemas[permissionType].partial().parse(permissions), }, - }; + }); - await updateRoleByName(roleName, mergedUpdates); - logger.info(`Updated '${roleName}' role prompts 'USE' permission to: ${value}`); + Object.entries(permissions).forEach(([permission, value]) => + logger.info( + `Updated '${roleName}' role ${permissionType} '${permission}' permission to: ${value}`, + ), + ); } catch (error) { - logger.error('Failed to update USER role prompts USE permission:', error); + logger.error(`Failed to update ${roleName} role ${permissionType} permissions:`, error); } } +const updatePromptsAccess = (roleName, permissions) => + updateAccessPermissions(roleName, PermissionTypes.PROMPTS, permissions); + +const updateBookmarksAccess = (roleName, permissions) => + updateAccessPermissions(roleName, PermissionTypes.BOOKMARKS, permissions); + /** * Initialize default roles in the system. * Creates the default roles (ADMIN, USER) if they don't exist in the database. @@ -123,4 +139,5 @@ module.exports = { initializeRoles, updateRoleByName, updatePromptsAccess, + updateBookmarksAccess, }; diff --git a/api/models/schema/roleSchema.js b/api/models/schema/roleSchema.js index 0387f44ad..ebd1d0bc4 100644 --- a/api/models/schema/roleSchema.js +++ b/api/models/schema/roleSchema.js @@ -8,6 +8,12 @@ const roleSchema = new mongoose.Schema({ unique: true, index: true, }, + [PermissionTypes.BOOKMARKS]: { + [Permissions.USE]: { + type: Boolean, + default: true, + }, + }, [PermissionTypes.PROMPTS]: { [Permissions.SHARED_GLOBAL]: { type: Boolean, diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index 14db47556..104b0616f 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -8,7 +8,6 @@ const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); const { forkConversation } = require('~/server/utils/import/fork'); const { importConversations } = require('~/server/utils/import'); const { createImportLimiters } = require('~/server/middleware'); -const { updateTagsForConversation } = require('~/models/ConversationTag'); const getLogStores = require('~/cache/getLogStores'); const { sleep } = require('~/server/utils'); const { logger } = require('~/config'); @@ -174,18 +173,4 @@ router.post('/fork', async (req, res) => { } }); -router.put('/tags/:conversationId', async (req, res) => { - try { - const conversationTags = await updateTagsForConversation( - req.user.id, - req.params.conversationId, - req.body.tags, - ); - res.status(200).json(conversationTags); - } catch (error) { - logger.error('Error updating conversation tags', error); - res.status(500).send('Error updating conversation tags'); - } -}); - module.exports = router; diff --git a/api/server/routes/tags.js b/api/server/routes/tags.js index 289ee5c8f..c9f637c47 100644 --- a/api/server/routes/tags.js +++ b/api/server/routes/tags.js @@ -1,13 +1,21 @@ const express = require('express'); +const { PermissionTypes, Permissions } = require('librechat-data-provider'); const { getConversationTags, updateConversationTag, createConversationTag, deleteConversationTag, + updateTagsForConversation, } = require('~/models/ConversationTag'); -const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); +const { requireJwtAuth, generateCheckAccess } = require('~/server/middleware'); +const { logger } = require('~/config'); + const router = express.Router(); + +const checkBookmarkAccess = generateCheckAccess(PermissionTypes.BOOKMARKS, [Permissions.USE]); + router.use(requireJwtAuth); +router.use(checkBookmarkAccess); /** * GET / @@ -24,7 +32,7 @@ router.get('/', async (req, res) => { res.status(404).end(); } } catch (error) { - console.error('Error getting conversation tags:', error); + logger.error('Error getting conversation tags:', error); res.status(500).json({ error: 'Internal server error' }); } }); @@ -40,7 +48,7 @@ router.post('/', async (req, res) => { const tag = await createConversationTag(req.user.id, req.body); res.status(200).json(tag); } catch (error) { - console.error('Error creating conversation tag:', error); + logger.error('Error creating conversation tag:', error); res.status(500).json({ error: 'Internal server error' }); } }); @@ -60,7 +68,7 @@ router.put('/:tag', async (req, res) => { res.status(404).json({ error: 'Tag not found' }); } } catch (error) { - console.error('Error updating conversation tag:', error); + logger.error('Error updating conversation tag:', error); res.status(500).json({ error: 'Internal server error' }); } }); @@ -80,9 +88,29 @@ router.delete('/:tag', async (req, res) => { res.status(404).json({ error: 'Tag not found' }); } } catch (error) { - console.error('Error deleting conversation tag:', error); + logger.error('Error deleting conversation tag:', error); res.status(500).json({ error: 'Internal server error' }); } }); +/** + * PUT /convo/:conversationId + * Updates the tags for a conversation. + * @param {Object} req - Express request object + * @param {Object} res - Express response object + */ +router.put('/convo/:conversationId', async (req, res) => { + try { + const conversationTags = await updateTagsForConversation( + req.user.id, + req.params.conversationId, + req.body.tags, + ); + res.status(200).json(conversationTags); + } catch (error) { + logger.error('Error updating conversation tags', error); + res.status(500).send('Error updating conversation tags'); + } +}); + module.exports = router; diff --git a/api/server/services/AppService.spec.js b/api/server/services/AppService.spec.js index 3a0db5f23..90ca6f975 100644 --- a/api/server/services/AppService.spec.js +++ b/api/server/services/AppService.spec.js @@ -24,6 +24,7 @@ jest.mock('./Files/Firebase/initialize', () => ({ jest.mock('~/models/Role', () => ({ initializeRoles: jest.fn(), updatePromptsAccess: jest.fn(), + updateBookmarksAccess: jest.fn(), })); jest.mock('./ToolService', () => ({ loadAndFormatTools: jest.fn().mockReturnValue({ diff --git a/api/server/services/start/interface.js b/api/server/services/start/interface.js index 2f627ce0e..aaa4db84b 100644 --- a/api/server/services/start/interface.js +++ b/api/server/services/start/interface.js @@ -1,5 +1,5 @@ -const { SystemRoles, removeNullishValues } = require('librechat-data-provider'); -const { updatePromptsAccess } = require('~/models/Role'); +const { SystemRoles, Permissions, removeNullishValues } = require('librechat-data-provider'); +const { updatePromptsAccess, updateBookmarksAccess } = require('~/models/Role'); const { logger } = require('~/config'); /** @@ -24,10 +24,12 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol sidePanel: interfaceConfig?.sidePanel ?? defaults.sidePanel, privacyPolicy: interfaceConfig?.privacyPolicy ?? defaults.privacyPolicy, termsOfService: interfaceConfig?.termsOfService ?? defaults.termsOfService, + bookmarks: interfaceConfig?.bookmarks ?? defaults.bookmarks, prompts: interfaceConfig?.prompts ?? defaults.prompts, }); - await updatePromptsAccess(roleName, loadedInterface.prompts); + await updatePromptsAccess(roleName, { [Permissions.USE]: loadedInterface.prompts }); + await updateBookmarksAccess(roleName, { [Permissions.USE]: loadedInterface.bookmarks }); let i = 0; const logSettings = () => { diff --git a/api/server/services/start/interface.spec.js b/api/server/services/start/interface.spec.js index 3cf56799b..8a07ef572 100644 --- a/api/server/services/start/interface.spec.js +++ b/api/server/services/start/interface.spec.js @@ -1,9 +1,10 @@ -const { SystemRoles } = require('librechat-data-provider'); +const { SystemRoles, Permissions } = require('librechat-data-provider'); const { updatePromptsAccess } = require('~/models/Role'); const { loadDefaultInterface } = require('./interface'); jest.mock('~/models/Role', () => ({ updatePromptsAccess: jest.fn(), + updateBookmarksAccess: jest.fn(), })); describe('loadDefaultInterface', () => { @@ -13,7 +14,7 @@ describe('loadDefaultInterface', () => { await loadDefaultInterface(config, configDefaults); - expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, true); + expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, { [Permissions.USE]: true }); }); it('should call updatePromptsAccess with false when prompts is false', async () => { @@ -22,7 +23,9 @@ describe('loadDefaultInterface', () => { await loadDefaultInterface(config, configDefaults); - expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, false); + expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, { + [Permissions.USE]: false, + }); }); it('should call updatePromptsAccess with undefined when prompts is not specified in config', async () => { @@ -31,7 +34,9 @@ describe('loadDefaultInterface', () => { await loadDefaultInterface(config, configDefaults); - expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, undefined); + expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, { + [Permissions.USE]: undefined, + }); }); it('should call updatePromptsAccess with undefined when prompts is explicitly undefined', async () => { @@ -40,6 +45,8 @@ describe('loadDefaultInterface', () => { await loadDefaultInterface(config, configDefaults); - expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, undefined); + expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, { + [Permissions.USE]: undefined, + }); }); }); diff --git a/client/src/components/Bookmarks/BookmarkEditDialog.tsx b/client/src/components/Bookmarks/BookmarkEditDialog.tsx index 3b85edce4..2764cf4b8 100644 --- a/client/src/components/Bookmarks/BookmarkEditDialog.tsx +++ b/client/src/components/Bookmarks/BookmarkEditDialog.tsx @@ -1,12 +1,17 @@ -import React, { useRef, useState, Dispatch, SetStateAction } from 'react'; +import React, { useRef, Dispatch, SetStateAction } from 'react'; import { TConversationTag, TConversation } from 'librechat-data-provider'; import OGDialogTemplate from '~/components/ui/OGDialogTemplate'; -import { OGDialog, OGDialogClose } from '~/components/ui/'; +import { useConversationTagMutation } from '~/data-provider'; +import { NotificationSeverity } from '~/common'; +import { useToastContext } from '~/Providers'; +import { OGDialog } from '~/components/ui'; +import { Spinner } from '~/components/svg'; import BookmarkForm from './BookmarkForm'; import { useLocalize } from '~/hooks'; -import { Spinner } from '../svg'; +import { logger } from '~/utils'; type BookmarkEditDialogProps = { + context: string; bookmark?: TConversationTag; conversation?: TConversation; tags?: string[]; @@ -16,6 +21,7 @@ type BookmarkEditDialogProps = { }; const BookmarkEditDialog = ({ + context, bookmark, conversation, tags, @@ -24,9 +30,40 @@ const BookmarkEditDialog = ({ setOpen, }: BookmarkEditDialogProps) => { const localize = useLocalize(); - const [isLoading, setIsLoading] = useState(false); const formRef = useRef(null); + const { showToast } = useToastContext(); + const mutation = useConversationTagMutation({ + context, + tag: bookmark?.tag, + options: { + onSuccess: (_data, vars) => { + showToast({ + message: bookmark + ? localize('com_ui_bookmarks_update_success') + : localize('com_ui_bookmarks_create_success'), + }); + setOpen(false); + logger.log('tag_mutation', 'tags before setting', tags); + if (setTags && vars.addToConversation === true) { + const newTags = [...(tags || []), vars.tag].filter( + (tag) => tag !== undefined, + ) as string[]; + setTags(newTags); + logger.log('tag_mutation', 'tags after', newTags); + } + }, + onError: () => { + showToast({ + message: bookmark + ? localize('com_ui_bookmarks_update_error') + : localize('com_ui_bookmarks_create_error'), + severity: NotificationSeverity.ERROR, + }); + }, + }, + }); + const handleSubmitForm = () => { if (formRef.current) { formRef.current.dispatchEvent(new Event('submit', { cancelable: true, bubbles: true })); @@ -40,26 +77,23 @@ const BookmarkEditDialog = ({ showCloseButton={false} main={ } buttons={ - - - + } /> diff --git a/client/src/components/Bookmarks/BookmarkForm.tsx b/client/src/components/Bookmarks/BookmarkForm.tsx index c5c96f414..ea35b423c 100644 --- a/client/src/components/Bookmarks/BookmarkForm.tsx +++ b/client/src/components/Bookmarks/BookmarkForm.tsx @@ -1,41 +1,39 @@ import React, { useEffect } from 'react'; +import { QueryKeys } from 'librechat-data-provider'; import { Controller, useForm } from 'react-hook-form'; +import { useQueryClient } from '@tanstack/react-query'; import type { - TConversationTag, TConversation, + TConversationTag, TConversationTagRequest, } from 'librechat-data-provider'; -import { cn, removeFocusOutlines, defaultTextProps } from '~/utils/'; +import { cn, removeFocusOutlines, defaultTextProps, logger } from '~/utils'; +import { Checkbox, Label, TextareaAutosize } from '~/components/ui'; import { useBookmarkContext } from '~/Providers/BookmarkContext'; import { useConversationTagMutation } from '~/data-provider'; -import { Checkbox, Label, TextareaAutosize } from '~/components/ui/'; -import { useLocalize, useBookmarkSuccess } from '~/hooks'; -import { NotificationSeverity } from '~/common'; import { useToastContext } from '~/Providers'; +import { useLocalize } from '~/hooks'; type TBookmarkFormProps = { + tags?: string[]; bookmark?: TConversationTag; conversation?: TConversation; - onOpenChange: React.Dispatch>; formRef: React.RefObject; - setIsLoading: React.Dispatch>; - tags?: string[]; - setTags?: (tags: string[]) => void; + setOpen: React.Dispatch>; + mutation: ReturnType; }; const BookmarkForm = ({ - bookmark, - conversation, - onOpenChange, - formRef, - setIsLoading, tags, - setTags, + bookmark, + mutation, + conversation, + setOpen, + formRef, }: TBookmarkFormProps) => { const localize = useLocalize(); + const queryClient = useQueryClient(); const { showToast } = useToastContext(); const { bookmarks } = useBookmarkContext(); - const mutation = useConversationTagMutation(bookmark?.tag); - const onSuccess = useBookmarkSuccess(conversation?.conversationId || ''); const { register, @@ -46,56 +44,47 @@ const BookmarkForm = ({ formState: { errors }, } = useForm({ defaultValues: { - tag: bookmark?.tag || '', - description: bookmark?.description || '', - conversationId: conversation?.conversationId || '', + tag: bookmark?.tag ?? '', + description: bookmark?.description ?? '', + conversationId: conversation?.conversationId ?? '', addToConversation: conversation ? true : false, }, }); useEffect(() => { - if (bookmark) { - setValue('tag', bookmark.tag || ''); - setValue('description', bookmark.description || ''); + if (bookmark && bookmark.tag) { + setValue('tag', bookmark.tag); + setValue('description', bookmark.description ?? ''); } }, [bookmark, setValue]); const onSubmit = (data: TConversationTagRequest) => { + logger.log('tag_mutation', 'BookmarkForm - onSubmit: data', data); if (mutation.isLoading) { return; } if (data.tag === bookmark?.tag && data.description === bookmark?.description) { return; } + if (data.tag != null && (tags ?? []).includes(data.tag)) { + showToast({ + message: localize('com_ui_bookmarks_create_exists'), + status: 'warning', + }); + return; + } + const allTags = + queryClient.getQueryData([QueryKeys.conversationTags]) ?? []; + if (allTags.some((tag) => tag.tag === data.tag)) { + showToast({ + message: localize('com_ui_bookmarks_create_exists'), + status: 'warning', + }); + return; + } - setIsLoading(true); - mutation.mutate(data, { - onSuccess: () => { - showToast({ - message: bookmark - ? localize('com_ui_bookmarks_update_success') - : localize('com_ui_bookmarks_create_success'), - }); - setIsLoading(false); - onOpenChange(false); - if (setTags && data.addToConversation) { - const newTags = [...(tags || []), data.tag].filter( - (tag) => tag !== undefined, - ) as string[]; - setTags(newTags); - onSuccess(newTags); - } - }, - onError: () => { - showToast({ - message: bookmark - ? localize('com_ui_bookmarks_update_error') - : localize('com_ui_bookmarks_create_error'), - severity: NotificationSeverity.ERROR, - }); - setIsLoading(false); - }, - }); + mutation.mutate(data); + setOpen(false); }; return ( @@ -175,10 +164,11 @@ const BookmarkForm = ({ )} />