Conversation
- Add `DownloadQuery` DTO with optional `disposition` field to `src/api/dto/files.rs` - Add `download_disposition_from_query` helper in `access.rs` that maps `inline`/`attachment`/empty to `DownloadDisposition`, returning a validation error for unsupported values - Thread `disposition` parameter through `download` and `team_download` route handlers and into `download_response` - Propagate `disposition` through `download_in_scope_with_file_and_audit` and `download_in_scope_with_range_and_file` in `file_service` - Remove the now-redundant `build_download_outcome` wrapper in `file_service/download/build.rs`, calling `build_download_outcome_with_disposition_and_range` directly - Add `download_shared_file_with_disposition_and_range` and `download_shared_folder_file_with_disposition_and_range` in `share_service/content.rs`; refactor old `_with_range` functions to delegate to the new ones with `Attachment` as default - Export the new share service functions from `share_service/mod.rs` - Wire disposition into `download_shared` and `download_shared_folder_file_handler` in `share_public.rs` - Add tests in `test_files.rs` covering default attachment, explicit inline, explicit attachment, and invalid disposition values, plus a CSP sandbox test for dangerous MIME types served inline - Extend `test_shares.rs` to assert `Content-Disposition` header on share downloads and cover inline/invalid disposition variants
- Add `POST /api/v1/files/{id}/resource-handle` and `POST /api/v1/teams/{team_id}/files/{id}/resource-handle` endpoints
- Introduce `FileResourceHandleRequest` DTO with purpose, delivery_mode, and representation fields
- Add `FileResourceHandle` response type with identity, request, and delivery sub-structures
- Add enums for delivery mode, purpose, representation, credentials, conditional headers, and redirect policy
- Implement `resolve_file_resource_handle` service in a new `resource_handle` module
- Resolve representation automatically for non-browser-renderable images (HEIC, RAW, TIFF, etc.) in `Auto` mode
- Use presigned URLs with cross-origin policy when object storage presigned download is enabled
- Fall back to same-origin download URL with credentials and conditional headers for local/non-presigned storage
- Force same-origin path for sandboxed MIME types (HTML, etc.) even when presigned is configured
- Return `image/webp` MIME type and scoped etag for derived image-preview and thumbnail representations
- Register new routes in both personal file scope and team file scope
- Add inline route comments documenting the web app use case for every file route
- Broaden CSP `media-src` to allow `http:` and `https:` origins for presigned audio/video URLs
- Register all new DTO types and operations in the OpenAPI spec
- Add unit tests covering query string preservation, browser-renderable image detection, presigned vs same-origin branching, auto representation selection, and validation errors for unsupported processors
- Add integration tests for personal and team resource handle endpoints including scope isolation rejection
…eview and playback Replace ad-hoc path and factory props with unified FilePreviewResources interface. Consolidate resource resolution, authentication, and lifecycle management into a structured handle system. - **Resource abstraction**: Add `FilePreviewResources` interface bundling paths, resolution logic, and action factories (preview links, WOPI, streams, archives) into a single prop - **Resource handles**: Introduce `ReadyFileResourceHandle` with explicit identity, request config (credentials, conditional headers, redirect policy), and delivery metadata - **File resource utilities**: Add `fileResource.ts` with builders for authenticated downloads, derived resources (thumbnails, previews), and preview link caching - **Path to resource migration**: Replace path/factory props across preview components with resource handles. Update `BlobImagePreview`, `VideoPreview`, `ImagePreviewPanel`, text/code/JSON/PDF/markdown/CSV/XML previews, music player, and share views - **Authentication flow**: Update `authenticatedResource.ts` and `useBlobUrl`/`useTextContent` to consume resource handles with explicit credential and conditional header policies - **Music player refactor**: Replace direct paths with resource handles in queue building and playback preparation. Add async resource resolution to `buildDirectMusicQueue` - **Backend integration**: Add `fileService.resolveResourceHandle` API client and `useFileContentResource` hook for on-demand resource resolution - **Audio preview removal**: Drop built-in audio preview mode. Audio files now play through music player only - **Test coverage**: Update 300+ test assertions to reflect resource-based APIs
Prevent music playback from restarting when active queue track metadata is updated with the same resource. Skip range probes for resources that may redirect cross-origin to avoid unnecessary auth checks. **Changes:** - Stabilize playback during metadata updates by keying tracks on resource path instead of full track object - Extract track resource, name, and translate function into refs to preserve playback state across renders - Skip range probes for resources with `may_cross_origin` redirect policy while still refreshing auth - Add `resourceRedirectPolicy` helper to determine redirect handling requirements - Add test coverage for metadata refresh scenario and cross-origin resource preparation
… hooks Split large useFilePreviewDialogModel hook into focused, testable units: - Extract capability detection into usePreviewCapabilities - Extract open mode selection into usePreviewOpenMode - Extract chrome/layout state into usePreviewDialogChromeState - Extract session resources into usePreviewSessionResources - Refactor dialog state reducer with explicit action types - Add comprehensive test coverage for all extracted hooks - Replace toggleExpanded with setExpanded action for clearer semantics - Track fileId in state to determine reset conditions - Replace resetForFile boolean with fileId comparison
Restructured the file preview component directory to improve code organization and maintainability: - **capabilities/**: File type detection, format support, and open-with configuration - Moved `file-capabilities.ts`, `fileCapabilityData.ts`, `types.ts` - Moved `video-browser-config.ts`, `openWithLabel.ts` - **dialog/**: Preview dialog UI and state management - Moved `FilePreviewDialog.tsx`, `FilePreviewBody.tsx` - Moved dialog state hooks and logic - Moved `FilePreviewPanel.tsx`, `FilePreviewMethodChooser.tsx` - Moved `UnsavedChangesGuard.tsx` - **navigation/**: Image preview navigation logic - Moved `imagePreviewNavigation.ts` - **resources/**: Resource management utilities - Moved `filePreviewResources.ts` - **shared/**: Common preview components - Moved `PreviewError.tsx`, `PreviewLoadingState.tsx` - Moved `PreviewSurface.tsx`, `PreviewUnavailable.tsx` - Moved `AnimatedCollapsible.tsx` - **viewers/**: Format-specific preview components - `archive/`: Archive file viewer components - `external/`: External viewer integration (`UrlTemplatePreview`, `EmbeddedWebAppPreview`) - `image/`: Image viewer components (`BlobImagePreview`, `ImagePreviewPanel`) - `pdf/`: PDF viewer component - `text/`: Text-based viewers (code, JSON, XML, CSV, Markdown) - `video/`: Video player component - `wopi/`: WOPI protocol integration Updated all import paths across components, tests, and page modules to reflect the new structure.
…nCoordinator Extract scattered storage event echo, cache invalidation, and refresh decision logic into a dedicated `storageMutationCoordinator` module with explicit decision types. - Add `storageMutationCoordinator.ts` with pure decision functions: `decideRemoteStorageMutation`, `decideUploadQueueSettledRefresh`, `decideVirtualStorageViewRefresh`, `decideFolderTreeStorageRefresh` - Add `beginLocalStorageDeleteMutation` and `beginLocalStorageMoveMutation` returning rollback/publish handles, replacing raw `rememberStorageDeleteEchoes`/`forgetStorageEventEchoes` call sites - Add `fileResourceCacheInvalidation.ts` with `invalidateFileResourceCachesForMutation` and `invalidateAllFileResourceCaches`, consolidating blob URL and text content invalidation into one call - Replace `folder-tree-move` custom DOM event with `publishStorageChange` via `beginLocalStorageMoveMutation`, making folder tree updates flow through the storage change bus - Simplify `ImagePreviewPanel` backend preview resource: build it directly from `resources.paths.imagePreview` using `derivedFileResource` instead of routing through `useFileContentResource` - Simplify `useStorageChangeEvents` by delegating all branching logic to coordinator decision objects and replacing inline helper functions - Add `fileResourceCacheKeysForMutation` helper to `fileResource.ts` for consistent cache key enumeration - Add unit tests for `storageMutationCoordinator` covering all decision functions and mutation lifecycle - Add unit tests for `fileResourceCacheInvalidation` covering full and partial invalidation paths - Update `VersionHistoryDialog`, `CategoryBrowserPage`, `SearchBrowserPage`, and `crudSlice` to use the new coordinator and invalidation APIs
Replace string path parameters with derivedFileResource objects in preview component tests to align with new resource-based API. - Replace path strings with derivedFileResource objects in PdfPreview tests - Update CsvTablePreview tests to use resource objects with deliveryMode - Migrate JsonPreview, MarkdownPreview, and XmlPreview tests to resource API - Convert TextCodePreview tests from path-based to resource-based calls - Add fileService path helper methods in FileBrowserDialogs test mock - Import derivedFileResource utility across all preview test files - Update useBlobUrl and useTextContent assertions to expect resource objects
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough该 PR 新增文件资源句柄与下载处置接口，统一前端资源模型与预览契约，并重构分享、音乐与存储变更协调链路。 Changes文件资源与预览重构
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend-panel/src/pages/FileBrowserPage.test.tsx (1)
1952-1976: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win这个用例的订阅和 fake timers 缺少
finally保护，失败时会污染后续测试。当前如果中途断言失败，
unsubscribe()和vi.useRealTimers()可能不执行，后续用例会被连带污染。🔧 建议修复
it("moves items, publishes storage updates, and shows the formatted move toast", async () => { const { subscribeStorageChange } = await import("`@/lib/storageChangeBus`"); const storageEvents: unknown[] = []; const unsubscribe = subscribeStorageChange((event) => { storageEvents.push(event); }); - render(<FileBrowserPage />); - - vi.useFakeTimers(); - - fireEvent.click(screen.getByRole("button", { name: "move-selection" })); - - await Promise.resolve(); - await Promise.resolve(); - expect(mockState.store.moveToFolder).toHaveBeenCalledWith([7], [8], 20); - await vi.advanceTimersByTimeAsync(FILE_BROWSER_FEEDBACK_DURATION_MS); - await Promise.resolve(); - await Promise.resolve(); - vi.useRealTimers(); - unsubscribe(); + try { + render(<FileBrowserPage />); + vi.useFakeTimers(); + + fireEvent.click(screen.getByRole("button", { name: "move-selection" })); + + await Promise.resolve(); + await Promise.resolve(); + expect(mockState.store.moveToFolder).toHaveBeenCalledWith([7], [8], 20); + await vi.advanceTimersByTimeAsync(FILE_BROWSER_FEEDBACK_DURATION_MS); + await Promise.resolve(); + await Promise.resolve(); + } finally { + vi.useRealTimers(); + unsubscribe(); + } expect(storageEvents).toContainEqual( expect.objectContaining({ folder_ids: [8], kind: "folder.updated", }), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/pages/FileBrowserPage.test.tsx` around lines 1952 - 1976, The test "moves items, publishes storage updates, and shows the formatted move toast" is missing finally block protection for cleanup operations. If any assertion fails before the unsubscribe() and vi.useRealTimers() calls are reached, these cleanup operations will not execute, leaving fake timers and subscriptions active for subsequent tests. Wrap the test body in a try...finally block, moving the unsubscribe() call and vi.useRealTimers() into the finally block to guarantee they always execute regardless of test outcome.
🧹 Nitpick comments (9)
frontend-panel/src/components/folders/folder-tree/useFolderTreeController.ts (1)
372-390: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value每次 storage event 都重建
folderParentIdsByIdMap。当前实现在每个事件到来时都从
nodeMap创建新 Map。考虑到 storage 事件频率通常不高，这不是严重问题，但如果nodeMap很大且事件频繁，可以考虑将这个 Map 提前用useMemo缓存。不过鉴于这是事件回调内的一次性计算，目前的实现是可接受的。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/components/folders/folder-tree/useFolderTreeController.ts` around lines 372 - 390, The `folderParentIdsById` Map is being recreated on every storage event within the useEffect hook in useFolderTreeController. To optimize this, extract the Map creation logic into a separate useMemo hook that depends only on nodeMap, then pass the memoized Map to the decideFolderTreeStorageRefresh function call instead of creating it inline. This prevents unnecessary Map reconstructions when the nodeMap hasn't changed.frontend-panel/src/lib/storageMutationCoordinator.ts (1)
255-259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
decideVirtualStorageViewRefresh的 search 分支有冗余逻辑。最后两个 return 路径效果相同——无论是否为 tag 事件都返回
hasResourceReference(event)。可以简化：- if (isTagChangeEvent(event)) { - return hasResourceReference(event); - } return hasResourceReference(event);不过这不影响正确性，只是代码冗余。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/lib/storageMutationCoordinator.ts` around lines 255 - 259, The decideVirtualStorageViewRefresh function has redundant logic where both the if block checking isTagChangeEvent and the subsequent return statement execute the same operation—returning hasResourceReference(event). Remove the unnecessary if condition and the isTagChangeEvent check, keeping only the single return statement that calls hasResourceReference(event), since the outcome is identical regardless of whether the event is a tag change event or not.frontend-panel/src/hooks/useStorageChangeEvents.ts (1)
69-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
refreshStorageUsage的 switch 缺少default分支的 exhaustive check。TypeScript 在联合类型完全覆盖时会推断不会走到 default，但加个 exhaustive check 能防止将来扩展
StorageUsageRefreshDecision时漏掉分支：建议的改进
function refreshStorageUsage(decision: StorageUsageRefreshDecision) { const { refreshUser } = useAuthStore.getState(); switch (decision) { case "none": return; case "personal_quota": void refreshUser({ fields: ["quota"] }); return; case "teams": reloadTeamsForCurrentUser(); return; case "all": void refreshUser(); reloadTeamsForCurrentUser(); return; + default: + decision satisfies never; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/hooks/useStorageChangeEvents.ts` around lines 69 - 84, The switch statement in the refreshStorageUsage function lacks an exhaustive check for the StorageUsageRefreshDecision union type. Add a default case at the end of the switch statement that assigns the decision parameter to a variable of type never, which will cause a TypeScript compile error if new cases are added to StorageUsageRefreshDecision without being handled in the switch statement.src/api/routes/frontend.rs (1)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议添加注释说明 media-src 放宽的原因。
connect-src 有注释解释允许 http(s) 的原因（presigned upload/download），media-src 的改动也是出于同样的考量，但缺少对应说明。
💬 建议添加注释
// presigned upload / download 可能直接命中外部对象存储或 remote follower， // 这里必须允许浏览器向任意 http(s) 终点发起 XHR/fetch/WebSocket 连接。 "connect-src 'self' http: https: ws: wss: blob:; ", + // 同上，音视频播放可能使用 presigned URL 直接从对象存储加载媒体资源。 "media-src 'self' blob: http: https:; ",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/routes/frontend.rs` at line 33, The media-src Content Security Policy directive in the frontend.rs file has been relaxed to include blob:, http:, and https: sources, but it lacks an explanatory comment. Add a comment above or on the same line as the media-src directive explaining that the relaxation is necessary for presigned upload/download functionality, mirroring the pattern and reasoning documented in the nearby connect-src directive comment to maintain consistency and clarity for future maintainers.frontend-panel/src/lib/fileResource.test.ts (1)
8-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win这组测试没压到 previewLink 缓存分支，回归会悄悄溜进来。
建议补上
cachePreviewLinkResource/readCachedPreviewLinkResource/clearPreviewLinkResourceCache的用例，至少覆盖：过期条目淘汰、etag精确读取、skew 边界（expires_at - 10s）这三条核心分支。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/lib/fileResource.test.ts` around lines 8 - 79, The test file currently lacks coverage for the preview link resource cache functions. Add new test cases for cachePreviewLinkResource, readCachedPreviewLinkResource, and clearPreviewLinkResourceCache to cover three critical branches: expired entry eviction (verify that expired preview link resources are properly removed from cache), etag precise reading (verify that etag values are correctly retrieved and matched), and skew boundary handling (verify cache behavior at the expires_at minus 10 seconds boundary to ensure proper lifecycle management). These test cases should use the previewLinkResource metadata structure already referenced in the existing tests.frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.ts (1)
8-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win“文件切换重置”用例缺少脏状态断言，回归会继续漏。
这个用例名叫“file-scoped UI state”，但没覆盖
isDirty/confirmOpen。把它们加进断言，才能卡住这次的状态串文件问题。建议补测
it("resets file-scoped UI state when the file changes", () => { const selected = filePreviewDialogUiReducer( initialFilePreviewDialogUiState, { type: "selectOpenMethod", mode: "builtin.markdown" }, ); + const dirty = filePreviewDialogUiReducer(selected, { + type: "setDirty", + isDirty: true, + }); + const confirming = filePreviewDialogUiReducer(dirty, { + type: "setConfirmOpen", + confirmOpen: true, + }); - const expanded = filePreviewDialogUiReducer(selected, { + const expanded = filePreviewDialogUiReducer(confirming, { type: "setExpanded", expanded: true, }); const reset = filePreviewDialogUiReducer(expanded, { type: "syncMode", fileId: 7, preferredMode: "builtin.code", }); expect(reset).toMatchObject({ forceOpenMethodChooser: false, hasConfirmedInitialMode: false, hasManualExpanded: false, isExpanded: false, + isDirty: false, + confirmOpen: false, mode: "builtin.code", }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.ts` around lines 8 - 30, The test case "resets file-scoped UI state when the file changes" is incomplete as it only verifies a subset of the UI state properties after the syncMode action. The expect statement needs to be expanded to include assertions for the dirty state fields (isDirty and confirmOpen or similar state properties) to ensure they are properly reset when the file changes. Add these additional property checks to the toMatchObject call to verify all file-scoped state is correctly reset during the syncMode action with a new fileId.frontend-panel/src/components/files/preview/viewers/video/VideoPreview.tsx (1)
111-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift把资源鉴权和流会话策略从组件层挪走。
这里把
prepareAuthenticatedResource和createMediaStreamSession的分支决策写在展示组件里，后续很容易在其他 viewer 继续复制，策略一致性会被打散。建议下沉到 service 或专用 hook（例如统一输出resolvedPath/loading/error/retry），组件只负责渲染和交互。As per coding guidelines "Components must not directly infer remote resource authentication, caching, retry, refresh, or consistency policies; delegate to service layer".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/components/files/preview/viewers/video/VideoPreview.tsx` around lines 111 - 125, The VideoPreview component currently contains branching logic for deciding between createMediaStreamSession and prepareAuthenticatedResource within the component itself. Extract this resource authentication and session strategy logic out of the component into a dedicated service or custom hook that encapsulates the resolution strategy and returns a unified interface with resolvedPath, loading, error, and retry states. The VideoPreview component should then simply consume this hook or service, delegating all authentication, session creation, and error handling responsibilities away from the presentation layer. This prevents duplication across other viewer components and maintains consistent policy handling across the application.Source: Coding guidelines
frontend-panel/src/components/music/MusicPlayerHost.test.tsx (1)
80-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win把
resource从unknown收紧成明确类型，别让测试契约失焦。这里用
unknown会让无效资源对象也能混进队列，测试会放过真实契约破坏。建议收紧到ReturnType<typeof testTrackResource>（或项目里的资源类型别名）。♻️ 建议修改
+type TestTrackResource = ReturnType<typeof testTrackResource>; + function testTrackResource(path: string, mimeType = "audio/mpeg") { return derivedFileResource(path, { deliveryMode: "direct_url", mimeType, }); } function withTrackResource< Track extends { mimeType: string; path: string; - resource?: unknown; + resource?: TestTrackResource; }, ->(track: Track): Omit<Track, "resource"> & { resource: unknown } { +>(track: Track): Omit<Track, "resource"> & { resource: TestTrackResource } { return { ...track, resource: track.resource ?? testTrackResource(track.path, track.mimeType), }; } function installQueue( tracks: Array<{ mimeType: string; path: string; - resource?: unknown; + resource?: TestTrackResource; [key: string]: unknown; }>, ) { mockState.state.queue = tracks.map((track) => withTrackResource(track)); }Also applies to: 95-125, 161-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/components/music/MusicPlayerHost.test.tsx` at line 80, The `resource` property is declared with type `unknown` which is too loose and allows invalid resource objects to pass through the queue without proper type validation. Replace the `unknown` type annotation with a specific type like `ReturnType<typeof testTrackResource>` or the appropriate resource type used in the project. This tightens the type contract and ensures the test properly validates actual resource shapes rather than accepting any arbitrary values. Apply this fix to the resource property declarations in the MusicPlayerHost test file at all locations where this pattern appears.frontend-panel/src/pages/ShareViewPage.tsx (1)
99-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win同一套 share 资源路径分支写了两遍，后面很容易漂移。抽成一个构造器。
resolveShareResourceHandle和previewResources里的 download/imagePreview/thumbnail 分支完全重复。建议提取成单一 helper，避免后续改一处漏一处。♻️ 建议修改
+function buildSharePreviewPaths( + info: NonNullable<ReturnType<typeof useShareViewPageController>["info"]>, + token: string, + fileId: number, +) { + return info.share_type === "file" + ? { + download: shareService.downloadPath(token), + imagePreview: shareService.imagePreviewPath(token), + thumbnail: shareService.thumbnailPath(token), + } + : { + download: shareService.downloadFolderPath(token, fileId), + imagePreview: shareService.folderFileImagePreviewPath(token, fileId), + thumbnail: shareService.folderFileThumbnailPath(token, fileId), + }; +} + const resolveShareResourceHandle = useCallback<ResolveFileResourceHandle>( (_fileId, request) => { if (!retainedPreviewFile || !info) { return Promise.reject(new Error("share resource is unavailable")); } - - const downloadPath = - info.share_type === "file" - ? shareService.downloadPath(token) - : shareService.downloadFolderPath(token, retainedPreviewFile.id); - const imagePreviewPath = - info.share_type === "file" - ? shareService.imagePreviewPath(token) - : shareService.folderFileImagePreviewPath(token, retainedPreviewFile.id); - const thumbnailPath = - info.share_type === "file" - ? shareService.thumbnailPath(token) - : shareService.folderFileThumbnailPath(token, retainedPreviewFile.id); + const { download: downloadPath, imagePreview: imagePreviewPath, thumbnail: thumbnailPath } = + buildSharePreviewPaths(info, token, retainedPreviewFile.id); ... const previewResources = useMemo<FilePreviewResources | null>(() => { if (!retainedPreviewFile || !info) return null; - const downloadPath = ... - const imagePreviewPath = ... - const thumbnailPath = ... + const { download: downloadPath, imagePreview: imagePreviewPath, thumbnail: thumbnailPath } = + buildSharePreviewPaths(info, token, retainedPreviewFile.id);As per coding guidelines "Cross-API, cross-store, and cross-event-source business logic must be consolidated into testable hooks, coordinators, or resource modules (not scattered in useEffect across pages and components)"。
Also applies to: 150-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend-panel/src/pages/ShareViewPage.tsx` around lines 99 - 114, The code in resolveShareResourceHandle and previewResources contains duplicated conditional logic for constructing share resource paths (downloadPath, imagePreviewPath, and thumbnailPath) based on whether info.share_type is "file" or not. Extract this path construction logic into a single helper function that takes the share type, token, and optionally the file id as parameters and returns an object containing the three paths. Then replace both occurrences of the duplicated conditional branches with calls to this helper function to ensure consistency and make future updates easier to maintain.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.ts`:
- Around line 49-64: The syncMode case in the reducer is not resetting the
isDirty and confirmOpen fields when switching to a different file (when
resetForFile is true), causing the dirty state from the previous file to carry
over to the next file. Add isDirty and confirmOpen fields to the returned state
object in the syncMode case, following the same pattern as the other fields: set
them to false when resetForFile is true, otherwise preserve their current values
from state. This ensures that when switching files, both the UI state and dirty
flags are properly reset to prevent stale "unsaved changes" confirmations from
the previous file appearing on the new file.
In
`@frontend-panel/src/components/files/preview/dialog/usePreviewCapabilities.ts`:
- Around line 39-48: The useMemo hook for baseProfile currently requires both
previewAppsLoaded and thumbnailSupportLoaded to be true before calling
detectFilePreviewProfile. This causes the entire preview capability to be
unavailable when thumbnail support loading fails and stays false. Remove the
thumbnailSupportLoaded check from the condition in the useMemo dependency so
that preview profile detection only requires previewAppsLoaded to be true,
allowing thumbnail support to be optional and degrade gracefully rather than
blocking all preview functionality.
In
`@frontend-panel/src/components/files/preview/dialog/usePreviewSessionResources.ts`:
- Around line 78-87: The `stableArchiveManifestLoader` useCallback is narrowing
the options parameter type to only include `signal`, which removes the
`filenameEncoding` property. This breaks the type contract because downstream
code in useArchivePreviewState.ts calls loadManifest with both signal and
filenameEncoding parameters. Fix this by updating the options parameter type
annotation in the stableArchiveManifestLoader callback to include the full set
of properties (both signal and filenameEncoding) that
archiveManifestLoaderRef.current expects, ensuring the type definition matches
what is actually passed by callers.
In `@frontend-panel/src/components/music/MusicPlayerHost.tsx`:
- Around line 607-609: The trackKey is being constructed with trackResourcePath
which contains a temporary signed URL that changes when expired, causing
useEffect dependencies at L721 to re-trigger unnecessarily and interrupt
playback. Replace the temporary URL in the trackKey construction with a stable
identifier that persists across URL refreshes. Instead of using
trackResourcePath or track.path (which may contain the temporary request.url),
use a stable cache key or logical path that represents the actual resource and
won't change when the temporary signature expires. This ensures the trackKey
remains constant for the same resource regardless of URL refreshing.
In `@frontend-panel/src/hooks/useBlobUrl.ts`:
- Around line 637-643: The dependency array around line 686 is missing the
values that determine credentials and conditional headers behavior. When the
resource changes in a way that affects the output of resourceCredentials() or
resourceConditionalHeaders() functions, the cache key or request doesn't
invalidate because these derived values aren't tracked in the dependency list.
Add the results of resourceCredentials(resource, shouldSendResourceCredentials)
and resourceConditionalHeaders(resource) to the dependency array, or ensure that
resource itself and shouldSendResourceCredentials are included so that any
change in how these functions compute their values will trigger a re-fetch.
In `@frontend-panel/src/hooks/useFileResource.ts`:
- Around line 8-18: Remove the duplicate type definitions for
FileResourceRepresentation and ResolveFileResourceHandleRequest that are
currently defined inline in the file. Instead, import FileResourceRepresentation
and FileResourceHandleRequest directly from `@/types/api`. If the local name
ResolveFileResourceHandleRequest needs to be preserved for compatibility, create
a type alias that maps to FileResourceHandleRequest from the imported types.
This ensures the type definitions stay synchronized with the backend schema
changes rather than silently drifting.
In `@frontend-panel/src/lib/fileResource.ts`:
- Around line 153-160: The previewLinkCache grows indefinitely because expired
preview link entries are never removed. When an exact match is found but fails
the isPreviewLinkUsable check (indicating it has expired), the expired entry
should be deleted from previewLinkCache using its key before proceeding.
Additionally, review the cache write logic around line 178 where entries are
stored: ensure that not only is expiresAtMs greater than 0, but also that the
entry has not already expired by checking if the current time is before
expiresAtMs, and avoid caching entries that are already expired. This will
prevent stale entries from accumulating in the cache during long sessions.
In `@frontend-panel/src/lib/musicPlayer.ts`:
- Around line 156-159: The buildDirectMusicQueue function uses Promise.all which
causes the entire music queue to fail if any single buildDirectMusicTrack
rejects, preventing playback from starting. Replace Promise.all with
Promise.allSettled to allow the queue to preserve successfully resolved tracks
even when some track building operations fail, letting the calling code handle
missing tracks at the activeTrack level instead of failing the entire queue.
In `@frontend-panel/src/pages/file-browser/useFileBrowserContextValue.ts`:
- Around line 104-110: The buildDirectMusicQueue async operation in the file
browser lacks error handling and race condition protection. Add error handling
to the promise chain (either with .catch() or wrap in try-catch) to handle
potential rejections from buildDirectMusicQueue, and implement race condition
protection by tracking the request sequence (using a timestamp or request ID) to
ensure only the most recent click's playback result is applied, preventing
earlier requests from overwriting later ones when responses arrive out of order.
In `@frontend-panel/src/services/fileService.ts`:
- Around line 36-47: Remove the duplicate type definitions for
FileResourcePurpose, FileResourceRepresentation, and FileResourceHandleRequest
from fileService.ts, as these types already exist in api.generated.ts. Instead,
import these three types directly from api.generated.ts at the top of the
fileService.ts file to follow the rule against manually redefining shared
interface types.
In `@src/services/file_service/resource_handle.rs`:
- Around line 340-359: The current logic uses OR between extension whitelist and
MIME whitelist, which allows files with renderable extensions but non-renderable
MIME types (like a file named `.jpg` with `image/heic` MIME) to be incorrectly
marked as renderable. Fix this by inverting the logic: first check that the MIME
type is NOT a generic `image/*` type that isn't in the approved list, then
additionally verify that the file meets BOTH the extension whitelist AND MIME
whitelist requirements. Change the OR operator (||) in the matches! macro to AND
(&&) so both conditions must be satisfied. Additionally, add a regression test
case that validates the scenario where the extension is renderable but the MIME
type is not renderable, confirming it is properly rejected.
---
Outside diff comments:
In `@frontend-panel/src/pages/FileBrowserPage.test.tsx`:
- Around line 1952-1976: The test "moves items, publishes storage updates, and
shows the formatted move toast" is missing finally block protection for cleanup
operations. If any assertion fails before the unsubscribe() and
vi.useRealTimers() calls are reached, these cleanup operations will not execute,
leaving fake timers and subscriptions active for subsequent tests. Wrap the test
body in a try...finally block, moving the unsubscribe() call and
vi.useRealTimers() into the finally block to guarantee they always execute
regardless of test outcome.
---
Nitpick comments:
In
`@frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.ts`:
- Around line 8-30: The test case "resets file-scoped UI state when the file
changes" is incomplete as it only verifies a subset of the UI state properties
after the syncMode action. The expect statement needs to be expanded to include
assertions for the dirty state fields (isDirty and confirmOpen or similar state
properties) to ensure they are properly reset when the file changes. Add these
additional property checks to the toMatchObject call to verify all file-scoped
state is correctly reset during the syncMode action with a new fileId.
In `@frontend-panel/src/components/files/preview/viewers/video/VideoPreview.tsx`:
- Around line 111-125: The VideoPreview component currently contains branching
logic for deciding between createMediaStreamSession and
prepareAuthenticatedResource within the component itself. Extract this resource
authentication and session strategy logic out of the component into a dedicated
service or custom hook that encapsulates the resolution strategy and returns a
unified interface with resolvedPath, loading, error, and retry states. The
VideoPreview component should then simply consume this hook or service,
delegating all authentication, session creation, and error handling
responsibilities away from the presentation layer. This prevents duplication
across other viewer components and maintains consistent policy handling across
the application.
In
`@frontend-panel/src/components/folders/folder-tree/useFolderTreeController.ts`:
- Around line 372-390: The `folderParentIdsById` Map is being recreated on every
storage event within the useEffect hook in useFolderTreeController. To optimize
this, extract the Map creation logic into a separate useMemo hook that depends
only on nodeMap, then pass the memoized Map to the
decideFolderTreeStorageRefresh function call instead of creating it inline. This
prevents unnecessary Map reconstructions when the nodeMap hasn't changed.
In `@frontend-panel/src/components/music/MusicPlayerHost.test.tsx`:
- Line 80: The `resource` property is declared with type `unknown` which is too
loose and allows invalid resource objects to pass through the queue without
proper type validation. Replace the `unknown` type annotation with a specific
type like `ReturnType<typeof testTrackResource>` or the appropriate resource
type used in the project. This tightens the type contract and ensures the test
properly validates actual resource shapes rather than accepting any arbitrary
values. Apply this fix to the resource property declarations in the
MusicPlayerHost test file at all locations where this pattern appears.
In `@frontend-panel/src/hooks/useStorageChangeEvents.ts`:
- Around line 69-84: The switch statement in the refreshStorageUsage function
lacks an exhaustive check for the StorageUsageRefreshDecision union type. Add a
default case at the end of the switch statement that assigns the decision
parameter to a variable of type never, which will cause a TypeScript compile
error if new cases are added to StorageUsageRefreshDecision without being
handled in the switch statement.
In `@frontend-panel/src/lib/fileResource.test.ts`:
- Around line 8-79: The test file currently lacks coverage for the preview link
resource cache functions. Add new test cases for cachePreviewLinkResource,
readCachedPreviewLinkResource, and clearPreviewLinkResourceCache to cover three
critical branches: expired entry eviction (verify that expired preview link
resources are properly removed from cache), etag precise reading (verify that
etag values are correctly retrieved and matched), and skew boundary handling
(verify cache behavior at the expires_at minus 10 seconds boundary to ensure
proper lifecycle management). These test cases should use the
previewLinkResource metadata structure already referenced in the existing tests.
In `@frontend-panel/src/lib/storageMutationCoordinator.ts`:
- Around line 255-259: The decideVirtualStorageViewRefresh function has
redundant logic where both the if block checking isTagChangeEvent and the
subsequent return statement execute the same operation—returning
hasResourceReference(event). Remove the unnecessary if condition and the
isTagChangeEvent check, keeping only the single return statement that calls
hasResourceReference(event), since the outcome is identical regardless of
whether the event is a tag change event or not.
In `@frontend-panel/src/pages/ShareViewPage.tsx`:
- Around line 99-114: The code in resolveShareResourceHandle and
previewResources contains duplicated conditional logic for constructing share
resource paths (downloadPath, imagePreviewPath, and thumbnailPath) based on
whether info.share_type is "file" or not. Extract this path construction logic
into a single helper function that takes the share type, token, and optionally
the file id as parameters and returns an object containing the three paths. Then
replace both occurrences of the duplicated conditional branches with calls to
this helper function to ensure consistency and make future updates easier to
maintain.
In `@src/api/routes/frontend.rs`:
- Line 33: The media-src Content Security Policy directive in the frontend.rs
file has been relaxed to include blob:, http:, and https: sources, but it lacks
an explanatory comment. Add a comment above or on the same line as the media-src
directive explaining that the relaxation is necessary for presigned
upload/download functionality, mirroring the pattern and reasoning documented in
the nearby connect-src directive comment to maintain consistency and clarity for
future maintainers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f668d536-704b-4876-bd8e-2552ea1b74c5
📒 Files selected for processing (148)
frontend-panel/src/components/admin/settings/adminSettingsContentShared.tsxfrontend-panel/src/components/common/BatchActionBar.tsxfrontend-panel/src/components/files/FilePreview.test.tsxfrontend-panel/src/components/files/FilePreview.tsxfrontend-panel/src/components/files/FileTypeIcon.test.tsxfrontend-panel/src/components/files/FileTypeIcon.tsxfrontend-panel/src/components/files/VersionHistoryDialog.test.tsxfrontend-panel/src/components/files/VersionHistoryDialog.tsxfrontend-panel/src/components/files/preview/MusicPreview.test.tsxfrontend-panel/src/components/files/preview/MusicPreview.tsxfrontend-panel/src/components/files/preview/capabilities/file-capabilities.test.tsfrontend-panel/src/components/files/preview/capabilities/file-capabilities.tsfrontend-panel/src/components/files/preview/capabilities/fileCapabilityData.tsfrontend-panel/src/components/files/preview/capabilities/openWithLabel.tsfrontend-panel/src/components/files/preview/capabilities/types.tsfrontend-panel/src/components/files/preview/capabilities/video-browser-config.test.tsfrontend-panel/src/components/files/preview/capabilities/video-browser-config.tsfrontend-panel/src/components/files/preview/dialog/FilePreviewBody.test.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewBody.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewDialog.test.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewDialog.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewFileSummary.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewMethodChooser.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewPanel.test.tsxfrontend-panel/src/components/files/preview/dialog/FilePreviewPanel.tsxfrontend-panel/src/components/files/preview/dialog/UnsavedChangesGuard.test.tsxfrontend-panel/src/components/files/preview/dialog/UnsavedChangesGuard.tsxfrontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.tsfrontend-panel/src/components/files/preview/dialog/filePreviewDialogState.tsfrontend-panel/src/components/files/preview/dialog/useFilePreviewDialogModel.test.tsxfrontend-panel/src/components/files/preview/dialog/useFilePreviewDialogModel.tsfrontend-panel/src/components/files/preview/dialog/usePreviewCapabilities.test.tsxfrontend-panel/src/components/files/preview/dialog/usePreviewCapabilities.tsfrontend-panel/src/components/files/preview/dialog/usePreviewDialogChromeState.test.tsxfrontend-panel/src/components/files/preview/dialog/usePreviewDialogChromeState.tsfrontend-panel/src/components/files/preview/dialog/usePreviewOpenMode.test.tsxfrontend-panel/src/components/files/preview/dialog/usePreviewOpenMode.tsfrontend-panel/src/components/files/preview/dialog/usePreviewSessionResources.test.tsxfrontend-panel/src/components/files/preview/dialog/usePreviewSessionResources.tsfrontend-panel/src/components/files/preview/navigation/imagePreviewNavigation.test.tsfrontend-panel/src/components/files/preview/navigation/imagePreviewNavigation.tsfrontend-panel/src/components/files/preview/resources/filePreviewResources.tsfrontend-panel/src/components/files/preview/shared/AnimatedCollapsible.tsxfrontend-panel/src/components/files/preview/shared/PreviewError.test.tsxfrontend-panel/src/components/files/preview/shared/PreviewError.tsxfrontend-panel/src/components/files/preview/shared/PreviewLoadingState.tsxfrontend-panel/src/components/files/preview/shared/PreviewSurface.tsxfrontend-panel/src/components/files/preview/shared/PreviewUnavailable.test.tsxfrontend-panel/src/components/files/preview/shared/PreviewUnavailable.tsxfrontend-panel/src/components/files/preview/useFilePreviewDialogModel.tsfrontend-panel/src/components/files/preview/viewers/archive/ArchivePreview.test.tsxfrontend-panel/src/components/files/preview/viewers/archive/ArchivePreview.tsxfrontend-panel/src/components/files/preview/viewers/archive/ArchivePreviewContent.tsxfrontend-panel/src/components/files/preview/viewers/archive/archivePreviewFormatCapabilities.test.tsfrontend-panel/src/components/files/preview/viewers/archive/archivePreviewFormatCapabilities.tsfrontend-panel/src/components/files/preview/viewers/archive/archivePreviewTypes.tsfrontend-panel/src/components/files/preview/viewers/archive/archivePreviewUtils.test.tsfrontend-panel/src/components/files/preview/viewers/archive/archivePreviewUtils.tsfrontend-panel/src/components/files/preview/viewers/archive/useArchivePreviewState.test.tsxfrontend-panel/src/components/files/preview/viewers/archive/useArchivePreviewState.tsfrontend-panel/src/components/files/preview/viewers/external/EmbeddedWebAppPreview.tsxfrontend-panel/src/components/files/preview/viewers/external/UrlTemplatePreview.test.tsxfrontend-panel/src/components/files/preview/viewers/external/UrlTemplatePreview.tsxfrontend-panel/src/components/files/preview/viewers/image/BlobImagePreview.test.tsxfrontend-panel/src/components/files/preview/viewers/image/BlobImagePreview.tsxfrontend-panel/src/components/files/preview/viewers/image/ImagePreviewPanel.test.tsxfrontend-panel/src/components/files/preview/viewers/image/ImagePreviewPanel.tsxfrontend-panel/src/components/files/preview/viewers/image/useImagePreviewTransform.test.tsxfrontend-panel/src/components/files/preview/viewers/image/useImagePreviewTransform.tsfrontend-panel/src/components/files/preview/viewers/pdf/PdfPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/pdf/PdfPreview.tsxfrontend-panel/src/components/files/preview/viewers/text/CodePreviewEditor.test.tsxfrontend-panel/src/components/files/preview/viewers/text/CodePreviewEditor.tsxfrontend-panel/src/components/files/preview/viewers/text/CsvTablePreview.test.tsxfrontend-panel/src/components/files/preview/viewers/text/CsvTablePreview.tsxfrontend-panel/src/components/files/preview/viewers/text/JsonPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/text/JsonPreview.tsxfrontend-panel/src/components/files/preview/viewers/text/MarkdownPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/text/MarkdownPreview.tsxfrontend-panel/src/components/files/preview/viewers/text/TextCodePreview.test.tsxfrontend-panel/src/components/files/preview/viewers/text/TextCodePreview.tsxfrontend-panel/src/components/files/preview/viewers/text/TextCodePreviewToolbar.tsxfrontend-panel/src/components/files/preview/viewers/text/XmlPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/text/XmlPreview.tsxfrontend-panel/src/components/files/preview/viewers/text/codePreviewPrism.test.tsfrontend-panel/src/components/files/preview/viewers/text/codePreviewPrism.tsfrontend-panel/src/components/files/preview/viewers/text/prismClassNames.test.tsfrontend-panel/src/components/files/preview/viewers/text/prismClassNames.tsfrontend-panel/src/components/files/preview/viewers/video/VideoPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/video/VideoPreview.tsxfrontend-panel/src/components/files/preview/viewers/wopi/WopiPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/wopi/WopiPreview.tsxfrontend-panel/src/components/files/preview/viewers/wopi/wopiSessionResource.tsfrontend-panel/src/components/files/useUploadAreaManager.tsfrontend-panel/src/components/folders/FolderTree.test.tsxfrontend-panel/src/components/folders/folder-tree/useFolderTreeController.tsfrontend-panel/src/components/music/MusicPlayerHost.test.tsxfrontend-panel/src/components/music/MusicPlayerHost.tsxfrontend-panel/src/hooks/useBlobUrl.tsfrontend-panel/src/hooks/useFileResource.tsfrontend-panel/src/hooks/useStorageChangeEvents.tsfrontend-panel/src/hooks/useTextContent.tsfrontend-panel/src/lib/authenticatedResource.test.tsfrontend-panel/src/lib/authenticatedResource.tsfrontend-panel/src/lib/fileResource.test.tsfrontend-panel/src/lib/fileResource.tsfrontend-panel/src/lib/fileResourceCacheInvalidation.test.tsfrontend-panel/src/lib/fileResourceCacheInvalidation.tsfrontend-panel/src/lib/musicPlayer.test.tsfrontend-panel/src/lib/musicPlayer.tsfrontend-panel/src/lib/pwaWarmupLoaders.test.tsfrontend-panel/src/lib/pwaWarmupLoaders.tsfrontend-panel/src/lib/resourceRequest.test.tsfrontend-panel/src/lib/resourceRequest.tsfrontend-panel/src/lib/storageMutationCoordinator.test.tsfrontend-panel/src/lib/storageMutationCoordinator.tsfrontend-panel/src/pages/CategoryBrowserPage.test.tsxfrontend-panel/src/pages/CategoryBrowserPage.tsxfrontend-panel/src/pages/FileBrowserPage.test.tsxfrontend-panel/src/pages/FileBrowserPage.tsxfrontend-panel/src/pages/SearchBrowserPage.test.tsxfrontend-panel/src/pages/SearchBrowserPage.tsxfrontend-panel/src/pages/ShareViewPage.test.tsxfrontend-panel/src/pages/ShareViewPage.tsxfrontend-panel/src/pages/admin/AdminSettingsPage.test.tsxfrontend-panel/src/pages/file-browser/FileBrowserDialogs.test.tsxfrontend-panel/src/pages/file-browser/FileBrowserDialogs.tsxfrontend-panel/src/pages/file-browser/useFileBrowserBatchActions.tsxfrontend-panel/src/pages/file-browser/useFileBrowserContextValue.tsfrontend-panel/src/pages/file-browser/useFileBrowserDragAndDrop.tsfrontend-panel/src/services/api.generated.tsfrontend-panel/src/services/fileService.tsfrontend-panel/src/stores/fileStore/crudSlice.tsfrontend-panel/src/stores/musicPlayerStore.tssrc/api/dto/files.rssrc/api/openapi.rssrc/api/routes/files/access.rssrc/api/routes/files/mod.rssrc/api/routes/frontend.rssrc/api/routes/share_public.rssrc/services/file_service/download/build.rssrc/services/file_service/mod.rssrc/services/file_service/resource_handle.rssrc/services/share_service/content.rssrc/services/share_service/mod.rstests/test_files.rstests/test_shares.rstests/test_team_space.rs
💤 Files with no reviewable changes (5)
- frontend-panel/src/components/files/preview/MusicPreview.test.tsx
- frontend-panel/src/components/files/preview/MusicPreview.tsx
- frontend-panel/src/components/files/preview/capabilities/types.ts
- frontend-panel/src/components/files/preview/useFilePreviewDialogModel.ts
- frontend-panel/src/components/files/preview/capabilities/fileCapabilityData.ts
… resource handling - Extract `useVideoPreviewResource` hook from `VideoPreview` component to centralize video resource resolution logic with retry support - Remove frontend build job from CI workflow and eliminate build artifact dependencies across jobs - Add comprehensive cache validation for preview-link resources with expiry checks and eviction logic - Improve resource handle identity tracking by using file ID only instead of combining with resource path - Refactor type exports moving `FileResourceHandleRequest` and related types from `fileService.ts` to API type definitions - Enhance browser image rendering detection to reject non-renderable image MIME types with renderable extensions - Add exhaustive checks for storage refresh decision switches - Improve music queue building with `Promise.allSettled` to handle partial failures gracefully - Optimize hook dependencies by memoizing derived values in `useFolderTreeController` and `useBlobUrl` - Add request sequence tracking to prevent stale music playback operations - Enhance build script fallback HTML with configurable metadata placeholders - Update CSP policy comments to clarify media source requirements for presigned preview links - Reset preview dialog dirty state and confirmation modal state on file changes - Remove redundant thumbnail support loading check from preview capabilities detection - Refactor share resource path construction into dedicated helper function - Improve test reliability with proper timer cleanup and type annotations - Format and align test assertions for better readability across Rust test suite
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend-panel/src/components/files/preview/viewers/video/useVideoPreviewResource.ts`:
- Around line 38-48: The dependency chain between the two useEffect hooks (the
one at lines 38-48 that calls setResourceVersion and the one at lines 50-86 that
depends on resourceKey) causes the second effect to execute twice per input
change: once with the old resourceKey and once with the new resourceKey,
resulting in duplicate createMediaStreamSession calls. Refactor the logic to
avoid this double execution by either consolidating the effects, synchronizing
the state updates, or restructuring the dependency arrays so that the resource
parsing effect only runs once per actual input change rather than triggering on
intermediate state transitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bab44d8-a209-4ed3-9c8a-9eb99e313b6e
📒 Files selected for processing (29)
.github/workflows/rust.ymlbuild.rsfrontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.tsfrontend-panel/src/components/files/preview/dialog/filePreviewDialogState.tsfrontend-panel/src/components/files/preview/dialog/usePreviewCapabilities.tsfrontend-panel/src/components/files/preview/dialog/usePreviewSessionResources.tsfrontend-panel/src/components/files/preview/viewers/video/VideoPreview.tsxfrontend-panel/src/components/files/preview/viewers/video/useVideoPreviewResource.tsfrontend-panel/src/components/folders/folder-tree/useFolderTreeController.tsfrontend-panel/src/components/music/MusicPlayerHost.test.tsxfrontend-panel/src/components/music/MusicPlayerHost.tsxfrontend-panel/src/hooks/useBlobUrl.tsfrontend-panel/src/hooks/useFileResource.tsfrontend-panel/src/hooks/useStorageChangeEvents.tsfrontend-panel/src/lib/fileResource.test.tsfrontend-panel/src/lib/fileResource.tsfrontend-panel/src/lib/musicPlayer.tsfrontend-panel/src/lib/storageMutationCoordinator.tsfrontend-panel/src/pages/FileBrowserPage.test.tsxfrontend-panel/src/pages/ShareViewPage.tsxfrontend-panel/src/pages/file-browser/useFileBrowserContextValue.tsfrontend-panel/src/services/fileService.tsfrontend-panel/src/types/api.tssrc/api/routes/files/access.rssrc/api/routes/files/mod.rssrc/api/routes/frontend.rssrc/services/file_service/resource_handle.rstests/test_files.rstests/test_team_space.rs
💤 Files with no reviewable changes (2)
- .github/workflows/rust.yml
- frontend-panel/src/lib/storageMutationCoordinator.ts
✅ Files skipped from review due to trivial changes (1)
- build.rs
🚧 Files skipped from review as they are similar to previous changes (19)
- frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.test.ts
- src/api/routes/frontend.rs
- tests/test_files.rs
- frontend-panel/src/pages/file-browser/useFileBrowserContextValue.ts
- tests/test_team_space.rs
- frontend-panel/src/components/folders/folder-tree/useFolderTreeController.ts
- src/api/routes/files/mod.rs
- frontend-panel/src/components/files/preview/dialog/usePreviewCapabilities.ts
- frontend-panel/src/components/files/preview/dialog/filePreviewDialogState.ts
- frontend-panel/src/pages/ShareViewPage.tsx
- frontend-panel/src/components/files/preview/dialog/usePreviewSessionResources.ts
- src/services/file_service/resource_handle.rs
- frontend-panel/src/components/music/MusicPlayerHost.test.tsx
- frontend-panel/src/hooks/useStorageChangeEvents.ts
- frontend-panel/src/hooks/useBlobUrl.ts
- frontend-panel/src/lib/fileResource.ts
- src/api/routes/files/access.rs
- frontend-panel/src/lib/musicPlayer.ts
- frontend-panel/src/pages/FileBrowserPage.test.tsx
Simplify video preview resource management by removing redundant version tracking and ref-based input comparison. Extend resource request utilities with credential and conditional header policy helpers, and add resource handle resolution support. Improve test coverage across file previews, music player, browser pages, and file service with comprehensive validation of resource resolution, deletion mutation coordination, and adjacent image navigation. - Remove resourceVersion state and inputsRef from useVideoPreviewResource - Add test for single stream session per resource change in VideoPreview - Add test for omitting unresolvable direct music tracks in musicPlayer - Add resource policy validation tests in musicPlayer for share scope - Implement resourceCredentials, resourceConditionalHeaders helpers in resourceRequest - Add readyFileResourceFromRequest utility for promoting resource requests to ready handles - Add resource handle resolution tests across ShareViewPage scenarios - Add local storage delete mutation coordination tests in CategoryBrowserPage and SearchBrowserPage - Add resolveResourceHandle tests in fileService for personal and team scopes - Update image navigation test to use HEIC format in ShareViewPage
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend-panel/src/components/files/preview/viewers/video/useVideoPreviewResource.ts`:
- Around line 32-34: The resourceKey constructed on line 32 in
useVideoPreviewResource.ts only includes requestPath, resourceMode, and
retryCount, which causes stale resolvedPath values to be reused when resource or
createMediaStreamSession dependencies change but those three properties remain
unchanged. Fix this by including additional identifying information (such as
relevant properties or identifiers from resource and createMediaStreamSession)
in the resourceKey construction on line 32, ensuring that changes to these
dependencies result in a different key that prevents cached stale paths from
being incorrectly reused on line 34.
In `@frontend-panel/src/pages/ShareViewPage.test.tsx`:
- Around line 21-24: The resolve function's request parameter is currently typed
as the loose Record<string, string>, which does not provide proper type safety
and allows field typos or contract drift to go uncaught by TypeScript. Replace
the Record<string, string> type with the proper backend schema request type by
importing it from `@/types/api` using import type syntax. Identify the correct
request type from `@/types/api` that corresponds to this resource handle request
and update the resolve function signature accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a089bbfa-49cb-4c39-a5e0-13007d561abc
📒 Files selected for processing (8)
frontend-panel/src/components/files/preview/viewers/video/VideoPreview.test.tsxfrontend-panel/src/components/files/preview/viewers/video/useVideoPreviewResource.tsfrontend-panel/src/lib/musicPlayer.test.tsfrontend-panel/src/lib/resourceRequest.test.tsfrontend-panel/src/pages/CategoryBrowserPage.test.tsxfrontend-panel/src/pages/SearchBrowserPage.test.tsxfrontend-panel/src/pages/ShareViewPage.test.tsxfrontend-panel/src/services/fileService.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend-panel/src/components/files/preview/viewers/video/VideoPreview.test.tsx
- frontend-panel/src/lib/musicPlayer.test.ts
Prevent video preview resource from reloading when input identity changes by introducing identity tracking mechanism. - Add input identity ref and version tracking to detect prop changes - Split effects to handle identity changes separately from resource loading - Reset retry count when resource or stream session function changes - Update resource key to include identity version for proper cache invalidation - Fix type annotation in ShareViewPage test for FileResourceHandleRequest
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors several file preview and file management components to improve modularity, maintainability, and testability. The main changes include reorganizing file preview capabilities into a dedicated subdirectory, updating resource and capability types, and consolidating cache invalidation logic for file resource mutations. Additionally, the music file preview component has been removed, and related tests and imports have been updated accordingly.
File Preview and Capability Refactoring:
Moved
file-capabilitiesand related types fromfrontend-panel/src/components/files/preview/to a newcapabilitiessubdirectory, updating all imports and tests to reference the new location. This improves code organization and clarity. [1] [2] [3] [4]Updated the import path for
CodePreviewEditorto reflect a new location underviewers/text/, ensuring consistency with the new file structure.File Preview Resource and Dialog API Changes:
FilePreviewandFilePreviewDialogcomponents to use a newresourcesprop of typeFilePreviewResourcesinstead of multiple individual props for paths and factory functions. This simplifies the API and centralizes resource management. All usages and tests have been updated to match the new interface. [1] [2] [3] [4] [5] [6]File Resource Cache Invalidation Improvements:
invalidateFileResourceCachesForMutationutility. This replaces separate calls toinvalidateBlobUrlandinvalidateTextContentin version history logic and tests, making cache management more robust and maintainable. [1] [2] [3] [4] [5] [6]Batch Action Bar and Storage Mutation Coordination:
BatchActionBarto use a newbeginLocalStorageDeleteMutationcoordinator, replacing manual echo tracking and rollback logic. This streamlines mutation management and error handling. [1] [2] [3]Other Notable Changes:
MusicPreviewcomponent and all related logic, indicating a possible deprecation or replacement of music file preview functionality.Summary by CodeRabbit
发行说明
disposition=inline/attachment，并增强对应安全兜底。