mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: require a usable route before downloading the original artifact file
A shared link to a non-snapshotted code-execution office artifact strips source/user and deletes filepath while keeping file_id (share sanitization + applyShareFileRoute). The preview-panel download gate treated that lone file_id as sufficient, so it routed to an empty useCodeOutputDownload fetch and downloaded nothing instead of falling back to the preview-content blob. Take the original-file branch only when useAttachmentLink can actually fetch: a non-empty filepath (http target, share route, or code-output URL) or full local-file metadata (isLocallyStoredSource + file_id + user). Export isLocallyStoredSource from LogLink so the panel reuses the same predicate.
This commit is contained in:
parent
7cf6e5bbc4
commit
149a0dce9d
3 changed files with 72 additions and 5 deletions
|
|
@ -2,7 +2,10 @@ import React, { useState } from 'react';
|
|||
import { Button } from '@librechat/client';
|
||||
import { Download, CircleCheckBig } from 'lucide-react';
|
||||
import type { Artifact } from '~/common';
|
||||
import { useAttachmentLink } from '~/components/Chat/Messages/Content/Parts/LogLink';
|
||||
import {
|
||||
useAttachmentLink,
|
||||
isLocallyStoredSource,
|
||||
} from '~/components/Chat/Messages/Content/Parts/LogLink';
|
||||
import useArtifactProps from '~/hooks/Artifacts/useArtifactProps';
|
||||
import { isPreviewOnlyArtifact } from '~/utils/artifacts';
|
||||
import { useCodeState } from '~/Providers/EditorContext';
|
||||
|
|
@ -21,9 +24,20 @@ const DownloadArtifact = ({ artifact }: { artifact: Artifact }) => {
|
|||
* text artifacts keep the blob path: their `content` IS the file (and
|
||||
* reflects any in-panel edits). */
|
||||
const { download } = artifact;
|
||||
const downloadOriginalFile =
|
||||
isPreviewOnlyArtifact(artifact.type) &&
|
||||
(download?.filepath != null || download?.file_id != null);
|
||||
/* Only take the original-file branch when `useAttachmentLink` can
|
||||
* actually fetch something: a usable `filepath` (http target, share
|
||||
* route, or code-output URL) OR enough metadata for the local-file
|
||||
* API path (`isLocallyStoredSource` + file_id + user). A shared link
|
||||
* to a non-snapshotted code-execution artifact strips source/user and
|
||||
* deletes filepath while keeping file_id; without this guard that lone
|
||||
* file_id would route to an empty fetch and download nothing instead
|
||||
* of falling back to the preview-content blob. */
|
||||
const hasUsableRoute =
|
||||
(download?.filepath != null && download.filepath !== '') ||
|
||||
(download?.file_id != null &&
|
||||
download?.user != null &&
|
||||
isLocallyStoredSource(download?.source));
|
||||
const downloadOriginalFile = isPreviewOnlyArtifact(artifact.type) && hasUsableRoute;
|
||||
const { handleDownload: downloadAttachment } = useAttachmentLink({
|
||||
href: download?.filepath ?? '',
|
||||
filename: artifact.title ?? fileName,
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ jest.mock('~/Providers/EditorContext', () => ({
|
|||
|
||||
jest.mock('~/components/Chat/Messages/Content/Parts/LogLink', () => ({
|
||||
useAttachmentLink: () => ({ handleDownload: mockFileDownload }),
|
||||
isLocallyStoredSource: (source?: string) =>
|
||||
['local', 'firebase', 's3', 'cloudfront', 'azure_blob'].includes(source ?? ''),
|
||||
}));
|
||||
|
||||
const officeArtifact: Artifact = {
|
||||
|
|
@ -48,6 +50,37 @@ const htmlArtifact: Artifact = {
|
|||
content: '<h1>hello</h1>',
|
||||
};
|
||||
|
||||
/* Shared link to a non-snapshotted code-execution office artifact: share
|
||||
* sanitization strips source/user and `applyShareFileRoute` deletes
|
||||
* filepath, leaving only file_id. There is no route to fetch the
|
||||
* original, so the panel must fall back to the preview-content blob. */
|
||||
const sharedNoRouteArtifact: Artifact = {
|
||||
id: 'tool-artifact-fid-2',
|
||||
lastUpdateTime: 0,
|
||||
type: TOOL_ARTIFACT_TYPES.PRESENTATION,
|
||||
title: 'deck.pptx',
|
||||
content: '<html><body>slide text scrape</body></html>',
|
||||
download: {
|
||||
file_id: 'fid-2',
|
||||
},
|
||||
};
|
||||
|
||||
/* Locally-stored office artifact with no filepath but full local-file
|
||||
* metadata: the API download path (isLocallyStoredSource + file_id +
|
||||
* user) can still fetch the original. */
|
||||
const localMetadataArtifact: Artifact = {
|
||||
id: 'tool-artifact-fid-3',
|
||||
lastUpdateTime: 0,
|
||||
type: TOOL_ARTIFACT_TYPES.SPREADSHEET,
|
||||
title: 'book.xlsx',
|
||||
content: '<html><body>sheet scrape</body></html>',
|
||||
download: {
|
||||
file_id: 'fid-3',
|
||||
source: 'local',
|
||||
user: 'user-3',
|
||||
},
|
||||
};
|
||||
|
||||
describe('DownloadArtifact', () => {
|
||||
let createObjectURL: jest.Mock;
|
||||
let revokeObjectURL: jest.Mock;
|
||||
|
|
@ -92,4 +125,24 @@ describe('DownloadArtifact', () => {
|
|||
expect(createObjectURL).toHaveBeenCalledTimes(1);
|
||||
expect(mockFileDownload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the preview blob when an office artifact has only a lone file_id (no usable route)', async () => {
|
||||
render(<DownloadArtifact artifact={sharedNoRouteArtifact} />);
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button'));
|
||||
});
|
||||
// No filepath/share route and no local metadata: must NOT call the
|
||||
// empty attachment fetch; serialize the preview content instead.
|
||||
expect(mockFileDownload).not.toHaveBeenCalled();
|
||||
expect(createObjectURL).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('downloads the original via the local-file path when filepath is absent but local metadata is present', async () => {
|
||||
render(<DownloadArtifact artifact={localMetadataArtifact} />);
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button'));
|
||||
});
|
||||
expect(mockFileDownload).toHaveBeenCalledTimes(1);
|
||||
expect(createObjectURL).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ interface AttachmentLinkOptions {
|
|||
* Files with these sources are stored on the LibreChat server and should
|
||||
* use the /api/files/download endpoint instead of direct URL access.
|
||||
*/
|
||||
const isLocallyStoredSource = (source?: string): boolean => {
|
||||
export const isLocallyStoredSource = (source?: string): boolean => {
|
||||
if (!source) {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue