Skip to content

Fix MultiValueDictKeyError when radio group is unselected in save_set…#366

Open
jamesmulcahy wants to merge 1 commit intoNSPManager:develfrom
jamesmulcahy:fix-optimistic-mode-bug
Open

Fix MultiValueDictKeyError when radio group is unselected in save_set…#366
jamesmulcahy wants to merge 1 commit intoNSPManager:develfrom
jamesmulcahy:fix-optimistic-mode-bug

Conversation

@jamesmulcahy
Copy link
Copy Markdown
Contributor

…tings

Radio button groups (optimistic_mode, show_screensaver_inside/outside_temperature, turn_on_behavior) are absent from POST data when neither option is pre-selected (e.g. fresh install with no stored value). Use .get() with safe defaults instead of hard dict access to avoid MultiValueDictKeyError.

…tings

Radio button groups (optimistic_mode, show_screensaver_inside/outside_temperature,
turn_on_behavior) are absent from POST data when neither option is pre-selected
(e.g. fresh install with no stored value). Use .get() with safe defaults instead
of hard dict access to avoid MultiValueDictKeyError.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jamesmulcahy
Copy link
Copy Markdown
Contributor Author

After upgrading to the latest beta, I updated some settings in the UI, and hit save, and hit this error in Django, as it's assuming some fields are already set.

Environment:


Request Method: POST
Request URL: http://nspm.manque.net/save_settings

Django Version: 5.1.1
Python Version: 3.12.5
Installed Applications:
['web',
 'django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django_components.safer_staticfiles',
 'django_components']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware']



Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/django/utils/datastructures.py", line 84, in __getitem__
    list_ = super().__getitem__(key)
            ^^^^^^^^^^^^^^^^^^^^^^^^

During handling of the above exception ('optimistic_mode'), another exception occurred:
  File "/usr/local/lib/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/app/nspanelmanager/web/views.py", line 772, in save_settings
    name="optimistic_mode", value=request.POST["optimistic_mode"] == "optimistic"
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/utils/datastructures.py", line 86, in __getitem__
    raise MultiValueDictKeyError(key)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Exception Type: MultiValueDictKeyError at /save_settings
Exception Value: 'optimistic_mode'

Copy link
Copy Markdown
Collaborator

@tpanajott tpanajott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice patch, thanks. Please see review for a few quick things before merging.

set_setting_value(
name="show_screensaver_inside_temperature",
value=request.POST["show_screensaver_inside_temperature"],
value=request.POST.get("show_screensaver_inside_temperature", "False"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to keep this to "True". We always want to show inside default as default.

set_setting_value(
name="show_screensaver_outside_temperature",
value=request.POST["show_screensaver_outside_temperature"],
value=request.POST.get("show_screensaver_outside_temperature", "False"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to keep this to "True". We always want to show outside default as default.

set_setting_value(
name="optimistic_mode", value=request.POST["optimistic_mode"] == "optimistic"
name="optimistic_mode",
value=request.POST.get("optimistic_mode", "") == "optimistic",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to keep optimistic mode as the default. So, adding "optimistic" to be the default in case it was not found would seem appropriate. Or perhaps even better, checking of "optimistic_mode" in request.POST before trying to access it as it would then not change a value in case it was missing in the request but actually set in the stored settings.

@tpanajott
Copy link
Copy Markdown
Collaborator

Having thought a bit more about this i find it strange that this issue would pop up. Sure it needs to be fixed but it shouldn't happen. When the manager first starts it will go through the database and remove any setting that exists but is no longer used and will set the default value in case it does not exist. This functionality can be seen here:

void MqttManagerConfig::populate_default_and_clean() {

It seems there may be some issue with the check in the Django template if it should check a radio box or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants