Issue 305

[b-gran]

Create independent APIs for

• Retrieving, creating, modifying, and deleting documents (what's currently in operations.js)
• Generating URIs from a model

[Zertz]

This is a smart change! Just make this ternary a single line, for consistency.

[Zertz]

We should probably just throw here rather than use assert as a side effect

[Zertz]

Also, _.isString instead of typeof

[Zertz]

That's a heroic PR!

The one failing test is here, looks like something might have changed in the error response. Edit: We're getting failures on master as well, I'm guessing something changed yet again in how mongoose handles errors. Feel free to have a look at it, but it can be fixed separately too.

Great work, I added a few comments here and there, but looks good overall 👍

[b-gran]

Thanks! Locally, I'm working on updates to the operations API that so that each op returns a Promise -- it's still failing a couple tests though, but I should be able to sort it out in the morning.

By the way, great work on this library!

[Zertz]

I'm keeping an eye out on this, let me know if you have any questions!

[coveralls]

Coverage increased (+0.4%) to 99.468% when pulling 144fa51 on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[b-gran]

Just realized the tests from #317 didn't make it in -- just added them.

[coveralls]

Coverage increased (+0.4%) to 99.468% when pulling 55d0599 on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[coveralls]

Coverage increased (+0.4%) to 99.465% when pulling 49a4345 on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[b-gran]

Hey @Zertz, what do you think about moving to ES6 modules? The tooling supports it, but it means modifying the tests (which have nested require() calls, so can't generally be converted) like this:

const restify = require('../express-restify-mongoose').default

The changes are in a separate branch here if you want to see how it looks. That branch passes tests btw.

[coveralls]

Coverage increased (+0.4%) to 99.459% when pulling cb0819b on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[coveralls]

Coverage increased (+0.4%) to 99.459% when pulling cb0819b on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[coveralls]

Coverage increased (+0.4%) to 99.465% when pulling 9b68b45 on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[b-gran]

So I looked at the test that's failing and in 4.8.0 mongoose added support for selecting fields by array (the way the test attempts).

Should I update the test to document that new feature?

BTW here's the relevant test

Edit: it was actually an mquery update that added support for select()ing arrays and just got merged into mongoose:
mongoosejs/mquery@063d389

[coveralls]

Coverage increased (+0.4%) to 99.465% when pulling b180b4d on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[coveralls]

Coverage increased (+0.4%) to 99.465% when pulling 50e9d5d on b-gran:issue-305 into e8e4063 on florianholzapfel:master.

[OmgImAlexis]

@b-gran is this still being worked on?

[Zertz]

I love the idea here but it seems like work has stalled and quite a few merge conflicts have risen and the scope of changes is quite large. Closing for now, let me know if there is interest in picking up the work.