fix(frontend): load root folders on mount so cold first add isn't falsely rejected#661
Open
timk75 wants to merge 1 commit into
Open
fix(frontend): load root folders on mount so cold first add isn't falsely rejected#661timk75 wants to merge 1 commit into
timk75 wants to merge 1 commit into
Conversation
…sely rejected The root folders store is only ever populated by an explicit load() call. Several views read rootFoldersStore.folders synchronously to gate UI, but never load it on a cold SPA start: - AddNewView's addToLibrary() guard reported "Root folder not configured" and redirected to /settings on the first add after a fresh page load; the redirect populated the store, so a back+retry then succeeded. - AudiobooksView's hasRootFolderConfigured drove a false "Root Folder Not Configured" empty-state for users whose library is empty. - AudiobookDetailView's path preview fell back to the legacy outputPath. Load the store on mount in each of these views (mirroring the existing LibraryImportView / UnmatchedFilesModal pattern). Also stop wiping already-loaded folders when a reload throws, so a transient API failure no longer masquerades as "no root folders configured". Adds a regression test asserting AddNewView fetches root folders on mount.
therobbiedavis
force-pushed
the
fix/cold-add-root-folder
branch
from
58f21db to
6e8c024
Compare
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
2 participants
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.
Summary
Fixes the cold-start "Root folder not configured" false negative. After a fresh page load (or after the app has been idle long enough that the SPA is re-initialised), the first attempt to add an audiobook reports "Root folder not configured" and redirects to Settings — even though a root folder is configured. Going browser-back and trying again works, which is the tell-tale symptom.
Root cause.
rootFoldersStoreis only ever populated by an explicitload()call. Several views readrootFoldersStore.folderssynchronously to gate UI, but nothing loads the store on a cold SPA start — notApp.vue, not the router, not these views'onMounted. So the guard sees an empty list and assumes no folder is configured. The redirect to/settingshappens to load the store, which is why the second attempt succeeds.The same un-loaded-store read affects two more views (found while tracing the root cause).
Changes
Changed
AddNewView.vue— load root folders inonMountedbeforeaddToLibrary()'s guard can read them. This is the primary cold-add fix.AudiobooksView.vue—hasRootFolderConfigureddrives a "Root Folder Not Configured" empty-state CTA; load the store on mount so an empty library doesn't falsely show it.AudiobookDetailView.vue— the path-preview computed falls back torootFoldersStore.defaultFolder; load the store on mount so a cold load previews the correct destination instead of the legacyoutputPath.LibraryImportView.vueandUnmatchedFilesModal.vue(if (folders.length === 0) await load()).Fixed
stores/rootFolders.ts—load()no longer resetsfoldersto[]inside itscatch. Previously a transient API failure on reload wiped already-loaded folders, making a failed load indistinguishable from "no folders configured".foldersstill starts as[], so a first-ever failure naturally yields an empty list.Added
AddNewView.spec.tsasserting the view fetches root folders on mount.Testing
vue-tsc --noEmitandeslintclean on all changed files.Notes
AddNewView.vuealso accepts a configuredapplicationSettings.outputPathas an alternative to a root folder; that path was already loaded on mount, so only the root-folder half was failing.load()has no in-flight de-dupe; theif (folders.length === 0)caller-side guard keeps this to a single fetch per cold load, consistent with the existing call sites. A store-level loading guard would be a reasonable follow-up but is out of scope here.