fix: CreateAuthorizationResourceOptions type definition#1530
fix: CreateAuthorizationResourceOptions type definition#1530mxp-qk wants to merge 1 commit 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!
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.