fix: validate effective_at against new invalid_at when both updated#13
fix: validate effective_at against new invalid_at when both updated#13Washio20 wants to merge 1 commit intostayforge:mainfrom
Conversation
…loses stayforge#6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughOpenAPI documentation updated to clarify cross-field time validation constraints between effective_at and invalid_at fields in card schemas. Descriptions now specify that effective_at must precede invalid_at, and PATCH operations validate both fields against their new values when updated together. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #6 by adding documentation to clarify the validation behavior when both effective_at and invalid_at are updated together. The key clarification is that when both fields are provided in the same request, they are validated against each other's new values (from the request body) rather than against the currently stored values in the database.
Changes:
- Updated field descriptions in
CardPropertiesschema to document the cross-field validation betweeneffective_atandinvalid_at - Enhanced the PATCH endpoint's 422 error description to explain time validation behavior
- Applied consistent changes to both
openapi.yamlandopenapi.json
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| openapi.yaml | Added documentation for time validation in CardProperties schema and PATCH 422 error response |
| openapi.json | Mirrored the documentation changes from openapi.yaml in JSON format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| When updating both `effective_at` and `invalid_at` in a single PATCH request, | ||
| `invalid_at` is validated against the new `effective_at` value provided in | ||
| the request body (not the currently stored `effective_at`). |
There was a problem hiding this comment.
The description mentions "When updating both effective_at and invalid_at in a single PATCH request", but since CardProperties is also used by CardInput for POST requests, this wording might be confusing. Consider either:
- Using more generic wording like "When both fields are provided in the same request" (similar to the 422 error description), or
- Explicitly mentioning both POST and PATCH operations if the validation applies to both.
This would make the documentation clearer for API consumers using the POST endpoint.
| }, | ||
| "422": { | ||
| "description": "Unprocessable Entity - Request is well-formed but contains semantic errors.\nError codes: `unprocessable_entity`\n", | ||
| "description": "Unprocessable Entity - Request is well-formed but contains semantic errors.\nFor time validation: `effective_at` must be earlier than `invalid_at`. When both fields are provided in the same request, they are validated against each other (the new values), not against the currently stored values.\nError codes: `unprocessable_entity`\n", |
There was a problem hiding this comment.
The time validation documentation was added only to the PATCH endpoint's 422 error description, but the CardProperties schema is also used by the POST endpoint (via CardInput). If the time validation logic applies to POST requests as well (when both effective_at and invalid_at are provided), consider adding similar documentation to the POST endpoint's 422 error description (line 318 in openapi.json). This would ensure consistency and clarity for API consumers using either endpoint.
| "format": "date-time", | ||
| "nullable": true, | ||
| "description": "Effective time (when the card becomes active).\nMust be the current time or a future time.\n**⚠️Default: `UTC`**, recommended to use the format `2025-02-16T12:00:00+01:00` to ensure the correct time zone.\nAbout ISO 8601: https://en.wikipedia.org/wiki/ISO_8601\n" | ||
| "description": "Effective time (when the card becomes active).\nMust be the current time or a future time, and must be earlier than `invalid_at`.\nWhen updating both `effective_at` and `invalid_at` in a single PATCH request, `effective_at` is validated against the new `invalid_at` value provided in the request body (not the currently stored `invalid_at`).\n**⚠️Default: `UTC`**, recommended to use the format `2025-02-16T12:00:00+01:00` to ensure the correct time zone.\nAbout ISO 8601: https://en.wikipedia.org/wiki/ISO_8601\n" |
There was a problem hiding this comment.
The description mentions "When updating both effective_at and invalid_at in a single PATCH request", but since CardProperties is also used by CardInput for POST requests, this wording might be confusing. Consider either:
- Using more generic wording like "When both fields are provided in the same request" (similar to the 422 error description), or
- Explicitly mentioning both POST and PATCH operations if the validation applies to both.
This would make the documentation clearer for API consumers using the POST endpoint.
| "format": "date-time", | ||
| "nullable": true, | ||
| "description": "Invalidation time (when the card becomes invalid/expires).\nMust be the current time or a future time.\n**⚠️Default: `UTC`**, recommended to use the format `2025-02-16T12:00:00+01:00` to ensure the correct time zone.\nAbout ISO 8601: https://en.wikipedia.org/wiki/ISO_8601\n" | ||
| "description": "Invalidation time (when the card becomes invalid/expires).\nMust be the current time or a future time, and must be later than `effective_at`.\nWhen updating both `effective_at` and `invalid_at` in a single PATCH request, `invalid_at` is validated against the new `effective_at` value provided in the request body (not the currently stored `effective_at`).\n**⚠️Default: `UTC`**, recommended to use the format `2025-02-16T12:00:00+01:00` to ensure the correct time zone.\nAbout ISO 8601: https://en.wikipedia.org/wiki/ISO_8601\n" |
There was a problem hiding this comment.
The description mentions "When updating both effective_at and invalid_at in a single PATCH request", but since CardProperties is also used by CardInput for POST requests, this wording might be confusing. Consider either:
- Using more generic wording like "When both fields are provided in the same request" (similar to the 422 error description), or
- Explicitly mentioning both POST and PATCH operations if the validation applies to both.
This would make the documentation clearer for API consumers using the POST endpoint.
| For time validation: `effective_at` must be earlier than `invalid_at`. | ||
| When both fields are provided in the same request, they are validated | ||
| against each other (the new values), not against the currently stored values. |
There was a problem hiding this comment.
The time validation documentation was added only to the PATCH endpoint's 422 error description, but the CardProperties schema is also used by the POST endpoint (via CardInput). If the time validation logic applies to POST requests as well (when both effective_at and invalid_at are provided), consider adding similar documentation to the POST endpoint's 422 error description (lines 236-242 in openapi.yaml). This would ensure consistency and clarity for API consumers using either endpoint.
| When updating both `effective_at` and `invalid_at` in a single PATCH request, | ||
| `effective_at` is validated against the new `invalid_at` value provided in | ||
| the request body (not the currently stored `invalid_at`). |
There was a problem hiding this comment.
The description mentions "When updating both effective_at and invalid_at in a single PATCH request", but since CardProperties is also used by CardInput for POST requests, this wording might be confusing. Consider either:
- Using more generic wording like "When both fields are provided in the same request" (similar to the 422 error description), or
- Explicitly mentioning both POST and PATCH operations if the validation applies to both.
This would make the documentation clearer for API consumers using the POST endpoint.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openapi.json (1)
660-671:⚠️ Potential issue | 🟡 MinorClarify the 422 example message to avoid “Device” confusion.
This is a card endpoint, but the example message still begins with “Device validation failed,” which can mislead API consumers. Consider wording that explicitly refers to card validation or is neutral.
📝 Suggested wording
- "message": "Device validation failed or invalid effective_at/invalid_at" + "message": "Validation failed: invalid effective_at/invalid_at or device association"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.json` around lines 660 - 671, Update the 422 example value under the "unprocessable_entity" example (the JSON example object referencing components/schemas/Error) so the error message no longer says "Device validation failed"; change it to mention "card validation failed" or a neutral phrasing like "Validation failed or invalid effective_at/invalid_at" to match the card endpoint and avoid confusion. Ensure the updated message stays under the same example key "unprocessable_entity" and keeps the same error code "unprocessable_entity".openapi.yaml (1)
236-251:⚠️ Potential issue | 🟡 MinorPOST 422 description not updated — inconsistency with
openapi.jsonAccording to the AI summary,
openapi.jsonhad itsPOST /v1/cards/{organization_id}422 response expanded to include the cross-field time validation constraint and an updated example error message. The equivalent change is missing here inopenapi.yaml, leaving the two spec files out of sync.📄 Suggested update to align with `openapi.json`
'422': - description: 'Unprocessable Entity - Request is well-formed but contains - semantic errors. - - Error codes: `unprocessable_entity` - - ' + description: 'Unprocessable Entity - Request is well-formed but contains + semantic errors. + + For time validation: `effective_at` must be earlier than `invalid_at`. + When both fields are provided in the same request, they are validated + against each other (the new values), not against the currently stored values. + + Error codes: `unprocessable_entity` + + ' content: application/json: schema: $ref: '#/components/schemas/Error' examples: unprocessable_entity: value: code: unprocessable_entity - message: Device validation failed + message: Device validation failed or invalid effective_at/invalid_at🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 236 - 251, Update the POST /v1/cards/{organization_id} 422 response in openapi.yaml to match openapi.json: expand the description to include the cross-field time validation constraint text, update the example under content->application/json->examples->unprocessable_entity to the new error message, and ensure the response still references the Error schema; locate the POST path entry for /v1/cards/{organization_id} and modify the '422' response block (schema $ref and examples key unprocessable_entity) to reflect the same wording and example as openapi.json.
🧹 Nitpick comments (1)
openapi.yaml (1)
5543-5547: PATCH-specific wording in sharedCardPropertiesmay confuse POST consumers
CardPropertiesis inherited by bothCardInput(used byPOST /v1/cards) andCardUpdate(used byPATCH /v1/cards/{organization_id}/{card_number}). The phrase "When updating botheffective_atandinvalid_atin a single PATCH request" is scoped to PATCH, but developers reading the field description viaCardInputsee PATCH-specific semantics that don't apply to card creation (where there are no stored values to contrast against).Consider generalising the wording to be operation-agnostic:
✏️ Suggested wording (applies naturally to both POST and PATCH)
- When updating both `effective_at` and `invalid_at` in a single PATCH request, - `effective_at` is validated against the new `invalid_at` value provided in - the request body (not the currently stored `invalid_at`). + When both `effective_at` and `invalid_at` are provided in the same request, + they are validated against each other (the supplied values), not against + any previously stored value.Apply the equivalent change to the
invalid_atdescription (lines 5563-5565).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 5543 - 5547, The shared CardProperties field descriptions include PATCH-specific language that can confuse POST consumers; update the effective_at description (and similarly invalid_at) to operation-agnostic wording such as: "Must be the current time or a future time and earlier than invalid_at. If both effective_at and invalid_at are provided in the same request, validation is performed against the values supplied in that request (not stored values)." Ensure you change the descriptions in CardProperties so CardInput (POST /v1/cards) and CardUpdate (PATCH /v1/cards/{organization_id}/{card_number}) inherit the clarified, request-scoped semantics for effective_at and invalid_at.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
openapi.jsonopenapi.yaml
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@openapi.json`:
- Around line 660-671: Update the 422 example value under the
"unprocessable_entity" example (the JSON example object referencing
components/schemas/Error) so the error message no longer says "Device validation
failed"; change it to mention "card validation failed" or a neutral phrasing
like "Validation failed or invalid effective_at/invalid_at" to match the card
endpoint and avoid confusion. Ensure the updated message stays under the same
example key "unprocessable_entity" and keeps the same error code
"unprocessable_entity".
In `@openapi.yaml`:
- Around line 236-251: Update the POST /v1/cards/{organization_id} 422 response
in openapi.yaml to match openapi.json: expand the description to include the
cross-field time validation constraint text, update the example under
content->application/json->examples->unprocessable_entity to the new error
message, and ensure the response still references the Error schema; locate the
POST path entry for /v1/cards/{organization_id} and modify the '422' response
block (schema $ref and examples key unprocessable_entity) to reflect the same
wording and example as openapi.json.
---
Nitpick comments:
In `@openapi.yaml`:
- Around line 5543-5547: The shared CardProperties field descriptions include
PATCH-specific language that can confuse POST consumers; update the effective_at
description (and similarly invalid_at) to operation-agnostic wording such as:
"Must be the current time or a future time and earlier than invalid_at. If both
effective_at and invalid_at are provided in the same request, validation is
performed against the values supplied in that request (not stored values)."
Ensure you change the descriptions in CardProperties so CardInput (POST
/v1/cards) and CardUpdate (PATCH /v1/cards/{organization_id}/{card_number})
inherit the clarified, request-scoped semantics for effective_at and invalid_at.
Summary
effective_atandinvalid_atfield descriptions inCardPropertiesschema to clarify thateffective_atmust be earlier thaninvalid_at422error description to explicitly describe this time validation behavioropenapi.yamlandopenapi.jsonCloses #6
Test plan
openapi.yamlis valid YAMLopenapi.jsonis valid JSONeffective_atdescription now mentions validation againstinvalid_atinvalid_atdescription now mentions validation againsteffective_at🤖 Generated with Claude Code
Summary by CodeRabbit