-
Notifications
You must be signed in to change notification settings - Fork 2
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 new classes for PassageUser, CreateUserArgs, and UpdateUserArgs #90
Conversation
|
||
use OpenAPI\Client\Model\CreateUserRequest; | ||
|
||
class CreateUserArgs extends CreateUserRequest |
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.
couldn't get class_alias
to resolve the imports correctly, so just went with creating new classes. These are only used with the new function signatures. All of the old ones still use the same codegen'd classes.
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.
Why is this needed?
use OpenAPI\Client\Model\CreateUserRequest as CreateUserArgs;
wasn't working in the Passage.php file?
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.
class CreateUserArgs extends CreateUserRequest
creates a new class with the name, which is available to be imported/used by any dev external to this file
use OpenAPI\Client\Model\CreateUserRequest as CreateUserArgs;
only allows the CreateUserArgs
name to be used within the file the line is written in
@@ -263,4 +267,15 @@ public function revokeRefreshTokens(string $userId): void | |||
throw PassageError::fromApiException($e); | |||
} | |||
} | |||
|
|||
private function castUserInfoToPassageUser(UserInfo $userInfo): PassageUser |
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.
duplicated just to keep this function private and to not have to create a whole base class to inherit from just for this.
4836636
to
837fae4
Compare
} catch (PassageError $e) { | ||
throw $e->getPrevious(); | ||
} | ||
} | ||
|
||
private function cast(object $object, string $class): mixed |
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.
I'm confused by this. Why is it was expecting one class but receiving another? For instance, why would createUser not expect CreateUserRequest
? This is the class that is genreated.
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.
it's because of the class aliasing. the new functions take a different class name. in the other languages it was as easy as type aliasing but i tried php's class_alias
, which didn't seem to work to allow an end user to use the aliased class names. so i had to create new classes that are just copies of the generated classes with different names. because of that, it was necessary to cast the classes otherwise it threw an error at runtime
use OpenAPI\Client\Model\UserInfo as PassageUser; | ||
use OpenAPI\Client\Model\UpdateUserRequest as UpdateUserArgs; | ||
use OpenAPI\Client\Model\CreateUserRequest as CreateUserArgs; |
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.
This wasn't working in the Passage.php file so you created the workaround?
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.
that was working, but only within the scope of this file. it didn't actually alias the class names for an external dev
What's New?
Screenshots (if appropriate):
Type of change
Checklist:
Additional context