Skip to content

Conversation

@dplewis
Copy link
Member

@dplewis dplewis commented Jan 25, 2023

New Pull Request Checklist

Issue Description

Similar to parse-community/parse-server#8128

Closes: Added additional test case for parse-community/parse-server#8335

Approach

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title fix: localDatastore query.containedIn not working when field is an array fix: LocalDatastore query.containedIn not working when field is an array Jan 25, 2023
@parse-github-assistant
Copy link

parse-github-assistant bot commented Jan 25, 2023

Thanks for opening this pull request!

  • ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Base: 99.89% // Head: 99.89% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (7bce5fc) compared to base (e9a2cc9).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #1666   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files          61       61           
  Lines        5973     5977    +4     
  Branches     1367     1369    +2     
=======================================
+ Hits         5967     5971    +4     
  Misses          6        6           
Impacted Files Coverage Δ
src/OfflineQuery.js 99.68% <100.00%> (+<0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dplewis dplewis requested a review from a team January 25, 2023 04:17
@mtrezza mtrezza changed the title fix: LocalDatastore query.containedIn not working when field is an array fix: LocalDatastore Parse.Query.containedIn not working when field is an array Jan 25, 2023
@mtrezza mtrezza changed the title fix: LocalDatastore Parse.Query.containedIn not working when field is an array fix: Local datastore Parse.Query.containedIn not working when field is an array Jan 25, 2023
@mtrezza
Copy link
Member

mtrezza commented Jan 25, 2023

Thanks for the PR; could you rephrase the PR title to make it easier to understand in the changelog what this actually fixes?

The 2 references are related to LiveQuery - is this also LiveQuery related?

@dplewis
Copy link
Member Author

dplewis commented Jan 25, 2023

@mtrezza The LocalDatastore is internally code copied from LiveQuery meaning queries that LiveQuery supports the LocalDatastore should support too. Those are just additional test cases that doesn't exist server side.

@mtrezza
Copy link
Member

mtrezza commented Jan 25, 2023

How would you best describe the fix in src/OfflineQuery.js?

Local datastore not working properly for Parse.Query.containedIn where field is an array

Or something more specific?

@dplewis
Copy link
Member Author

dplewis commented Jan 25, 2023

How would you best describe the fix in src/OfflineQuery.js?

It's the same fix as the QueryTools.js in the latest 5.4.0 release of the server

https://github.com/parse-community/parse-server/pull/8128/files#diff-13c52f9da4d37c7dc0bd3e9e845716de38a13e2a8632465ea167422762d5a9b6

@dplewis
Copy link
Member Author

dplewis commented Jan 26, 2023

Or something more specific?

For non array fields you are comparing a single value against an array. This fix checks a array against an array recursively. As I write this I realized that nonContainedIn is broken and will return always return an object.

Without this fix we would have to rely on.

haystack.indexOf(needle) > -1

Array.indexOf(Array) will always return -1

@mtrezza
Copy link
Member

mtrezza commented Jan 26, 2023

The other PR is called "fix: liveQuery with containedIn not working when object field is an array". So how about this:

fix: Local datastore query with containedIn not working when field is an array

What do you conclude from the nonContainedIn? I haven't looked into the logic so far...

@mtrezza mtrezza changed the title fix: Local datastore Parse.Query.containedIn not working when field is an array fix: Local datastore query with containedIn not working when field is an array Jan 26, 2023
@dplewis
Copy link
Member Author

dplewis commented Jan 26, 2023

@mtrezza I write a few additional test cases, I also think there might be an issue with containedBy queries.

@dplewis dplewis requested a review from a team January 26, 2023 22:21
@dplewis
Copy link
Member Author

dplewis commented Jan 26, 2023

@mtrezza This is ready for review

@mtrezza
Copy link
Member

mtrezza commented Jan 26, 2023

Re-running CI

I'm curious about these fails: https://github.com/parse-community/Parse-SDK-JS/actions/runs/4019619971/jobs/6906679057; it seems a connection error, do you have any idea what that could be since you've been looking at the flaky tests?

@dplewis
Copy link
Member Author

dplewis commented Jan 27, 2023

it seems a connection error, do you have any idea what that could be since you've been looking at the flaky tests?

@mtrezza I suspect it might be an issue with reconfigureServer. allowCustomObjectId tests seem most common but I have seen eventuallyQueue tests also fail with his error. You can see where the first occurrence of the error is thrown.

@mtrezza mtrezza merged commit 2391bff into parse-community:alpha Jan 27, 2023
parseplatformorg pushed a commit that referenced this pull request Jan 27, 2023
# [4.0.0-alpha.6](4.0.0-alpha.5...4.0.0-alpha.6) (2023-01-27)

### Bug Fixes

* Local datastore query with `containedIn` not working when field is an array ([#1666](#1666)) ([2391bff](2391bff))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0-alpha.6

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jan 27, 2023
parseplatformorg pushed a commit that referenced this pull request Jan 31, 2023
## [4.0.1-beta.1](4.0.0...4.0.1-beta.1) (2023-01-31)

### Bug Fixes

* Local datastore query with `containedIn` not working when field is an array ([#1666](#1666)) ([2391bff](2391bff))
* Request execution time keeps increasing over time when using `Parse.Object.extend` ([#1682](#1682)) ([f555c43](f555c43))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.1-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jan 31, 2023
parseplatformorg pushed a commit that referenced this pull request Jan 31, 2023
## [4.0.1](4.0.0...4.0.1) (2023-01-31)

### Bug Fixes

* Local datastore query with `containedIn` not working when field is an array ([#1666](#1666)) ([2391bff](2391bff))
* Request execution time keeps increasing over time when using `Parse.Object.extend` ([#1682](#1682)) ([f555c43](f555c43))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.1

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants