Fixed sort on None fields

[maingoh]

Closes #98

The current fix allows choices to contains None values and put them in first position like this: [{'id': None, 'text': None}, {'id': 'bar', 'text': 'bar'} ,{'id': 'foo', 'text': 'foo'}, ...]. I hope it doesn't break somewhere later, maybe we don't want to add None values inside choices ? I am not sure where the choices are used. It seems it is for the autocompletion of the field when creating a filter but the completion doesn't seem to work on my side (maybe there is another bug somewhere ?).

At least this doesn't raise an exception :)

[coveralls]

Coverage remained the same at 95.914% when pulling a34e9886ed8f3c3c621c733e86303f441c1d061b on maingoh:fixed_nullable_fields into 4f1575a67e751330606a9a3ee4f33eda5214b44e on modlinltd:develop.

[coveralls]

Coverage decreased (-0.008%) to 95.906% when pulling 4029e12d5e50203c19a18afa5991de3c1e0a7713 on maingoh:fixed_nullable_fields into 85c282b0e4078af8972d88fab44857f95597ed74 on modlinltd:develop.

[asfaltboy]

Hi @maingoh , thanks for submitting the PR 👍! I assume you have a use case where this sorting is useful (or an exception is raised otherwise); I've just seen your ticket #98, thank you for reporting; could you please add a test case?

As for the autocomplete, this would work using select2 if installed. It used to be required as part of the library, but we decided to stop requiring it as it's too invasive, hard to maintain compatibility throughout jQuery versions, etc... Though it's been a while, I believe you should be able to enable this functionality by installing a package like applegrew/django-select2 and ensure the path of the frontend resources matches the related settings.

Please let me know if I can help with clarifying anything else.

[maingoh]

So to be clearer, when I just click on a model name where the advanced filters are activated and the admin launches the list of entries, there is an ajax method that call the django-advanced-filters view and get a list of all possible choice in advance for each field, probably for the case where I want to click on the "Advanced filter" button to create a filter. This is this ajax call that raise an exception (It seems that I cannot avoid it). The fact is that I don't have any filter on my nullable field (and don't need to create some) but I have some entries in my DB where the field value is null, so the endpoint called by the ajax request try to aggregate this None value. So this is why I don't know what should be the best fix to avoid this exception :) Do we want to enable None values in the choices ? Also if select2 is not installed, should we still do an ajax request to get those choices or are they totally useless ? I didn't have time to dig a lot into the code so probably my reasoning is wrong ^^.

And no worries if the autocomplete doesn't work, I don't need it much but thank you for the explanation !

[slorg1]

Hi @asfaltboy, I ran into the same issue as @maingoh and I was going to post a PR when I noticed his fix: could this be accepted?

Thank you!!

[asfaltboy]

Hi @slorg1 I think this can be merged, I'm just waiting for a test case.

If you would consider contributing, think it should be relatively straightforward to add a scenario where a nullable field contains a null value and fails without this fix, and passes after it's applied.

[davidlougheed]

+1 for this. I'll try and write a test case for the PR sometime soon.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 31044934a5de2388b65f35fb2ef485ea6667d965 into 85c282b0e4078af8972d88fab44857f95597ed74 - view on LGTM.com

new alerts:

• 1 for Unused import