🏷️ fix: Editing Bookmark Descriptions (#10685)

* fix: exclude the currently edited bookmark from duplicate checks

* ci: Add comprehensive tests for BookmarkForm component

- Introduced a new test suite for the BookmarkForm component to validate bookmark editing functionality.
- Implemented tests for various scenarios including editing descriptions, renaming tags, and handling duplicate tags.
- Ensured proper feedback through toasts for error cases and successful submissions.
- Mocked necessary hooks and context to isolate component behavior during tests.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>

client/src/components/Bookmarks/BookmarkForm.tsx

@@ -72,7 +72,7 @@ const BookmarkForm = ({
     }
     const allTags =
       queryClient.getQueryData<TConversationTag[]>([QueryKeys.conversationTags]) ?? [];
-    if (allTags.some((tag) => tag.tag === data.tag)) {
+    if (allTags.some((tag) => tag.tag === data.tag && tag.tag !== bookmark?.tag)) {
       showToast({
         message: localize('com_ui_bookmarks_create_exists'),
         status: 'warning',

client/src/components/Bookmarks/__tests__/BookmarkForm.test.tsx

@@ -0,0 +1,499 @@
+import React, { createRef } from 'react';
+import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
+import '@testing-library/jest-dom/extend-expect';
+import BookmarkForm from '../BookmarkForm';
+import type { TConversationTag } from 'librechat-data-provider';
+
+const mockMutate = jest.fn();
+const mockShowToast = jest.fn();
+const mockGetQueryData = jest.fn();
+const mockSetOpen = jest.fn();
+
+jest.mock('~/hooks', () => ({
+  useLocalize: () => (key: string, params?: Record<string, unknown>) => {
+    const translations: Record<string, string> = {
+      com_ui_bookmarks_title: 'Title',
+      com_ui_bookmarks_description: 'Description',
+      com_ui_bookmarks_edit: 'Edit Bookmark',
+      com_ui_bookmarks_new: 'New Bookmark',
+      com_ui_bookmarks_create_exists: 'This bookmark already exists',
+      com_ui_bookmarks_add_to_conversation: 'Add to current conversation',
+      com_ui_bookmarks_tag_exists: 'A bookmark with this title already exists',
+      com_ui_field_required: 'This field is required',
+      com_ui_field_max_length: `${params?.field || 'Field'} must be less than ${params?.length || 0} characters`,
+    };
+    return translations[key] || key;
+  },
+}));
+
+jest.mock('@librechat/client', () => {
+  const ActualReact = jest.requireActual<typeof import('react')>('react');
+  return {
+    Checkbox: ({
+      checked,
+      onCheckedChange,
+      value,
+      ...props
+    }: {
+      checked: boolean;
+      onCheckedChange: (checked: boolean) => void;
+      value: string;
+    }) =>
+      ActualReact.createElement('input', {
+        type: 'checkbox',
+        checked,
+        onChange: (e: React.ChangeEvent<HTMLInputElement>) => onCheckedChange(e.target.checked),
+        value,
+        ...props,
+      }),
+    Label: ({ children, ...props }: { children: React.ReactNode }) =>
+      ActualReact.createElement('label', props, children),
+    TextareaAutosize: ActualReact.forwardRef<
+      HTMLTextAreaElement,
+      React.TextareaHTMLAttributes<HTMLTextAreaElement>
+    >((props, ref) => ActualReact.createElement('textarea', { ref, ...props })),
+    Input: ActualReact.forwardRef<HTMLInputElement, React.InputHTMLAttributes<HTMLInputElement>>(
+      (props, ref) => ActualReact.createElement('input', { ref, ...props }),
+    ),
+    useToastContext: () => ({
+      showToast: mockShowToast,
+    }),
+  };
+});
+
+jest.mock('~/Providers/BookmarkContext', () => ({
+  useBookmarkContext: () => ({
+    bookmarks: [],
+  }),
+}));
+
+jest.mock('@tanstack/react-query', () => ({
+  useQueryClient: () => ({
+    getQueryData: mockGetQueryData,
+  }),
+}));
+
+jest.mock('~/utils', () => ({
+  cn: (...classes: (string | undefined | null | boolean)[]) => classes.filter(Boolean).join(' '),
+  logger: {
+    log: jest.fn(),
+  },
+}));
+
+const createMockBookmark = (overrides?: Partial<TConversationTag>): TConversationTag => ({
+  _id: 'bookmark-1',
+  user: 'user-1',
+  tag: 'Test Bookmark',
+  description: 'Test description',
+  createdAt: '2024-01-01T00:00:00.000Z',
+  updatedAt: '2024-01-01T00:00:00.000Z',
+  count: 1,
+  position: 0,
+  ...overrides,
+});
+
+const createMockMutation = (isLoading = false) => ({
+  mutate: mockMutate,
+  isLoading,
+  isError: false,
+  isSuccess: false,
+  data: undefined,
+  error: null,
+  reset: jest.fn(),
+  mutateAsync: jest.fn(),
+  status: 'idle' as const,
+  variables: undefined,
+  context: undefined,
+  failureCount: 0,
+  failureReason: null,
+  isPaused: false,
+  isIdle: true,
+  submittedAt: 0,
+});
+
+describe('BookmarkForm - Bookmark Editing', () => {
+  const formRef = createRef<HTMLFormElement>();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockGetQueryData.mockReturnValue([]);
+  });
+
+  describe('Editing only the description (tag unchanged)', () => {
+    it('should allow submitting when only the description is changed', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'My Bookmark',
+        description: 'Original description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const descriptionInput = screen.getByRole('textbox', { name: /description/i });
+
+      await act(async () => {
+        fireEvent.change(descriptionInput, { target: { value: 'Updated description' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).toHaveBeenCalledWith(
+          expect.objectContaining({
+            tag: 'My Bookmark',
+            description: 'Updated description',
+          }),
+        );
+      });
+      expect(mockShowToast).not.toHaveBeenCalled();
+      expect(mockSetOpen).toHaveBeenCalledWith(false);
+    });
+
+    it('should not submit when both tag and description are unchanged', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'My Bookmark',
+        description: 'Same description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).not.toHaveBeenCalled();
+      });
+      expect(mockSetOpen).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('Renaming a tag to an existing tag (should show error)', () => {
+    it('should show error toast when renaming to an existing tag name (via allTags)', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'Original Tag',
+        description: 'Description',
+      });
+
+      const otherBookmark = createMockBookmark({
+        _id: 'bookmark-2',
+        tag: 'Existing Tag',
+        description: 'Other description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark, otherBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const tagInput = screen.getByLabelText('Edit Bookmark');
+
+      await act(async () => {
+        fireEvent.change(tagInput, { target: { value: 'Existing Tag' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockShowToast).toHaveBeenCalledWith({
+          message: 'This bookmark already exists',
+          status: 'warning',
+        });
+      });
+      expect(mockMutate).not.toHaveBeenCalled();
+      expect(mockSetOpen).not.toHaveBeenCalled();
+    });
+
+    it('should show error toast when renaming to an existing tag name (via tags prop)', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'Original Tag',
+        description: 'Description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          tags={['Existing Tag', 'Another Tag']}
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const tagInput = screen.getByLabelText('Edit Bookmark');
+
+      await act(async () => {
+        fireEvent.change(tagInput, { target: { value: 'Existing Tag' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockShowToast).toHaveBeenCalledWith({
+          message: 'This bookmark already exists',
+          status: 'warning',
+        });
+      });
+      expect(mockMutate).not.toHaveBeenCalled();
+      expect(mockSetOpen).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('Renaming a tag to a new tag (should succeed)', () => {
+    it('should allow renaming to a completely new tag name', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'Original Tag',
+        description: 'Description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const tagInput = screen.getByLabelText('Edit Bookmark');
+
+      await act(async () => {
+        fireEvent.change(tagInput, { target: { value: 'Brand New Tag' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).toHaveBeenCalledWith(
+          expect.objectContaining({
+            tag: 'Brand New Tag',
+            description: 'Description',
+          }),
+        );
+      });
+      expect(mockShowToast).not.toHaveBeenCalled();
+      expect(mockSetOpen).toHaveBeenCalledWith(false);
+    });
+
+    it('should allow keeping the same tag name when editing (not trigger duplicate error)', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'My Bookmark',
+        description: 'Original description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const descriptionInput = screen.getByRole('textbox', { name: /description/i });
+
+      await act(async () => {
+        fireEvent.change(descriptionInput, { target: { value: 'New description' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).toHaveBeenCalledWith(
+          expect.objectContaining({
+            tag: 'My Bookmark',
+            description: 'New description',
+          }),
+        );
+      });
+      expect(mockShowToast).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('Validation interaction between different data sources', () => {
+    it('should check both tags prop and allTags query data for duplicates', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'Original Tag',
+        description: 'Description',
+      });
+
+      const queryDataBookmark = createMockBookmark({
+        _id: 'bookmark-query',
+        tag: 'Query Data Tag',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark, queryDataBookmark]);
+
+      render(
+        <BookmarkForm
+          tags={['Props Tag']}
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const tagInput = screen.getByLabelText('Edit Bookmark');
+
+      await act(async () => {
+        fireEvent.change(tagInput, { target: { value: 'Props Tag' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockShowToast).toHaveBeenCalledWith({
+          message: 'This bookmark already exists',
+          status: 'warning',
+        });
+      });
+      expect(mockMutate).not.toHaveBeenCalled();
+    });
+
+    it('should not trigger mutation when mutation is loading', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'My Bookmark',
+        description: 'Description',
+      });
+
+      mockGetQueryData.mockReturnValue([existingBookmark]);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation(true) as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const descriptionInput = screen.getByRole('textbox', { name: /description/i });
+
+      await act(async () => {
+        fireEvent.change(descriptionInput, { target: { value: 'Updated description' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).not.toHaveBeenCalled();
+      });
+    });
+
+    it('should handle empty allTags gracefully', async () => {
+      const existingBookmark = createMockBookmark({
+        tag: 'My Bookmark',
+        description: 'Description',
+      });
+
+      mockGetQueryData.mockReturnValue(null);
+
+      render(
+        <BookmarkForm
+          bookmark={existingBookmark}
+          mutation={
+            createMockMutation() as ReturnType<
+              typeof import('~/data-provider').useConversationTagMutation
+            >
+          }
+          setOpen={mockSetOpen}
+          formRef={formRef}
+        />,
+      );
+
+      const tagInput = screen.getByLabelText('Edit Bookmark');
+
+      await act(async () => {
+        fireEvent.change(tagInput, { target: { value: 'New Tag' } });
+      });
+
+      await act(async () => {
+        fireEvent.submit(formRef.current!);
+      });
+
+      await waitFor(() => {
+        expect(mockMutate).toHaveBeenCalledWith(
+          expect.objectContaining({
+            tag: 'New Tag',
+          }),
+        );
+      });
+    });
+  });
+});