Skip to content

fix: allow clearing organization domains by sending empty arrays#522

Merged
gjtorikian merged 2 commits intoworkos:mainfrom
jinzhongjia:update-organization
Mar 20, 2026
Merged

fix: allow clearing organization domains by sending empty arrays#522
gjtorikian merged 2 commits intoworkos:mainfrom
jinzhongjia:update-organization

Conversation

@jinzhongjia
Copy link
Contributor

Summary

  • Remove omitempty from Domains and DomainData JSON tags in UpdateOrganizationOpts
  • In Go, json:",omitempty" omits both nil and empty slices ([]string{}), making it impossible to send an empty array to the API to clear an organization's domains
  • After this fix:
    • nil → serializes as null (no change on API side)
    • []string{} → serializes as [] (clears domains)
    • []string{"foo.com"} → serializes as ["foo.com"] (sets domains)

Test plan

  • go test ./pkg/organizations/... passes

@jinzhongjia jinzhongjia requested review from a team as code owners March 20, 2026 05:34
@jinzhongjia jinzhongjia requested a review from stanleyphu March 20, 2026 05:34
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 20, 2026

Greptile Summary

This PR removes omitempty from the Domains and DomainData JSON tags in UpdateOrganizationOpts to enable callers to send an empty array to the WorkOS API and thereby clear an organization's domains. The fix is logically correct for the stated goal, but has a side-effect: when either field is nil (not set by the caller), the JSON payload now includes "domains": null and "domain_data": null rather than omitting those keys entirely, which is a behavioral change for every existing caller that updates an organization without touching domains.

  • Behavioral change for nil fields: Previously a name-only update sent {"name": "New Name"}; after this change it sends {"name": "New Name", "domains": null, "domain_data": null}. If the WorkOS API interprets null as "clear this field," all existing callers could silently lose their domains.
  • No regression test for the fix: The test suite has no case with an empty Domains or DomainData slice, so there is no automated proof that the fix actually works and no guard against future regressions.

Confidence Score: 3/5

  • The fix resolves the stated issue but introduces a possible silent regression where nil domain fields now serialize as null, which may have unintended API-side effects for existing callers.
  • The change is minimal (two JSON tag removals) and the intent is clear. However, the null-vs-absent ambiguity is a genuine risk that depends entirely on undocumented API behavior, and there is no new test covering the fixed scenario to build confidence.
  • pkg/organizations/client.go — the UpdateOrganizationOpts struct and how the API handles null vs. absent domain fields.

Important Files Changed

Filename Overview
pkg/organizations/client.go Removes omitempty from Domains and DomainData JSON tags in UpdateOrganizationOpts to allow sending empty arrays; introduces a risk that nil fields now serialize as null instead of being omitted, potentially affecting callers who only update non-domain fields.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant SDK as workos-go SDK
    participant API as WorkOS API

    Note over Caller,API: Before fix (omitempty)
    Caller->>SDK: UpdateOrganization({Name: "X", Domains: nil})
    SDK->>API: PUT /organizations/{id} {"name":"X"}
    Caller->>SDK: UpdateOrganization({Name: "X", Domains: []})
    SDK->>API: PUT /organizations/{id} {"name":"X"} (empty slice omitted — cannot clear)

    Note over Caller,API: After fix (no omitempty)
    Caller->>SDK: UpdateOrganization({Name: "X", Domains: nil})
    SDK->>API: PUT /organizations/{id} {"name":"X","domains":null,"domain_data":null}
    Caller->>SDK: UpdateOrganization({Name: "X", Domains: []})
    SDK->>API: PUT /organizations/{id} {"name":"X","domains":[],"domain_data":null} ✓ clears domains
Loading

Comments Outside Diff (1)

  1. pkg/organizations/client_test.go, line 470-605 (link)

    P2 No test case for the actual bug being fixed

    The test suite doesn't include a case that specifically exercises the new behavior (sending an empty array to clear domains). All existing test cases use non-empty Domains or DomainData slices, so they would have passed before this change too. Adding a test case that sets Domains: []string{} or DomainData: []OrganizationDomainData{} and verifies the request body actually contains "domains":[] / "domain_data":[] would prevent regressions and confirm the fix is working as intended.

Last reviewed commit: "refactor: remove omi..."

Adds a custom MarshalJSON to UpdateOrganizationOpts that distinguishes
between nil slices (omit from request) and empty slices (send [] to
clear domains). The previous approach of simply removing omitempty
would send null for these fields on every update call, even when the
caller doesn't intend to modify domains.
@gjtorikian
Copy link
Contributor

Hey thanks for pointing this out.

However, the fix of removing omitempty from the struct tags introduces a breaking change: every UpdateOrganization call that doesn't set Domains or DomainData will now send "domains": null and "domain_data": null in the request body. Previously those fields were omitted entirely.

For example, a caller doing:

  client.UpdateOrganization(ctx, organizations.UpdateOrganizationOpts{
      Organization: "org_123",
      Name:         "New Name",
  })

Now needs to shape it as {"name": "New Name", "domains": null, "domain_data": null}

This means the API receives explicit null values on every update, even when the caller has no intention of touching domains. Whether that causes problems depends on how the API handles null vs. absent, but it's a contract change for every existing user of this method.

I took a different approach: a custom MarshalJSON that keeps omitempty on the tags but overrides marshaling to distinguish nil (omit the field) from [] (send an empty array). This preserves the existing behavior for all callers while solving your use case.

  // Clear domains — sends "domain_data": []
  opts := organizations.UpdateOrganizationOpts{
      Organization: "org_123",
      DomainData:   []organizations.OrganizationDomainData{},
  }

  // Just rename — domain_data is omitted entirely
  opts := organizations.UpdateOrganizationOpts{
      Organization: "org_123",
      Name:         "New Name",
  }

Will release this as a patch version now.

@gjtorikian gjtorikian merged commit a315391 into workos:main Mar 20, 2026
5 checks passed
@gjtorikian
Copy link
Contributor

Will release this as a patch version now.

Actually, it'll go out as 6.5.0 -- there were a few other changes waiting.

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.

2 participants