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

feat: Add comment to MongoDB query via Parse.Query.comment #2088

Merged
merged 10 commits into from
Mar 11, 2024

Conversation

Meglali20
Copy link
Contributor

@Meglali20 Meglali20 commented Feb 19, 2024

Copy link

parse-github-assistant bot commented Feb 19, 2024

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.

@Meglali20 Meglali20 changed the title Initial commit to add comment to a query (for mongodb profiler) Add comment to a query Feb 19, 2024
@mtrezza
Copy link
Member

mtrezza commented Mar 3, 2024

CI fails with:

  1. Parse Query can query and send a comment
  • Error: Invalid parameter for query: comment

I believe the tests are not running with the latest alpha version of Parse Server because the package-lock file usually specifies a commit hash so it's locked to a specific version. Running npm i -DE parse-server@7.0.0-alpha.23 should fix that. However, there's #1979 which we should fix in the coming days, so some tests may fail that are unrelated to this PR.

@Meglali20
Copy link
Contributor Author

Yes, I was expecting this error. for me, all tests passed when I used parse server as local package after making changes locally related to parse-community/parse-server#8928

@mtrezza
Copy link
Member

mtrezza commented Mar 5, 2024

Could you upgrade Parse Server like I've mentioned?

@mtrezza mtrezza changed the title Add comment to a query feat: Add comment to MongoDB query via Parse.Query.comment Mar 9, 2024
@mtrezza
Copy link
Member

mtrezza commented Mar 9, 2024

There has been some issue with the package-lock file; I've reverted it and ran npm i -DE parse-server@7.0.0-alpha.25.

@mtrezza
Copy link
Member

mtrezza commented Mar 9, 2024

I believe we need to upgrade the Parse JS SDK to support only Node 18 and 20, the same node versions as Parse Server 7. In a separate PR, before we can merge this PR. #2063

@mtrezza
Copy link
Member

mtrezza commented Mar 9, 2024

Tests are failing because the Parse JS SDK test suite needs to be updated for Parse Server 7. For example, allowClientClassCreation needs to be set to true. #1979

@Meglali20
Copy link
Contributor Author

When running tests locally most of them pass, did you figure out the issue for this?

@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2024

Did you download the latest commit of this branch? I would be surprised if it passed locally. They likely pass when testing with an older version of Parse Server, but not with the latest alpha version as in this branch.

@Meglali20
Copy link
Contributor Author

You're correct tests are failing, didn't notice you changed to alpha 25 I had to run npm i -DE parse-server@7.0.0-alpha.25 after that tests started failing.

@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2024

Yes, I recommend to pull the latest commit in this PR to your local clone, so that you use the same code, delete your current node_modules directory, then run just npm i. Otherwise you may see different errors.

Copy link

codecov bot commented Mar 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (86600bc) to head (bc08c7f).
Report is 1 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff            @@
##             alpha     #2088   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           61        61           
  Lines         6173      6186   +13     
  Branches      1494      1498    +4     
=========================================
+ Hits          6173      6186   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2024

Tests are passing now; could you check the codecov? It seems that the new tests are not testing all lines of the new feature. After that it should be good to merge.

src/ParseQuery.js Outdated Show resolved Hide resolved
src/ParseQuery.js Outdated Show resolved Hide resolved
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! Codecov is fixed and all tests pass, thanks for the PR!

@mtrezza mtrezza merged commit a970913 into parse-community:alpha Mar 11, 2024
9 checks passed
parseplatformorg pushed a commit that referenced this pull request Mar 11, 2024
# [5.0.0-alpha.3](5.0.0-alpha.2...5.0.0-alpha.3) (2024-03-11)

### Features

* Add comment to MongoDB query via `Parse.Query.comment` ([#2088](#2088)) ([a970913](a970913))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-alpha.3

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Mar 11, 2024
parseplatformorg pushed a commit that referenced this pull request Mar 17, 2024
# [5.0.0-beta.1](4.3.1...5.0.0-beta.1) (2024-03-17)

### Bug Fixes

* Calling `Parse.Object.relation.add` multiple times adds only the first object ([#2078](#2078)) ([0f98117](0f98117))

### Features

* Add comment to MongoDB query via `Parse.Query.comment` ([#2088](#2088)) ([a970913](a970913))
* Add compatibility with Parse Server 7 ([#2089](#2089)) ([86600bc](86600bc))
* Add support for Node 20, remove support for Node 14 and 16 ([#2063](#2063)) ([74eb4d5](74eb4d5))

### BREAKING CHANGES

* Parse JS SDK 5 requires Parse Server 7 and is incompatible with Parse Server 6. ([86600bc](86600bc))
* Removes support for Node 14 and 16. ([74eb4d5](74eb4d5))
@parseplatformorg
Copy link
Contributor

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

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Mar 17, 2024
parseplatformorg pushed a commit that referenced this pull request Mar 17, 2024
# [5.0.0](4.3.1...5.0.0) (2024-03-17)

### Bug Fixes

* Calling `Parse.Object.relation.add` multiple times adds only the first object ([#2078](#2078)) ([0f98117](0f98117))

### Features

* Add comment to MongoDB query via `Parse.Query.comment` ([#2088](#2088)) ([a970913](a970913))
* Add compatibility with Parse Server 7 ([#2089](#2089)) ([86600bc](86600bc))
* Add support for Node 20, remove support for Node 14 and 16 ([#2063](#2063)) ([74eb4d5](74eb4d5))

### BREAKING CHANGES

* Parse JS SDK 5 requires Parse Server 7 and is incompatible with Parse Server 6. ([86600bc](86600bc))
* Removes support for Node 14 and 16. ([74eb4d5](74eb4d5))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 17, 2024
mtrezza added a commit to mtrezza/Parse-SDK-JS that referenced this pull request Mar 23, 2024
…rification

* alpha:
  chore(release): 5.0.0-beta.1 [skip ci]
  chore(release): 5.0.0-alpha.3 [skip ci]
  feat: Add comment to MongoDB query via `Parse.Query.comment` (parse-community#2088)
  chore(release): 5.0.0-alpha.2 [skip ci]
  feat: Add compatibility with Parse Server 7 (parse-community#2089)
  chore(release): 5.0.0-alpha.1 [skip ci]
  feat: Add support for Node 20, remove support for Node 14 and 16 (parse-community#2063)
  refactor: Upgrade ws from 8.15.1 to 8.16.0 (parse-community#2087)
  ci: Remove manual caching for `actions/setup-node` (parse-community#2064)
  chore(release): 4.3.1-alpha.2 [skip ci]
  fix: Calling `Parse.Object.relation.add` multiple times adds only the first object (parse-community#2078)
  refactor: Upgrade ws from 8.15.0 to 8.15.1 (parse-community#2074)
  chore(release): 4.3.1 [skip ci]
  release
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