-
Notifications
You must be signed in to change notification settings - Fork 214
feat: Add Self Service Profiles endpoints #795
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
base: main
Are you sure you want to change the base?
Conversation
…iles # Conflicts: # src/API/Management.php # src/Contract/API/ManagementInterface.php
kishore7snehil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Claude finished @kishore7snehil's task —— View job Code Review Progress
Overall Assessment❌ This PR requires minor fixes before merging. I've identified a typo in the test file, an HTTP method case inconsistency, and a redundant null check that should be addressed to maintain code quality and consistency with the rest of the codebase. |
|
|
||
| test('revokeSsoTicket() issues an appropriate request', function(): void { | ||
| $id = uniqid(); | ||
| $prfileId = uniqid(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in variable name: $prfileId should be $profileId to match the parameter name in the method signature and improve code readability.
| ->method('PUT') | ||
| ->addPath(['self-service-profiles', $id, 'custom-text', $language, $page]) | ||
| ->withOptions($options) | ||
| ->withBody($body ?? []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant null coalescing operator: The $body ?? [] is unnecessary since $body has already been filtered and trimmed on line 169. After filtering, $body will be an array (empty array if null was passed). This redundant check adds confusion about the actual state of $body.
| ])->isString(); | ||
|
|
||
| return $this->getHttpClient() | ||
| ->method('PUT') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP method case inconsistency: This method uses 'PUT' in uppercase while all other methods in the class use lowercase ('post', 'get', 'delete', 'patch'). For consistency with the rest of the codebase, this should be 'put' in lowercase.
Changes
Adding the Self Service Profiles API endpoints to the Management API. This has been added in other language versions, but not in the PHP library.
References
Self Service Profiles
Testing
All new endpoints have 100% code coverage.
Contributor Checklist