Intent question

[jaredahern]

I'm doing a significant revamp to this filter, which will include things like more easily overridden widgets and fields, as well as support for multiple select. In the process of doing this, I ran into a point of confusion regarding our class variables - partially caused by my prior incorporated revisions. Basically, what do people expect field_name and parameter_name to do in the case of relations that span multiple models?

Example: you want to single-filter a Book model by publisher__country, where Book.publisher is a FK to Publisher.pk and Publisher.country is a FK to Country.pk.

As far as I can tell, this is the current situation:

• On the filter class, we set field_name to be "country" and parameter_name to be "publisher__country".
• Name of field to be queried on ultimate model ("country") -> field_name.
• Name of GET parameter to be used in query ("publisher__country__exact" or similar) -> an instance-level parameter_name. A "base" version of this may be defined at the class level ("publisher__country"), which will be used to build the final version, or field_name will be the base.
• Name of the queryset field lookup ("publisher__country__exact") -> same version of parameter_name as above.

But I think this is a bit sub-optimal, and confusing. Overwriting the class-level parameter_name is confusing and a bit contrary to how SimpleListFilter works, which complicates the factory version; parameter_name is dual-used and doesn't allow custom non-lookup GET parameters; and while we only have README docs for field_name, it doesn't actually handle the nested relations, leading to user confusion.

I propose that the filter should work as follows:

• On the filter class, we set field_name to be "publisher__country". If we don't want to use the auto-generated parameter_name for the GET parameter, specify that too.
• Name of field to be queried on ultimate model ("country") -> a property calculated from field_name.
• Name of GET parameter to be used in query ("publisher__country__exact", or something totally different) -> the SimpleListFilter-level parameter_name (calculated from field_name if omitted).
• Name of the queryset field lookup ("publisher__country__exact") -> another property calculated from field_name.
• (Note that the factory version might slightly change as well, but I think in a way that would be backwards-compatible.)

Alternatively, we could use something other than field_name as the main variable, clearly indicating that its old use is depreciated. Since we don't really even document how to handle nested relations now (I'll address that as well), I'm not too worried about breakage in a pre-1.0 version, but we can handle that however seems best.

What say you all?

[farhan0581]

what about the name of the field name in the html widget that appears in the panel ? will field_name do the job ? Also I am also fine to merge the fields and use only one of them if possible. The idea is to keep this as simple as possible while defining the filter class.

[jaredahern]

what about the name of the field name in the html widget that appears in the panel ?

Good point. Right now it is controlled via the title class attribute. Should that default to field_name if no separate title is provided?

Also I am also fine to merge the fields and use only one of them if possible. The idea is to keep this as simple as possible while defining the filter class.

Great, thanks for the feedback! I'll put together a PR for you to review.

[ndunn219]

@jaredahern, are you testing this against Django 3.2? If so, please keep in mind issue #53 as well.

[jaredahern]

Sure, I can test there too, @ndunn219 . I'm mostly done with my fixes, but I have one last bug that I'm trying to sort out before I create a PR. I haven't looked at it in great detail, but hopefully support for Django 3.2 won't be too complex of a change.