-
Notifications
You must be signed in to change notification settings - Fork 1
Fix Zod schema error for optional wp:featuredmedia fields and add schema validation to smoke tests #940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: hawkrives <464441+hawkrives@users.noreply.github.com>
Co-authored-by: hawkrives <464441+hawkrives@users.noreply.github.com>
Co-authored-by: hawkrives <464441+hawkrives@users.noreply.github.com>
|
@copilot Please add this to the existing St. Olaf smoke tests. As we don't control the upstream endpoint, we can't rely solely on unit tests to ensure that this continues to work in production. |
Co-authored-by: hawkrives <464441+hawkrives@users.noreply.github.com>
Added explicit validation for
This ensures the WordPress JSON feed parsing with optional wp:featuredmedia fields continues to work in production. Commit: 51e5c25 |
|
@copilot For each endpoint which has a Zod schema, I'd like to have a smoke test which ensures that the response is valid according to the schema. |
Co-authored-by: hawkrives <464441+hawkrives@users.noreply.github.com>
Created The script:
This ensures responses match the schema definitions and will catch any future regressions. Commit: 0a3c962 |
4f64a9e to
ca54c55
Compare
| ;; | ||
|
|
||
| "/v1/news/named/stolaf" | "/v1/news/named/krlx") | ||
| # Validate news endpoints with Zod schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment doesn't add much value.
| echo "validating $route with Zod schema" | ||
| RESPONSE=$(curl --silent --fail "localhost:3000$route") | ||
| # Validate against the Zod schema | ||
| echo "$RESPONSE" | node dist/scripts/validate-schema.js "$route" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only place in this shell script where we depend on the script having somehow gotten to this path. We should not begin the tests unless the script is present here.
Are the Zod checks applied on ccc output (responses we send to apps) or input (responses we get from upstreams?)?
If they're applied on upstreams, then this test is also not actually testing what we want — in that case it's testing that… the output happens to also conform to the input spec? Which doesn't apply to other types of routes like food menus.
Why not just let these two routes
Problem
The
/v1/news/named/stolafendpoint was returning 500 errors due to Zod schema validation failures. The WordPress JSON API sometimes returnswp:featuredmediaarray items with incomplete data, wheremedia_type,media_details, andsource_urlfields are undefined.Solution
Made the following fields optional in the
WpJsonFeedEntrySchema:media_type: Changed from required to.optional()media_details: Changed from required to.optional()source_url: Changed from required to.optional()Updated
convertWpJsonItemToStoryto safely handle these optional fields using optional chaining (?.) and added explicit null coalescing to ensurefeaturedImagedefaults tonullwhen fields are missing.Testing
Unit Tests
Added comprehensive test coverage in
source/feeds/wp-json.test.ts:All tests pass (7 new tests added). Schemas are exported from source and imported in tests to avoid duplication.
Smoke Tests
Created a schema validation system for smoke tests:
scripts/validate-schema.ts): Validates API responses against their Zod schemas by dynamically importing the schema modules/v1/news/named/stolafand/v1/news/named/krlxendpoints now validate responses against their actual Zod schemas instead of manual JSON structure checksThis provides strong protection against regressions since we don't control the upstream APIs. The validation uses the same Zod schemas that the application uses, ensuring smoke tests validate against the actual contract.
Impact
This fix is backwards compatible. Posts with complete featured media information continue to work as before, while posts with incomplete or missing featured media fields now parse successfully without throwing validation errors.
Fixes #908
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.