6585 admin dropdown uses modeladmin ordering #806

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

fisadev commented Feb 23, 2013

Solves ticket 6585:

https://code.djangoproject.com/ticket/6585

(uses the modeladmin ordering if provided, else falls back to model ordering)

Also added 2 people to the AUTHORS (contributors on this and past tickets)

Contributor

fisadev commented Feb 23, 2013

(pull request done as part of #sprints-django-ar :)

'to_field_name': self.rel.field_name,
}
defaults.update(kwargs)
+
+ if 'queryset' not in kwargs:
@ramiro

ramiro Mar 2, 2013

Member

If I understand correctly. This isn't needed.

See ramiro/django@master...6585-admin-dropdown-uses-modeladmin-ordering for a WIP review.

@fisadev

fisadev Mar 4, 2013

Contributor

I understand it's needed. You have two different scenarios: the admin building the form field, or another thing building the form field. The default behaviour is this code, which uses the model ordering (Meta....). But you can override specifying the queryset kwarg, and that is what the admin does (changes to options.py) to override the Meta ordering with the ModelAdmin ordering. You can't expect to always have the argument, because of the situations when the field is being build outside the admin. And you can't ignore the argument because of the situations when the admin is building the field, asking for a specific queryset. Am I wrong?

@fisadev

fisadev Mar 4, 2013

Contributor

more info: the added tests should expose both scenarios, so it would be a good thing to run them with and without this, to be sure.

@fisadev

fisadev Mar 4, 2013

Contributor

update: my bad, was missunderstanding what was being done with defaults. You are right, that change isn't needed, and the tests prove that also.

Member

ramiro commented Mar 4, 2013

Merged in d9330d5.. Thanks!

@ramiro ramiro closed this Mar 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment