Skip to content

Commit

Permalink
🏷️ fix: Address Statefulness Issues for Bookmarks (#3590)
Browse files Browse the repository at this point in the history
* refactor: optimize tag methods, remove rebuild

* refactor(tags): add lean db operations, fix updateTagsForConversation, remove rebuild button, only send convoId once

* refactor: Update BookmarkMenu to use Constants.NEW_CONVO constant for comparison

* style: Update BookmarkMenu styles and constants, use theming

* refactor: move tags query from package to client workspace

* refactor: optimize ConversationTag document creation and update logic

* style: Update BookmarkMenuItems to use theming

* refactor: JSDocs + try/catch for conversation tags API routes

* refactor: Update BookmarkNav theming classes and new data provider location

* fix: statefulness of conversation bookmarks
- move non-mutation hook to hooks/Conversation
- remove use of deprecated global convo
- update convo infinite data as well as current convo state upon successful tag add

* refactor: Update BookmarkMenu styles and constants, use theming

* refactor: Add lean option to ConversationTag deletion query

* fix(BookmarkTable): position order rendering esp. when new tag is created

* refactor: Update useBookmarkSucess to useBookmarkSuccess for consistency

* refactor: Update ConversationTag creation logic to increment count only if addToConversation is true

* style: theming
  • Loading branch information
danny-avila authored Aug 9, 2024
1 parent 6ea2628 commit 016ed86
Show file tree
Hide file tree
Showing 28 changed files with 607 additions and 521 deletions.
426 changes: 203 additions & 223 deletions api/models/ConversationTag.js

Large diffs are not rendered by default.

13 changes: 11 additions & 2 deletions api/server/routes/convos.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,17 @@ router.post('/fork', async (req, res) => {
});

router.put('/tags/:conversationId', async (req, res) => {
const tag = await updateTagsForConversation(req.user.id, req.params.conversationId, req.body);
res.status(200).json(tag);
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;
82 changes: 63 additions & 19 deletions api/server/routes/tags.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,88 @@
const express = require('express');

const {
getConversationTags,
updateConversationTag,
createConversationTag,
deleteConversationTag,
rebuildConversationTags,
} = require('~/models/ConversationTag');
const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
const router = express.Router();
router.use(requireJwtAuth);

/**
* GET /
* Retrieves all conversation tags for the authenticated user.
* @param {Object} req - Express request object
* @param {Object} res - Express response object
*/
router.get('/', async (req, res) => {
const tags = await getConversationTags(req.user.id);

if (tags) {
res.status(200).json(tags);
} else {
res.status(404).end();
try {
const tags = await getConversationTags(req.user.id);
if (tags) {
res.status(200).json(tags);
} else {
res.status(404).end();
}
} catch (error) {
console.error('Error getting conversation tags:', error);
res.status(500).json({ error: 'Internal server error' });
}
});

/**
* POST /
* Creates a new conversation tag for the authenticated user.
* @param {Object} req - Express request object
* @param {Object} res - Express response object
*/
router.post('/', async (req, res) => {
const tag = await createConversationTag(req.user.id, req.body);
res.status(200).json(tag);
});

router.post('/rebuild', async (req, res) => {
const tag = await rebuildConversationTags(req.user.id);
res.status(200).json(tag);
try {
const tag = await createConversationTag(req.user.id, req.body);
res.status(200).json(tag);
} catch (error) {
console.error('Error creating conversation tag:', error);
res.status(500).json({ error: 'Internal server error' });
}
});

/**
* PUT /:tag
* Updates an existing conversation tag for the authenticated user.
* @param {Object} req - Express request object
* @param {Object} res - Express response object
*/
router.put('/:tag', async (req, res) => {
const tag = await updateConversationTag(req.user.id, req.params.tag, req.body);
res.status(200).json(tag);
try {
const tag = await updateConversationTag(req.user.id, req.params.tag, req.body);
if (tag) {
res.status(200).json(tag);
} else {
res.status(404).json({ error: 'Tag not found' });
}
} catch (error) {
console.error('Error updating conversation tag:', error);
res.status(500).json({ error: 'Internal server error' });
}
});

/**
* DELETE /:tag
* Deletes a conversation tag for the authenticated user.
* @param {Object} req - Express request object
* @param {Object} res - Express response object
*/
router.delete('/:tag', async (req, res) => {
const tag = await deleteConversationTag(req.user.id, req.params.tag);
res.status(200).json(tag);
try {
const tag = await deleteConversationTag(req.user.id, req.params.tag);
if (tag) {
res.status(200).json(tag);
} else {
res.status(404).json({ error: 'Tag not found' });
}
} catch (error) {
console.error('Error deleting conversation tag:', error);
res.status(500).json({ error: 'Internal server error' });
}
});

module.exports = router;
14 changes: 8 additions & 6 deletions client/src/components/Bookmarks/BookmarkForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { cn, removeFocusOutlines, defaultTextProps } from '~/utils/';
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 = {
bookmark?: TConversationTag;
Expand All @@ -31,10 +31,11 @@ const BookmarkForm = ({
tags,
setTags,
}: TBookmarkFormProps) => {
const { showToast } = useToastContext();
const localize = useLocalize();
const mutation = useConversationTagMutation(bookmark?.tag);
const { showToast } = useToastContext();
const { bookmarks } = useBookmarkContext();
const mutation = useConversationTagMutation(bookmark?.tag);
const onSuccess = useBookmarkSuccess(conversation?.conversationId || '');

const {
register,
Expand Down Expand Up @@ -82,6 +83,7 @@ const BookmarkForm = ({
(tag) => tag !== undefined,
) as string[];
setTags(newTags);
onSuccess(newTags);
}
},
onError: () => {
Expand Down Expand Up @@ -172,9 +174,9 @@ const BookmarkForm = ({
/>
)}
/>
<label
<button
aria-label={localize('com_ui_bookmarks_add_to_conversation')}
className="form-check-label text-token-text-primary w-full cursor-pointer"
htmlFor="addToConversation"
onClick={() =>
setValue('addToConversation', !getValues('addToConversation'), {
shouldDirty: true,
Expand All @@ -184,7 +186,7 @@ const BookmarkForm = ({
<div className="flex select-none items-center">
{localize('com_ui_bookmarks_add_to_conversation')}
</div>
</label>
</button>
</div>
)}
</div>
Expand Down
45 changes: 27 additions & 18 deletions client/src/components/Bookmarks/BookmarkItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { cn } from '~/utils';
type MenuItemProps = {
tag: string | React.ReactNode;
selected: boolean;
ctx: 'header' | 'nav';
count?: number;
handleSubmit: (tag: string) => Promise<void>;
icon?: React.ReactNode;
Expand All @@ -15,6 +16,7 @@ type MenuItemProps = {

const BookmarkItem: FC<MenuItemProps> = ({
tag,
ctx,
selected,
count,
handleSubmit,
Expand All @@ -34,47 +36,54 @@ const BookmarkItem: FC<MenuItemProps> = ({
overflowWrap: 'anywhere',
};

const renderIcon = () => {
if (icon) {
return icon;
}
if (isLoading) {
return <Spinner className="size-4" />;
}
if (selected) {
return <BookmarkFilledIcon className="size-4" />;
}
return <BookmarkIcon className="size-4" />;
};

const ariaLabel =
ctx === 'header' ? `${selected ? 'Remove' : 'Add'} bookmark for ${tag}` : (tag as string);

return (
<div
<button
aria-label={ariaLabel}
role="menuitem"
className={cn(
'group m-1.5 flex w-[225px] cursor-pointer gap-2 rounded px-2 py-2.5 !pr-3 text-sm !opacity-100 focus:ring-0 radix-disabled:pointer-events-none radix-disabled:opacity-50',
'hover:bg-black/5 dark:hover:bg-white/5',
highlightSelected && selected && 'bg-black/5 dark:bg-white/5',
'group m-1.5 flex w-[225px] cursor-pointer gap-2 rounded bg-transparent px-2 py-2.5 !pr-3 text-sm !opacity-100 focus:ring-0 radix-disabled:pointer-events-none radix-disabled:opacity-50',
highlightSelected && selected ? 'bg-surface-secondary' : '',
ctx === 'header' ? 'hover:bg-header-hover' : 'hover:bg-surface-hover',
)}
tabIndex={-1}
{...rest}
onClick={clickHandler}
>
<div className="flex grow items-center justify-between gap-2">
<div className="flex items-center gap-2">
{icon ? (
icon
) : isLoading ? (
<Spinner className="size-4" />
) : selected ? (
<BookmarkFilledIcon className="size-4" />
) : (
<BookmarkIcon className="size-4" />
)}
{renderIcon()}
<div style={breakWordStyle}>{tag}</div>
</div>

{count !== undefined && (
<div className="flex items-center justify-end">
<span
className={cn(
'ml-auto w-7 min-w-max whitespace-nowrap rounded-md bg-white px-2.5 py-0.5 text-center text-xs font-medium leading-5 text-gray-600',
'dark:bg-gray-800 dark:text-white',
)}
className="ml-auto w-7 min-w-max whitespace-nowrap rounded-md bg-surface-secondary px-2.5 py-0.5 text-center text-xs font-medium leading-5 text-text-secondary"
aria-hidden="true"
>
{count}
</span>
</div>
)}
</div>
</div>
</button>
);
};

export default BookmarkItem;
5 changes: 4 additions & 1 deletion client/src/components/Bookmarks/BookmarkItems.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import type { FC } from 'react';
import { useBookmarkContext } from '~/Providers/BookmarkContext';
import BookmarkItem from './BookmarkItem';
interface BookmarkItemsProps {
ctx: 'header' | 'nav';
tags: string[];
handleSubmit: (tag: string) => Promise<void>;
header: React.ReactNode;
highlightSelected?: boolean;
}

const BookmarkItems: FC<BookmarkItemsProps> = ({
ctx,
tags,
handleSubmit,
header,
Expand All @@ -19,9 +21,10 @@ const BookmarkItems: FC<BookmarkItemsProps> = ({
return (
<>
{header}
<div className="my-1.5 h-px bg-black/10 dark:bg-white/10" role="none" />
<div className="my-1.5 h-px" role="none" />
{bookmarks.map((bookmark) => (
<BookmarkItem
ctx={ctx}
key={bookmark.tag}
tag={bookmark.tag}
selected={tags.includes(bookmark.tag)}
Expand Down
8 changes: 4 additions & 4 deletions client/src/components/Chat/Input/Files/FileContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ const FileContainer = ({
const fileType = getFileType(file.type);

return (
<div className="group relative inline-block text-sm text-black/70 dark:text-white/90">
<div className="relative overflow-hidden rounded-xl border border-gray-200 dark:border-gray-600">
<div className="w-60 p-2 dark:bg-gray-600">
<div className="group relative inline-block text-sm text-text-primary">
<div className="relative overflow-hidden rounded-xl border border-border-medium">
<div className="w-60 bg-surface-active p-2">
<div className="flex flex-row items-center gap-2">
<FilePreview file={file} fileType={fileType} className="relative" />
<div className="overflow-hidden">
<div className="truncate font-medium">{file.filename}</div>
<div className="truncate text-gray-300">{fileType.title}</div>
<div className="truncate text-text-secondary">{fileType.title}</div>
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions client/src/components/Chat/Input/OptionsPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default function OptionsPopover({
{presetsDisabled ? null : (
<Button
type="button"
className="h-auto w-[150px] justify-start rounded-md border border-gray-300/50 bg-transparent px-2 py-1 text-xs font-medium font-normal text-black hover:bg-gray-100 hover:text-black focus:ring-1 focus:ring-green-500/90 dark:border-gray-500/50 dark:bg-transparent dark:text-white dark:hover:bg-gray-600 dark:focus:ring-white"
className="h-auto w-[150px] justify-start rounded-md border border-gray-300/50 bg-transparent px-2 py-1 text-xs font-normal text-black hover:bg-gray-100 hover:text-black focus:ring-1 focus:ring-ring-primary dark:border-gray-500/50 dark:bg-transparent dark:text-white dark:hover:bg-gray-600 dark:focus:ring-white"
onClick={saveAsPreset}
>
<Save className="mr-1 w-[14px]" />
Expand All @@ -77,7 +77,7 @@ export default function OptionsPopover({
<Button
type="button"
className={cn(
'ml-auto h-auto bg-transparent px-3 py-2 text-xs font-medium font-normal text-black hover:bg-gray-100 hover:text-black dark:bg-transparent dark:text-white dark:hover:bg-gray-700 dark:hover:text-white',
'ml-auto h-auto bg-transparent px-3 py-2 text-xs font-normal text-black hover:bg-gray-100 hover:text-black dark:bg-transparent dark:text-white dark:hover:bg-gray-700 dark:hover:text-white',
removeFocusOutlines,
)}
onClick={closePopover}
Expand Down
Loading

0 comments on commit 016ed86

Please sign in to comment.