Skip to content

Conversation

@flovilmart
Copy link
Contributor

Adds support for $eq, $ne and $nin when querying on relation keys

@flovilmart flovilmart changed the title Issue/1349 🚧 Issue/1349 Apr 3, 2016
@flovilmart flovilmart changed the title 🚧 Issue/1349 WIP: Make equalTo and notEqual to work on relations Apr 3, 2016
@flovilmart flovilmart force-pushed the issue/1349 branch 2 times, most recently from 8fc2451 to 20a70ea Compare April 4, 2016 00:34
@flovilmart flovilmart changed the title WIP: Make equalTo and notEqual to work on relations Make notEqual work on relations Apr 4, 2016
@codecov-io
Copy link

Current coverage is 93.08%

Merging #1350 into master will increase coverage by +0.01% as of c28253d

@@            master   #1350   diff @@
======================================
  Files           84      84       
  Stmts         5345    5382    +37
  Branches       981     991    +10
  Methods          0       0       
======================================
+ Hit           4975    5010    +35
  Partial          9       9       
- Missed         361     363     +2

Review entire Coverage Diff as of c28253d

Powered by Codecov. Updated on successful CI builds.

@drew-gross
Copy link
Contributor

This looks like it's going to not work if you use mix and match equalTo, notEqualtTo, containedIn, and notContainedIn all in the same query. Have you checked for that?

@flovilmart
Copy link
Contributor Author

@drew-gross yes you're definitely right, I didn't check for that. I'll need to map on all $eq, $ne, $in, $nin

@lolobosse
Copy link

Hey,

@drew-gross yes, it was another consequences of my further tests (that I just finished now (you were quicker)), the server do not like the combinations as well. Do you need some further test cases to fix that quicker?

When do you think it could go live?

@drew-gross
Copy link
Contributor

@lolobosse I think @flovilmart is already working on this, but If you posted a test case similar to the one in this PR, he would probably find that helpful!

@flovilmart
Copy link
Contributor Author

@drew-gross I posted the support, let me know what you think

@drew-gross
Copy link
Contributor

This looks like it should do the trick 💯

@drew-gross drew-gross merged commit 0e3636d into master Apr 4, 2016
@drew-gross drew-gross deleted the issue/1349 branch April 4, 2016 18:05
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.

6 participants