Skip to content

Commit ba14913

Browse files
asdekasdek
committed
refactor: standardize resource ID handling and improve flow file management
- Updated resource ID handling to ensure consistent string coercion for numeric IDs across components. - Enhanced sorting of resources in the FlowForm to use alphabetical order based on path. - Improved resource attachment logic in the FlowFilesAttachResourcesDialog to avoid deduplication of descendants. - Introduced utility functions for converting REST resource entries to a consistent format for Apollo cache. - Adjusted upload handling to ensure proper type consistency between REST and GraphQL responses. - Refactored promote functionality to align with updated resource ID handling and improve clarity in parameters.
1 parent 8830ae7 commit ba14913

9 files changed

Lines changed: 233 additions & 98 deletions

‎frontend/src/features/flows/files/flow-files-attach-resources-dialog.tsx‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { FolderInput, Loader2, Search, X } from 'lucide-react';
22
import { useCallback, useMemo, useState } from 'react';
33

4-
import { dedupeOverlappingPaths, FileManager, type FileNode } from '@/components/file-manager';
4+
import { FileManager, type FileNode } from '@/components/file-manager';
55
import { Button } from '@/components/ui/button';
66
import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog';
77
import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from '@/components/ui/empty';
@@ -44,11 +44,14 @@ const FlowFilesAttachResourcesDialogBody = ({
4444

4545
// Map selected paths back to resource ids on submit. Built lazily — the lookup
4646
// is only walked once per attach, so a per-render `Map` allocation is wasteful.
47+
// `resource.id` is canonically numeric in the cache (see `resources-rest.ts`)
48+
// even though codegen types it as `string`; coerce here so downstream callers
49+
// that rely on the `string` contract (`useFlowFilesAttachResources`) stay safe.
4750
const pathToIdRef = useMemo(() => {
4851
const map = new Map<string, string>();
4952

5053
for (const resource of resources) {
51-
map.set(resource.path, resource.id);
54+
map.set(resource.path, String(resource.id));
5255
}
5356

5457
return map;
@@ -63,11 +66,16 @@ const FlowFilesAttachResourcesDialogBody = ({
6366
return;
6467
}
6568

66-
// Drop descendants of any picked directory so the backend doesn't
67-
// re-process them — a recursive copy of the parent already covers them.
69+
// Forward every explicitly selected path's ID to the backend. Earlier
70+
// versions deduped descendants of any picked directory on the assumption
71+
// that the backend would copy directory trees recursively — it does not
72+
// (`flowfiles.CopyResourcesToFlow` only `MkdirAll`s a directory ref and
73+
// file refs are copied individually). Until that becomes recursive on
74+
// the backend, the user must multi-select a folder together with its
75+
// children to attach the contents.
6876
const ids: string[] = [];
6977

70-
for (const path of dedupeOverlappingPaths(selectedPaths)) {
78+
for (const path of selectedPaths) {
7179
const id = pathToIdRef.get(path);
7280

7381
if (id) {

‎frontend/src/features/flows/files/flow-files-utils.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,24 @@ import { CONTAINER_PATH_PREFIX, RESOURCES_PATH_PREFIX, UPLOADS_PATH_PREFIX } fro
77

88
export type FlowFile = FlowFileFragmentFragment;
99

10+
/**
11+
* Wire shape of `models.FlowFile` (REST JSON, snake_case). The internal
12+
* `FlowFile` alias mirrors the GraphQL camelCase fragment for use in the
13+
* FileManager UI. Current consumers of `FlowFilesResponse` only read
14+
* `files.length` and `files[0].name`, so no conversion helper is needed yet.
15+
*/
1016
export interface FlowFilesResponse {
11-
files: Array<FlowFile>;
17+
files: RestFlowFile[];
1218
total: number;
1319
}
1420

15-
/** Mirrors `models.ResourceEntry` from the backend (REST shape, not GraphQL). */
16-
export interface ResourceEntry {
17-
createdAt: string;
18-
id: number;
19-
isDir: boolean;
21+
export interface RestFlowFile {
22+
id: string;
23+
is_dir: boolean;
24+
modified_at: string;
2025
name: string;
2126
path: string;
2227
size: number;
23-
updatedAt: string;
24-
userId: number;
2528
}
2629

2730
const ROOT_PREFIXES = [`${UPLOADS_PATH_PREFIX}/`, `${CONTAINER_PATH_PREFIX}/`, `${RESOURCES_PATH_PREFIX}/`];

‎frontend/src/features/flows/files/use-flow-files-attach-resources.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,27 @@
11
import { useCallback, useState } from 'react';
22
import { toast } from 'sonner';
33

4+
import { resourceIdsToWire } from '@/features/resources/resources-rest';
45
import { api, getApiErrorMessage } from '@/lib/axios';
56

67
import type { FlowFilesResponse } from './flow-files-utils';
78

89
import { FLOW_FILES_ATTACH_RESOURCES_API_PATH, RESOURCES_TARGET_DIRECTORY } from './flow-files-constants';
910

1011
interface AttachOptions {
12+
/** GraphQL `UserResource.id` values (strings). Converted at the REST boundary. */
1113
ids: string[];
1214
shouldOverwrite: boolean;
1315
}
1416

17+
/**
18+
* Wire shape of `models.AddResourcesRequest`. Backend expects `ids` as
19+
* uint64 (JSON numbers); see {@link resourceIdsToWire} for the bridge from
20+
* the GraphQL `string` ID convention used internally.
21+
*/
1522
interface AttachResourcesRequestBody {
1623
force: boolean;
17-
ids: string[];
24+
ids: number[];
1825
}
1926

2027
interface UseFlowFilesAttachResourcesParams {
@@ -46,20 +53,34 @@ export const useFlowFilesAttachResources = ({
4653
return false;
4754
}
4855

56+
let numericIds: number[];
57+
58+
try {
59+
numericIds = resourceIdsToWire(ids);
60+
} catch (error) {
61+
// Non-numeric IDs indicate an upstream cache contract bug, not a user
62+
// mistake. Surface a developer-friendly toast and bail out loudly.
63+
const description = error instanceof Error ? error.message : 'Invalid resource IDs.';
64+
65+
toast.error('Attach failed', { description });
66+
67+
return false;
68+
}
69+
4970
setIsAttaching(true);
5071

5172
try {
5273
await api.post<FlowFilesResponse, AttachResourcesRequestBody>(
5374
FLOW_FILES_ATTACH_RESOURCES_API_PATH(flowId),
5475
{
5576
force: shouldOverwrite,
56-
ids,
77+
ids: numericIds,
5778
},
5879
{ timeout: 0 },
5980
);
6081

6182
toast.success('Resources attached', {
62-
description: `Copied ${ids.length} ${ids.length === 1 ? 'item' : 'items'} to ${RESOURCES_TARGET_DIRECTORY}`,
83+
description: `Copied ${numericIds.length} ${numericIds.length === 1 ? 'item' : 'items'} to ${RESOURCES_TARGET_DIRECTORY}`,
6384
});
6485
onSuccess?.();
6586

‎frontend/src/features/flows/files/use-flow-files-promote.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { useCallback, useState } from 'react';
22
import { toast } from 'sonner';
33
import { z } from 'zod';
44

5-
import { api, getApiErrorMessage } from '@/lib/axios';
5+
import type { RestResourceList } from '@/features/resources/resources-rest';
66

7-
import type { ResourceEntry } from './flow-files-utils';
7+
import { api, getApiErrorMessage } from '@/lib/axios';
88

99
import { FLOW_FILES_PROMOTE_API_PATH } from './flow-files-constants';
1010

@@ -23,44 +23,48 @@ export type FlowFilesPromoteFormValues = z.infer<typeof flowFilesPromoteFormSche
2323
interface PromoteRequestBody {
2424
destination: string;
2525
force: boolean;
26-
sourcePath: string;
26+
source: string;
2727
}
2828

2929
interface UseFlowFilesPromoteParams {
3030
flowId: null | string;
31-
onSuccess?: (entry: ResourceEntry) => void;
3231
}
3332

3433
interface UseFlowFilesPromoteResult {
3534
isPromoting: boolean;
36-
promote: (sourcePath: string, values: FlowFilesPromoteFormValues) => Promise<boolean>;
35+
promote: (source: string, values: FlowFilesPromoteFormValues) => Promise<boolean>;
3736
}
3837

3938
const PROMOTE_OVERWRITE_HINT = 'Resource already exists — enable "Overwrite" to replace it';
4039

4140
/**
4241
* Wraps the "promote flow file → user resource" REST call (`POST /files/to-resources`)
43-
* with toast notifications and a loading flag. Returns a `promote(sourcePath, values)`
42+
* with toast notifications and a loading flag. Returns a `promote(source, values)`
4443
* callback that resolves to `true` on success so the dialog can decide whether to close.
44+
*
45+
* The endpoint accepts both single files and directories — the backend always returns
46+
* a `ResourceList` covering every entry it created or updated. The response payload
47+
* itself is discarded: the resource library Apollo cache is kept in sync via the
48+
* `resourceAdded` / `resourceUpdated` GraphQL subscriptions.
4549
*/
46-
export const useFlowFilesPromote = ({ flowId, onSuccess }: UseFlowFilesPromoteParams): UseFlowFilesPromoteResult => {
50+
export const useFlowFilesPromote = ({ flowId }: UseFlowFilesPromoteParams): UseFlowFilesPromoteResult => {
4751
const [isPromoting, setIsPromoting] = useState(false);
4852

4953
const promote = useCallback(
50-
async (sourcePath: string, { destination, shouldOverwrite }: FlowFilesPromoteFormValues): Promise<boolean> => {
54+
async (source: string, { destination, shouldOverwrite }: FlowFilesPromoteFormValues): Promise<boolean> => {
5155
if (!flowId) {
5256
return false;
5357
}
5458

5559
setIsPromoting(true);
5660

5761
try {
58-
const response = await api.post<ResourceEntry, PromoteRequestBody>(
62+
await api.post<RestResourceList, PromoteRequestBody>(
5963
FLOW_FILES_PROMOTE_API_PATH(flowId),
6064
{
6165
destination: destination.trim(),
6266
force: shouldOverwrite,
63-
sourcePath,
67+
source,
6468
},
6569
{ timeout: 0 },
6670
);
@@ -69,10 +73,6 @@ export const useFlowFilesPromote = ({ flowId, onSuccess }: UseFlowFilesPromotePa
6973
description: `Stored at ${destination.trim()} in your resource library`,
7074
});
7175

72-
if (response.status === 'success' && response.data) {
73-
onSuccess?.(response.data);
74-
}
75-
7676
return true;
7777
} catch (error) {
7878
const description = getApiErrorMessage(error, 'Failed to save resource', {
@@ -86,7 +86,7 @@ export const useFlowFilesPromote = ({ flowId, onSuccess }: UseFlowFilesPromotePa
8686
setIsPromoting(false);
8787
}
8888
},
89-
[flowId, onSuccess],
89+
[flowId],
9090
);
9191

9292
return {

‎frontend/src/features/flows/flow-form.tsx‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,13 @@ export const FlowForm = ({
9191

9292
const fileInputRef = useRef<HTMLInputElement>(null);
9393

94+
// Resources are rendered as a hierarchy: alphabetical sort by full path
95+
// produces the right ordering for siblings at every depth (parents before
96+
// their descendants, peers in alphabetical order). Each row's nesting level
97+
// is then derived from the slash count and rendered as a left indent so the
98+
// user can visually trace files into their parent directories.
9499
const sortedResources = useMemo(
95-
() => [...resources].sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()),
100+
() => [...resources].sort((a, b) => a.path.localeCompare(b.path, undefined, { sensitivity: 'base' })),
96101
[resources],
97102
);
98103

@@ -109,6 +114,8 @@ export const FlowForm = ({
109114
);
110115
}, [sortedResources, resourceSearch]);
111116

117+
const isResourceSearchActive = resourceSearch.trim().length > 0;
118+
112119
const filteredTemplates = useMemo(() => {
113120
if (!templateSearch.trim()) {
114121
return templates;
@@ -159,21 +166,20 @@ export const FlowForm = ({
159166
const resourceIds = useWatch({ control, name: 'resourceIds' });
160167

161168
const updateResourceIds = useCallback(
162-
(updater: ((current: string[]) => string[]) | string[]) => {
169+
(updater: ((current: string[]) => string[]) | Array<number | string>) => {
163170
const current = getValues('resourceIds') ?? [];
164171
const raw = typeof updater === 'function' ? updater(current) : updater;
165-
// TODO(backend): drop the `String(id)` coercion once the REST `/resources/`
166-
// endpoints return `id` as a string (matching the GraphQL `ID` scalar). Until
167-
// then this is a safety net for stray numeric ids that would fail zod's
168-
// `z.array(z.string())` and silently block submission.
172+
// Canonical cache shape carries `id` as a number (see `resources-rest.ts`),
173+
// while zod enforces `z.array(z.string())` on the form state. String-coerce
174+
// every incoming value so the form always holds a consistent string array.
169175
const next = raw.map((id) => String(id));
170176
setValue('resourceIds', next, { shouldDirty: true, shouldValidate: true });
171177
},
172178
[getValues, setValue],
173179
);
174180

175181
const flowResources = useMemo<UserResourceFragmentFragment[]>(() => {
176-
const byId = new Map(resources.map((item) => [item.id, item]));
182+
const byId = new Map(resources.map((item) => [String(item.id), item]));
177183

178184
return resourceIds
179185
.map((id) => byId.get(id))
@@ -182,7 +188,7 @@ export const FlowForm = ({
182188

183189
const upload = useResourcesUpload({
184190
onSuccess: (uploaded) => {
185-
const ids = uploaded.items.map((item) => item.id);
191+
const ids = uploaded.items.map((item) => String(item.id));
186192

187193
if (ids.length === 0) {
188194
return;
@@ -333,11 +339,12 @@ export const FlowForm = ({
333339
>
334340
{flowResources.map((resource) => {
335341
const Icon = resource.isDir ? Folder : FileText;
342+
const resourceId = String(resource.id);
336343

337344
return (
338345
<div
339346
className="bg-muted/50 flex max-w-full items-center gap-1.5 rounded-md border px-2 py-1 text-xs"
340-
key={resource.id}
347+
key={resourceId}
341348
title={resource.path}
342349
>
343350
<Icon className="text-muted-foreground size-3.5 shrink-0" />
@@ -348,7 +355,7 @@ export const FlowForm = ({
348355
aria-label={`Remove ${resource.name}`}
349356
className="text-muted-foreground hover:text-destructive ml-0.5 flex shrink-0 items-center justify-center"
350357
disabled={isFormDisabled}
351-
onClick={() => handleRemoveAttachment(resource.id)}
358+
onClick={() => handleRemoveAttachment(resourceId)}
352359
type="button"
353360
>
354361
<X className="size-3.5" />
@@ -640,33 +647,42 @@ export const FlowForm = ({
640647
</DropdownMenuItem>
641648
) : (
642649
filteredResources.map((resource) => {
643-
const isSelected = resourceIds.includes(resource.id);
650+
const resourceId = String(resource.id);
651+
const isSelected = resourceIds.includes(resourceId);
644652
const Icon = resource.isDir ? Folder : FileText;
653+
// Depth derived from the path's slash count; ignored while a
654+
// search query is active so matches don't appear orphaned
655+
// beneath hidden ancestors.
656+
const depth = isResourceSearchActive
657+
? 0
658+
: resource.path.split('/').length - 1;
645659

646660
return (
647661
<DropdownMenuItem
648-
key={resource.id}
662+
key={resourceId}
649663
onSelect={(event) => {
650664
event.preventDefault();
651665

652666
if (isFormDisabled) {
653667
return;
654668
}
655669

656-
handleToggleAttachment(resource.id);
670+
handleToggleAttachment(resourceId);
657671
}}
672+
style={{ paddingLeft: `${0.5 + depth * 0.875}rem` }}
658673
>
659674
<div className="flex w-full min-w-0 items-center gap-2">
660675
<Icon className="text-muted-foreground size-4 shrink-0" />
661676
<div className="flex min-w-0 flex-1 flex-col">
662677
<span className="truncate">
663678
{resource.name}
664679
</span>
665-
{resource.path !== resource.name && (
666-
<span className="text-muted-foreground truncate text-xs">
667-
{resource.path}
668-
</span>
669-
)}
680+
{isResourceSearchActive &&
681+
resource.path !== resource.name && (
682+
<span className="text-muted-foreground truncate text-xs">
683+
{resource.path}
684+
</span>
685+
)}
670686
</div>
671687
{isSelected && (
672688
<Check className="ml-auto size-4 shrink-0" />

0 commit comments

Comments
 (0)