updated with some api consolidation and changes#2596
Conversation
📝 WalkthroughWalkthroughThe PR renames stream-token based viewing to view-token based viewing across backend and frontend code, adds a dedicated inline view endpoint, updates media streaming to require media-only requests, and rewires viewer URL generation and token plumbing. It also tightens user permission normalization. ChangesStream-to-view token migration
UserEdit Permission Fix
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/http/middleware.go (1)
146-151: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
/rawto the fast-path skip list.
GET /rawnow maps to the samepublicDownloadHandleras/resources/download, but it still falls through toFileInfoFasterFuncand view-token minting here. That adds avoidable I/O and cache churn on a hot download path.🤖 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 `@backend/http/middleware.go` around lines 146 - 151, The fast-path skip list in the middleware path check does not include the new GET /raw route, so requests still fall through to FileInfoFasterFunc and view-token minting. Update the conditional in the middleware that checks r.Method and r.URL.Path to treat /raw the same as /resources/download, using the existing path-matching logic alongside the other download/view exclusions.
🧹 Nitpick comments (11)
frontend/src/components/prompts/UserEdit.vue (1)
334-346: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect null/undefined handling.
Logic is sound: strict checks avoid misclassifying valid falsy/empty permission objects. Optionally, the repeated
=== null || === undefinedpairs could be simplified using== null(which is true for bothnullandundefinedin JS) to reduce verbosity.♻️ Optional simplification
- if ((user.permissions === null || user.permissions === undefined) && user.account?.permissions !== null && user.account?.permissions !== undefined) { + if (user.permissions == null && user.account?.permissions != null) { user.permissions = { ...user.account.permissions }; - if (user.permissions.download === null || user.permissions.download === undefined) { + if (user.permissions.download == null) { user.permissions.download = true; } } - if (user.permissions === null || user.permissions === undefined) { + if (user.permissions == null) { user.permissions = this.defaultPermissions(); }🤖 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/src/components/prompts/UserEdit.vue` around lines 334 - 346, In normalizeFormUser, the null/undefined handling is already correct, but the repeated strict checks for user.permissions and user.account.permissions can be simplified. Update the permission-null checks in this method to use a single nullish comparison pattern so the code stays equivalent while being less verbose, and keep the existing behavior in UserEdit.vue unchanged.CHANGELOG.md (1)
32-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant/conflicting endpoint notes in same changelog block.
Line 34 still says "new resources/stream endpoint" while lines 38-39 describe moving the stream API to
/media/streamand splitting non-media viewing to/api/resources/view. Both bullets describe the same endpoint evolution within the same v2.0.0 "Notes" section, which reads as contradictory to end users reading the changelog.📝 Suggested consolidation
**Notes**: - new dropdown and input styles - - new resources/stream endpoint - user updates are more granular, don't include entire user payload. - `user.id` has been moved to a backend property and all frontend apis now query users by username. Swagger has been updated. - removed legacy and deprecated properties from API responses and generated config output - Moved stream api to `/media/stream` - `/api/media/stream` is audio/video only (range-based chunking). Non-media inline viewing uses `GET /api/resources/view`. Both endpoints use the same `viewToken` from file metadata.🤖 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 `@CHANGELOG.md` around lines 32 - 39, The changelog Notes section contains duplicate/conflicting endpoint bullets, so consolidate the streaming/viewing changes into a single clear entry. Update the relevant bullets in CHANGELOG.md to describe the final API split using the existing stream and view endpoint wording, and remove or rewrite the older “new resources/stream endpoint” note so it does not contradict the later `/media/stream` and `/api/resources/view` descriptions.frontend/src/utils/mediaFile.js (1)
1-4: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a few more common audio/video extensions.
MEDIA_EXTENSIONSomits some common formats (e.g..3gp,.flv,.ts,.mpg/.mpeg,.m2ts). Files with these extensions will fall through to the non-media/resources/viewpath instead of the range-based/media/streamendpoint, degrading seek/scrub UX for those formats.♻️ Suggested extension additions
const MEDIA_EXTENSIONS = new Set([ '.mp3', '.mp4', '.m4a', '.flac', '.wav', '.ogg', '.oga', '.opus', '.webm', '.mkv', '.avi', '.mov', '.wmv', '.m4v', '.aac', '.wma', + '.3gp', '.flv', '.ts', '.mpg', '.mpeg', '.m2ts', ])🤖 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/src/utils/mediaFile.js` around lines 1 - 4, MEDIA_EXTENSIONS is missing several common audio/video formats, so add the suggested extensions (such as .3gp, .flv, .ts, .mpg/.mpeg, and .m2ts) to the MEDIA_EXTENSIONS set in mediaFile.js. Keep the existing isMediaFile logic unchanged so files with these extensions correctly route to /media/stream instead of falling through to /resources/view.frontend/src/api/media.js (2)
105-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct unit tests for the new
media.jsexports.
getStreamURL/getStreamURLPublicare only exercised indirectly via mocks inresources.view.test.js; their own validation branches (missing source/viewToken) and param construction aren't directly tested.🤖 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/src/api/media.js` around lines 105 - 142, Add direct unit coverage for the new media helpers in media.js: create tests for getStreamURL and getStreamURLPublic that verify the missing-source and missing-viewToken validation branches throw as expected, and that the returned URL includes the correct params built via getApiPath/getPublicApiPath. Use the function names getStreamURL and getStreamURLPublic so the tests stay anchored even if surrounding code changes.
105-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate URL-building logic across
media.jsandresources.js.
getStreamURL/getStreamURLPublichere are structurally identical togetRawViewURL/getRawViewURLPublicinresources.js(same validation, same params shape, same try/catch/notify pattern) — only the endpoint string differs (media/streamvsresources/view). Consider extracting a shared helper to avoid the two paths drifting apart on future fixes (e.g. sessionId handling, error messaging).♻️ Example consolidation
+function buildResourceURL(endpoint, source, path, viewToken, errorLabel) { + if (!source) throw new Error('no source provided') + if (!viewToken) throw new Error('view token required') + try { + const params = { source, file: path, viewToken, sessionId: state.sessionId } + return window.origin + getApiPath(endpoint, params) + } catch (err) { + notify.showError(err.message || errorLabel) + throw err + } +} + +export function getStreamURL(source, path, viewToken) { + return buildResourceURL('media/stream', source, path, viewToken, 'Error getting stream URL') +}🤖 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/src/api/media.js` around lines 105 - 142, The URL-building and validation logic in getStreamURL and getStreamURLPublic is duplicated with the equivalent helpers in resources.js, so future fixes can drift between the two paths. Extract the shared source/path/viewToken/sessionId construction and notify/error handling into a common helper, then have getStreamURL and getStreamURLPublic call it while only passing the endpoint-specific path (media/stream vs resources/view).frontend/src/api/resources.view.test.js (1)
1-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend coverage to shareInfo and download-fallback branches.
Current tests only cover the non-share, viewToken-present paths.
getViewURL's share-based routing (getRawViewURLPublic) and theallowDownloadFallback=truebranch (getDownloadURL/getDownloadURLPublic) aren't exercised here, despite being core to the view-token migration's contract.🤖 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/src/api/resources.view.test.js` around lines 1 - 57, The getViewURL tests currently miss the share-based and download-fallback routing paths, so extend this suite to cover those branches. Add cases that use the mocked store.state.shareInfo and getters.isShare to verify getRawViewURLPublic is used when sharing is enabled, and add allowDownloadFallback=true scenarios to exercise getDownloadURL/getDownloadURLPublic behavior. Use getViewURL and the existing mocked `@/api/media` helpers to confirm the returned URLs switch correctly across these branches.frontend/src/views/files/ThreeJs.vue (2)
365-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate sibling-token matching logic across files.
resolveViewTokenForPathreimplements essentially the same name/path matching heuristic asviewTokenForSiblinginfrontend/src/utils/htmlPreview.ts. Consolidating into a single shared utility would let both the correctness fix and any future refinement (nested-path handling) be applied once.🤖 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/src/views/files/ThreeJs.vue` around lines 365 - 374, The sibling token lookup logic is duplicated between resolveViewTokenForPath in ThreeJs.vue and viewTokenForSibling in htmlPreview.ts, so please consolidate the name/path matching into one shared utility and have resolveViewTokenForPath call it instead of reimplementing the heuristic. Keep the existing fbdata.path and parentDirItems behavior intact, but route both callers through the shared helper so any future nested-path or matching fixes only need to be made once.
388-393: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
getAllfallback.
parsed.searchParams.get("file") || parsed.searchParams.getAll("file")[0]—get()already returns the first value of a repeated param, sogetAll("file")[0]never differs fromget("file")when a match exists; this fallback is dead 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/src/views/files/ThreeJs.vue` around lines 388 - 393, The URL parsing in ThreeJs.vue has a redundant fallback in the file lookup logic: `parsed.searchParams.get("file") || parsed.searchParams.getAll("file")[0]` in the URL handling path. Update the code in the `resourceViewUrl`/URL parsing block to use only `searchParams.get("file")`, since `get()` already returns the first matching value and the `getAll()[0]` branch is dead code. Keep the existing behavior otherwise, just simplify the `filePath` assignment and related return flow.backend/http/view.go (1)
39-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the duplicated token/validation preamble shared with
streamHandler.
viewHandlerandstreamHandler(backend/http/stream.go, Lines 130-159) share nearly identical logic: archive/algo rejection, single-file enforcement,viewTokenextraction,SanitizePath,validateViewGrant,GetScopeForSourceName, andJoinPathAsUnix. LikewisepublicViewHandlermirrorspublicStreamHandler. Extracting a shared helper that returns(source, scopedPath, status, error)would remove the duplication and prevent the two flows from drifting (e.g. one endpoint tightening validation while the other lags).As per path instructions: "reduce duplication where there is clear duplication found, use consolidated functions."
🤖 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 `@backend/http/view.go` around lines 39 - 66, The `viewHandler` preamble is duplicated with `streamHandler` and the public variants, so extract the shared archive/algo rejection, single-file check, token extraction, path sanitization, grant validation, scope lookup, and scoped-path construction into a common helper. Use the existing `viewHandler`, `streamHandler`, `publicViewHandler`, `publicStreamHandler`, and `validateViewGrant` symbols to centralize the logic and have both handlers call that helper before invoking their respective file-serving functions.Source: Path instructions
backend/http/stream_grant.go (1)
85-95: 🚀 Performance & Scalability | 🔵 TrivialPer-file view-grant minting on every listing may bloat the shared cache.
attachViewTokensForDirectorymints a fresh random token (cryptoRandomHex) and writes aViewGrantsCacheentry for every non-directory child on each listing request. For large directories or frequent re-listings, this is O(n) crypto + cache writes per request and can accumulate many short-lived entries in the global 15-minute cache. Consider reusing a deterministic/idempotent grant per (user, source, path) window or bounding cache size, if listings can be large or hot.🤖 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 `@backend/http/stream_grant.go` around lines 85 - 95, The per-file minting in attachViewTokensForDirectory is creating a new RandomHex token and ViewGrantsCache entry for every non-directory child on each listing, which can bloat the shared cache and add O(n) work per request. Update the flow around mintViewGrant and attachViewTokensForDirectory so grants are reused deterministically or idempotently for the same (user, source, path) within the cache window, or otherwise bound the number of cached entries created during listings. Keep the existing childPath-based lookup and ViewToken assignment behavior, but avoid generating fresh grants for every repeated listing.Source: Path instructions
backend/http/httpRouter.go (1)
140-159: 📐 Maintainability & Code Quality | 🔵 TrivialConsider consolidating repeated timeout+helper wrapping.
The
withTimeout(mediaHandlerTimeout, withUserHelper(...))/withTimeout(mediaHandlerTimeout, withHashFileHelper(...))pattern is repeated 7 times across these two route blocks. A small local helper would reduce duplication and make future timeout tuning a single-point change.♻️ Suggested consolidation
+ mediaTimeout := func(fn handleFunc) handleFunc { return withTimeout(mediaHandlerTimeout, withUserHelper(fn)) } + mediaTimeoutPublic := func(fn handleFunc) handleFunc { return withTimeout(mediaHandlerTimeout, withHashFileHelper(fn)) } + - api.HandleFunc("GET /resources/view", withTimeout(mediaHandlerTimeout, withUserHelper(viewHandler))) + api.HandleFunc("GET /resources/view", mediaTimeout(viewHandler)) ... - publicApi.HandleFunc("GET /resources/view", withTimeout(mediaHandlerTimeout, withHashFileHelper(publicViewHandler))) + publicApi.HandleFunc("GET /resources/view", mediaTimeoutPublic(publicViewHandler))As per path instructions, "reduce duplication where there is clear duplication found, use consolidated functions."
Also applies to: 206-212
🤖 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 `@backend/http/httpRouter.go` around lines 140 - 159, Repeated timeout-plus-helper wrapping for resource view/preview routes is duplicated across the api and publicApi registrations, so extract a small local helper to centralize this pattern. Update the route setup around withTimeout, withUserHelper, and withHashFileHelper to use that helper for the view/preview handlers, keeping the timeout values and handler behavior unchanged while making future timeout updates a single-point change.Source: Path instructions
🤖 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/src/utils/htmlPreview.ts`:
- Around line 49-62: The sibling-token fallback in viewTokenForSibling is
matching by basename and endsWith, which can return a token for the wrong path
and cause getViewURL to build an invalid request. Update the lookup in
htmlPreview.ts so it only uses tokens from the resolved directory, preferably by
requiring an exact entry.path match (or equivalent directory-scoped check)
before falling back, while keeping the existing viewToken return behavior
unchanged.
In `@frontend/src/views/files/ThreeJs.vue`:
- Around line 341-374: The texture URL lookup in ThreeJs.vue is too strict:
resolveTextureUrl can rewrite bare names into nested paths, but
resolveViewTokenForPath only matches the current file or flat siblings, so
getViewURL() ends up without a token for nested assets. Update resourceViewUrl
and resolveViewTokenForPath to recognize rewritten texture paths (for example,
modelDir/textures/... and other nested sibling locations), and add a fallback
path when no exact sibling token exists so texture loading can still resolve.
Keep the existing ThreeJs.vue helpers and resourcesApi.getViewURL flow, but make
sure the lookup can derive a usable token or fallback for nested asset layouts.
---
Outside diff comments:
In `@backend/http/middleware.go`:
- Around line 146-151: The fast-path skip list in the middleware path check does
not include the new GET /raw route, so requests still fall through to
FileInfoFasterFunc and view-token minting. Update the conditional in the
middleware that checks r.Method and r.URL.Path to treat /raw the same as
/resources/download, using the existing path-matching logic alongside the other
download/view exclusions.
---
Nitpick comments:
In `@backend/http/httpRouter.go`:
- Around line 140-159: Repeated timeout-plus-helper wrapping for resource
view/preview routes is duplicated across the api and publicApi registrations, so
extract a small local helper to centralize this pattern. Update the route setup
around withTimeout, withUserHelper, and withHashFileHelper to use that helper
for the view/preview handlers, keeping the timeout values and handler behavior
unchanged while making future timeout updates a single-point change.
In `@backend/http/stream_grant.go`:
- Around line 85-95: The per-file minting in attachViewTokensForDirectory is
creating a new RandomHex token and ViewGrantsCache entry for every non-directory
child on each listing, which can bloat the shared cache and add O(n) work per
request. Update the flow around mintViewGrant and attachViewTokensForDirectory
so grants are reused deterministically or idempotently for the same (user,
source, path) within the cache window, or otherwise bound the number of cached
entries created during listings. Keep the existing childPath-based lookup and
ViewToken assignment behavior, but avoid generating fresh grants for every
repeated listing.
In `@backend/http/view.go`:
- Around line 39-66: The `viewHandler` preamble is duplicated with
`streamHandler` and the public variants, so extract the shared archive/algo
rejection, single-file check, token extraction, path sanitization, grant
validation, scope lookup, and scoped-path construction into a common helper. Use
the existing `viewHandler`, `streamHandler`, `publicViewHandler`,
`publicStreamHandler`, and `validateViewGrant` symbols to centralize the logic
and have both handlers call that helper before invoking their respective
file-serving functions.
In `@CHANGELOG.md`:
- Around line 32-39: The changelog Notes section contains duplicate/conflicting
endpoint bullets, so consolidate the streaming/viewing changes into a single
clear entry. Update the relevant bullets in CHANGELOG.md to describe the final
API split using the existing stream and view endpoint wording, and remove or
rewrite the older “new resources/stream endpoint” note so it does not contradict
the later `/media/stream` and `/api/resources/view` descriptions.
In `@frontend/src/api/media.js`:
- Around line 105-142: Add direct unit coverage for the new media helpers in
media.js: create tests for getStreamURL and getStreamURLPublic that verify the
missing-source and missing-viewToken validation branches throw as expected, and
that the returned URL includes the correct params built via
getApiPath/getPublicApiPath. Use the function names getStreamURL and
getStreamURLPublic so the tests stay anchored even if surrounding code changes.
- Around line 105-142: The URL-building and validation logic in getStreamURL and
getStreamURLPublic is duplicated with the equivalent helpers in resources.js, so
future fixes can drift between the two paths. Extract the shared
source/path/viewToken/sessionId construction and notify/error handling into a
common helper, then have getStreamURL and getStreamURLPublic call it while only
passing the endpoint-specific path (media/stream vs resources/view).
In `@frontend/src/api/resources.view.test.js`:
- Around line 1-57: The getViewURL tests currently miss the share-based and
download-fallback routing paths, so extend this suite to cover those branches.
Add cases that use the mocked store.state.shareInfo and getters.isShare to
verify getRawViewURLPublic is used when sharing is enabled, and add
allowDownloadFallback=true scenarios to exercise
getDownloadURL/getDownloadURLPublic behavior. Use getViewURL and the existing
mocked `@/api/media` helpers to confirm the returned URLs switch correctly across
these branches.
In `@frontend/src/components/prompts/UserEdit.vue`:
- Around line 334-346: In normalizeFormUser, the null/undefined handling is
already correct, but the repeated strict checks for user.permissions and
user.account.permissions can be simplified. Update the permission-null checks in
this method to use a single nullish comparison pattern so the code stays
equivalent while being less verbose, and keep the existing behavior in
UserEdit.vue unchanged.
In `@frontend/src/utils/mediaFile.js`:
- Around line 1-4: MEDIA_EXTENSIONS is missing several common audio/video
formats, so add the suggested extensions (such as .3gp, .flv, .ts, .mpg/.mpeg,
and .m2ts) to the MEDIA_EXTENSIONS set in mediaFile.js. Keep the existing
isMediaFile logic unchanged so files with these extensions correctly route to
/media/stream instead of falling through to /resources/view.
In `@frontend/src/views/files/ThreeJs.vue`:
- Around line 365-374: The sibling token lookup logic is duplicated between
resolveViewTokenForPath in ThreeJs.vue and viewTokenForSibling in
htmlPreview.ts, so please consolidate the name/path matching into one shared
utility and have resolveViewTokenForPath call it instead of reimplementing the
heuristic. Keep the existing fbdata.path and parentDirItems behavior intact, but
route both callers through the shared helper so any future nested-path or
matching fixes only need to be made once.
- Around line 388-393: The URL parsing in ThreeJs.vue has a redundant fallback
in the file lookup logic: `parsed.searchParams.get("file") ||
parsed.searchParams.getAll("file")[0]` in the URL handling path. Update the code
in the `resourceViewUrl`/URL parsing block to use only
`searchParams.get("file")`, since `get()` already returns the first matching
value and the `getAll()[0]` branch is dead code. Keep the existing behavior
otherwise, just simplify the `filePath` assignment and related return flow.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 62709b15-1c84-44cb-af50-98115b931740
📒 Files selected for processing (31)
CHANGELOG.mdbackend/common/utils/cache.gobackend/common/utils/stream_grant.gobackend/http/httpRouter.gobackend/http/middleware.gobackend/http/middleware_test.gobackend/http/resource.gobackend/http/stream.gobackend/http/stream_grant.gobackend/http/stream_grant_test.gobackend/http/stream_range.gobackend/http/stream_range_test.gobackend/http/view.gobackend/indexing/iteminfo/fileinfo.gofrontend/src/api/media.jsfrontend/src/api/resources.jsfrontend/src/api/resources.view.test.jsfrontend/src/components/files/Icon.vuefrontend/src/components/files/ListingItem.vuefrontend/src/components/prompts/UserEdit.vuefrontend/src/store/mutations.jsfrontend/src/store/types.tsfrontend/src/utils/htmlPreview.test.jsfrontend/src/utils/htmlPreview.tsfrontend/src/utils/mediaFile.jsfrontend/src/utils/mediaFile.test.jsfrontend/src/views/files/DocViewer.vuefrontend/src/views/files/EpubViewer.vuefrontend/src/views/files/ListingView.vuefrontend/src/views/files/Preview.vuefrontend/src/views/files/ThreeJs.vue
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/views/files/ThreeJs.vue (1)
397-438: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the redundant
modelDirfetch when the token is already indexed.
Icon.vuealready passesviewTokenandparentDirItemsintoThreeJs, so the model token is usually available beforeprefetchAssetViewTokens()runs. Keep the/textureslookup, but avoid the extramodelDirlisting request on every thumbnail.🤖 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/src/views/files/ThreeJs.vue` around lines 397 - 438, The prefetch path in prefetchAssetViewTokens is doing a redundant fetch for the model directory even though Icon.vue already supplies parentDirItems and a viewToken that can populate the cache first. Update prefetchAssetViewTokens (and, if needed, fetchDirectoryTokens) to skip the modelDir listing request when the model token for normalizeAssetPath(this.fbdata.path) is already present in viewTokenByPath, while still keeping the /textures fetch.
🤖 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.
Nitpick comments:
In `@frontend/src/views/files/ThreeJs.vue`:
- Around line 397-438: The prefetch path in prefetchAssetViewTokens is doing a
redundant fetch for the model directory even though Icon.vue already supplies
parentDirItems and a viewToken that can populate the cache first. Update
prefetchAssetViewTokens (and, if needed, fetchDirectoryTokens) to skip the
modelDir listing request when the model token for
normalizeAssetPath(this.fbdata.path) is already present in viewTokenByPath,
while still keeping the /textures fetch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e631f5c0-b058-40ed-8991-026ff6911778
⛔ Files ignored due to path filters (3)
backend/swagger/docs/docs.gois excluded by!backend/swagger/**backend/swagger/docs/swagger.jsonis excluded by!backend/swagger/**backend/swagger/docs/swagger.yamlis excluded by!backend/swagger/**
📒 Files selected for processing (5)
CHANGELOG.mdfrontend/src/components/files/Icon.vuefrontend/src/utils/htmlPreview.test.jsfrontend/src/utils/htmlPreview.tsfrontend/src/views/files/ThreeJs.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/files/Icon.vue
1 participant
Description
According to the contributing guide, A PR should contain:
Additional Details
Summary by CodeRabbit
New Features
/api/media/stream(and public equivalent), enforcing audio/video-only streaming.Bug Fixes
/rawand/api/rawdownload routes.