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

Allow overriding the base queryset to be used in Snippets IndexView #10275

Closed
wants to merge 15 commits into from

Conversation

laymonage
Copy link
Member

@laymonage laymonage commented Mar 28, 2023

Part of wagtail/rfcs#85. Incorporates #10271.

This PR extracts a get_base_queryset() method to the generic IndexView (inspired by ModelAdmin's WMABaseView.get_base_queryset() for the initial logic of the queryset, before any filters or search are applied. The API is surfaced in SnippetViewSet as get_queryset(request) to mirror ModelAdmin.get_queryset(request) API. On the Snippets IndexView.get_base_queryset(), we first try to call SnippetViewSet.get_queryset(self.request). If the result is None (the default), the original implementation in the generic IndexView is used.

Please check the following:

  • Do the tests still pass?1
  • Does the code comply with the style guide?
    • Run make lint from the Wagtail root.
  • For Python changes: Have you added tests to cover the new/fixed behaviour?
  • For new features: Has the documentation been updated accordingly?

Please describe additional details for testing this change.

Continuing the test example in #10256:

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


class PersonViewSet(SnippetViewSet):
    icon = "user"
    list_per_page = 2
    chooser_per_page = 1
    admin_url_namespace = "some_namespace"
    base_url_path = "deep/within/the/admin"
    chooser_admin_url_namespace = "my_chooser_namespace"
    chooser_base_url_path = "choose/wisely"
    # this will generate exact filters
    list_filter = ["job_title", "first_name"]
    # alternatively, this will generate filters with lookups
    list_filter = {
        "job_title": ["contains"],
        "first_name": ["contains"],
        "first_published_at": ["date"],
    }

    def get_queryset(self, request):
        return self.model._default_manager.all().exclude(job_title__contains="[HIDDEN]")


register_snippet(Person, viewset=PersonViewSet)

If you create a Person object with a job title that contains [HIDDEN], it won't be shown on the listing view.

Footnotes

  1. Development Testing

@squash-labs
Copy link

squash-labs bot commented Mar 28, 2023

Manage this branch in Squash

Test this branch here: https://laymonagecustom-snippets-get-q-egkin.squash.io

@laymonage laymonage force-pushed the custom-snippets-get-queryset branch from 879a0a7 to 4add057 Compare April 4, 2023 12:02
Copy link
Collaborator

@gasman gasman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@@ -22,20 +22,27 @@
from wagtail.test.utils import WagtailTestUtils


class TestCustomIcon(WagtailTestUtils, TestCase):
class BaseSnippetViewSetTests(WagtailTestUtils, TestCase):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this encompasses the search tests that now have to be done as a TransactionTestCase instead as of #10208 - will find out when I rebase... - but if it does, I think the work needed to untangle that might outweigh the benefit of pulling out these methods, in which case I might skip this commit.

@gasman
Copy link
Collaborator

gasman commented Apr 14, 2023

Merged in fbd3cca

@gasman gasman closed this Apr 14, 2023
gasman added a commit that referenced this pull request Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants