Skip to content

feat!: Generate AdminPortal from OpenAPI spec - #1629

Open
gjtorikian wants to merge 1 commit into
mainfrom
oagen/own-admin-portal
Open

feat!: Generate AdminPortal from OpenAPI spec#1629
gjtorikian wants to merge 1 commit into
mainfrom
oagen/own-admin-portal

Conversation

@gjtorikian

Copy link
Copy Markdown
Contributor

Description

This PR generates the AdminPortal category in the Node SDK from the OpenAPI spec.

It renames one field:

Breaking — field renamed:

  ┌────────────────────────┬────────────────────────────┐
  │     Before (main)      │       After (branch)       │
  ├────────────────────────┼────────────────────────────┤
  │ adminEmails?: string[] │ itContactEmails?: string[] │
  └────────────────────────┴────────────────────────────┘

Because of this, the PR should be treated as a breaking change.

@gjtorikian
gjtorikian requested review from a team as code owners June 17, 2026 01:36
@gjtorikian
gjtorikian requested a review from dandorman June 17, 2026 01:36
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR migrates the AdminPortal module to be fully generated by oagen from the OpenAPI spec, introducing proper serializer/deserializer pairs, a new DomainVerificationIntentOptions type, and the breaking rename of adminEmailsitContactEmails on both the model and wire interfaces.

  • Breaking rename: GenerateLink.adminEmailsitContactEmails (and admin_emailsit_contact_emails on the wire), documented in the PR description; serializer correctly maps the new field name.
  • New domainVerification intent option: IntentOptions and IntentOptionsResponse gain an optional domainVerification / domain_verification sub-object with its own serializer, guarded by null checks consistent with the SSO pattern.
  • Test coverage regressed: The generated test suite replaces detailed per-intent round-trip assertions with a single happy-path check and toBeDefined() guards on all serializer tests, leaving the key breaking-change field and all error paths unverified (see existing review threads for details).

Confidence Score: 5/5

  • Safe to merge — the runtime serialization logic is correct and the breaking rename is consistently applied across interfaces, serializers, fixtures, and JSDoc.
  • The generated serializers faithfully map all camelCase model fields to their snake_case wire equivalents, including the renamed itContactEmails field and the new domainVerification sub-object. The open issues (weak test assertions, dead generate-link-intent.interface.ts, and the no-op payload assignment) were raised in prior review threads and do not affect runtime correctness.
  • serializers.spec.ts — the generated serializer tests pass wire-format fixtures to serialize functions that expect model-format inputs, making the tests vacuous. admin-portal.spec.ts dropped all per-intent and error-path coverage.

Important Files Changed

Filename Overview
src/admin-portal/admin-portal.ts Core generated class now delegates serialization/deserialization to dedicated serializers; logic is clean and type-safe.
src/admin-portal/admin-portal.spec.ts Integration test reduced to a single happy-path case that does not exercise the renamed itContactEmails field, optional intent, or any error responses. Coverage was significantly regressed from the original hand-written suite.
src/admin-portal/serializers.spec.ts Serializer tests pass wire-format fixtures (snake_case) to serialize functions that expect model-format inputs (camelCase), so most fields silently resolve to undefined. All assertions were downgraded to toBeDefined(), masking the incorrect behaviour.
src/admin-portal/serializers/generate-link.serializer.ts Correctly renames admin_emails to it_contact_emails in the serialized output and removes the now-unused deserialize direction.
src/admin-portal/interfaces/generate-link.interface.ts Renames adminEmails to itContactEmails on both model and wire interfaces; imports GenerateLinkIntent from the shared common module.
src/admin-portal/interfaces/generate-link-intent.interface.ts Generator-produced duplicate of the common GenerateLinkIntent constant/type that is never imported by any file in the admin-portal module — dead code.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant AdminPortal
    participant serializeGenerateLink
    participant serializeIntentOptions
    participant WorkOS API
    participant deserializePortalLinkResponse

    Caller->>AdminPortal: generateLink(options: GenerateLink)
    AdminPortal->>serializeGenerateLink: serializeGenerateLink(options)
    serializeGenerateLink->>serializeIntentOptions: serializeIntentOptions(options.intentOptions)
    serializeIntentOptions-->>serializeGenerateLink: "IntentOptionsResponse { sso?, domain_verification? }"
    serializeGenerateLink-->>AdminPortal: GenerateLinkResponse (wire body)
    AdminPortal->>WorkOS API: POST /portal/generate_link (body: GenerateLinkResponse)
    WorkOS API-->>AdminPortal: PortalLinkResponseWire { link }
    AdminPortal->>deserializePortalLinkResponse: deserializePortalLinkResponse(data)
    deserializePortalLinkResponse-->>AdminPortal: "PortalLinkResponse { link }"
    AdminPortal-->>Caller: "PortalLinkResponse { link }"
Loading

Reviews (2): Last reviewed commit: "Generate AdminPortal from OpenAPI spec" | Re-trigger Greptile

Comment on lines +1 to +14
// This file is auto-generated by oagen. Do not edit.

export const GenerateLinkIntent = {
SSO: 'sso',
DSync: 'dsync',
AuditLogs: 'audit_logs',
LogStreams: 'log_streams',
DomainVerification: 'domain_verification',
CertificateRenewal: 'certificate_renewal',
BringYourOwnKey: 'bring_your_own_key',
} as const;

export type GenerateLinkIntent =
(typeof GenerateLinkIntent)[keyof typeof GenerateLinkIntent];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Unused generated file — never imported

This file duplicates src/common/interfaces/generate-link-intent.interface.ts exactly, but nothing imports it. generate-link.interface.ts still imports GenerateLinkIntent from ../../common/interfaces/generate-link-intent.interface, and interfaces/index.ts does not re-export this file. The generator appears to have produced the file but did not update the import path in generate-link.interface.ts to use the local copy, leaving this file as dead code.

Comment on lines 19 to 58
describe('SSOIntentOptionsSerializer', () => {
it('round-trips through serialize/deserialize', () => {
it('serializes correctly', () => {
const fixture = ssoIntentOptionsFixture as SSOIntentOptionsResponse;
const deserialized = deserializeSSOIntentOptions(fixture);
const reserialized = serializeSSOIntentOptions(deserialized);
expect(reserialized).toEqual(expect.objectContaining(fixture));
const serialized = serializeSSOIntentOptions(fixture as any);
expect(serialized).toBeDefined();
});
});

describe('DomainVerificationIntentOptionsSerializer', () => {
it('serializes correctly', () => {
const fixture =
domainVerificationIntentOptionsFixture as DomainVerificationIntentOptionsResponse;
const serialized = serializeDomainVerificationIntentOptions(fixture as any);
expect(serialized).toBeDefined();
});
});

describe('IntentOptionsSerializer', () => {
it('round-trips through serialize/deserialize', () => {
it('serializes correctly', () => {
const fixture = intentOptionsFixture as IntentOptionsResponse;
const deserialized = deserializeIntentOptions(fixture);
const reserialized = serializeIntentOptions(deserialized);
expect(reserialized).toEqual(expect.objectContaining(fixture));
const serialized = serializeIntentOptions(fixture as any);
expect(serialized).toBeDefined();
});
});

describe('GenerateLinkSerializer', () => {
it('round-trips through serialize/deserialize', () => {
it('serializes correctly', () => {
const fixture = generateLinkFixture as GenerateLinkResponse;
const deserialized = deserializeGenerateLink(fixture);
const reserialized = serializeGenerateLink(deserialized);
expect(reserialized).toEqual(expect.objectContaining(fixture));
const serialized = serializeGenerateLink(fixture as any);
expect(serialized).toBeDefined();
});
});

describe('PortalLinkResponseSerializer', () => {
it('round-trips through serialize/deserialize', () => {
it('deserializes correctly', () => {
const fixture = portalLinkResponseFixture as PortalLinkResponseWire;
const deserialized = deserializePortalLinkResponse(fixture);
const reserialized = serializePortalLinkResponse(deserialized);
expect(reserialized).toEqual(expect.objectContaining(fixture));
expect(deserialized).toBeDefined();
});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Tests only verify existence, not correctness

All serializer tests were downgraded from round-trip assertions (expect(reserialized).toEqual(expect.objectContaining(fixture))) to expect(serialized).toBeDefined(). A call that returns {} or an object where every field is undefined would still pass. Notably, the renamed field itContactEmailsit_contact_emails (the key breaking change in this PR) is never verified to round-trip correctly through any of these tests.

Comment on lines +46 to +51
async generateLink(options: GenerateLink): Promise<PortalLinkResponse> {
const payload = options;
const { data } = await this.workos.post<
PortalLinkResponseWire,
GenerateLinkResponse
>('/portal/generate_link', serializeGenerateLink(payload));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The intermediate const payload = options; assignment adds no value — options can be passed directly to serializeGenerateLink.

Suggested change
async generateLink(options: GenerateLink): Promise<PortalLinkResponse> {
const payload = options;
const { data } = await this.workos.post<
PortalLinkResponseWire,
GenerateLinkResponse
>('/portal/generate_link', serializeGenerateLink(payload));
async generateLink(options: GenerateLink): Promise<PortalLinkResponse> {
const { data } = await this.workos.post<
PortalLinkResponseWire,
GenerateLinkResponse
>('/portal/generate_link', serializeGenerateLink(options));

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!

@gjtorikian
gjtorikian marked this pull request as draft June 18, 2026 19:06
@gjtorikian gjtorikian added the autogenerated Autogenerated code or content label Jun 18, 2026
@gjtorikian
gjtorikian marked this pull request as ready for review August 24, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autogenerated Autogenerated code or content

Development

Successfully merging this pull request may close these issues.

1 participant