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

FindAndModify + upsert #939

Closed
wants to merge 8 commits into from

Conversation

Globegitter
Copy link
Member

I know there is the current discussion going on about findOrCreateEach (
https://github.com/balderdashy/waterline/issues/931) so it might not have been the wisest choice to add a findAndModifyEach but I needed a findAndModify functionality (see http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/) and it seemed like a good solution to put it such a way that other people can use it as well.

Usage is Model.findAndModify(search critieras, values, [options, cb]);

where-as default options look like that:

{
  upsert: false, // on true automatically inserts the values if nothing is found
  new: true, // if false returns the model before it was updated/created
  mergeArrays: false // quite a special case that I needed and I am still testing in my app. When any of the values given contain an array it is merged to the found object rather than just replaced.
}

I added passing unit tests as well as adapter-tests, see balderdashy/waterline-adapter-tests#47
There is something strange going on, even though the findAndModify passes the unit tests and works in my app (via npm i git+https://github.com/Globegitter/waterline.git#findAndModify) the adapter-tests do not pass and throw the same error throughout:

9) Semantic Interface .findAndModfiy() should return a model instance:
     TypeError: undefined is not a function
      at Context.<anonymous> (Git/waterline/node_modules/waterline-adapter-tests/interfaces/semantic/findAndModify.test.js:73:20)
      at Test.Runnable.run (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runnable.js:194:15)
      at Runner.runTest (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:355:10)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:401:12
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:281:14)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:290:7
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:234:23)
      at Immediate._onImmediate (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:258:5)
      at processImmediate [as _immediateCallback] (timers.js:361:17)

I have no idea why this happens or how to fix this. So apart from if this actually gets merged, anyone got any idea of why that could be?

Also it does not work completely in my app via sails-postgresql, I seem to be getting this error. I am not sure if that is related to my code or to sails-postgresql.

db_1     | ERROR:  syntax error at or near "LIMIT" at character 140
db_1     | STATEMENT:  UPDATE "subject" AS "subject" SET "researchIds" = $1, "text" = $2, "createdAt" = $3, "updatedAt" = $4  WHERE LOWER("subject"."text") = $5  LIMIT 1 RETURNING *

@dmarcelino
Copy link
Member

@Globegitter, I think the chances of this being merged to master are slim, you will probably get a more favourable audience if you PR this against 0.11 branch (still depending on the outcome of balderdashy/waterline#931). The idea is that 0.10.x is being used to fix bugs and new features are to be added to 0.11.

@Globegitter
Copy link
Member Author

Ok will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants