Skip to content

AssertionError: To use StatusField, the model 'MyModel' must have a STATUS choices class attribute. #29

Closed
chronossc opened this Issue Mar 5, 2013 · 8 comments

4 participants

@chronossc

Hello Carl, I found a possible issue, maybe in Django it self, but that we should handle here.

On StatusField we have a contribute_to_class method, that set choices as cls.STATUS.

On Django it is called at https://github.com/django/django/blob/1.4.5/django/db/models/base.py#L217

def add_to_class(cls, name, value):
    if hasattr(value, 'contribute_to_class'):
        value.contribute_to_class(cls, name)
    else:
        setattr(cls, name, value)

This 'add_to_class' is called at https://github.com/django/django/blob/1.4.5/django/db/models/base.py#L98 in this case:

# Add all attributes to the class.
for obj_name, obj in attrs.items():
    new_class.add_to_class(obj_name, obj)

Ok, and your method is at https://github.com/carljm/django-model-utils/blob/master/model_utils/fields.py#L57:

def contribute_to_class(self, cls, name):
    if not cls._meta.abstract and self.check_for_status:
        assert hasattr(cls, 'STATUS'), \
            "To use StatusField, the model '%s' must have a STATUS choices class attribute." \
            % cls.__name__
        self._choices = cls.STATUS
        if not self.has_default():
            self.default = tuple(cls.STATUS)[0][0]  # set first as default
    super(StatusField, self).contribute_to_class(cls, name

Well, the issue is that StatusField.contribute_to_class check for cls.STATUS, but because and logic on Django you can't assert that STATUS exists on new_class because attrs.items() is a dict, not SortedDict or list. You don't have order and should not check for other attributes.

Fortunately we have 'no_check_for_status' argument that you use for South, but aniway we are handling that stuff wrong because 'contribute_to_class' can't expect on this django loop at base.py#L98. May if we change it to use 'class_prepared' signal we do right thing?

@carljm
Owner
carljm commented Mar 7, 2013

Good catch. Yes, I think doing this work in the class_prepared signal would be better (so contribute_to_class just registers the signal handler.)

@chronossc

Today I have found another thing we can do. I saw that StatusField do not register get__display method. Afaik is field that should register? I was wrong?

@carljm
Owner
carljm commented Mar 8, 2013

Hmm, why doesn't it? Is it because choices are set later instead of being available in the field __init__? Yeah, that's probably something worth fixing too.

@chronossc

Just for records, who have this issue need to set all instances of StatusField to have no_check_for_status=True in a migration file, or it will not run.

Carl, maybe you can change REAME to have this tip?

@carljm
Owner
carljm commented Mar 11, 2013

The South introspection rules at the bottom of fields.py should ensure that no_check_for_status is True for generated migrations on any normal StatusField. If you are using the undocumented no_check_for_status argument yourself as a workaround for this issue, then that breaks, but I don't want to document that, I'd rather fix this bug so that nobody needs to use no_check_for_status in that way it's not intended for.

@howieweiner

Hi. I just caught out by this when doing a South migration. For anyone else who has this issue, the simplest solution is to change the field to models.Charfield(choices=... ). StatusField, which is essentially what StatusField creates. South migration works fine.

@treyhunner
Collaborator

i'm trying to add support for Python 3.3 and I believe this bug is causing the tests to fail for me:

AssertionError: To use StatusField, the model 'StatusFieldDefaultFilled' must have a STATUS choice
@carljm
Owner
carljm commented Apr 8, 2013

Yes, this bug is a dict-ordering dependency, so hash randomization in Python 3.3 probably causes it to fail. This should probably just be fixed prior to doing the Python 3 port, it's not that difficult to fix. The South stuff in this thread is all red herrings AFAICT, the needed fix here is simply to do the actual work in a class_prepared signal registered from contribute_to_class, rather than in contribute_to_class itself, so we can be sure the STATUS attribute is available.

@treyhunner treyhunner added a commit to treyhunner/django-model-utils that referenced this issue Apr 9, 2013
@treyhunner treyhunner Fix StatusField and MonitorField bugs
Fixes #29.
3932ae4
@treyhunner treyhunner added a commit that closed this issue Apr 9, 2013
@treyhunner treyhunner Fix StatusField bug
Fixes #29
59e484e
@treyhunner treyhunner closed this in 59e484e Apr 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.