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

Overriding modeladmin's get_queryset not respected by Edit-, Inspect & DeleteView #21

Open
robmoorman opened this issue Dec 3, 2019 · 4 comments
Labels
good first issue Good for newcomers type:Enhancement New feature or request

Comments

@robmoorman
Copy link

robmoorman commented Dec 3, 2019

It's common to use a filtered set of models in order to apply view restrictions on list views, e.g.

Issue

File wagtail/contrib/modeladmin/options.py

def get_queryset(self, request):
    """
    Returns a QuerySet of all model instances that can be edited by the
    admin site.
    """
    qs = self.model._default_manager.get_queryset()
    ordering = self.get_ordering(request)
    if ordering:
        qs = qs.order_by(*ordering)
    return qs

Example override:

def get_queryset(self, request):
    qs = super().get_queryset(request)
    qs = qs.filter(user=request.user)
    return qs

If you try to change the id of an edit url (which shouldn't be allowed) you can still enter the details of that object. From my point of view it's caused by this piece of code:

class InstanceSpecificView(WMABaseView):

    instance_pk = None
    pk_quoted = None
    instance = None

    def __init__(self, model_admin, instance_pk):
        super().__init__(model_admin)
        self.instance_pk = unquote(instance_pk)
        self.pk_quoted = quote(self.instance_pk)
        filter_kwargs = {}
        filter_kwargs[self.pk_attname] = self.instance_pk
        object_qs = model_admin.model._default_manager.get_queryset().filter(
            **filter_kwargs)
        self.instance = get_object_or_404(object_qs)

Possible solution

The id/pk should be filtered from the get_queryset instead and respect it possible overrides.

@robmoorman robmoorman added the type:Bug Something isn't working label Dec 3, 2019
@engAmirEng
Copy link

engAmirEng commented May 9, 2023

@gasman This is serious

@gasman gasman added the good first issue Good for newcomers label May 9, 2023
@disconnect821
Copy link

Would like to give this a try.

@engAmirEng
Copy link

engAmirEng commented May 19, 2023

@gasman @disconnect821 I'm on it

@laymonage
Copy link
Collaborator

The documentation does mention that the get_queryset function is a customisation point for the IndexView and makes no mention that the same queryset is used for other views: https://docs.wagtail.org/en/stable/reference/contrib/modeladmin/indexview.html#modeladmin-get-queryset

Changing this behaviour might be unexpected (although unlikely) for existing users. That said, I'm inclined to classify this as an enhancement request rather than a bug.

As per wagtail/rfcs#85, I'm moving this to the wagtail-modeladmin repo. I believe the same is also the case with snippets, so I'll file a separate issue for that.

@laymonage laymonage added type:Enhancement New feature or request and removed type:Bug Something isn't working labels Jul 26, 2023
@laymonage laymonage transferred this issue from wagtail/wagtail Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type:Enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants