feat: Enhance Favorites functionality with validation, cleanup, and improved error handling

This commit is contained in:
Marco Beretta 2025-12-02 00:15:07 +01:00
parent c006630a45
commit f6881d91f1
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
5 changed files with 128 additions and 36 deletions

View file

@ -1,5 +1,8 @@
const { updateUser, getUserById } = require('~/models'); const { updateUser, getUserById } = require('~/models');
const MAX_FAVORITES = 50;
const MAX_STRING_LENGTH = 256;
const updateFavoritesController = async (req, res) => { const updateFavoritesController = async (req, res) => {
try { try {
const { favorites } = req.body; const { favorites } = req.body;
@ -13,10 +16,30 @@ const updateFavoritesController = async (req, res) => {
return res.status(400).json({ message: 'Favorites must be an array' }); return res.status(400).json({ message: 'Favorites must be an array' });
} }
if (favorites.length > MAX_FAVORITES) {
return res.status(400).json({ message: `Maximum ${MAX_FAVORITES} favorites allowed` });
}
for (const fav of favorites) { for (const fav of favorites) {
const hasAgent = !!fav.agentId; const hasAgent = !!fav.agentId;
const hasModel = !!(fav.model && fav.endpoint); const hasModel = !!(fav.model && fav.endpoint);
if (fav.agentId && fav.agentId.length > MAX_STRING_LENGTH) {
return res
.status(400)
.json({ message: `agentId exceeds maximum length of ${MAX_STRING_LENGTH}` });
}
if (fav.model && fav.model.length > MAX_STRING_LENGTH) {
return res
.status(400)
.json({ message: `model exceeds maximum length of ${MAX_STRING_LENGTH}` });
}
if (fav.endpoint && fav.endpoint.length > MAX_STRING_LENGTH) {
return res
.status(400)
.json({ message: `endpoint exceeds maximum length of ${MAX_STRING_LENGTH}` });
}
if (!hasAgent && !hasModel) { if (!hasAgent && !hasModel) {
return res.status(400).json({ return res.status(400).json({
message: 'Each favorite must have either agentId or model+endpoint', message: 'Each favorite must have either agentId or model+endpoint',

View file

@ -3,8 +3,8 @@ import { EarthIcon, Pin, PinOff } from 'lucide-react';
import { isAgentsEndpoint, isAssistantsEndpoint } from 'librechat-data-provider'; import { isAgentsEndpoint, isAssistantsEndpoint } from 'librechat-data-provider';
import { useModelSelectorContext } from '../ModelSelectorContext'; import { useModelSelectorContext } from '../ModelSelectorContext';
import { CustomMenuItem as MenuItem } from '../CustomMenu'; import { CustomMenuItem as MenuItem } from '../CustomMenu';
import { useFavorites, useLocalize } from '~/hooks';
import type { Endpoint } from '~/common'; import type { Endpoint } from '~/common';
import { useFavorites } from '~/hooks';
import { cn } from '~/utils'; import { cn } from '~/utils';
interface EndpointModelItemProps { interface EndpointModelItemProps {
@ -14,6 +14,7 @@ interface EndpointModelItemProps {
} }
export function EndpointModelItem({ modelId, endpoint, isSelected }: EndpointModelItemProps) { export function EndpointModelItem({ modelId, endpoint, isSelected }: EndpointModelItemProps) {
const localize = useLocalize();
const { handleSelectModel } = useModelSelectorContext(); const { handleSelectModel } = useModelSelectorContext();
const { isFavoriteModel, toggleFavoriteModel, isFavoriteAgent, toggleFavoriteAgent } = const { isFavoriteModel, toggleFavoriteModel, isFavoriteAgent, toggleFavoriteAgent } =
useFavorites(); useFavorites();
@ -94,6 +95,7 @@ export function EndpointModelItem({ modelId, endpoint, isSelected }: EndpointMod
</div> </div>
<button <button
onClick={handleFavoriteClick} onClick={handleFavoriteClick}
aria-label={isFavorite ? localize('com_ui_unpin') : localize('com_ui_pin')}
className={cn( className={cn(
'rounded-md p-1 hover:bg-surface-hover', 'rounded-md p-1 hover:bg-surface-hover',
isFavorite ? 'visible' : 'invisible group-hover:visible', isFavorite ? 'visible' : 'invisible group-hover:visible',

View file

@ -1,19 +1,21 @@
import { useMemo, memo, type FC, useCallback, useEffect, useRef } from 'react'; import { useMemo, memo, type FC, useCallback, useEffect, useRef } from 'react';
import throttle from 'lodash/throttle'; import throttle from 'lodash/throttle';
import { ChevronRight } from 'lucide-react'; import { ChevronRight } from 'lucide-react';
import { TConversation } from 'librechat-data-provider'; import { useRecoilValue } from 'recoil';
import { Spinner, useMediaQuery } from '@librechat/client'; import { Spinner, useMediaQuery } from '@librechat/client';
import { TConversation, PermissionTypes, Permissions } from 'librechat-data-provider';
import { List, AutoSizer, CellMeasurer, CellMeasurerCache } from 'react-virtualized'; import { List, AutoSizer, CellMeasurer, CellMeasurerCache } from 'react-virtualized';
import { useLocalize, TranslationKeys, useFavorites, useHasAccess } from '~/hooks';
import FavoritesList from '~/components/Nav/Favorites/FavoritesList'; import FavoritesList from '~/components/Nav/Favorites/FavoritesList';
import { useLocalize, TranslationKeys, useFavorites } from '~/hooks';
import { groupConversationsByDate, cn } from '~/utils'; import { groupConversationsByDate, cn } from '~/utils';
import Convo from './Convo'; import Convo from './Convo';
import store from '~/store';
interface ConversationsProps { interface ConversationsProps {
conversations: Array<TConversation | null>; conversations: Array<TConversation | null>;
moveToTop: () => void; moveToTop: () => void;
toggleNav: () => void; toggleNav: () => void;
containerRef: React.RefObject<HTMLDivElement | List>; containerRef: React.RefObject<List>;
loadMoreConversations: () => void; loadMoreConversations: () => void;
isLoading: boolean; isLoading: boolean;
isSearchLoading: boolean; isSearchLoading: boolean;
@ -88,10 +90,27 @@ const Conversations: FC<ConversationsProps> = ({
setIsChatsExpanded, setIsChatsExpanded,
}) => { }) => {
const localize = useLocalize(); const localize = useLocalize();
const search = useRecoilValue(store.search);
const { favorites, isLoading: isFavoritesLoading } = useFavorites(); const { favorites, isLoading: isFavoritesLoading } = useFavorites();
const isSmallScreen = useMediaQuery('(max-width: 768px)'); const isSmallScreen = useMediaQuery('(max-width: 768px)');
const convoHeight = isSmallScreen ? 44 : 34; const convoHeight = isSmallScreen ? 44 : 34;
const hasAccessToAgents = useHasAccess({
permissionType: PermissionTypes.AGENTS,
permission: Permissions.USE,
});
const hasAccessToMarketplace = useHasAccess({
permissionType: PermissionTypes.MARKETPLACE,
permission: Permissions.USE,
});
const showAgentMarketplace = hasAccessToAgents && hasAccessToMarketplace;
// Determine if FavoritesList will render content
const shouldShowFavorites =
!search.query && (isFavoritesLoading || favorites.length > 0 || showAgentMarketplace);
const filteredConversations = useMemo( const filteredConversations = useMemo(
() => rawConversations.filter(Boolean) as TConversation[], () => rawConversations.filter(Boolean) as TConversation[],
[rawConversations], [rawConversations],
@ -104,7 +123,10 @@ const Conversations: FC<ConversationsProps> = ({
const flattenedItems = useMemo(() => { const flattenedItems = useMemo(() => {
const items: FlattenedItem[] = []; const items: FlattenedItem[] = [];
items.push({ type: 'favorites' }); // Only include favorites row if FavoritesList will render content
if (shouldShowFavorites) {
items.push({ type: 'favorites' });
}
items.push({ type: 'chats-header' }); items.push({ type: 'chats-header' });
if (isChatsExpanded) { if (isChatsExpanded) {
@ -118,7 +140,7 @@ const Conversations: FC<ConversationsProps> = ({
} }
} }
return items; return items;
}, [groupedConversations, isLoading, isChatsExpanded]); }, [groupedConversations, isLoading, isChatsExpanded, shouldShowFavorites]);
// Store flattenedItems in a ref for keyMapper to access without recreating cache // Store flattenedItems in a ref for keyMapper to access without recreating cache
const flattenedItemsRef = useRef(flattenedItems); const flattenedItemsRef = useRef(flattenedItems);
@ -214,7 +236,11 @@ const Conversations: FC<ConversationsProps> = ({
</button> </button>
); );
} else if (item.type === 'header') { } else if (item.type === 'header') {
rendering = <DateLabel groupName={item.groupName} isFirst={index === 2} />; // First date header index depends on whether favorites row is included
// With favorites: [favorites, chats-header, first-header] → index 2
// Without favorites: [chats-header, first-header] → index 1
const firstHeaderIndex = shouldShowFavorites ? 2 : 1;
rendering = <DateLabel groupName={item.groupName} isFirst={index === firstHeaderIndex} />;
} else if (item.type === 'convo') { } else if (item.type === 'convo') {
rendering = ( rendering = (
<MemoizedConvo conversation={item.convo} retainView={moveToTop} toggleNav={toggleNav} /> <MemoizedConvo conversation={item.convo} retainView={moveToTop} toggleNav={toggleNav} />
@ -230,7 +256,18 @@ const Conversations: FC<ConversationsProps> = ({
</CellMeasurer> </CellMeasurer>
); );
}, },
[cache, flattenedItems, moveToTop, toggleNav, clearFavoritesCache, isSmallScreen], [
cache,
flattenedItems,
moveToTop,
toggleNav,
clearFavoritesCache,
isSmallScreen,
isChatsExpanded,
localize,
setIsChatsExpanded,
shouldShowFavorites,
],
); );
const getRowHeight = useCallback( const getRowHeight = useCallback(
@ -264,7 +301,7 @@ const Conversations: FC<ConversationsProps> = ({
<AutoSizer> <AutoSizer>
{({ width, height }) => ( {({ width, height }) => (
<List <List
ref={containerRef as React.RefObject<List>} ref={containerRef}
width={width} width={width}
height={height} height={height}
deferredMeasurementCache={cache} deferredMeasurementCache={cache}

View file

@ -1,5 +1,6 @@
import { useCallback, useEffect, useState, useMemo, memo, lazy, Suspense, useRef } from 'react'; import { useCallback, useEffect, useState, useMemo, memo, lazy, Suspense, useRef } from 'react';
import { useRecoilValue } from 'recoil'; import { useRecoilValue } from 'recoil';
import { List } from 'react-virtualized';
import { AnimatePresence, motion } from 'framer-motion'; import { AnimatePresence, motion } from 'framer-motion';
import { Skeleton, useMediaQuery } from '@librechat/client'; import { Skeleton, useMediaQuery } from '@librechat/client';
import { PermissionTypes, Permissions } from 'librechat-data-provider'; import { PermissionTypes, Permissions } from 'librechat-data-provider';
@ -100,7 +101,7 @@ const Nav = memo(
}, [data?.pages]); }, [data?.pages]);
const outerContainerRef = useRef<HTMLDivElement>(null); const outerContainerRef = useRef<HTMLDivElement>(null);
const conversationsRef = useRef<HTMLDivElement>(null); const conversationsRef = useRef<List | null>(null);
const { moveToTop } = useNavScrolling<ConversationListResponse>({ const { moveToTop } = useNavScrolling<ConversationListResponse>({
setShowLoading, setShowLoading,

View file

@ -1,7 +1,11 @@
import { useEffect } from 'react'; import { useEffect, useCallback } from 'react';
import { useRecoilState } from 'recoil'; import { useRecoilState } from 'recoil';
import store from '~/store'; import { useToastContext } from '@librechat/client';
import type { Favorite } from '~/store/favorites';
import { useGetFavoritesQuery, useUpdateFavoritesMutation } from '~/data-provider'; import { useGetFavoritesQuery, useUpdateFavoritesMutation } from '~/data-provider';
import { useLocalize } from '~/hooks';
import { logger } from '~/utils';
import store from '~/store';
/** /**
* Hook for managing user favorites (pinned agents and models). * Hook for managing user favorites (pinned agents and models).
@ -14,7 +18,24 @@ import { useGetFavoritesQuery, useUpdateFavoritesMutation } from '~/data-provide
* @returns Object containing favorites state and helper methods for * @returns Object containing favorites state and helper methods for
* adding, removing, toggling, reordering, and checking favorites. * adding, removing, toggling, reordering, and checking favorites.
*/ */
/**
* Cleans favorites array to only include canonical shapes (agentId or model+endpoint).
*/
const cleanFavorites = (favorites: Favorite[]): Favorite[] =>
favorites.map((f) => {
if (f.agentId) {
return { agentId: f.agentId };
}
if (f.model && f.endpoint) {
return { model: f.model, endpoint: f.endpoint };
}
return f;
});
export default function useFavorites() { export default function useFavorites() {
const localize = useLocalize();
const { showToast } = useToastContext();
const [favorites, setFavorites] = useRecoilState(store.favorites); const [favorites, setFavorites] = useRecoilState(store.favorites);
const getFavoritesQuery = useGetFavoritesQuery(); const getFavoritesQuery = useGetFavoritesQuery();
const updateFavoritesMutation = useUpdateFavoritesMutation(); const updateFavoritesMutation = useUpdateFavoritesMutation();
@ -29,15 +50,21 @@ export default function useFavorites() {
} }
}, [getFavoritesQuery.data, setFavorites]); }, [getFavoritesQuery.data, setFavorites]);
const saveFavorites = (newFavorites: typeof favorites) => { const saveFavorites = useCallback(
const cleaned = newFavorites.map((f) => { async (newFavorites: typeof favorites) => {
if (f.agentId) return { agentId: f.agentId }; const cleaned = cleanFavorites(newFavorites);
if (f.model && f.endpoint) return { model: f.model, endpoint: f.endpoint }; setFavorites(cleaned);
return f; try {
}); await updateFavoritesMutation.mutateAsync(cleaned);
setFavorites(cleaned); } catch (error) {
updateFavoritesMutation.mutate(cleaned); logger.error('Error updating favorites:', error);
}; showToast({ message: localize('com_ui_error'), status: 'error' });
// Refetch to resync state with server
getFavoritesQuery.refetch();
}
},
[setFavorites, updateFavoritesMutation, showToast, localize, getFavoritesQuery],
);
const addFavoriteAgent = (agentId: string) => { const addFavoriteAgent = (agentId: string) => {
if (favorites.some((f) => f.agentId === agentId)) return; if (favorites.some((f) => f.agentId === agentId)) return;
@ -89,25 +116,27 @@ export default function useFavorites() {
}; };
/** /**
* Reorder favorites and persist the new order to the server. * Reorder favorites and optionally persist the new order to the server.
* This combines state update and persistence to avoid race conditions * This combines state update and persistence to avoid race conditions
* where the closure captures stale state. * where the closure captures stale state.
*/ */
const reorderFavorites = (newFavorites: typeof favorites, persist = false) => { const reorderFavorites = useCallback(
const cleaned = newFavorites.map((f) => { async (newFavorites: typeof favorites, persist = false) => {
if (f.agentId) { const cleaned = cleanFavorites(newFavorites);
return { agentId: f.agentId }; setFavorites(cleaned);
if (persist) {
try {
await updateFavoritesMutation.mutateAsync(cleaned);
} catch (error) {
console.error('Error reordering favorites:', error);
showToast({ message: localize('com_ui_error'), status: 'error' });
// Refetch to resync state with server
getFavoritesQuery.refetch();
}
} }
if (f.model && f.endpoint) { },
return { model: f.model, endpoint: f.endpoint }; [setFavorites, updateFavoritesMutation, showToast, localize, getFavoritesQuery],
} );
return f;
});
setFavorites(cleaned);
if (persist) {
updateFavoritesMutation.mutate(cleaned);
}
};
return { return {
favorites, favorites,