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

ChoiceFilter callable choices #710

Closed
teelee7133 opened this issue May 11, 2017 · 7 comments
Closed

ChoiceFilter callable choices #710

teelee7133 opened this issue May 11, 2017 · 7 comments
Milestone

Comments

@teelee7133
Copy link

Hello,

For ChoiceFilter (and other related ones), is it possible, or will it make sense, to have the 'choices' callable to behave like the ChoiceField in django, where 'it is evaluated each time the field’s form is initialized'?

https://docs.djangoproject.com/en/1.11/ref/forms/fields/#choicefield

(I am happy to draft a PR if this sounds a reasonable feature request)

Thanks!

@carltongibson
Copy link
Owner

... or will it make sense...

That's the question isn't is? 🙂

Can you explain your use-case a bit so we can see? Thanks!

@teelee7133
Copy link
Author

Indeed! 🙂

I think currently in ChoiceFilter, choices parameter accepts a callable. But if datetime function or ORM functions are used within that callable, then, ideally, it would be best the choices data to be materialized when the FilterSet instance is created, instead of when the FilterSet class is created.

Some naive example:
Consider some reporting system with the following models (I have not tested the syntax):

class AccountTransactionType(models.Model):
    name = models.CharField(max_length=100)

class AccountTransaction(models.Model):
    transaction_type=models.ForeighKey(AccountTransactionType)
    transaction_date=models.DateTimeField()
    #....

# ------------
def recent_type_choice_fn():
    # Lets get some more recent account type
    return AccountTransaction.objects.filter(
        transaction_date__gt=(timezone.now() - timezone.delta(days=30))
    ).values_list('transaction_type', 'transaction_type__name')

class AccountingTransactionFilter(django_filter.FilterSet):
    transaction_type = ChoiceFilter(
        choices=recent_type_choice_fn, 
        #....
    )
   

Here we want to show transaction type with recent entries e.g. last 30 days, in the filter. However, choices for AccountingTransactionFilter will be fixed when the class is created. If we leave django server running for a month, the choices won't change. In order to have refreshed choices, either ChoiceFilter have to be subclassed, or restart django server...

Let me know of your thoughts, thanks!

@rpkilby
Copy link
Collaborator

rpkilby commented May 12, 2017

Yep. The problem is choice resolution during the field instantiation. Could you try installing the branch from #680? I believe that this incidentally fixes the issue you're describing here.

@rpkilby
Copy link
Collaborator

rpkilby commented May 12, 2017

It might not also be a bad idea for me to add a test in 680 ensuring that choices are resolved during FilterSet form creation instead of during filter instantiation.

@teelee7133
Copy link
Author

Yep rpkilby, I gave PR #680 a quick test, and it looks like the PR solves the issue. 👍 Thanks for the PR!

When will it likely be released? 🙂

@carltongibson
Copy link
Owner

When will it likely be released? 🙂

It'll be a week or two (at least). I need to finish 1.0.3 and then look at the proposals for 1.1 (which includes this one.) If you need it before then I'd suggest installing from the branch

@carltongibson carltongibson added this to the Version 1.1 milestone May 16, 2017
@teelee7133
Copy link
Author

version 1.1 sounds good, thanks!

rpkilby pushed a commit to rpkilby/django-filter that referenced this issue Jul 25, 2017
rpkilby pushed a commit to rpkilby/django-filter that referenced this issue Aug 22, 2017
carltongibson pushed a commit that referenced this issue Oct 19, 2017
* Add test cases from PR 639. Thanks @michael-mri!

* Fix tests for null choice iterator

* Add more null value tests

* Add null choice to various (Model)ChoiceFields

* Fix django < 1.11 field iterator handling

* Resolve #710 - add test for lazy callable choices
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

No branches or pull requests

3 participants