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

Make notEqual work on relations #1350

Merged
merged 3 commits into from
Apr 4, 2016
Merged

Make notEqual work on relations #1350

merged 3 commits into from
Apr 4, 2016

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.

None yet

5 participants