feat(generated): Authorization (batch c64ce1e7) - #447
Conversation
|
| /** The ID of the group role assignment the role was derived from, or null if direct. */ | ||
| public ?string $groupRoleAssignmentId, | ||
| /** The group the role was derived from, or null if direct. */ | ||
| public ?UserRoleAssignmentSourceGroup $group, |
There was a problem hiding this comment.
Required group argument breaks callers
If a consumer constructs UserRoleAssignmentSource with the previously valid two arguments, the new $group parameter is required even when there is no group. Upgrading will cause that call to throw an ArgumentCountError. Defaulting the nullable parameter to null would preserve those calls.
| public ?UserRoleAssignmentSourceGroup $group, | |
| public ?UserRoleAssignmentSourceGroup $group = null, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/Resource/UserRoleAssignmentSource.php
Line: 20
Comment:
**Required group argument breaks callers**
If a consumer constructs `UserRoleAssignmentSource` with the previously valid two arguments, the new `$group` parameter is required even when there is no group. Upgrading will cause that call to throw an `ArgumentCountError`. Defaulting the nullable parameter to `null` would preserve those calls.
```suggestion
public ?UserRoleAssignmentSourceGroup $group = null,
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| "type": "direct", | ||
| "group_role_assignment_id": null | ||
| "group_role_assignment_id": null, | ||
| "group": null |
There was a problem hiding this comment.
Group assignments lack test coverage
The assignment fixtures contain only "group": null, and no test uses the new fixture with a non-null group. Add a group-derived assignment case that checks decoding and serialization through UserRoleAssignmentSource; otherwise a regression in the new nested-group path could pass the test suite.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/Fixtures/list_user_role_assignment.json
Line: 18
Comment:
**Group assignments lack test coverage**
The assignment fixtures contain only `"group": null`, and no test uses the new fixture with a non-null group. Add a group-derived assignment case that checks decoding and serialization through `UserRoleAssignmentSource`; otherwise a regression in the new nested-group path could pass the test suite.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Regenerated SDK from spec changes.
Triggered by workos/openapi-spec@e2873c2