Skip to content

fix: CreateAuthorizationResourceOptions type definition#1530

Open
mxp-qk wants to merge 1 commit intoworkos:mainfrom
mxp-qk:patch-1
Open

fix: CreateAuthorizationResourceOptions type definition#1530
mxp-qk wants to merge 1 commit intoworkos:mainfrom
mxp-qk:patch-1

Conversation

@mxp-qk
Copy link

@mxp-qk mxp-qk commented Mar 22, 2026

Description

Fix CreateAuthorizationResourceOptions type definition to accept no parent as mentioned in the doc

Related issue: #1529

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@mxp-qk mxp-qk requested review from a team as code owners March 22, 2026 20:50
@mxp-qk mxp-qk requested a review from mattgd March 22, 2026 20:50
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 22, 2026

Greptile Summary

This PR fixes a type-level bug where CreateAuthorizationResourceOptions only allowed resources to be created with a parent (via parentResourceId or parentResourceExternalId/parentResourceTypeSlug), even though the WorkOS FGA API supports parentless resources. The fix adds BaseCreateAuthorizationResourceOptions as a third variant in the discriminated union, aligning the TypeScript types with the documented API behaviour.

  • The one-line union change in authorization-resource.interface.ts is correct and complete.
  • The existing serializer (serializeCreateResourceOptions) already handles the no-parent path correctly through conditional spreads ('parentResourceId' in options), so no runtime changes were needed.
  • No test case was added to the createResource suite to assert that the serializer omits all parent fields when none are supplied — this is a small gap worth closing.
  • BaseCreateAuthorizationResourceOptions remains unexported, which is fine; consumers interact with it through the exported union type.

Confidence Score: 5/5

  • Safe to merge — the one-line type fix is correct and the serializer already handles the new no-parent case at runtime.
  • The change is a single-line type-union addition that correctly unblocks a valid API use case. The serializer was already written defensively with 'parentResourceId' in options guards, so no runtime behaviour changes. The only gap is a missing unit-test case, which is a non-blocking follow-up.
  • No files require special attention.

Important Files Changed

Filename Overview
src/authorization/interfaces/authorization-resource.interface.ts Adds BaseCreateAuthorizationResourceOptions to the CreateAuthorizationResourceOptions union so resources can be created without a parent; the existing serializer already handles this case correctly via conditional spreads.

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."
Loading

Reviews (1): Last reviewed commit: "fix: CreateAuthorizationResourceOptions ..." | Re-trigger Greptile

Comment on lines 44 to 47
export type CreateAuthorizationResourceOptions =
| BaseCreateAuthorizationResourceOptions
| CreateOptionsWithParentResourceId
| CreateOptionsWithParentExternalId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant