Closed isaacaskew closed 1 year ago
I will reformat the title to use the proper commit message syntax.
@dplewis Is this something you'd want to review?
@dplewis Before we merge this, do you have any idea how the test results are reported here? So we know that they pass before merging. This hasn't migrated to GitHub Actions yet and is still using Travis CI.
MIght have to reenable travis and pay the bill. I'm currenly working on migrating this to Github Action
@mtrezza , @dplewis I forked this repo and tested it against these Parse Server Versions:
3.10.0 - [CircleCI run] [GitHub Changes] - Tests: 598, Assertions: 1334, Errors: 22, Failures: 4, Skipped: 1
5.4.3 - [CircleCI run] [GitHub Changes] - Tests: 598, Assertions: 1292, Errors: 29, Failures: 5, Skipped: 1
6.0.0 - [CircleCI run] [GitHub Changes] - Tests: 598, Assertions: 1272, Errors: 36, Failures: 5, Skipped: 1
There were 5 failures:
1) Parse\Test\ParseCloudTest::testGetJobsData
Failed asserting that 0 matches expected 3.
/Users/isaac/Development/parse-php-sdk/tests/Parse/ParseCloudTest.php:124
2) Parse\Test\ParseSessionFixationTest::testCookieIdChangedForAnonymous
Failed asserting that '' is not equal to "".
/Users/isaac/Development/parse-php-sdk/tests/Parse/ParseSessionFixationTest.php:51
3) Parse\Test\ParseSessionFixationTest::testCookieIdChangedForAnonymousToRegistered
Failed asserting that '' is not equal to "".
/Users/isaac/Development/parse-php-sdk/tests/Parse/ParseSessionFixationTest.php:65
4) Parse\Test\ParseSessionStorageTest::testIsUsingParseSession
Failed asserting that false is true.
/Users/isaac/Development/parse-php-sdk/tests/Parse/ParseSessionStorageTest.php:48
5) Parse\Test\ParseUserTest::testCountUsers
Failed asserting that 1 matches expected 3.
/Users/isaac/Development/parse-php-sdk/tests/Parse/ParseUserTest.php:587
Note that there were only 4 errors when run against Parse 3, and 5 errors in the other two. Let me know if there's anything about the server in particular I need to configure for these other ones to pass - otherwise I can try and help fix these once I understand a bit more of the context around a few of these functions.
In the meantime, Parse 6 running with my tests doesn't introduce any new test failures, so the 3 tests I added pass - we can see here that the pipeline run with my changes has the same number of errors as before, the 5 above. I chose Parse 6 as the server to run the tests against since Parse 6 is the version that introduced the new endpoint that I needed the client to talk to. I'm not in a rush to merge this PR so I'm game to help get the other tests passing and everything running clean against every version before we merge my feature in.
I can target the changes against a new major version of Parse PHP SDK as well - that might make sense since this method wouldn't work for any folks who upgrade their PHP client after this change is released but are using Parse Server versions < 6.
We've released the new SDK version 2.0.0. I've rebased this PR, let's see how tests are doing.
To your point, I think we should add a compatibility table to the README for Parse Server, to indicate which Parse Server versions this SDK is compatible with. For now, this is running against the alpha branch of Parse Server:
With all tests passing, we could assume that it's compatible with all relevant previous Parse Server versions - given that we don't support Parse Server 4 anymore, with Parse Server 5 currently being in LTS.
If you have any additions for loginAs
in https://github.com/parse-community/parse-php-sdk/pull/491 please add them here. Once you are satisfied with this PR let us know and we can go ahead and merge it.
Changelog entry and version bump are not needed as part of this PR, auto-release will do that.
Patch coverage: 100.00
% and no project coverage change.
Comparison is base (
dfe2957
) 98.47% compared to head (3e559b2
) 98.48%.:exclamation: Current head 3e559b2 differs from pull request most recent head b6a6371. Consider uploading reports for the commit b6a6371 to get more accurate results
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
I suppose we can merge this, it seems that #491 contains the same tests as this PR?
@mtrezza I think this is good to merge. You want to try auto release with this one?
Yes, let's give it a try! I'll just copy your fixes from #498 before that, so we should also see the docs auto-generated.
🎉 This change has been released in version 2.1.0
New Pull Request Checklist
Closes: #485
Issue Description
Later versions of Parse Server allow for the
loginAs
method. Given auserId
and the master key, one can become logged in as the user associated to theuserId
provided. This PR adds the PHP Parse SDK method to call the Parse Server endpoint.Approach
To implement this method, I've added a function similar to
login
but modified to only require theuserId
, as the master key is already assumed when using this method. Tests similar to thelogin
tests have been added and modified to cover the cases of a success or a failure with nouserId
provided.