Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added filters that filter multiple values at once using multiwidgets and multivaluefields #1164

Closed

Conversation

jonathan-golorry
Copy link

This is a pretty basic implementation, but it can do a few fancy things like efficiently filtering to customers with at least one completed, unreturned purchase:

class CustomerFilter(filters.FilterSet):
    purchases = filters.RelatedMultiFilter(
        related_names=["completed", "returned"], distinct=True
    )
    class Meta:
        model = Customer
        fields = {}
Customer.objects.filter(purchases__complete=True, purchases__returned=False).distinct()
SELECT DISTINCT "customers"."id"
FROM "customers"
INNER JOIN "purchases" ON ("customers"."id" = "purchases"."customer_id")
WHERE ("purchases"."completed" = True and "purchases"."returned" = False);

Currently django-filter with two separate separate filters would return all customers with a completed purchase and an unreturned purchase (with no guarantee that they are the same purchase). See #745 and #1077 :

class CustomerFilter(filters.FilterSet):
    class Meta:
        model = Customer
        fields = {
            "purchases__completed": ["exact"],
            "purchases__returned": ["exact"],
        }
Customer.objects.filter(purchases__complete=True.filter(purchases__returned=False).distinct()
SELECT DISTINCT "customers"."id"
FROM "customers"
INNER JOIN "purchases" ON ("customers"."id" = "purchases"."customer_id")
INNER JOIN "purchases" T3 ON ("customers"."id" = T3."customer_id")
WHERE ("purchases"."completed" = True and T3."returned" = False);

This is similar to RelatedFilter in philipn/django-rest-framework-filters/pull/199, but without the use of subqueries which can be extremely slow for this sort of lookup.

class CustomerFilter(filters.FilterSet):
    purchases = filters.RelatedFilter(
        "PurchaseFilter", queryset=Purchase.objects.all(), lookups=["completed", "returned"]
    )
    class Meta:
        model = Product
subquery = Purchase.objects.filter(completed=True).filter(returned=False).values('pk')
Customer.objects.filter(purchases__in=subquery).distinct()
SELECT DISTINCT "customers"."id"
FROM "customers"
INNER JOIN "purchases" ON ("customers"."id" = "purchases"."customer_id")
WHERE "purchases"."id" IN (
    SELECT U0."id"
    FROM "purchases" U0
    WHERE (U0."completed" = True AND U0."returned" = False)
);

Having done all this, I'm not sure a MultiWidget is a good way to group filters. The main issue is that validation errors are always described at the MultiWidget level instead of the subwidget level (see https://stackoverflow.com/questions/12245887/how-to-render-validation-errors-in-a-multivaluefield-multiwidget-per-field). This makes validation error messages largely useless. Unfortunately this is a fairly core issue in django and not easily fixable.

The django-rest-framework-filters approach of having a separate form for each RelatedFilter might be better.

If people really like this MultiWidget approach, the remaining work is:

  • Handle related_names that result in a ModelChoiceField (currently errors due to no queryset)
  • Change the SuffixedMultiWidget template to show suffixes to the user
  • Rewrite MultiValueField.clean to raise more useful validation errors. The best approach is probably to to make errors a dictionary, but I'm not sure how to set the keys or how Form will interpret a dictionary.
  • Write tests for fields.

I use django-rest-framework, so I don't have much experience with widgets, fields, and forms.

@rpkilby rpkilby mentioned this pull request Jan 28, 2020
2 tasks
@carltongibson
Copy link
Owner

Closing in favour of #1167.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants