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

Straighten removing strategy from collection field #1177

Merged
merged 1 commit into from
Jul 24, 2015

Conversation

malarzm
Copy link
Member

@malarzm malarzm commented Jul 23, 2015

Fixes #1174. This commit finishes removing "unused" strategy from collection field. People who are still using strategy combined with collection and annotation mapping will now start getting error because AbstractField no longer has that property (which was the reason why our tests were green but YML/XML mappings suffered as reported in #1174)

@redthor could you maybe run your test suite against this PR?

@malarzm malarzm added the Bug label Jul 23, 2015
@malarzm malarzm added this to the 1.0.0 milestone Jul 23, 2015
@redthor
Copy link

redthor commented Jul 24, 2015

The test still fails but neither driver contains strategy:

1) Doctrine\ODM\MongoDB\Tests\Functional\Ticket\GH1174Test::compareDrivers
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     'isInverseSide' => false
+    'options' => Array ()
+    'value' => null
 )

I then checked out your branch in our dev code and it appears to be all working.

So my test case is perhaps not something we need?

Thanks for looking into this.

@malarzm
Copy link
Member Author

malarzm commented Jul 24, 2015

So my test case is perhaps not something we need?

I added this assert: https://github.com/doctrine/mongodb-odm/pull/1177/files#diff-6a4e49bb800973ef639a55d44888ff43R50 I'd say the rest is out of scope of this PR. But anyway thank you very much for that test, I would spend a lot more time looking for the culprit if not for your test :)

@jmikola
Copy link
Member

jmikola commented Jul 24, 2015

LGTM. I don't have any recollection of strategy being used for the collection field type. Both it and hash are treated like basic fields where any changes are persisted with $set. The collection strategies only apply when dealing with EmbedMany and ReferenceMany relationships.

malarzm added a commit that referenced this pull request Jul 24, 2015
Straighten removing strategy from collection field
@malarzm malarzm merged commit e8aceaf into doctrine:master Jul 24, 2015
@malarzm malarzm deleted the gh1174 branch July 24, 2015 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants