[fix] Fixed MultiValueDictKeyError on empty device form submission by Srinath0916 · Pull Request #1173 · openwisp/openwisp-controller

Srinath0916 · 2025-12-08T19:02:35Z

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.
N/A I have updated the documentation.

Reference to Existing Issue

Description of Changes

Fixed the MultiValueDictKeyError that happened when you click "Add another Configuration" on a new device and hit save without filling anything in.

The problem was in the ConfigForm.get_temp_model_instance() method - it was trying to access organization, name, and mac_address from the form data without checking if they exist first. Changed it to use .get() so it handles empty fields properly and shows validation errors instead of crashing.

Added a test case to verify the fix works.

Screenshot

See issue #1057 for the error screenshot. After the fix, the form now shows proper validation errors instead of crashing.

DragnEmperor · 2025-12-09T05:37:39Z

openwisp_controller/config/tests/test_admin.py

        config = self._create_config(organization=self._get_org())
        self._verify_template_queries(config, 10)

+    def test_empty_device_form_with_config_inline(self):


Can you please try running this test without the fix and check if it fails or not?
Also I don't think it is necessary to mention the issue in the docstring, a simple description of what it does is enough, you can take other tests as reference

Hi @DragnEmperor , Thanks for the feedback! I've updated the test to properly reproduce the bug by omitting the device fields from the POST data, which matches the actual scenario from the issue.

I tried running it locally but hit a Twisted import error with my Python version. However, the test logic is clear: without the fix, accessing self.data["organization"] on a missing key raises MultiValueDictKeyError, and with the fix using .get(), it's handled gracefully.

CI should catch it if there's any issue. Let me know if you need anything else changed.

Also updated the docstring as you suggested.

Hey @DragnEmperor , I've fixed the CI issues from the previous run. The workflow needs approval to run the tests again. Could you approve it when you get a chance?

Srinath0916 · 2025-12-09T16:04:26Z

Hey @DragnEmperor , really sorry for the inconvenience, I had to fix the commit message format to match the project guidelines (added the [fix] prefix). Could you approve the workflow one more time?
should be good to go now.

DragnEmperor · 2025-12-09T19:04:21Z

.github/workflows/ci.yml

+        if: ${{ failure() && steps.deps.conclusion == 'success' }}
+        run: |
+          if [ -f geckodriver.log ]; then
+            cat geckodriver.log
+          else
+            echo "geckodriver.log not found"
+          fi


why is this change required? IMO any unrelated changes should be taken up if there is a dedicated issue for the same. Else, would be difficult to track the changes.

You're right, my bad! I added this while debugging some test failures but it's not really needed for this PR. Removed it in the latest commit. Thank you for addressing

Regarding the checks - all of them should pass except Python 3.9. This seems to be an existing issue on the main branch and not related to my changes. I encountered the same Python 3.9 failure in my previous PR as well, and it was also discussed in the community group.

I've opened PR #1174 to drop Python 3.9 support from openwisp-controller, which should resolve this CI issue for all pending PRs once merged.

Hi @DragnEmperor , PR #1174 has been merged which drops Python 3.9 support. Could you re-run the checks on this PR? All tests should pass now. Thanks!

Fixes openwisp#1057 Fixed MultiValueDictKeyError when submitting empty device form with configuration inline. Changed direct dictionary access to use .get() method with defaults to allow proper validation.

…rove test

coveralls · 2025-12-10T11:30:04Z

coverage: 98.608% (-0.003%) from 98.611%
when pulling c0096f2 on Srinath0916:fix-multivaluedictkeyerror-1057
into 03ddc6c on openwisp:master.

Srinath0916 · 2025-12-10T11:30:20Z

Hi @pandafy , @DragnEmperor , all checks are passing now, This PR is ready for review.

DragnEmperor · 2025-12-10T12:37:45Z

Hi @pandafy , @DragnEmperor , all checks are passing now, This PR is ready for review.

Hi @Srinath0916 , I would suggest to have a look once again through all the changes done to ensure any unrelated changes are not there. I see some changes in workflow yaml file.

Srinath0916 · 2025-12-10T14:17:11Z

Thanks @DragnEmperor ! I've reviewed all changes in this PR,

The MultiValueDictKeyError fix includes: - config/admin.py (the main fix) - config/base/config.py (related changes) - config/tests/test_admin.py (test for the fix)

The workflow yaml changes include: - Geckodriver logging removal (as you said) and Python 3.9 related changes from updating with master (PR #1174)

Should I clean up the workflow file to keep only the geckodriver removal you asked for and remove the Python 3.9 changes? I want to make sure the tests still pass."

DragnEmperor · 2025-12-10T16:30:25Z

Thanks @DragnEmperor ! I've reviewed all changes in this PR,

The MultiValueDictKeyError fix includes: - config/admin.py (the main fix) - config/base/config.py (related changes) - config/tests/test_admin.py (test for the fix)

The workflow yaml changes include: - Geckodriver logging removal (as you said) and Python 3.9 related changes from updating with master (PR #1174)

Should I clean up the workflow file to keep only the geckodriver removal you asked for and remove the Python 3.9 changes? I want to make sure the tests still pass."

@Srinath0916 I meant to keep the ci.yml as it is, no changes at all, currently I see there is some removal of code

…rror fix

Srinath0916 · 2025-12-10T16:39:44Z

Thanks @DragnEmperor ! I've reverted ci.yml to match master exactly. Now the PR contains only the MultiValueDictKeyError fix files with no workflow changes. Ready for review!

DragnEmperor · 2025-12-10T16:54:12Z

LGTM

Srinath0916 · 2025-12-10T17:37:55Z

Thank you @DragnEmperor , Really appreciate your patience and detailed feedback throughout this process.

Srinath0916 · 2025-12-11T10:27:42Z

Hi @pandafy , This PR has been approved, and all checks are passing. Could you help merge it when you get a chance? Thanks.

pandafy · 2025-12-12T15:21:31Z

openwisp_controller/config/admin.py

+            if not config:
+                # The request does not contain vaild data to create a temporary
+                # Device instance. Thus, we cannot validate the templates.
+                # The Device validation will be handled by DeviceAdmin.
+                # Therefore, we don't need to raise any error here.
+                return


We shall return early if the data does not contain organization.

Thanks @pandafy , for the review and the improvement,
Really appreciate the feedback and approval.

pandafy

I have made a small improvement in 81f0780.

LGTM!

nemesifier · 2025-12-19T17:40:51Z

openwisp_controller/config/tests/test_admin.py

+        # Should show validation errors instead
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "errorlist")
+        # Verify no device was created


Let's avoid adding too many comments, it's not useful, I am going to remove some.

Srinath0916 force-pushed the fix-multivaluedictkeyerror-1057 branch from ee88b78 to 76cb583 Compare December 8, 2025 19:07

DragnEmperor requested changes Dec 9, 2025

View reviewed changes

Srinath0916 requested a review from DragnEmperor December 9, 2025 07:21

Srinath0916 force-pushed the fix-multivaluedictkeyerror-1057 branch 2 times, most recently from 19170b9 to 21c1fb1 Compare December 9, 2025 15:50

DragnEmperor requested changes Dec 9, 2025

View reviewed changes

Srinath0916 requested a review from DragnEmperor December 10, 2025 08:25

pandafy added this to OpenWISP Contributor's Board Dec 10, 2025

github-project-automation bot moved this to In progress in OpenWISP Contributor's Board Dec 10, 2025

pandafy moved this from In progress to Needs review in OpenWISP Contributor's Board Dec 10, 2025

Srinath0916 added 2 commits December 10, 2025 16:22

[fix] Handle empty form submission in device add

026436c

Fixes openwisp#1057 Fixed MultiValueDictKeyError when submitting empty device form with configuration inline. Changed direct dictionary access to use .get() method with defaults to allow proper validation.

[fix] Fix Device.organization.RelatedObjectDoesNotExist error and imp…

db53750

…rove test

Srinath0916 force-pushed the fix-multivaluedictkeyerror-1057 branch from e0f61e4 to d152703 Compare December 10, 2025 10:52

[change] Remove unrelated geckodriver logging changes

94f9cd2

Srinath0916 force-pushed the fix-multivaluedictkeyerror-1057 branch from d152703 to 94f9cd2 Compare December 10, 2025 11:06

[change] Revert ci.yml to match master - keep only MultiValueDictKeyE…

0ab2651

…rror fix

DragnEmperor previously approved these changes Dec 10, 2025

View reviewed changes

github-project-automation bot moved this from Needs review to In progress in OpenWISP Contributor's Board Dec 10, 2025

[chores] Made improvements to the logic

81f0780

pandafy dismissed DragnEmperor’s stale review via 81f0780 December 12, 2025 15:19

pandafy reviewed Dec 12, 2025

View reviewed changes

pandafy previously approved these changes Dec 12, 2025

View reviewed changes

pandafy moved this from In progress to Needs review in OpenWISP Contributor's Board Dec 12, 2025

Srinath0916 mentioned this pull request Dec 16, 2025

[feature] Invalidate VPN cache when organization config variables change #1177

Open

4 tasks

nemesifier assigned Srinath0916 Dec 19, 2025

nemesifier added the bug label Dec 19, 2025

nemesifier changed the title ~~Fix MultiValueDictKeyError on empty device form submission 1057~~ [fix] Fixed MultiValueDictKeyError on empty device form submission Dec 19, 2025

nemesifier reviewed Dec 19, 2025

View reviewed changes

nemesifier dismissed pandafy’s stale review via 97b9ddd December 19, 2025 17:41

[chores] Removed redundant comments

c0096f2

nemesifier force-pushed the fix-multivaluedictkeyerror-1057 branch from 97b9ddd to c0096f2 Compare December 19, 2025 17:54

nemesifier approved these changes Dec 19, 2025

View reviewed changes

github-project-automation bot moved this from Needs review to In progress in OpenWISP Contributor's Board Dec 19, 2025

nemesifier merged commit 828dfb3 into openwisp:master Dec 19, 2025
13 checks passed

github-project-automation bot moved this from In progress to Done in OpenWISP Contributor's Board Dec 19, 2025

Uh oh!

Conversation

Srinath0916 commented Dec 8, 2025 • edited by nemesifier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reference to Existing Issue

Description of Changes

Screenshot

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Srinath0916 commented Dec 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Srinath0916 Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Srinath0916 commented Dec 10, 2025

Uh oh!

DragnEmperor commented Dec 10, 2025

Uh oh!

Srinath0916 commented Dec 10, 2025

Uh oh!

DragnEmperor commented Dec 10, 2025

Uh oh!

Srinath0916 commented Dec 10, 2025

Uh oh!

DragnEmperor commented Dec 10, 2025

Uh oh!

Srinath0916 commented Dec 10, 2025

Uh oh!

Srinath0916 commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandafy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Srinath0916 commented Dec 8, 2025 •

edited by nemesifier

Loading

Srinath0916 Dec 9, 2025 •

edited

Loading

coveralls commented Dec 10, 2025 •

edited

Loading