Using MultiValueDict breaks LinkWidget

[pajowu]

#1634 introduced using a MultiValueDict as the default empty value for the form data. This however broke using LinkWidget. If the FilterSet is initialised with a false-y data attribute, the FilterSets data attribute is set to an empty MultiValueDict. This gets passed to LinkWidgets value_from_datadict, which then sets its self.data attribute to it. This leads to LinkWidget.render_option passing a MultiValueDict to django.utils.http.urlencode which then renders the option values as lists (?price=%5B%27test-val1%27%5D, i.e. ?price=['test-val1'], instead of ?price=test-val1 for an empty option in the example below).

  def test_widget_multivaluedict(self):
      choices = (
          ("", "---------"),
          ("test-val1", "test-label1"),
          ("test-val2", "test-label2"),
      )

      w = LinkWidget(choices=choices)
      w.value_from_datadict(MultiValueDict(), {}, "price")
      self.assertHTMLEqual(
          w.render("price", ""),
          """
          <ul>
              <li><a class="selected" href="?price=">All</a></li>
              <li><a href="?price=test-val1">test-label1</a></li>
              <li><a href="?price=test-val2">test-label2</a></li>
          </ul>""",
      )

fails as follows:

======================================================================
FAIL: test_widget_multivaluedict (tests.test_widgets.LinkWidgetTests.test_widget_multivaluedict)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "[...]/django-filter/tests/test_widgets.py", line 156, in test_widget_multivaluedict
    self.assertHTMLEqual(
  File "[...]/env/lib/python3.12/site-packages/django/test/testcases.py", line 1044, in assertHTMLEqual
    self.fail(self._formatMessage(msg, standardMsg))
AssertionError: <ul>
<li>
<a class="selected" href="?price=%5B%27%27%5D">
All
</a>
</li><li>
<a  [truncated]... != <ul>
<li>
<a class="selected" href="?price=">
All
</a>
</li><li>
<a href="?price [truncated]...
  <ul>
  <li>
- <a class="selected" href="?price=%5B%27%27%5D">
?                                  ------------

+ <a class="selected" href="?price=">
  All
  </a>
  </li><li>
- <a href="?price=%5B%27test-val1%27%5D">
?                 ------         ------

+ <a href="?price=test-val1">
  test-label1
  </a>
  </li><li>
- <a href="?price=%5B%27test-val2%27%5D">
?                 ------         ------

+ <a href="?price=test-val2">
  test-label2
  </a>
  </li>
  </ul>

----------------------------------------------------------------------

[carltongibson]

Hmmm. Prior to the change the default was a dict which has the same behaviour when passed to value_from_datadict():

>>> from django.utils.datastructures import MultiValueDict
>>> d = MultiValueDict()
>>> d.get("prices")
>>> {}.get("prices")

i.e. data should be None in both cases no? 🤔

[pajowu]

I don't fully understand your comment, but just in case:

The problem is not the .get-Method, but the self.data attribute getting set which is then used further in render_option:

django-filter/django_filters/widgets.py

Lines 59 to 65 in 3656174

	data = self.data.copy()
	data[name] = option_value
	selected = data == self.data or option_value in selected_choices
	try:
	url = data.urlencode()
	except AttributeError:
	url = urlencode(data)

Urlencode then encodes the lists which MultiValueDict returns as their string-representation (as seen in the test output)

[pajowu]

>>> from django.utils.http import MultiValueDict, urlencode
>>>
>>> d = MultiValueDict()
>>> d['prices'] = ''
>>> urlencode(d)
'prices=%5B%27%27%5D'
>>>
>>>
>>> d = {}
>>> d["prices"] = ""
>>> urlencode(d)
'prices='

[carltongibson]

Ok, so we'll likely have to cast to a dict there. The data should be a multidict, so LinkWidget should handle that.

Good spot.

Fancy making a PR with the regression test and tweak?

[carltongibson]

@pajowu Actually, it looks like we want a QueryDict in this case (which provides the urlencode() method.

Do you fancy trying the change from #1634 using that to see the effect?

(Possibly we can drop the try block, since data should no longer be a dict in any case)