mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🔗 fix: Render Shared Links Containing Steers (#14480)
* 🔗 fix: Render Shared Links Containing Steers
The /share/:shareId route mounts outside AuthContextProvider, so any
useAuthContext() on that tree throws and the whole page is replaced by the
route error boundary. SteerPart called it directly, and the MessageIcon tree it
renders reaches Endpoints/Icon, which called it too - so fixing only the first
still died on the icon.
Both now read the user atom instead. AuthContextProvider mirrors the user into
it, so authenticated rendering is unchanged, and the share route reads
undefined rather than throwing.
The two crash sites were invisible because the spec mocked both
~/hooks/AuthContext and MessageIcon. Both mocks are gone: the test seeds the
atom and renders the real icon tree, with a case covering the share route
having neither an auth context nor a user.
* fix: sort test imports and assert the real avatar title
The worktree has no node_modules, so the lint-staged sort-imports hook never
ran on the first commit and CI caught the drift.
The icon assertion also used the wrong value: Endpoints/Icon derives the title
from user.name ?? user.username, so the seeded user renders 'Danny', not the
username.
* fix: keep viewer identity off shared steer avatars
store.user is app-wide and survives navigation, so a signed-in viewer opening a
share link still has an identity in state — reading it for the avatar put the
viewer's face on the sharer's steer. The shared branch now renders the generic
avatar, mirroring Share/MessageIcon, while the label guard already handled the
text.
Also fixes the share test, which passed undefined into a defaulted parameter
and so seeded a user anyway, testing the signed-in path it claimed to exclude.
This commit is contained in:
parent
52fcc51b36
commit
ea643e8c9c
3 changed files with 71 additions and 20 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { memo, useMemo, useState, useCallback } from 'react';
|
||||
import { useAtomValue } from 'jotai';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { InfoHoverCard, ESide } from '@librechat/client';
|
||||
import { InfoHoverCard, ESide, UserIcon } from '@librechat/client';
|
||||
import type { TFile, TMessage } from 'librechat-data-provider';
|
||||
import type { TMessageIcon } from '~/common';
|
||||
import FilePreviewDialog from '~/components/Chat/Messages/Content/FilePreviewDialog';
|
||||
|
|
@ -10,7 +10,6 @@ import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite';
|
|||
import FileContainer from '~/components/Chat/Input/Files/FileContainer';
|
||||
import MessageIcon from '~/components/Chat/Messages/MessageIcon';
|
||||
import Image from '~/components/Chat/Messages/Content/Image';
|
||||
import { useAuthContext } from '~/hooks/AuthContext';
|
||||
import { fontSizeAtom } from '~/store/fontSize';
|
||||
import { useShareContext } from '~/Providers';
|
||||
import { useLocalize } from '~/hooks';
|
||||
|
|
@ -41,7 +40,9 @@ const SteerPart = memo(function SteerPart({
|
|||
createdAt?: number;
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
const { user } = useAuthContext();
|
||||
/** Read the atom rather than the auth context: AuthContextProvider mirrors the
|
||||
* user into it, and the public share route mounts outside that provider. */
|
||||
const user = useRecoilValue(store.user);
|
||||
const fontSize = useAtomValue(fontSizeAtom);
|
||||
const { isSharedConvo } = useShareContext();
|
||||
const usernameDisplay = useRecoilValue<boolean>(store.UsernameDisplay);
|
||||
|
|
@ -90,7 +91,24 @@ const SteerPart = memo(function SteerPart({
|
|||
>
|
||||
<div className="relative flex flex-shrink-0 flex-col items-center">
|
||||
<div className="flex h-6 w-6 items-center justify-center overflow-hidden rounded-full">
|
||||
<MessageIcon iconData={USER_ICON} />
|
||||
{isSharedConvo === true ? (
|
||||
/** The atom still holds the viewer's identity when a signed-in user opens
|
||||
* a share link, so rendering the identity-bearing avatar here would put
|
||||
* the viewer's face on the sharer's steer. Mirrors Share/MessageIcon. */
|
||||
<div
|
||||
style={{
|
||||
backgroundColor: 'rgb(121, 137, 255)',
|
||||
width: '20px',
|
||||
height: '20px',
|
||||
boxShadow: 'rgba(240, 246, 252, 0.1) 0px 0px 0px 1px',
|
||||
}}
|
||||
className="relative flex h-9 w-9 items-center justify-center rounded-sm p-1 text-white"
|
||||
>
|
||||
<UserIcon />
|
||||
</div>
|
||||
) : (
|
||||
<MessageIcon iconData={USER_ICON} />
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
<div className="user-turn relative flex w-11/12 flex-col">
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import type { TMessage } from 'librechat-data-provider';
|
||||
import SteerPart from '../SteerPart';
|
||||
import store from '~/store';
|
||||
|
||||
let mockShareContext: { isSharedConvo?: boolean; shareId?: string } = {};
|
||||
|
||||
|
|
@ -10,19 +12,10 @@ jest.mock('~/hooks', () => ({
|
|||
useLocalize: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/AuthContext', () => ({
|
||||
useAuthContext: () => ({ user: { name: 'Danny', username: 'danny' } }),
|
||||
}));
|
||||
|
||||
jest.mock('~/Providers', () => ({
|
||||
useShareContext: () => mockShareContext,
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Messages/MessageIcon', () => ({
|
||||
__esModule: true,
|
||||
default: () => <div data-testid="user-icon" />,
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Messages/ui/MessageTimestamp', () => ({
|
||||
__esModule: true,
|
||||
default: () => null,
|
||||
|
|
@ -53,11 +46,23 @@ jest.mock('~/components/Chat/Messages/Content/Image', () => ({
|
|||
default: ({ altText }: { altText: string }) => <img alt={altText} data-testid="steer-image" />,
|
||||
}));
|
||||
|
||||
function renderPart(files?: TMessage['files']) {
|
||||
/** Seeds the user atom rather than mocking `useAuthContext`, and renders the real
|
||||
* MessageIcon tree — mocking either one hid a crash on the share route, where
|
||||
* neither an auth context nor a user exists. */
|
||||
const SEEDED_USER = { name: 'Danny', username: 'danny' };
|
||||
|
||||
function renderPart(
|
||||
files?: TMessage['files'],
|
||||
/** `null` seeds nothing — passing `undefined` would fall back to the default and
|
||||
* silently test the signed-in state instead of the anonymous share route. */
|
||||
user: { name: string; username: string } | null = SEEDED_USER,
|
||||
) {
|
||||
return render(
|
||||
<RecoilRoot>
|
||||
<SteerPart steer="steered words" steerId="s1" createdAt={1} files={files} />
|
||||
</RecoilRoot>,
|
||||
<QueryClientProvider client={new QueryClient()}>
|
||||
<RecoilRoot initializeState={({ set }) => user && set(store.user, user as never)}>
|
||||
<SteerPart steer="steered words" steerId="s1" createdAt={1} files={files} />
|
||||
</RecoilRoot>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -72,6 +77,29 @@ describe('SteerPart author label', () => {
|
|||
expect(screen.queryByText('com_user_message')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders on the share route, where there is no auth context and no user', () => {
|
||||
/** Regression for #14474: `/share/:shareId` mounts outside AuthContextProvider,
|
||||
* so any `useAuthContext()` on this tree throws and the whole page is replaced
|
||||
* by the route error boundary. Both SteerPart and the MessageIcon tree it
|
||||
* renders used to call it. */
|
||||
mockShareContext = { isSharedConvo: true, shareId: 'share-1' };
|
||||
|
||||
expect(() => renderPart(undefined, null)).not.toThrow();
|
||||
expect(screen.getByText('steered words')).toBeInTheDocument();
|
||||
expect(screen.getByText('com_user_message')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('never renders the viewer identity on a shared steer avatar', () => {
|
||||
/** The user atom is app-wide and survives navigation, so a signed-in viewer
|
||||
* opening a share link still has an identity in state. The shared steer must
|
||||
* show the generic avatar regardless. */
|
||||
mockShareContext = { isSharedConvo: true, shareId: 'share-1' };
|
||||
renderPart(undefined, SEEDED_USER);
|
||||
|
||||
expect(screen.queryByTitle('Danny')).toBeNull();
|
||||
expect(screen.queryByText('Danny')).toBeNull();
|
||||
});
|
||||
|
||||
it('labels with the generic user message in the share view, never the viewer identity', () => {
|
||||
mockShareContext = { isSharedConvo: true, shareId: 'share-1' };
|
||||
renderPart();
|
||||
|
|
@ -104,7 +132,9 @@ describe('SteerPart presentation', () => {
|
|||
|
||||
it('presents the steer as a user message with an icon', () => {
|
||||
renderPart();
|
||||
expect(screen.getByTestId('user-icon')).toBeInTheDocument();
|
||||
/** Asserts the real avatar rather than a stubbed one — the previous mock was
|
||||
* what hid the auth-context crash inside this icon tree. */
|
||||
expect(screen.getByTitle('Danny')).toBeInTheDocument();
|
||||
expect(screen.getByText('steered words')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,10 +1,11 @@
|
|||
import React, { memo } from 'react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { UserIcon, useAvatar } from '@librechat/client';
|
||||
import type { IconProps } from '~/common';
|
||||
import MessageEndpointIcon from './MessageEndpointIcon';
|
||||
import { useAuthContext } from '~/hooks/AuthContext';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
import store from '~/store';
|
||||
|
||||
type ResolvedAvatar = { type: 'image'; src: string } | { type: 'fallback' };
|
||||
|
||||
|
|
@ -101,7 +102,9 @@ const UserAvatar = memo(
|
|||
UserAvatar.displayName = 'UserAvatar';
|
||||
|
||||
const Icon: React.FC<IconProps> = memo((props) => {
|
||||
const { user } = useAuthContext();
|
||||
/** Same reason as SteerPart: this renders on the unauthenticated share route,
|
||||
* where `useAuthContext` throws. The atom is the same value in the app. */
|
||||
const user = useRecoilValue(store.user);
|
||||
const { size = 30, isCreatedByUser } = props;
|
||||
|
||||
const avatarSrc = useAvatar(user);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue