fix(web): save the weather provider and PV rated power you choose - #1437
Merged
Merged
Conversation
Settings › Weather showed met_no and 10000 W when neither was set. An empty provider matched no option, so the browser showed the first one, and a missing rating showed the field's default. Save stores only fields that differ from what was rendered, so accepting what the form showed stored nothing: Core kept no provider and no rated power, and the site had no solar forecast. The form now shows what Core uses: no provider and an empty rated power. A select whose stored value is not one of its options shows its default instead of the first option. Fixes #1436. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Fixes #1436.
Problem
On v0.136.3-beta.1, segran2 chose
met_noand 10000 W under Settings › Weather and saved. The stored config kept"provider": ""and nopv_rated_w. Core therefore fetched no forecast, and the Plan showed a PV forecast of 0 kWh.Cause: 76f811b made Save store only fields whose value differs from what was rendered, so displayed defaults do not become settings. That rule assumes a displayed default is Core's default. Weather broke that assumption:
providermatches no<option>, so the browser shows the first one,met_no.pv_rated_wrendered the field default10000.Core uses neither value when the setting is missing. Accepting what the form showed was therefore "unchanged", and nothing was stored.
Change
selectField: a stored value that is not one of the options shows the declared default, not the first option.none, since Core starts no forecast without a provider. The PV rated field is empty when unset, since Core uses 0 then. It has a help text saying the solar forecast needs it or PV arrays.Evidence
shows a select's default when the stored value is not an optionandshows no provider and no rated power when none is setfail on master and pass here.npm testpasses 638/638 withLANG=C. With a Swedish locale,update-dialog.test.mjsfails on master too, because it formats "8 192".provider: "", a location, no rated power):provider=met_no pv_rated_w="10000". Saving without changes stores{"provider":"","latitude":55.8953,"longitude":13.4023}.provider=none pv_rated_w="". After choosingmet_noand entering 10000, Save stores{"provider":"met_no",…,"pv_rated_w":10000}.Screenshot of the branch's Weather tab: provider
none, PV rated empty, with help.Not changed: whether Core should start
met_noby itself for a site that has a location but no provider. That is a cold-start product choice for the owner.🤖 Generated with Claude Code
https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7