Skip to content

feat(admin): private file#151

Merged
99mini merged 7 commits intomainfrom
admin/release/private-file
May 5, 2026
Merged

feat(admin): private file#151
99mini merged 7 commits intomainfrom
admin/release/private-file

Conversation

@99mini
Copy link
Copy Markdown
Owner

@99mini 99mini commented May 5, 2026

No description provided.

@99mini 99mini self-assigned this May 5, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the file management API to a generated client, introduces a folder detail modal for viewing statistics, and adds visual indicators for private paths. The review feedback suggests optimizing the folder detail modal by consolidating redundant data-fetching calls, refining the dialog's modal behavior for better UX, and simplifying the file table by removing redundant sorting logic.

Comment on lines +54 to +88
const DirectoryCountValue = ({ path }: { path: string }) => {
const { data: directoryCount } = useSuspenseQuery({
...getV1FilesDetailOptions({ query: { path } }),
select: ({ data }) => data.directoryCount,
});

return <ValueColumn value={`${directoryCount}개`} />;
};

const LeafCountValue = ({ path }: { path: string }) => {
const { data: leafCount } = useSuspenseQuery({
...getV1FilesDetailOptions({ query: { path } }),
select: ({ data }) => data.leafCount,
});

return <ValueColumn value={`${leafCount}개`} />;
};

const TotalSizeValue = ({ path }: { path: string }) => {
const { data: totalSize } = useSuspenseQuery({
...getV1FilesDetailOptions({ query: { path } }),
select: ({ data }) => data.totalSize,
});

return <ValueColumn value={formatFileSize(totalSize)} />;
};

const FileCountValue = ({ path }: { path: string }) => {
const { data: fileCount } = useSuspenseQuery({
...getV1FilesDetailOptions({ query: { path } }),
select: ({ data }) => data.fileCount,
});

return <ValueColumn value={`${fileCount}개`} />;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Multiple sub-components (DirectoryCountValue, LeafCountValue, TotalSizeValue, and FileCountValue) are calling useSuspenseQuery with the exact same query options. Although React Query deduplicates these requests, it is cleaner and more efficient to fetch the data once in a single component (e.g., a FolderDetailContent component) and pass the data down to the rows. This also allows for a single Suspense boundary and a more consistent loading state for the entire detail block.

Comment on lines +20 to +21
<Dialog open={open} onOpenChange={onOpenChange} modal={false}>
<DialogContent className="max-w-sm" onInteractOutside={(e) => e.preventDefault()}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Dialog is configured with modal={false} while also attempting to prevent outside interaction via onInteractOutside. For a standard detail view, using modal={true} (the default) is usually preferred as it handles focus management and provides an overlay. If a non-modal behavior is truly intended, ensure that the interaction prevention doesn't lead to a confusing user experience where the background is visible but partially non-interactive.

* 파일 목록 테이블의 본문 컴포넌트
*/
const FileTableBody = ({ files, sortField, sortDirection, showAllColumns }: FileTableBodyProps) => {
const FileTableBody = ({ files, sortField, sortDirection, showAllColumns, onRequestDetail }: FileTableBodyProps) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The conditional logic inside FileTableBody (checking sortField and sortDirection) is redundant because the files array is already sorted in the parent FileList component. Currently, all branches of the if/else statements perform the exact same mapping operation. Consider simplifying this component by removing the sortField and sortDirection props and replacing the entire function body with a single .map() call over the files array to improve maintainability.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46aaf53518

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

queryKey: ['files', data.path ?? ''],
queryFn: () => readFiles(data),
queryFn: async () => {
const { data: res } = await getV1Files({ query: { path: data.path }, throwOnError: true });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Decode route path before querying file list

useFilesQuery now forwards data.path directly to getV1Files, but FilePage builds that value from location.pathname (encoded URL text). Without the previous decodeURIComponent step, paths containing escaped characters (e.g. spaces/non-ASCII) get encoded a second time (%20%2520), so the backend receives the wrong directory path and listing those folders fails.

Useful? React with 👍 / 👎.

…d $ref schemas in controller

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@99mini 99mini merged commit ff441f4 into main May 5, 2026
2 checks passed
@99mini 99mini deleted the admin/release/private-file branch May 5, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant