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

Matching queries with doesNotExist constraint #1250

Conversation

andrecardoso
Copy link
Contributor

I was trying LiveQuery using a query with a doesNotExist constraint and after some time debugging I've found out that QueryTools wasn't matching it correctly.

This pull request resolves this issue.

Feedback is appreciated.

Thanks.

@codecov-io
Copy link

Current coverage is 93.04%

Merging #1250 into master will increase coverage by +0.02% as of da4cbd7

@@            master   #1250   diff @@
======================================
  Files           83      83       
  Stmts         5287    5289     +2
  Branches       965     965       
  Methods          0       0       
======================================
+ Hit           4918    4921     +3
  Partial         10      10       
+ Missed         359     358     -1

Review entire Coverage Diff as of da4cbd7

Powered by Codecov. Updated on successful CI builds.

@@ -206,7 +206,9 @@ function matchesKeyConstraints(object, key, constraints) {
}
break;
case '$exists':
if (typeof object[key] === 'undefined') {
let propertyExists = typeof object[key] !== 'undefined';
let existenceIsRequired = constraints['$exists'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that true / false or a value or can be undefined/null??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my tests and understanding it will always be a boolean, because this query representation is done by the SDKs. It could be something different when the query is done via REST but this is not the case for LiveQuery right?

Anyway the logic would still work if constraints['$exists'] is null/undefined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is more, do we want it to work when it's null / undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum.. I see. Should I enclose it under an if (typeof constraints['$exists'] === "boolean")?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure :) that's the problem :P cc: @wangmengyan95

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want true/false only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrecardoso if you can address that concern we'll be able to merge that :)

@flovilmart
Copy link
Contributor

@andrecardoso just a little question but otherwise looks good

@facebook-github-bot
Copy link

@andrecardoso updated the pull request.

@drew-gross
Copy link
Contributor

I'm going to merge this and submit a separate PR that adds the typeof constraint.

@drew-gross drew-gross merged commit 3f8a1aa into parse-community:master Apr 1, 2016
@andrecardoso
Copy link
Contributor Author

Thanks guys!

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.

5 participants