Custom FilterSpecs

I really like the idea behind this, but I'd need to review the code closer and won't take the time to do it right now since it's a new feature. I'd rather focus on bugs preventing newforms-admin from being used.

Also see #3400 for another ideas about filters which could benefit from this scheme...

This ticket isn't critical to merge newforms-admin into trunk. Tagging with nfa-someday.

How about FilterSpec?.insert method to do prepend and FilterSpec?.register to do append? discussed somewhere, no?

I'm attaching my crack at this issue; in particular, I wanted to allow custom FilterSpecs that aren't associated with a field. This way, you can throw together a filter for fairly arbitrary queries.

Much of the field-specific code from FilterSpec has been moved out to a subclass, FieldFilterSpec; that should be used as the superclass for custom field-based filterspecs, while FilterSpec can be used for non-field-based ones.

The list_filter syntax gains two further options besides field names: a FilterSpec subclass, or a tuple of ('field_name', FieldFilterSpecSubclass).

An example combining all three:

list_filter = (
    'field1',
    ('field2', SomeFieldFilterSpec),
    SomeFilterSpec,
)

Custom FilterSpecs?, also allowing fieldless FilterSpecs?

Tweaked the summary and description.

^{that was me; sorry.}

updated version of my simple patch,

Honza_Kral: I modified the filterspec definition to allow for users to register their own filters in admin. The mechanism is simple - I just reverted the order of the registry so that newly registered specs will come first. That way if you register your own filter via FilterSpec.register, it will be used before the default one.

I'm not sure having a reversed registry is intuitive. It certainly makes overriding FilterSpecs? simple. Except if I were to register two FilterSpecs? I would expect them to be tested the order I add them, the same way it is done in the rest of Django. Maybe the FilterSpec?'s should be in a Priority Queue instead of a Queue?

I had created a solution which doesn't reverse the registry ticket:8330. But I'm realizing now that this wouldn't allow for overriding other filters like Boolean. I'll keep playing.

Suggestion: make FieldFilterSpec?'s similar to how widgets work in newforms admin.

Based on korpios' idea I made a patch that takes the idea further. With korpios' current solution you can add custom filters but not play with the queryset. Filters only based on field lookups is too little for complex administrations.

I provide a patch which lets you add, like korpios' suggested, custom filters like this:

list_filter = (
    'field1',
    ('field2', SomeFieldFilterSpec),
    SomeFilterSpec,
)

Furthermore, every FilterSpec? can implement get_query_set:

def SomeFilterSpec:
    def get_query_set(self, cl, qs):
        if self.params.has_key("custom_get_parameter"):
            return qs.filter(somefield__startswith = self.params["custom_get_parameter"])

Note that this is a trivial example that can also be achieved with field lookup parameters. But there are no limits of how to filter the result set.

Custom Filtersets (fieldless) with custom query set manipulation.

line 103 of filterspec_with_custom_queryset_against_1_0.diff should read:

+        super(RelatedFilterSpec, self).__init__(request, params, model, model_admin, f)

I believe there is a small bug with this patch. Line 31 of filterspec_with_custom_queryset_against_1_0.diff should be indented once more so that get_field() is only called when a field is defined in the list_filter. In the case that the list_filter item is a single callable item, the field variable is not set and get_field() should not be called for verification.

Also, any word on if this feature will make it into 1.1? I notice that there is no implemented listed on the status page. I'm using this feature frequently in my application, and would prefer to run a released version of Django instead of patching. :)

New path is diff versus v1.0.2 of django and includes fixes mentioned in the following comments:

10/08/08 20:08:44 changed by anonymous ¶ 12/12/08 09:58:24 changed by gerdemb ¶

Patch updated to latest HEAD passing all tests

I updated the patch to the latest HEAD and fixed the TODO comments. In order to pass the existing tests, I had to disable the use of custom GET params. In the original patch, the filterspec could consume a custom param and then return an arbitrary queryset based on this parameter, but to do this it silently removed the parameters that don't match a field name from the request. The problem is that there is a test to insure that any parameters that do not match field names are handled by forwarding to ?e=1 (testIncorrectLookupParameters) which failed when the previous patch silently removed them. Additionally, the code in the previous patch didn't handle filters on M2M relationships through an intermediate table because it assumed the first part of the search param would be a field name.

Developer discussion here: http://groups.google.com/group/django-developers/browse_thread/thread/eb08729417d50ef3

I am attaching a new patch against the latest HEAD. It's based on gerdemb's patch but now allows for consuming of GET parameters to allow for custom, non-field based querystring keys. The patch also implements a priority queue in a sense; FieldFilterSpec?.register() allows you to pass in high_priority=True in order to register the FieldFilterSpec? as high priority. This argument is True by default but passed in as False for all default filter specs. High priority specs are inserted before normal priority specs, but order is preserved amongst the two separate groups. Let me know what you think of that solution.

I'm also moving this to Accepted since it's on the 1.1 features list (forgive me if this is an incorrect decision). Also flagging it as Needs tests.

New patch against r9836

Please see 5833-against-9836-new-proper.patch and not 5833-against-9836-new.patch; I accidently diffed some additional, unrelated changes, and couldn't get trac to replace the old attachment.

I deleted the 5833-against-9836-new.patch to avoid confusion.

I don't see any documentation for this enhancement in any of the patches? Makes it hard to gets started on reviewing the code. In the absence of doc, tests would also at least show how the new function is intended to be used, but those too are still missing. If anyone who has worked on the patches here or has an interest in seeing this done could provide some of these missing pieces, that would help move this one along.

Without docs and tests, and with 1.1 feature freeze coming up soon, this isn't going to make 1.1.

Replying to jacob:

Without docs and tests, and with 1.1 feature freeze coming up soon, this isn't going to make 1.1.

I would like to get this committed, we can work on it during the PyCON sprint. It is really important for us that this ticket or #3400 gets into 1.1 and we are ready to put some work into it.

The feature freeze is tomorrow. Only bug fixes will be happening during the sprints at PyCon. I'm afraid this is just going to have to wait until 1.2.

Re-targeting for 1.2.

The validation in the latest patch is broken. Attaching a new version against 1.1/SVN which also fixes #11771.

New patch against 1.1/SVN

When you delete a record, querystring looses return_to_ parameter.

(BTW last patch seems not working for me)

Another place where it would be likeable to keep the filters are with admin actions

Just a small note: the patch 5883.patch should be modified in django/contrib/admin/views/main.py, line 200.

Currently it reads:

if new_qs: 

which fails if new_qs is empty QuerySet? (i.e. the filters output is empty). It works for me if I change it to

if new_qs is not False: 

#12173 raised the issue of ORs in filterspecs.