Opened 5 years ago

Last modified 5 years ago

#30543 closed Bug

admin.E108 is raised on fields accessible only via instance. — at Initial Version

Reported by: ajcsimons Owned by: nobody
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

#28490 caused a regression in 47016adbf54b54143d4cf052eeb29fc72d27e6b1, i.e.

  1. if hasattr(obj.model, item) returns false then we go straight to the else clause which returns the error,
  2. whereas before the else clause did another check for model._meta.get_field(item) and would only return the error if that raised a FieldDoesNotExist exception
  3. So how is it that hasattr(model, item) can return false, but model._meta.get_field(item) will return something meaning the check should not return an error?
  4. Answer: the field is a special one which is only valid to access on instances of the model (whose ModelAdmin we are verifying) and not the model class itself. An example of this is the PositionField from the django-positions library (which inherits from djangos models.IntegerField):
        def __get__(self, instance, owner):
            if instance is None:
                raise AttributeError("%s must be accessed via instance." % self.name)
            current, updated = getattr(instance, self.get_cache_name())
            return current if updated is None else updated
    
  5. So the simplification of the hasattr branch was valid, but the removal of the else branch to no longer check get_field doesn't throw before returning an error was not a refactor but a functionality altering change which makes this method return errors in cases where it used not to.

Change History (1)

by Mariusz Felisiak, 5 years ago

Attachment: 30543.diff added

fix

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