wagtail-nest / wagtail-modeladmin

Add any model in your project to the Wagtail admin. Formerly wagtail.contrib.modeladmin.
Other
22 stars 8 forks source link

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

Open robmoorman opened 4 years ago

robmoorman commented 4 years ago

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.

engAmirEng commented 1 year ago

@gasman This is serious

disconnect821 commented 1 year ago

Would like to give this a try.

engAmirEng commented 1 year ago

@gasman @disconnect821 I'm on it

laymonage commented 1 year ago

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.