fix(quality-profiles): stop hiding Maximum Age, and load a profile's qualities - #920
Open
m4bard wants to merge 1 commit into
Open
fix(quality-profiles): stop hiding Maximum Age, and load a profile's qualities#920m4bard wants to merge 1 commit into
m4bard wants to merge 1 commit into
Conversation
…qualities Two problems in the quality profile form. The second was found while writing the test for the first. Maximum Age was rendered behind v-if="formData.preferNewerReleases". It is not part of that checkbox. SearchResultScorer applies profile.MaximumAge as a hard reject whenever it is above zero, at :153 and :171, and never reads PreferNewerReleases at all. So unticking the box hid an input while the filter it sets stayed on, and the only route back to the number was to tick a box whose description offers a bonus for recent releases, which is the opposite of what someone turning off an age limit wants. The input now always renders. Opening an existing profile also lost its qualities. The watch that copies the profile into the form runs with immediate: true, so it calls initializeQualitiesFromProfile during setup, and that was a const arrow declared below it. Every mount carrying a profile threw ReferenceError there. Vue caught it, so nothing was visible except an empty quality list, and saving was then refused by the "select at least one quality" guard until the whole ladder was picked again by hand. parseQualityString had the same shape one level down and took over as soon as the first was fixed. Both are function declarations now, so they hoist. PreferNewerReleases still has no reader anywhere in the backend. Its description promises a recency bonus that no longer exists. I have left the checkbox alone rather than guess: removing it and implementing the bonus are both reasonable and the choice is yours. Say which and I will follow up. Also unchanged, and worth knowing: the form initialises preferNewerReleases to false while QualityProfile.cs:100 initialises it to true, so a profile made in the UI and one made from JSON that omits the key disagree. Nothing reads the field, so it has no effect today. I did not want to bury a default change in this. Controls: re-hiding Maximum Age fails one test; putting either helper back to a const arrow fails all four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEVQ7qDJLk5196MFeggWuA
Author
|
The quality-initialisation half of this is now filed on its own as #923, since it is the more serious of the two and worth being findable without reading a PR about a hidden input. |
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
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.
Fixes #895, and fixes a second problem in the same file that I found while writing the test for the first.
Maximum Age was hidden while it was still rejecting
Maximum Age (Days)rendered behindv-if="formData.preferNewerReleases". It is not part of that checkbox.SearchResultScorerappliesprofile.MaximumAgeas a hard reject whenever it is above zero, at:153and:171, and never readsPreferNewerReleasesat all.So unticking the box hid an input while the filter it sets stayed on, and the only route back to the number was to tick a box whose description offers a bonus for recent releases, which is the opposite of what someone turning off an age limit wants.
The input now always renders.
Opening a profile lost its qualities
The watch that copies the profile into the form runs with
{ immediate: true }, so it callsinitializeQualitiesFromProfileduring setup, and that was aconstarrow declared below it. Every mount carrying a profile threwReferenceErrorthere. Vue catches it, so the only symptom was an empty quality list, and saving was then refused by the "select at least one quality" guard until the whole ladder was picked again by hand.parseQualityStringhad the same shape one level down and took over as soon as the first was fixed. Both are function declarations now, so they hoist.I have written that up as its own issue, since it is a bigger problem than #895 and deserves to be findable on its own. It is in this PR because the two are in the same file and I could not test #895 without it. Say the word if you would rather review them separately and I will split it.
Left alone deliberately
PreferNewerReleasesstill has no reader anywhere in the backend. Its description promises a recency bonus that no longer exists. Removing the checkbox and implementing the bonus are both reasonable and the choice is yours, so I have not guessed. Say which and I will follow up.Also unchanged, and worth knowing: the form initialises
preferNewerReleasestofalsewhileQualityProfile.cs:100initialises it totrue, so a profile made in the UI and one made from JSON that omits the key disagree. Nothing reads the field, so it has no effect today, and I did not want to bury a default change in this.Tests
Four, in a new spec. There was no test that mounted this modal at all, which is how a component that threw on every mount with a profile went unnoticed.
Re-hiding Maximum Age fails one. Putting either helper back to a
constarrow fails all four.Worked through with Claude Code at my direction. The claims above were checked by running them rather than by reading, and I reviewed this before posting.