Repository navigation
fix(directory-sync): Add name to DirectoryUser - #1731
Open
gjtorikian wants to merge 1 commit into
Open
gjtorikian wants to merge 1 commit into
gjtorikian wants to merge 1 commit into
Conversation
The API returns a composed `name` on directory users, and the OpenAPI spec declares it as `string | null`, but `DirectoryUser` and `DirectoryUserResponse` did not model it, so both directory user deserializers dropped it. Identity providers that map only the composed name, and neither `first_name` nor `last_name`, left SDK consumers with no name at all for API responses and `dsync.user.*` events. Add `name` to both interfaces and carry it through `deserializeDirectoryUser` and `deserializeUpdatedEventDirectoryUser`, normalizing a missing value to `null` like the user deserializer does. Fixes #1700
Contributor
[Medium impact] The change reaches directory-user API results and directory-sync event workflows. The PR appears safe to merge, with a non-blocking request to test missing and null names. Findings
Fix with agent prompt### Issue 1
src/directory-sync/serializers/directory-user.serializer.ts:27
Both new tests supply a non-null `name`, so neither checks the new `?? null` fallback. Add cases with `name` omitted and explicitly null for both deserializers, and assert that the returned `name` is null. Otherwise, removing the fallback would leave these tests passing while older payloads return undefined.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds
Reviews (1) · Last reviewed commit: "fix(directory-sync): Add name to Directo..." · Reviewed by Greptile |
| firstName: directoryUser.first_name, | ||
| email: directoryUser.email, | ||
| lastName: directoryUser.last_name, | ||
| name: directoryUser.name ?? null, |
Contributor
There was a problem hiding this comment.
Both new tests supply a non-null name, so neither checks the new ?? null fallback. Add cases with name omitted and explicitly null for both deserializers, and assert that the returned name is null. Otherwise, removing the fallback would leave these tests passing while older payloads return undefined.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/directory-sync/serializers/directory-user.serializer.ts
Line: 27
Comment:
**Missing names go untested**
Both new tests supply a non-null `name`, so neither checks the new `?? null` fallback. Add cases with `name` omitted and explicitly null for both deserializers, and assert that the returned `name` is null. Otherwise, removing the fallback would leave these tests passing while older payloads return undefined.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
jonatascastro12
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The API returns a composed
nameon directory users and the OpenAPI spec declares it (DirectoryUser.name: string | null), butDirectoryUserandDirectoryUserResponsedid not model it, sodeserializeDirectoryUseranddeserializeUpdatedEventDirectoryUsersilently dropped it. Identity providers that map only the composed name leave SDK consumers with no name at all, for both API responses anddsync.user.*events.name: string | nulltoDirectoryUserandDirectoryUserResponse.null(same asdeserializeUser).nameto thedsync.user.createdwebhook fixture and thedsync.user.updatedevent test so both deserializers are covered.The spec also types
organization_idas a non-nullablestringwhile the SDK types itstring | null; that is left as is here since changing it would be a breaking type change.Fixes #1700
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.