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

Filter PageChooserPanel queryset #1250

Open
rmartins90 opened this issue Apr 29, 2015 · 26 comments
Open

Filter PageChooserPanel queryset #1250

rmartins90 opened this issue Apr 29, 2015 · 26 comments
Labels
component:Choosers Modal choosers for Page, Snippet and other models component:Frontend type:Enhancement

Comments

@rmartins90
Copy link

It would be great if you could filter Pages on PageChooserPanel using a queryset.

Something like:

PageChooserPanel('link_page', queryset=EventPage.objects.live().descendant_of(events_index)),
@MechanisM
Copy link

Yea! I thought about it aswell. For peeps at torchbox, it should be known for adding authors for blog posts in wagtail-torchbox 😄 Much better to jump directly to people pages.

@gasman
Copy link
Collaborator

gasman commented Apr 29, 2015

Interesting idea! Clearly you can't pass an arbitrary queryset as part of the URL to /admin/pages/chooser, so you'd probably have to pass it a reference to the original model field (something like /admin/pages/chooser/?target_field=demo.BlogPost.author) and have the chooser view extract the queryset from there. You'd have to make sure it also works when the field is part of an inline child rather than the base Page model - and if you want it to work within StreamField too (where the chooser could be any number of levels deep), that's another layer of complexity altogether...

You'd also need to define the queryset in a way that avoids evaluating it at server startup - it would probably have to be a callable (e.g. a lambda function) that returns a queryset.

In fact, now that I think about it some more - this is an unsolved problem within Django, too. limit_choices_to is Django's attempt at limiting the values of foreign key fields, but it doesn't work with arbitrary querysets (only filter dicts and Q objects), and it doesn't integrate fully with raw_id_fields in Django admin, which is Django admin's equivalent of Wagtail's PageChooserPanel. In other words - there's an opportunity here for someone to solve this problem for both Wagtail and Django :-)

But to bring this back down to earth... maybe this is all overkill, and the easy solution is to make the page chooser interface a bit smarter. If the page chooser is asking for an EventPage, and all of our EventPages exist in a single section, then there's no point dropping the user at the root level - we should just take them straight to that section.

@jorge-marques
Copy link

Maybe it would be possible to drop the user at the shallowest level where a page of the specified type appears? A simple list of pages would also work nicely for most cases. IMO, the search is main advantage over a simple select field.

@gasman
Copy link
Collaborator

gasman commented Dec 10, 2015

Related / alternative suggestion: #670

@nek4life
Copy link
Contributor

We're using this to select featured articles for a blog index page from a blogs child posts. It would be much less confusing for the blog author to start from the index page instead of the root of the tree, that way only the blog posts for that blog are shown. At the very least it would be lovely to have a boolean to select use currently page as the root of the tree, but having full queryset capabilities would be amazing.

@lb-
Copy link
Member

lb- commented Nov 24, 2017

For context, the suggestion in #670 proposes a function approach, eg. pass a function into PageChooserPanel that returns a list of pages. This would allow for a more robust solution, being able to filter by anything the developer needs like if pages have images attached.

@cnk
Copy link
Member

cnk commented Jan 31, 2018

Hey can I get some advice on how to approach fixing this issue? I need to have many of my snippet models scoped by site (I have a multitenanted app). So I need to be able to override the query done in wagtailsnippets/views/chooser.py's choose method.

I am currently monkey patching that method so that I can add a class method on my snippet class that contains the queryset for the chooser panel. Please see this gist. If this would be acceptable, I could work it into a patch for SnippetChooserPanel - and perhaps for PageChooserPanel as well

The last comment on this thread seemed to point to a solution that was more about passing the query override into the panel definition - more like this. The explicitness of that is rather nice - but would require more code tracing for me to explore a solution.

Comments?

@gasman
Copy link
Collaborator

gasman commented Feb 2, 2018

Thanks for looking into this @cnk! While the listing_queryset method is a very neat solution to this problem, I'd be wary of adding it to Wagtail's core functionality, as it's mixing model-level and view-level concerns a bit too closely.

Firstly, we want to avoid assuming that the editor is logged in through the same hostname as the site they're editing, as that closes off the possibility of having one central team responsible for multiple sites (I believe there are a few Wagtail installations in the wild working on that model).

Thinking about it more abstractly - listing_queryset here serves as a kind of validation of allowed values, similar to limit_choices_to on a ForeignKey. Passing the request to this method means that we can vary the set of allowed values in different situations, but to do that the developer has to pull the relevant information out of the request object themselves (request.site in this case, but it could potentially be any property of the request object) rather than being passed that information explicitly ("give me the set of valid authors for site X"). This makes life difficult for us as developers of Wagtail: we never told site authors which properties of the request are safe to check, so if we ever change the way that the chooser popup works in future, we have no way of knowing what we're able to change before we start breaking people's code.

It's hard to say how we can avoid this, but I think the first step is to make choosers more customisable so that, rather than an all-purpose SnippetChooserPanel, you'd be able to easily define a custom AuthorChooserPanel with whatever filters/parameters you need for that particular model. https://github.com/springload/wagtailmodelchoosers could be a good starting point for that...

@cnk
Copy link
Member

cnk commented Feb 5, 2018

Yes, my example of restricting to items associated with a specific site was just an example of the general case of "pass a function to do the restriction". I see your point about the request being an unknowable bag of attributes that might change so is a slightly risky thing to depend on in one's code.

I'll take a look at the wagtailmodelchoosers repo above. Hopefully it will give me some ideas on how to make more customizable choosers. Working from the Bakery Demo and some other GitHub issues, I have been trying to cobble together the UI I want for my ManyToMany relationship and frankly am getting very frustrated. If that doesn't get me what I need, I'll extract a demo project and ask for advice.

@Sam-Costigan
Copy link

Not quite the same as specifying the QuerySet per PageChooserPanel, but this issue is one of the top results for filtering the PageChooserPanel through any means so I thought it would be good to link to this relevant wagtail hook: https://docs.wagtail.io/en/v1.11.1/reference/hooks.html#construct-page-chooser-queryset

That hook might then be used like:

@hooks.register('construct_page_chooser_queryset')
def limit_page_chooser_to_site_specific_pages(pages, request):
    try:
        return pages.in_site(request.site)
    except Exception as e:
        # couldn't filter by site so return untouched pages queryset
        return pages

In order to achieve the site-limited query sets mentioned by @cnk.

@JorneVL
Copy link

JorneVL commented Mar 28, 2019

Isn't this something that can be implemented like the Django ModelAdmin function formfield_for_foreignkey?

I need this functionality for custom models, but this isn't possible because the ModelAdmin of Wagtail is custom.

@brylie
Copy link
Contributor

brylie commented Feb 9, 2022

For our use case, we want to select author objects for related articles but select only from Authors that have been published (live = True). The problem we are trying to circumvent is content editors accidentally:

  1. creating an author but only saving it as a draft
  2. creating an article and selecting the draft author
  3. when the user views the article, the author link leads to a 404 since the author wasn't published

Note, the global hook approach wouldn't work in our case as it would affect all PageChooserPanel querysets. We want to target a specific PageChooserPanel by passing in the desired queryset.

Alternatively, I've considered setting the default button state from "save draft" to "publish" for the Author form so no Author can be added in a draft state.

#4282

@ababic
Copy link
Contributor

ababic commented Feb 9, 2022

@brylie Can an author be unpublished while it has articles associated with it? If so, a widget that limits choice is only a partial solution in your case. Whatever author is selected, you still need to check if the related author is live before rendering a link to it on the article page.... and once you have that, do you need to restrict the chooser queryset any longer? On the one hand, it's helpful for editors to see relevant options only, but on the other, maybe both pages are a work-inprogress, and not being able to select the draft author for the article is a barrier that doesn't explain itself very well?

@brylie
Copy link
Contributor

brylie commented Feb 10, 2022

I believe specifying the PageChooserPanel queryset is a part of a holistic solution to the design problem.

As mentioned above, I'm also trying to figure out how to remove the "save as draft", and by extension, "unpublish", button from the "create author" form, since we don't want to save authors in the draft state or unpublish them. The template check for is_live may work in conjunction or may no longer be necessary if we constrain the input data.

@ababic
Copy link
Contributor

ababic commented Feb 10, 2022

@brylie okay, that all adds up. I just wanted to use your example to illustrate that related objects have their own lifecycle, affected both by direct and indirect actions, from editors and developers.

In over ten years of Django development, I have used limit_choices_to all of three times, and each time they ended up causing more headaches than they solved - because of this lifecycle issue: What is a 'valid' selection one moment can become 'invalid' the next - usually with no clear indication to the user of why. That issue is compounded by the fact that pages have drafts: what was a valid draft at the time of saving could be invalid by time that same draft is published.

As maintainers, we have a responsibility to prevent developers from shooting themselves in the foot. Maybe that can be done with heavy caveat-ing in the documentation, but my sense is that, even with that, it would still lead to a gradual influx of queries, false bug reports, and general dissatisfaction.

I honestly think the best solution here is to have some better base classes for developers to extend from... Then if they decide that overriding the queryset is worth the effort of jumping through a few hoops to create a custom panel/widget, they can do that easily enough.

@Sam-Costigan
Copy link

@brylie based on your requirements, and the fact you are considering removing the save as draft button, your author objects may be better off as regular Django models with a wagtail ModelAdmin, rather than extensions of the wagtail Page model. Is there any reason to keep them tied to a page?

@brylie
Copy link
Contributor

brylie commented Feb 14, 2022

That could be an excellent approach to define a regular Django model but might lack some of the benefits of the Wagtail page model. Particularly, the end-user needs to select a related entity among tens or hundreds of other entities. The Wagtail PageChooser is part of the core project and provides a friendly user experience.

The example use-case was meant to illustrate why a global hook approach may not be granular enough for some projects.

Potential foot guns aside, a queryset property on the PageChooser widget is an intuitive place for developers to specify the results they would like to show to the user.

@ababic
Copy link
Contributor

ababic commented Feb 14, 2022

@brylie For the time being, I think the best option is: https://pypi.org/project/wagtail-modelchooser/.

Particularly, the end-user needs to select a related entity among tens or hundreds of other entities. The Wagtail PageChooser is part of the core project and provides a friendly user experience.

You may well be aware of this already, but If you want a nice chooser experience, but don't really need any Page functionality, subclassing wagtail.search.index.Indexed and registering the model as a snippet will allow you to use SnippetChooserPanel - which automatically detects when wagtail.search.index.Indexed is used, and includes a search box to narrow down results.

@lb-
Copy link
Member

lb- commented Apr 21, 2022

https://github.com/wagtail/wagtail-generic-chooser is a good option if trying to do more complex page choosing scenarios.

The plan is, eventually, to bring parts of that code into Wagtail core.

It might be a good chance for others to use that library and see if it fits your needs, if so please report back or otherwise raise an issue on that repo.

Copied from #8401 (comment)

@lb- lb- added the component:Choosers Modal choosers for Page, Snippet and other models label Apr 28, 2022
@coredumperror
Copy link
Contributor

It might be a good chance for others to use that library and see if it fits your needs, if so please report back or otherwise raise an issue on that repo.

We recently switched to wagtail-generic-chooser for our custom choosers (largely to allow us to use a custom base queryset for the chooser listing), and it's been great! We were able to remove a bunch of much jankier code that we'd written to support this, and the whole thing feels a lot more solid, now.

Really hoping this functionality gets added to Wagtail core soon.

@ababic
Copy link
Contributor

ababic commented Nov 22, 2022

Another option to achieve this sort of thing is Proxy models.

  1. Define a proxy page model, subclassing the page type you're actually interested in (e.g. news.ArticlePage).
  2. Add a custom manager to the model and override get_queryset() to apply any filters you need.
  3. Add is_creatable = False to the model to prevent it from being used for other things.
  4. Pass the lone (important) proxy model to the chooser using the page_type argument.

You can do something similar for snippets / SnippetChooser too, but I haven't found a nice way to hide the menu item / separate listing just yet.

@gasman
Copy link
Collaborator

gasman commented Nov 28, 2022

Really hoping this functionality gets added to Wagtail core soon.

As of Wagtail 4 the core replacement for wagtail-generic-chooser is now in place - https://docs.wagtail.org/en/stable/extending/generic_views.html#chooserviewset

It isn't currently a like-for-like replacement for the page chooser, as it doesn't support tree-based navigation, but maybe it'll fit your needs.

@brylie
Copy link
Contributor

brylie commented Nov 29, 2022

Is there a recommended way to specify the queryset for a ChooserViewset, or do we override the get_object_list method?

@gasman
Copy link
Collaborator

gasman commented Nov 30, 2022

@brylie Overriding get_object_list is indeed the way to go.

@patillacode
Copy link

Is there an example to filter the queryset for a ChooserViewSet?

I have the ChooserViewSet and it is working as expected but I now need to filter out the available results.

I am trying to override the get_object_list method without success.

class ResourceChooserViewSet(ChooserViewSet):
    model = "cms.Resource"
    choose_one_text = "Choose a Resource"
    per_page = 20

    def get_object_list(self):
        pages = super().get_object_list()
        return pages.filter(live=True)

am I missing something?

@ababic
Copy link
Contributor

ababic commented Feb 20, 2024

@patillacode I would suspect you're missing some additional code to actually register your viewset with wagtail. When I want to override the chooser viewset for snippets, I usually need to do something like this in wagtail_hooks.py:

from wagtail.snippets.models import register_snippet
from wagtail.snippets.views.chooser import SnippetChooserViewSet
from wagtail.snippets.views.snippets import SnippetViewSet


class ResourceChooserViewSet(SnippetChooserViewSet):
    choose_one_text = "Choose a Resource"

    def get_object_list(self):
        pages = super().get_object_list()
        return pages.filter(live=True)


class ResourceViewSet(SnippetViewSet):
    model = Resource
    chooser_per_page = 20
    # This is how Wagtail knows to use the custom viewset defined above
    chooser_viewset_class = ResourceChooserViewSet
   

register_snippet(ResourceViewSet)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Choosers Modal choosers for Page, Snippet and other models component:Frontend type:Enhancement
Projects
None yet
Development

No branches or pull requests