Code

Opened 3 months ago

Closed 2 months ago

#21898 closed Cleanup/optimization (worksforme)

SingleObjectMixin should not require slug or pk if queryset is given

Reported by: tomc Owned by: nobody
Component: Generic views Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I might be missing an obvious design decision made here, but it seems to me that if you give a queryset or define get_queryset() in a class with the SingleObjectMixin, that I shouldn't need to override get_object() and essentially dump the same code in but without checking for slug or pk.

Simple use case: I'm rolling my own flatpages style app, different enough that I don't want to inherit from it. I'm not using the url as the pk as if someone typos the url the first time, they end up with two pages if they rename it. So my code ends up looking like this:

class PageDetail(DetailView):
    def get_queryset(self):
        return Page.objects.filter(url=self.kwargs.get('url'))

    def get_object(self, queryset=None):
        if queryset is None:
            queryset = self.get_queryset()

        try:
            obj = queryset.get()
        except ObjectDoesNotExist:
            raise Http404('No %(verbose_name)s found matching the query' %
                          {'verbose_name': queryset.model._meta.verbose_name})

        return obj

    def get_template_names(self):
        return [self.object.template_name]

I feel like I should be able to just set the queryset without overriding get_object()

Another option instead of removing the pk or slug check, might be to have a variable that sets the lookup field that can be anything, not just slug or pk, and if this isn't set, it reverts to the current behaviour

I should be able to provide a patch for either case if necessary.

Attachments (0)

Change History (1)

comment:1 Changed 2 months ago by mjtamlyn

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

I'm going to close this ticket, despite the fact I agree the API could be better. That said for your specific use case there are two reasonable options with the current API:

Firstly you can specify slug_url_kwarg = 'url and slug_field = 'url' and you will actually get the code you want without overriding any methods. There are some more complex use cases where this works, but I think in those situations it might be best to just override get_queryset.

There's an argument that we should provide a better set of tools than slug_url_kwarg/pk_url_kwarg etc for customising SingleObjectMixin, but that would be a separate ticket.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.