fix(Profile): Fixed the fetching of a workos profile. #294
Conversation
The `getProfile` method was not properly implemented and tested, wrote a test scenario for that to ensure that it works properly now.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Fixed implementation of getProfile() method in lib/SSO.php to correctly handle profile data retrieval and added comprehensive test coverage.
- Removed incorrect
json()method call on Profile object inlib/SSO.php - Simplified HTTP request handling by using
Client::requestdirectly instead of nested calls - Added new test method in
tests/WorkOS/SSOTest.phpto verify profile data matches expected fixture values - Improved error handling for missing profile data in API responses
2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
tests/WorkOS/SSOTest.php
Outdated
| { | ||
| $path = "sso/profile"; | ||
|
|
||
| $result = $this->profileAndTokenResponseFixture(); |
There was a problem hiding this comment.
logic: Using profileAndTokenResponseFixture() for a profile-only test may hide potential issues. Create a separate profileResponseFixture() that matches the actual API response for /sso/profile
There was a problem hiding this comment.
Thanks, this indeed introduced a bug. Had the proper endpoint now.
lib/SSO.php
Outdated
| 'sso/profile', | ||
| null, | ||
| null, | ||
| true |
There was a problem hiding this comment.
style: Document why the 'true' parameter is being passed to request()
There was a problem hiding this comment.
It's not done in other parts of the codebase, so I don't see why I should do it here. Not sure what the maintainers think?
SanderVerkuil
force-pushed
the
fix/sso-profile-improvements
branch
from
0772e07 to
f7dc162
Compare
SanderVerkuil
force-pushed
the
fix/sso-profile-improvements
branch
from
f7dc162 to
8764bce
Compare
1 participant
The
getProfilemethod was not properly implemented and tested, wrote a test scenario for that to ensure that it works properly now.Description
Fixed an issue because the
jsonmethod does not exist on the Profile class.Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.