Skip to content

feat: expand ContentResponse type to include additional API fields#3865

Closed
m-- wants to merge 3 commits intomainfrom
mgregory/3855-expand-content-response-type
Closed

feat: expand ContentResponse type to include additional API fields#3865
m-- wants to merge 3 commits intomainfrom
mgregory/3855-expand-content-response-type

Conversation

@m--
Copy link
Copy Markdown
Collaborator

@m-- m-- commented Mar 20, 2026

Summary

  • Expand ContentResponse and Revision types in packages/connect-cloud-api/src/types.ts to match the actual Connect Cloud API response shape
  • Add Author interface for embedded user info in responses
  • Add ContentUserRole interface for user roles on content
  • Update test mocks to include new required fields

Closes #3855

Test plan

  • TypeScript compiles without errors (npx tsc --noEmit)
  • All existing tests pass (npm test)

🤖 Generated with Claude Code

…3855)

Expand ContentResponse and Revision types to match the actual Connect
Cloud API response shape as revealed by the golden test fixtures.

- Add Author interface for embedded user info in responses
- Add ContentUserRole interface for user roles on content
- Expand Revision with source, runtime, and publish state fields
- Expand ContentResponse with author, timestamps, access control, etc.
- Update test mocks to include new required fields

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@m-- m-- requested a review from a team as a code owner March 20, 2026 23:22
@posit-connect-projects
Copy link
Copy Markdown
Contributor

posit-connect-projects Bot commented Mar 20, 2026

Claude finished @m--'s task in 1m 4s —— View job


Code Review

  • Gather context and review changed files
  • Analyze type definitions and test changes
  • Provide code review feedback

Overall: Looks good!

This PR properly expands the ContentResponse and Revision types to match the actual Connect Cloud API response shape. The golden test fixtures in testdata/ confirm the types now accurately represent real API responses.

Positive Aspects

  1. Well-organized type definitions - Clear section comments and logical grouping of related fields
  2. New interfaces - Author and ContentUserRole are cleanly defined and reused appropriately
  3. Test mocks updated - All test fixtures include the new required fields
  4. Pragmatic typing - The access: ContentAccess | string union with explanatory comment correctly handles the API returning "public" instead of the enum value

Minor Observations

  1. schedules?: unknown[] at types.ts:331 - If schedules have a known structure, consider adding a proper type. Fine to leave as unknown[] if not currently used.

  2. Nullable vs optional consistency - Fields like description?: string | null use both ? and | null. This is common for API types where the server explicitly returns null, so the pattern is acceptable.

  3. Breaking change note - The Revision interface changed some previously required fields (publish_log_channel, publish_result) to optional. This matches the actual API (as shown in the golden fixture where these are null), so this is a correctness fix rather than a concern.

Verification

The golden test fixtures (testdata/get_content.json, testdata/get_revision.json) show that:

  • Real API responses include all the new fields
  • publish_log_channel and publish_result can indeed be null
  • The access field can be "public" (a string not in the enum)

No issues found. Ready to merge.

publish_error_args?: Record<string, unknown> | null;
publish_error_details?: string | null;

// Other
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

do we care overly much about how this is organized? Other seems like kind of a weird category given that we have some uncategorized things hanging out at the top, too.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't really care about the organization, and would find it a bit frustrating to maintain.

I typically fallback to alphabetical, but that isn't as important either.

Comment thread packages/connect-cloud-api/src/types.ts Outdated
access: ContentAccess;
account_id: string;
title: string;
access: ContentAccess | string; // API may return "public" instead of enum value
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this makes me wonder if ContentAccess is the right type, if we're seeing public in the golden fixture.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If public is a valid value we should access it to ContentAccess

m-- and others added 2 commits March 26, 2026 16:29
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The API may return "private" or "public" shorthand values instead of
the longer form equivalents. This removes the need for the string
fallback type on ContentResponse.access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@m--
Copy link
Copy Markdown
Collaborator Author

m-- commented Mar 27, 2026

We're going to close this — we should add these types as we use and need them.

@m-- m-- closed this Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand ContentResponse type to include additional API fields

2 participants