Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added tests to show errors in django 1.10 #37

Closed
wants to merge 4 commits into from

Conversation

jbzdak
Copy link

@jbzdak jbzdak commented Aug 9, 2016

Adding this PR to show bug in django-enumfield in the newly released Django==1.10.

Problem was (most probably) caused by this changeset in Django repository: django/django@7f51876, and especially by the change to get_deferred_fields.

Let's consider this model:

class Person(models.Model):
    example = models.CharField(max_length=100, default="foo")
    status = EnumField(PersonStatus, default=PersonStatus.ALIVE)

EnumField users Python properties under the hood, that is: Person.status is a property, this causes check used by get_deferred_fields to always treat status as a deffered field (that is always "status" not in self.__dict__). This causes all kinds of fun, for example excludes status from fields that are updated.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 96.399% when pulling d3bb816 on i2biz:add-tests-for-new-django into 4f56bc1 on 5monkeys:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 96.399% when pulling 38af373 on i2biz:add-tests-for-new-django into 4f56bc1 on 5monkeys:master.

@listingmirror
Copy link

Ah this is perhaps why in a brand new model with EnumFIeld... .save() blows up without a force_insert=True....

@jbzdak
Copy link
Author

jbzdak commented Aug 11, 2016

@listingmirror I beileve so --- I also got exception on save() --- in this case cause was as follows:

  • save is called
  • EnumField is marked as deffered
  • save method set update_fields automatically (update_fields = all_fields - deferred_fields)
  • since we have update_fields save assumes that we are updating not inserting
  • We are updating and there is not pk set --- exception.

This might be a bug in Django, but it is triggered by get_deferred_fields.

If you need a working version: here is a version that works: https://github.com/i2biz/django-enumfield/tree/jbzdak/django110 (but please check the tests as some tests needed to be skipped due to changes I had to make).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 96.399% when pulling 28c7dd7 on i2biz:add-tests-for-new-django into 4f56bc1 on 5monkeys:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 96.399% when pulling a55c0af on i2biz:add-tests-for-new-django into 4f56bc1 on 5monkeys:master.

@listingmirror
Copy link

@jbzdak Based on no issues in Django issue tracker around save, and this is the only plugin breaking like this, I am going to make the claim the bug is here not in Django.

With that said, what is the right thing to do? I can spare some cycles to put together a PR.. but also debating just ripping this plugin out and doing something else. I don't personally care about any of the state transition stuff, just want a simple way to store enums without a separate database table to join to.

@ConorMcGee
Copy link

ConorMcGee commented Jan 19, 2017

@listingmirror I wouldn't be so sure - found at least one other project affected: jpwatts/django-positions#49

(I'm with you on feeling like ripping this out though)

kjagiello added a commit to kjagiello/django-enumfield that referenced this pull request Jun 20, 2017
@Swamii Swamii changed the base branch from master to feature/1.4 September 12, 2018 09:27
@Swamii
Copy link
Contributor

Swamii commented Sep 12, 2018

@jbzdak Thanks for your help. This test was included with #45 (previously #43).

@Swamii Swamii closed this Sep 12, 2018
Swamii added a commit to Swamii/django-enumfield that referenced this pull request May 20, 2019
…ure/2.0

* origin/forks/fcurella/enum34:
  Fix for issue mentioned in 5monkeys#37
  Run tests against Django 1.9+
  Define default value explicitly via __default__ attribute
  Support `--failfast` argument in run_tests
  Add changelog section to readme
  subclass IntEnum instead of Enum
  Fix hashing enum
  Test model initialization more precisely for different Django versions
  Use SubfieldBase for Django < 1.8
  Drop Django 1.5 support
  Change labels and transitions attr name; use... enum instead of int in field value; raise validation error when enum is not found by value or name (and support digit str), etc
  Update test config
  Add `.deconstruct()` for Django migrations
  Fix labels by renaming the attribute to `_labels`
  Use enum34 class

# Conflicts:
#	.travis.yml
#	Makefile
#	README.rst
#	django_enumfield/db/fields.py
#	run_tests.py
#	tox.ini
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants