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

fix change tracking for ComplexBaseFields #344

Merged
merged 1 commit into from
Jun 4, 2013

Conversation

paulswartz
Copy link
Contributor

No description provided.

@amcgregor
Copy link
Contributor

What about:

class Foo(Document):
    bar = ListField(StringField(), default=list)

instance = Foo()
instance.bar.append('foo')
instance.save()
instance.bar.append('bar')
del instance.bar[1]

print(instance._changed_fields)

Technically not changed?

@paulswartz
Copy link
Contributor Author

Technically no, but you'd have to store the original value somewhere and compare them, and that seems like a lot of extra work. This just lets the default behavior work, which doesn't mark the value as changed if it's the same as the current value.

@rozza
Copy link
Contributor

rozza commented Jun 3, 2013

@paulswartz thanks for the ticket - out of interest was this causing an issue for you? Marking for 0.8.2

@rozza
Copy link
Contributor

rozza commented Jun 3, 2013

@amcgregor the change tracking is very naive - I have scheduled for 0.9 to improve it - so you can do list operations without replacing the whole list.

@paulswartz
Copy link
Contributor Author

@rozza I have a mixin which does model change tracking, and I noticed that I was getting spurious changes recorded.

@rozza
Copy link
Contributor

rozza commented Jun 3, 2013

@paulswartz sounds interesting anything that should be in mongoengine or mongoengine extras :D 👍

@paulswartz
Copy link
Contributor Author

@rozza it's also pretty naive, but I'll make a note to look at breaking it out.

rozza added a commit that referenced this pull request Jun 4, 2013
Remove custom change tracking for ComplexBaseFields just use BaseField's one
@rozza rozza merged commit 3b60adc into MongoEngine:master Jun 4, 2013
@rozza
Copy link
Contributor

rozza commented Jun 4, 2013

Thanks @paulswartz for the code 👍 good spot.

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.

3 participants