fix(ui): delete-author dialog lists every author folder, not only the legacy path - #269
Open
jordanfelle wants to merge 2 commits into
Open
jordanfelle wants to merge 2 commits into
jordanfelle wants to merge 2 commits into
Conversation
… legacy path The delete dialog rendered a single `path` (the legacy Author.Path) in both the folder line and the "author folder and all of its content will be deleted" warning. An author can have separate audiobook and ebook folders (audiobookPath / ebookPath), and deleting with "delete files" removes all of them, so the dialog named the wrong - or only one - folder. Example: an ebook-only author whose page header showed /mnt/Media/Books/<name> got a dialog that named /mnt/Media/Audio Books/<name>. The connector already spreads the whole author resource into the component, so audiobookPath and ebookPath are available. List each distinct non-empty path among path/audiobookPath/ ebookPath, and repeat the existing warning once per folder (no new translation strings). The file count/size already comes from the author statistics and covers both formats.
…kFolder) in the delete dialog The previous commit listed audiobookPath / ebookPath from the author resource, but AuthorResource does not expose those: it maps Author.AudiobookPath / EbookPath to audiobookFolder / ebookFolder, and audiobookPath / ebookPath are null on the API response. So the dialog still only ever showed `path`. Checked against live data: for 2564 of 2774 authors the API has two distinct folders (audiobookFolder + ebookFolder), and for one author three. Use audiobookFolder and ebookFolder. Same de-duplication and one warning line per folder.
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.
Problem
The delete-author dialog rendered a single
path(the legacyAuthor.Path) in both the folder line and the "The author folder ... and all of its content will be deleted" warning. An author can have separate audiobook and ebook folders, and deleting with "delete files" removes all of them, so the dialog named only one folder.Seen live: an ebook-only author whose page header showed
/mnt/Media/Books/<name>got a delete dialog naming/mnt/Media/Audio Books/<name>; and after the first version of this PR, an author with both folders still showed one.Change
DeleteAuthorModalContentConnectorspreads the whole author resource into the component. The resource exposes the format folders asaudiobookFolder/ebookFolder(AuthorResourcemapsAuthor.AudiobookPath/EbookPathto them);audiobookPath/ebookPathare null on the API response. The dialog now lists each distinct, non-empty folder amongpath/audiobookFolder/ebookFolder, and repeats the existing warning once per folder (no new translation strings). The file count and size already come from the author statistics and cover both formats.Checked against live API data: 2564 of 2774 authors have two distinct folders and one has three, so this changes the dialog for nearly every author.
Correction to the first version of this PR
The first commit read
audiobookPath/ebookPath, which the API does not populate, so it never showed more thanpath. Adversarial review checked the code but not real data and missed it; caught by looking at the dialog on a live author.Testing
Syntax-checked with esbuild; the full webpack production build compiles. No frontend tests exist in this repo. Verified the field names and folder counts against the live
/api/v1/authorresponse; not yet re-verified in a browser after this fix.Known limitation
A folder is listed whenever it is configured on the author, whether or not it exists on disk (e.g. an ebook folder that was never created), so the warning can name a folder with nothing in it.