craigds / django-typed-models

polymorphic django models using automatic type-field downcasting
Other
220 stars 44 forks source link

TypedModel instance missing `get_type_display` method #47

Open thejcannon opened 5 years ago

thejcannon commented 5 years ago

Since choices doesn't have a truthy value, django.db.models.fields.Field doesn't add the get_FOO_display (code, docs) which in this case would be get_type_display.

IMO this is a Django bug (I'll file a ticket shortly), however opening an issue here for transparency and tracking (if they reject the ticket).

FYI django_model_utils works around this by adding dummy choices: https://github.com/jazzband/django-model-utils/blob/master/model_utils/fields.py#L75 So there is a (disappointing) workaround

thejcannon commented 5 years ago

Django ticket

thejcannon commented 5 years ago

The ticket was accepted, and a PR exists for it. That means this will be fixed in Django, and won't need any changes to this package 🎉

thejcannon commented 5 years ago

Hmm, I guess though if you wanted it to work in older Django versions you'd still need to make choices truthy. I suppose choices=True could work if you wanted something simple, or choices=[(0, "")], which would be more correct type-wise.

craigds commented 5 years ago

Tempted to just not fix it here rather than hack the choices, since that probably has other consequences.

What are you using get_type_display() for?

thejcannon commented 5 years ago

Rendering the human-readable choice. Type: {{ object.get_type_display() }}. Django also mentions it for tags like regroup: https://docs.djangoproject.com/en/2.1/ref/templates/builtins/#grouping-on-other-properties

It's a really nice method. If you don't want to hack the choices (which I don't blame you). Consider adding get_type_display manually to the TypeModel class?

craigds commented 5 years ago

Cool, that's a reasonable thing to do. I'll hold off till the Django ticket is addressed so I can guard the method addition on a particular django version (presumably 2.2, but maybe they'll backport it)