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

Fixed ListField deletion bug. #1318 #1435

Merged
merged 5 commits into from
Dec 22, 2016
Merged

Fixed ListField deletion bug. #1318 #1435

merged 5 commits into from
Dec 22, 2016

Conversation

JohnAD
Copy link
Contributor

@JohnAD JohnAD commented Dec 12, 2016

Fixes #1318

The BaseList supported the delitem operator in a manner
similar to DictList, however the key to a list is an index rather
than unique key. MongoDB does not support pulling from an array
by index; therefore the entire list must be marked as changed.
MongoDB reference: https://jira.mongodb.org/browse/SERVER-1014

@wojcikstefan
Copy link
Member

Thanks for this @JohnAD! Could you add a unit test and confirm it fails w/o your patch?

@wojcikstefan
Copy link
Member

@JohnAD I think you need to rebase against the latest master.

@JohnAD
Copy link
Contributor Author

JohnAD commented Dec 15, 2016

I will rebase. In fact, if you are okay with it, I would like to add comprehensive tests for all relevant list operators (other than those being tested already). I'll probably label the functions test_list_field_manipulative_operators and test_list_field_lexographic_operators. It will delay things only by a day or so.

@wojcikstefan
Copy link
Member

wojcikstefan commented Dec 15, 2016 via email

@JohnAD
Copy link
Contributor Author

JohnAD commented Dec 17, 2016

Should be ready.

@wojcikstefan
Copy link
Member

Thank you @JohnAD. The diff still shows code changes unrelated to this PR. Can you rebase against the upstream master?

@JohnAD
Copy link
Contributor Author

JohnAD commented Dec 19, 2016

That is odd. Did another rebase from master of this repo. Is it clean now?

@wojcikstefan
Copy link
Member

Yup, still an issue. Do you see the same commit history I'm seeing?

screen shot 2016-12-19 at 5 57 10 pm

Try this:

  1. git remote add upstream [email protected]:MongoEngine/mongoengine.git
  2. git fetch upstream
  3. git rebase upstream/master
  4. Manage any conflicts that arise.

JohnAD and others added 5 commits December 19, 2016 22:50
The BaseList supported the __delitem__ operator in a manner
similar to DictList, however the key to a list is an index rather
than unique key. MongoDB does not support pulling from an array
by index; therefore the entire list must be marked as changed.
MongoDB reference: https://jira.mongodb.org/browse/SERVER-1014
@JohnAD
Copy link
Contributor Author

JohnAD commented Dec 20, 2016

That is, IIRC, how I did the rebase with upstream. This time, I actually encountered circular merge conflicts on the very changes that I made. Odd. This time I did a git push origin --force when finished to see if I can clean up the head.

@wojcikstefan
Copy link
Member

This is great! Thank you for a comprehensive set of tests and for all the back-and-forths @JohnAD !

@wojcikstefan wojcikstefan merged commit b8454c7 into MongoEngine:master Dec 22, 2016
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.

2 participants