Hi Simon,
On Thursday, 10 September 2015 16:57:51 UTC+1, Simon Charette wrote:
>
> Hi Malcolm,
>
> > The system check checks that all the values returned from
> get_readonly_fields exist as class attributes on the ModelAdmin (or are
> callable/fields on model, neither of which helps here). With them being
> created via __getattr__, they don't.
>
> There might be something else going on here but I highly doubt checks are
> ran against the get_readonly_fields() return value since it's a method
> bound to a ModelAdmin instance and requires a `request` argument to be
> called in the first place.
>
>
My mistake, you're entirely correct. The SystemChecks check the
readonly_fields property, not the result of calling get_readonly_fields().
With the patch to check an instance rather than a class, perhaps I should
extend the checks to check the result of get_readonly_fields() rather than
just obj.readonly_fields. What do you think?
Malcolm
Simon
>
> Le jeudi 10 septembre 2015 06:27:13 UTC-4, Malcolm Box a écrit :
>>
>> Hi Simon,
>>
>> I've tried that, and it still fails the same system check.
>>
>> The system check checks that all the values returned from
>> get_readonly_fields exist as class attributes on the ModelAdmin (or are
>> callable/fields on model, neither of which helps here). With them being
>> created via __getattr__, they don't.
>>
>> I'm coming to the conclusion that the right behaviour is to run the
>> system check against an instance, not the class, since that's what the core
>> admin code uses.
>>
>> Thanks for the offer to review changes - I'll try to put a patch together
>> this week.
>>
>> Cheers,
>>
>> Malcolm
>>
>> On 9 September 2015 at 18:17, Simon Charette <chare...@gmail.com> wrote:
>>
>>> Hi Malcom,
>>>
>>> What I meant to suggest is to remove the fields from
>>> `fields`/`readonly_fields` and dynamically return them in the
>>> `get_(fields|readonly_fields)` fields method.
>>>
>>> e.g.
>>>
>>> class ThumbnailFieldsAdmin(models.ModelAdmin):
>>> fields = []
>>> readonly_fields = []
>>> thumbnail_fields = []
>>>
>>> def __getattr__(self, name):
>>> if name.endswith('_thumbnail'):
>>> return thumbnail_function
>>> raise AttributeError
>>>
>>> def get_fields(request, obj=None):
>>> fields = super(ThumbnailFieldsAdmin, self).get_fields(request,
>>> obj=obj)
>>> return self.thumbnail_fields + fields
>>>
>>> def get_readonly_fields(request, obj=None):
>>> readonly_fields = super(ThumbnailFieldsAdmin, self).get_fields(
>>> request, obj=obj)
>>> return readonly_fields + thumbnail_fields
>>>
>>> But I'm afraid that you'll have to rely on metaclass programming if you
>>> want the order of fields to be maintained somehow.
>>>
>>> > I therefore suspect that the check is actually borked, and it should
>>> be checking hasattr(instance, field_name) rather than hasattr(cls,
>>> field_name)
>>>
>>> The thing is checks are run against ModelAdmin classes and not the
>>> instances bound to the site they were registered to
>>> <https://github.com/django/django/blob/dae81c6ec62a76c1f28745ae3642c2d4a37ce259/django/contrib/admin/sites.py#L106-L110>.
>>>
>>> You could submit a feature request to actually run the test against the
>>> instances but since this is really and edge case you'd have to provide a
>>> patch/PR if you don't want the ticket to rot in Trac make it to Django 1.9
>>> which should enter feature freeze soon enough. I'd be glad to review your
>>> proposed changes if you're interested.
>>>
>>> Simon
>>>
>>>
>>> Le mercredi 9 septembre 2015 10:31:12 UTC-4, Malcolm Box a écrit :
>>>
>>>> Hi Simon,
>>>>
>>>> Thanks for the pointer, but I don't think that helps.
>>>>
>>>> The fields are already declared using the existing fields /
>>>> readonly_fields attributes on the ExampleAdmin class - and this is what
>>>> get_fields / get_readonly_fields return. The system check fails because
>>>> the
>>>> fields declared don't exist on the ExampleAdmin class nor on the model.
>>>> Here's the relevant lines from contrib/admin/checks.py:
>>>>
>>>> def _check_readonly_fields_item(self, cls, model, field_name,
>>>> label):
>>>> if callable(field_name):
>>>> return []
>>>> elif hasattr(cls, field_name):
>>>> return []
>>>> elif hasattr(model, field_name):
>>>> return []
>>>> else:
>>>> try:
>>>> model._meta.get_field(field_name)
>>>> except FieldDoesNotExist:
>>>> return [
>>>> checks.Error(
>>>> "The value of '%s' is not a callable, an
>>>> attribute of '%s', or an attribute of '%s.%s'." % (
>>>> label, cls.__name__, model._meta.app_label,
>>>> model._meta.object_name
>>>> ),
>>>> hint=None,
>>>> obj=cls,
>>>> id='admin.E035',
>>>> )
>>>> ]
>>>> else:
>>>> return []
>>>>
>>>> If the thumbnail fields were defined as methods on the ExampleAdmin,
>>>> all would be fine e.g.:
>>>>
>>>> class ExampleAdmin(models.ModelAdmin):
>>>> fields = ['image', 'image_thumbnail']
>>>>
>>>> def image_thumbnail(self, obj):
>>>> return "<img src="%s"/>" % obj.image.url
>>>>
>>>> That's fine, but if there's lots of image fields (with a variety of
>>>> names) spread over several ModelAdmin classes, then you end up with a lot
>>>> of duplicated code. The normal solution then is to refactor the code. And
>>>> that's where I get stuck - I can't see (short of metaclass programming!)
>>>> how to inject the methods into the class such that the system check
>>>> succeeds.
>>>>
>>>> I therefore suspect that the check is actually borked, and it should be
>>>> checking hasattr(instance, field_name) rather than hasattr(cls,
>>>> field_name)
>>>>
>>>> So I see three possibilities, in order of probability:
>>>>
>>>> 1. I'm being dumb, and there's an easy way to dynamically create
>>>> attributes on a ModelAdmin that passes system checks
>>>> 2. The system check is incorrect, and should allow dynamically
>>>> created attributes on ModelAdmin when validating fields
>>>> 3. Metaclass programming is really the right way to do this
>>>>
>>>>
>>>> Malcolm
>>>>
>>>> On 9 September 2015 at 02:23, Simon Charette <chare...@gmail.com>
>>>> wrote:
>>>>
>>>>> Hi Malcom!
>>>>>
>>>>> I would suggest you have a look at the ModelAdmin.get_fields()
>>>>> <https://docs.djangoproject.com/en/1.8/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_fields>
>>>>>
>>>>> method to append your thumbnail read-only field names.
>>>>>
>>>>> As documented
>>>>> <https://docs.djangoproject.com/en/1.8/ref/contrib/admin/#django.contrib.admin.ModelAdmin.fields>,
>>>>>
>>>>> just make sure you also override ModelAdmin.get_readonly_fields()
>>>>> <https://docs.djangoproject.com/en/1.8/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_readonly_fields>
>>>>>
>>>>> to return them as well.
>>>>>
>>>>> Cheers,
>>>>> Simon
>>>>>
>>>>> Le mardi 8 septembre 2015 13:31:47 UTC-4, Malcolm Box a écrit :
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I'm trying to add a dynamic method to a ModelAdmin so that I can have
>>>>>> automatically generated thumbnail fields for any image by simply adding
>>>>>> '<fieldname>_thumbnail' to the field definitions - which saves a lot of
>>>>>> code when there's a load of admin classes with image fields that need
>>>>>> thumbnails.
>>>>>>
>>>>>> e.g.
>>>>>>
>>>>>> class ThumbnailMixin(object):
>>>>>> def getattr(self, name):
>>>>>> if name.endswith('_thumbnail'):
>>>>>> # ... generate appropriate thumbnail method and return
>>>>>> return thumbnail_function
>>>>>> else:
>>>>>> raise AttributeError
>>>>>>
>>>>>> class ExampleAdmin(models.ModelAdmin, ThumbnailMixin):
>>>>>> fields = ['image_thumbnail', 'image', ...]
>>>>>>
>>>>>> This worked fine in Django 1.6/1.7, but it's now failing in 1.8 with
>>>>>> a SystemCheckError (admin.E035 / admin.E108). If I silence the error,
>>>>>> the
>>>>>> admin actually works fine, but that feels icky.
>>>>>>
>>>>>> The check fails because it checks for the field being on the
>>>>>> ExampleAdmin class, not on an instance - whereas the admin always uses
>>>>>> an
>>>>>> instance, so it works.
>>>>>>
>>>>>> What's the "right" way to do this in the 1.8 world - I've considered
>>>>>> a custom metaclass, but that feels like overkill for a simple task like
>>>>>> this. Should this even work, and if so is it a bug in the check
>>>>>> framework?
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> Malcolm
>>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to a topic in the
>>>>> Google Groups "Django users" group.
>>>>> To unsubscribe from this topic, visit
>>>>> https://groups.google.com/d/topic/django-users/lsDP5oUWOsw/unsubscribe
>>>>> .
>>>>> To unsubscribe from this group and all its topics, send an email to
>>>>> django-users...@googlegroups.com.
>>>>> To post to this group, send email to django...@googlegroups.com.
>>>>> Visit this group at http://groups.google.com/group/django-users.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/django-users/05249551-899b-4a64-b416-216c9f81d950%40googlegroups.com
>>>>>
>>>>> <https://groups.google.com/d/msgid/django-users/05249551-899b-4a64-b416-216c9f81d950%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>> .
>>>>>
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> --
>>> You received this message because you are subscribed to a topic in the
>>> Google Groups "Django users" group.
>>> To unsubscribe from this topic, visit
>>> https://groups.google.com/d/topic/django-users/lsDP5oUWOsw/unsubscribe.
>>> To unsubscribe from this group and all its topics, send an email to
>>> django-users...@googlegroups.com.
>>> To post to this group, send email to django...@googlegroups.com.
>>> Visit this group at http://groups.google.com/group/django-users.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/django-users/5c3d0da9-30bd-484e-9f39-6e011e0f5abc%40googlegroups.com
>>>
>>> <https://groups.google.com/d/msgid/django-users/5c3d0da9-30bd-484e-9f39-6e011e0f5abc%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>>
>>
--
You received this message because you are subscribed to the Google Groups
"Django users" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-users+unsubscr...@googlegroups.com.
To post to this group, send email to django-users@googlegroups.com.
Visit this group at http://groups.google.com/group/django-users.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-users/3d61cf0a-d047-47c4-853c-ef12154fb341%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
