Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Simon Charette]

Hi Malcolm,

> 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?

I think the checks should be run statically against ModelAdmin instances, 
that is only the properties of the instances should be checked and not the 
return value of methods that could depend on request properties such as the 
user.

Simon

Le jeudi 10 septembre 2015 13:32:32 UTC-4, Malcolm Box a écrit :
>
> 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  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 
 .
  
 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 ha

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Malcolm Box]

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  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 
>>> .
>>>  
>>> 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'

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Simon Charette]

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.

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  > 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 
>> .
>>  
>> 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 "" % 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 norm

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Malcolm Box]

Ticket filed: https://code.djangoproject.com/ticket/25374

On Thursday, 10 September 2015 11:27:13 UTC+1, Malcolm Box wrote:
>
> 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 <> 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 
>> .
>>  
>> 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 "" % 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:
>>>
>>>

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Malcolm Box]

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  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
> .
> 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 "" % 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

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Simon Charette]

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 
.
 
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 "" % 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  > wrote:
>
>> Hi Malcom!
>>
>> I would suggest you have a look at the ModelAdmin.get_fields() 
>> 
>>  
>> method to append your thumbnail read-only field names.
>>
>> As documented 
>> ,
>>  
>> just make sure you also override ModelAdmin.get_readonly_fields() 
>> 
>>  
>> to return them as well.
>>
>> Cheers,
>> Simon
>>
>> Le mardi 8 septembre 2015 13:31:47 

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Malcolm Box]

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 "" % 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  wrote:

> Hi Malcom!
>
> I would suggest you have a look at the ModelAdmin.get_fields()
> 
> method to append your thumbnail read-only field names.
>
> As documented
> ,
> just make sure you also override 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
>> '_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+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

Re: Adding dynamic methods to ModelAdmin fails in 1.8

[Simon Charette]

Hi Malcom!

I would suggest you have a look at the ModelAdmin.get_fields() 

 
method to append your thumbnail read-only field names.

As documented 
,
 
just make sure you also override 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 
> '_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 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/05249551-899b-4a64-b416-216c9f81d950%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Adding dynamic methods to ModelAdmin fails in 1.8

[Malcolm Box]

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 
'_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 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/987461ae-f455-44f7-9c61-935a6d412c49%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.