django-multiselectfield
django-multiselectfield copied to clipboard
Fix when selecting with a MSFList
Django==4.1.3, this is a serious error:
When selecting a model that has a multiselectfield, with a result from a former multiselectfield (MSFList), the DB conversion will fail, resulting in false negatives.
This patch fixes it. I use it in one of my projects. Until this PR gets merged, my work is available at https://gitea.ksol.io/karolyi/django-multiselectfield/commits/branch/master
Elaborating on how to reproduce:
>>> obj = SomeModel.objects.first()
>>> type(obj.some_multiselect_field)
multiselectfield.db.fields.MSFList
>>> SomeModel.objects.filter(pk=obj.pk, some_multiselect_field=obj.some_multiselect_field).count()
0
The result should be 1 instead of 0, obviously. Tracking the bug down, the DB conversion fails. It is very circumstantial to go into the ORM code to see what happens, but after hours of investigation, this PR fixes the bug.
@goinnn, please merge this.
guys, can we pick up the pace here? @tomasgarzon ?
bumperino
@blag ?
Should be good to go now.
I pushed a commit with the test code you provided:
def test_filter(self):
self.assertEqual(Book.objects.filter(tags__contains='sex').count(), 1)
self.assertEqual(Book.objects.filter(tags__contains='boring').count(), 0)
book = Book.objects.first()
self.assertQuerySetEqual(
Book.objects.filter(pk=book.pk),
Book.objects.filter(pk=book.pk, published_in=book.published_in),
)
but it's still failing:
$ (cd example; python manage.py test app.test_msf.MultiSelectTestCase.test_filter)
Found 1 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
F
======================================================================
FAIL: test_filter (app.test_msf.MultiSelectTestCase.test_filter)
----------------------------------------------------------------------
Traceback (most recent call last):
File "example/app/test_msf.py", line 91, in test_filter
self.assertQuerySetEqual(
File ".../django/test/testcases.py", line 1169, in assertQuerySetEqual
return self.assertEqual(list(items), values, msg=msg)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [<Book: My book 1>] != []
First list contains 1 additional elements.
First extra element 0:
<Book: My book 1>
- [<Book: My book 1>]
+ []
----------------------------------------------------------------------
Ran 1 test in 0.012s
FAILED (failures=1)
Destroying test database for alias 'default'...
When I change the filter to:
Book.objects.filter(pk=book.pk, published_in__contains=book.published_in)
the test passes.
¯\_(ツ)_/¯
contains ?!
I've got a test for this change in one of my projects that passes. Maybe I should port that over here?
Tests for this would be great, thank you.
I'll see what I can do - tomorrow.
Just make sure that when the test passes, you'll merge this in a timely fashion :) I've been waiting for this for almost 2 years now.
I make no promises - I do this all in my own free time and I have plenty of other things going on, as we all do.
But tests for anything and everything are much appreciated. 😄
On it now, it seems the changes introduced since my first patch broke the patch itself.
Need to figure out what's up here.
The culprit was https://github.com/goinnn/django-multiselectfield/commit/eadd5731b76918e46d45460f2efa655b7e62ae31, fixed that.
Some jobs failed to complete, looking into that.
The build for 4.1 has failed.
Support for 4.1 ended at April 5, 2023, removing from the tests.
Good to go now: https://github.com/karolyi/django-multiselectfield/actions/runs/9224962236
Thanks for your contribution!
Thanks. Good to see this module is taken care of by someone who still cares. :)