Adds support for multiple $in, pointer / relation queries on $or #769
Conversation
| let t = schema.getExpectedType(className, key); | ||
| let match = t ? t.match(/^relation<(.*)>$/) : false; | ||
| if (!match) { | ||
| return; |
There was a problem hiding this comment.
This should produce an inconsistent return type from the function, not sure if that is expected behavior or not. You are returning void or a Promise (later one down the line). Maybe convert to Promise.resolve(), but don't feel strongly.
There was a problem hiding this comment.
it's enclosed into a promise.then() , so that's pretty much the same as doing return Promise.resolve();
There was a problem hiding this comment.
Still an inconsistent return type from a closure(lambda?), but nature of JS wins here.
|
@flovilmart updated the pull request. |
|
@flovilmart updated the pull request. |
flovilmart
changed the title
| } | ||
| return Promise.resolve(); | ||
|
|
||
| return Object.keys(query).reduce((promise, key) => { |
There was a problem hiding this comment.
I'd suggest using Promise.all rather than reduce when possible, you should get slightly better perf that way, and IMO it's more readable.
There was a problem hiding this comment.
I was wondering about race conditions in that case, but that's fine for me
There was a problem hiding this comment.
all can lead to race conditions, but I think in this case it won't.
|
Can you prefer to use promises in tests instead of the backbone style callbacks? We are moving away from the backbone style. No need to spend a lot of time fixing this one, just for next tests. |
|
No problemo, I'll do the Promise.all and fix the tests. |
|
@flovilmart updated the pull request. |
flovilmart
force-pushed
the
flovilmart.fixFailingQueries
branch
from
ded60a6 to
d872f52
Compare
|
@flovilmart updated the pull request. |
Adds support for multiple $in, pointer / relation queries on $or
No description provided.