Add filter to support 1,2,3 syntax

[zoidyzoidzoid]

Based on discussion from #137

Using some of @kmwenja's work from his comment, and some from the original gist I had.

I considered using similar terms to Django for cleaning/validating the values, but I left it with the function names used in the comment where the cleaning was suggested.

[kmwenja]

Thanks for the shout-out 😄

[zoidyzoidzoid]

Credit where it's due. Yours allowed simple coercion, that I hadn't considered since I'd only used it with strings.

[zoidyzoidzoid]

Hi @carltongibson, is this the right direction? Sorry, if you were planning on looking at this already.

[carltongibson]

Hi @zoidbergwill. Thanks for the PR.

My initial thought is that this needs to be done at the widget level rather than the filter.

Look at the SelectMultiple widget — it is responsible for calling getlist on the request parameters QueryDict, and so get the list we are expecting from the standard ?param_list=value_1&param_list=value_2 syntax.

If we're going to add support for the 1,2,3 syntax then we need to match the behaviour there, rather than add a new filter type.

I know most probably want this for DRF related work, rather than building HTML forms, but some consideration should probably go into the render side of the equation too.

Also we need to document when/where/how devs would use the new widget.

[zoidyzoidzoid]

Thanks

I'll take a look at the SelectMultiple widget.

I only really have experience with the DRF-related uses for the filters, but I'll take a look at the HTML forms stuff and try my best to match the behaviour.

I've written some basic documentation for the filter that I haven't pushed yet, so I'll write some once the widget work is done too.

[carltongibson]

First pass I'd guess sublclass TextInput and just do the join on the value.

The ‘value’ given is not guaranteed to be valid input, therefore subclass implementations should program defensively. — The Widget Docs

[zoidyzoidzoid]

Thanks for all the help.

My initial thought is that this needs to be done at the widget level rather than the filter.

So I made a widget that matches the filter behaviour, so should I get rid of the filter? Will the widget be used when I send GET parameters to the filter in DRF?

Sorry, I tried to test the filter side with widgets, without any success. I have working tests for the widget though.

[zoidyzoidzoid]

@carltongibson Does this look more in the right direction?

[carltongibson]

Hey @zoidbergwill — Yes. That looks more or less what I had in mind.

I think people are going to want an example of how to specify the widget using the Filter.extra attribute, but other than that it looks good.

[zoidyzoidzoid]

Hi @carltongibson, is there a way to use the widget with a REST endpoint?

I've looked through the code and docs. I feel like I'm missing something.

Am I doing the logic at the wrong point in the widget?

[carltongibson]

Hey @zoidbergwill.

The flow goes FilterSet -> Filter -> Field -> Widget. Filters have an extras field that is passed down to the Field so if you set your Filter's extras.widget to your custom class that should get set.

You should be able to do this at declaration but if not override init, pull the filter and set it by hand.

(If you're getting stuck I'll need to see your code — what are you trying that's not working?)

[carltongibson]

Take another look here: https://django-filter.readthedocs.org/en/latest/usage.html#the-filter — the extra example there should work when modified

(Let me know how you get on: both for the docs here and in general.)

[zoidyzoidzoid]

So I was having some issues with testing this against an existing project, because of the Django forms.CharField's to_python function. It calls __unicode__ when sent a list.

I've added a simple CSVFilter, that uses a simples CSVField with a custom to_python field so the filter receives a proper list, it just feels wrong and dirty.

I can't see an easier solution with a better filter function to override or anything.

[zoidyzoidzoid]

Sorry @carltongibson, I missed your first of the two comments there.

[zoidyzoidzoid]

I'll push the field and filter stuff I've added, and I can add tests if it's gonna stay or I can rebase it out.

[carltongibson]

Ok, Thanks for the effort let me have a look at what you've got here and we'll see how to go forward.

[zoidyzoidzoid]

I tried writing tests, but there's nothing really worth testing that I added.

[carltongibson]

Hi @zoidbergwill,

So I was having some issues with testing this against an existing project, because of the Django forms.CharField's to_python function. It calls unicode when sent a list.

This confuses me. We're talking about adding a new widget for use with a SelectField on a MultipleChoiceFilter right. Where does the forms.CharField come from?

Can you put your FilterSet declaration either here, or in a test case (that you'd like to pass) or in Gist and I'll take a look? Thanks

[zoidyzoidzoid]

Sorry, I haven't had the time to work on this. I'll have time in the next few weeks.

[kewama]

Will the proposed code allow a developer to support syntax like this?

/resources/?id__in=1,2,3

or only something like:

/resources/?ids=1,2,3

What about support for __range lookup, e.g.;

/resources/?id__range=1,10

[gabn88]

This should really be added to make django-filter practical to use. In Ruby this feature has landed long ago in the default filtering gem.

[zoidyzoidzoid]

Gonna give this another try.

[carltongibson]

@zoidbergwill — OK. Cool. No stress. :-)

Rather than a filter, look at just creating a widget.

[carltongibson]

I'm going to close this pending the new version.