fix: CreateAuthorizationResourceOptions type definition#1530
fix: CreateAuthorizationResourceOptions type definition#1530nicknisi merged 2 commits intoworkos:mainfrom
Conversation
Greptile SummaryThis PR fixes a type-level bug where
Confidence Score: 5/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class BaseCreateAuthorizationResourceOptions {
+string externalId
+string name
+string|null description?
+string resourceTypeSlug
+string organizationId
}
class CreateOptionsWithParentResourceId {
+string parentResourceId
}
class CreateOptionsWithParentExternalId {
+string parentResourceExternalId
+string parentResourceTypeSlug
}
BaseCreateAuthorizationResourceOptions <|-- CreateOptionsWithParentResourceId
BaseCreateAuthorizationResourceOptions <|-- CreateOptionsWithParentExternalId
class CreateAuthorizationResourceOptions {
<<union type>>
BaseCreateAuthorizationResourceOptions
CreateOptionsWithParentResourceId
CreateOptionsWithParentExternalId
}
note for CreateAuthorizationResourceOptions "Before fix: only the two child types.\nAfter fix: base type added, allowing no-parent creation."
Reviews (1): Last reviewed commit: "fix: CreateAuthorizationResourceOptions ..." | Re-trigger Greptile |
| export type CreateAuthorizationResourceOptions = | ||
| | BaseCreateAuthorizationResourceOptions | ||
| | CreateOptionsWithParentResourceId | ||
| | CreateOptionsWithParentExternalId; |
There was a problem hiding this comment.
Missing test coverage for no-parent case
The type fix is correct and the serializer already handles the no-parent path (via 'parentResourceId' in options), but the createResource test suite in authorization.spec.ts has no test case that exercises creating a resource without any parent fields. It would be good to add one, for example:
it('creates an authorization resource without a parent', async () => {
fetchOnce(authorizationResourceFixture, { status: 201 });
await workos.authorization.createResource({
organizationId: testOrgId,
resourceTypeSlug: 'document',
externalId: 'doc-456',
name: 'Q4 Budget Report',
});
const body = fetchBody();
expect(body).not.toHaveProperty('parent_resource_id');
expect(body).not.toHaveProperty('parent_resource_external_id');
expect(body).not.toHaveProperty('parent_resource_type_slug');
});This confirms the serializer omits all parent fields when none are supplied.
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!
Covers the no-parent case enabled by the CreateAuthorizationResourceOptions type fix, verifying no parent fields are sent in the serialized payload.
|
@mxp-qk Thanks for this! |
Description
Fix
CreateAuthorizationResourceOptionstype definition to accept no parent as mentioned in the docRelated issue: #1529
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.