test: Add tests for isGet parameter in Cloud Code trigger beforeFind

[mtrezza]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Closes: #8736

There are 3 ways of getting an object, in all cases the isGet in the beforeFind trigger should be correct; correct here means unchanged compared to behavior in version 6.2.1, to ensure release 6.2.2 did not break that behavior.

Behavior in 6.2.1:

• Parse.Query.get --> isGet == false
• Parse.Query.find --> isGet == false
• Parse.Object.fetch --> isGet == true

Approach

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)

[parse-github-assistant]

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

[parse-github-assistant]

Thanks for opening this pull request!

[Moumouls]

@mtrezza i can see that we have a isGet test here

The issue only occur on parse object get ?

[Moumouls]

I checked, there is one failing test about the "get" but actually it seems that parse.get use the find endpoint.
We can detect that the payload have only an objectId and limit 1 to flag as a get. But i'm not sure if it's a good idea @mtrezza

Also i checked quickly but it seems that the issue was there before the patch.

Also i think it's a parse js sdk issue because, for example i use the graphql api and we didn't catch any issue in my company after the patch on hooks using isGet.

[mtrezza]

I've updated the description; rather then speculating what the behavior should be, I checked what the behavior was in 6.2.1 by running the same tests on a branch of the 6.2.1 tag, documented in #8738 (comment). That is the benchmark for the tests.

[mtrezza]

We can see that all 3 new tests pass in 6.2.2, so there is no indication of a change in behavior compared to 6.2.1.

@m12331 if there are no further scenarios to test, I'll go ahead and merge the additional tests in this PR and close #8736.

[Moumouls]

There is a failing test @mtrezza on get, but it's a Parse SDK issue, maybe all SDKs are impacted

[mtrezza]

There are currently no failing tests. All 3 tests pass, see #8738 (comment).

[m12331]

We can detect that the payload have only an objectId and limit 1 to flag as a get. But i'm not sure if it's a good idea @mtrezza

I think previously also arrays of ids would not trigger beforeFind. It would be good to somehow be able to distinguish the these two cases from “regular”/previous beforeFind

[m12331]

I think there is a distinction to be made here between Get (include id or array of id) and Find permissions. I think in cases where Get permissions was utilised in beforeFind in versions before 6.2.2 that req.isGet was true

[mtrezza]

I think previously also arrays of ids would not trigger beforeFind. It would be good to somehow be able to distinguish the these two cases from “regular”/previous beforeFind

In this PR and related issue we focus solely on whether there was change in behavior. If you have a specific test case you would like to add, to compare the behavior 6.2.1 and 6.2.2 please provide a code example and I'll be happy to run it on 6.2.1 and 6.2.2 so we can see whether there was a difference.

I think there is a distinction to be made here between Get (include id or array of id) and Find permissions

Also here, if there is a change in behavior, please provide a code example for a test, so we can compare the behavior.

[m12331]

Looks like you're right and I was off here. Thanks for the clarification

[mtrezza]

Thanks for reporting back, I'll go ahead and merge this PR.

[codecov]

Codecov Report

Patch coverage: 97.70% and project coverage change: -0.02% ⚠️

Comparison is base (2b3d4e5) 94.33% compared to head (b6f8504) 94.32%.
Report is 2 commits behind head on alpha.

❗ Current head b6f8504 differs from pull request most recent head cbc2463. Consider uploading reports for the commit cbc2463 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #8738      +/-   ##
==========================================
- Coverage   94.33%   94.32%   -0.02%     
==========================================
  Files         185      186       +1     
  Lines       14766    14773       +7     
==========================================
+ Hits        13930    13934       +4     
- Misses        836      839       +3     

Files Changed	Coverage Δ
src/rest.js	98.63% <95.45%> (-0.24%)	⬇️
src/RestQuery.js	95.70% <97.14%> (-0.14%)	⬇️
src/Auth.js	98.86% <100.00%> (-0.76%)	⬇️
src/Controllers/PushController.js	97.87% <100.00%> (ø)
src/Controllers/UserController.js	95.91% <100.00%> (ø)
src/RestWrite.js	94.88% <100.00%> (-0.14%)	⬇️
src/SharedRest.js	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

[parseplatformorg]

🎉 This change has been released in version 6.3.0-alpha.9

[parseplatformorg]

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

[parseplatformorg]

🎉 This change has been released in version 6.4.0-alpha.1

[parseplatformorg]

🎉 This change has been released in version 6.4.0