Opened 5 years ago

Closed 5 years ago

#13297 closed (duplicate)

Cannot specify 'pk' in model default ordering

Reported by: anonymous Owned by: gruszczy
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords: ordering pk
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I wanted to make an explicit default ordering over a model's primary key (it does make sense in my app), but manage.py validate complains:
"ordering" refers to "pk", a field that doesn't exist.

Specifying 'id' is fine with it.

Attachments (2)

13297_1.patch (1003 bytes) - added by gruszczy 5 years ago.
13297_2.patch (1.5 KB) - added by gruszczy 5 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Looks like a case where the 'pk' alias isn't being expanded correctly. I'm not sure we specifically document that this *should* be possible, but it would certainly be consistent.

comment:2 Changed 5 years ago by gruszczy

  • Owner changed from nobody to gruszczy

I can either change get_field in options to accept pk:

def get_field(self, name, many_to_many=True):

"""
Returns the requested field by name. Raises FieldDoesNotExist on error.
"""
if name == 'pk':

return self.pk

to_search = many_to_many and (self.fields + self.many_to_many) or self.fields
for f in to_search:

if f.name == name:

return f

raise FieldDoesNotExist('%s has no field named %r' % (self.object_name, name))

or check this inside get_validation_errors in django.core.management.validation. I don't know django core that well to decide, which way is better, but it seems to me, that get_field should return some good value also for 'pk'. If someone could make this decision, I will gladly make a patch and write some tests.

comment:3 Changed 5 years ago by gruszczy

Proper formatting of the code.

    def get_field(self, name, many_to_many=True):
        """
        Returns the requested field by name. Raises FieldDoesNotExist on error.
        """
        if name == 'pk':
            return self.pk
        to_search = many_to_many and (self.fields + self.many_to_many) or self.fields
        for f in to_search:
            if f.name == name:
                return f
        raise FieldDoesNotExist('%s has no field named %r' % (self.object_name, name))

comment:4 Changed 5 years ago by gruszczy

I am adding two patches. First changes get_field in options, second changes get_validation_errors in validation. Both also have test (which is only a model with ordering by pk specified). I hope one of them is good, if not, I will be happy to provide something better, if only I am advised what can be fixed in this case.

Changed 5 years ago by gruszczy

Changed 5 years ago by gruszczy

comment:5 Changed 5 years ago by kmtracey

  • Resolution set to duplicate
  • Status changed from new to closed

This was already reported in the still-open #8291.

Note: See TracTickets for help on using tickets.
Back to Top