API contract for OOO feature #249
Conversation
Summary by CodeRabbit
WalkthroughDocumentation updates in requests/README.md adjust the Requests API contract: the GET /requests example now includes a multi-line comment describing field remapping, and the POST /requests body fields are renamed from message/state to reason/status. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| /* Additional request objects | ||
| //NOTE : In case of dev=true the response would be send as above but in case of dev=false : | ||
| message --> reason, | ||
| state --> status, | ||
| reason --> comment | ||
| */ |
There was a problem hiding this comment.
There appears to be a field naming inconsistency in the API contract. The comment at lines 73-77 indicates that when dev=false, fields are mapped as:
message --> reason
state --> status
reason --> comment
However, in the field documentation at lines 141-142, reason and status are already defined as the primary field names for request creation.
To maintain a clear API contract, please:
- Clarify which field names are canonical in the API (either
message/stateorreason/status) - Update both the comment and field documentation to use consistent terminology
- Consider adding a note about backward compatibility if both naming conventions need to be supported
This will help prevent confusion for developers implementing against this API.
| /* Additional request objects | |
| //NOTE : In case of dev=true the response would be send as above but in case of dev=false : | |
| message --> reason, | |
| state --> status, | |
| reason --> comment | |
| */ | |
| /* Additional request objects | |
| //NOTE : For backward compatibility, the API supports two field naming conventions: | |
| Canonical field names: reason, status, comment | |
| Alternative field names (when dev=true): message, state, reason | |
| When dev=false, the alternative field names are mapped to their canonical equivalents. | |
| */ |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
requests/README.md (5)
192-193: Return 201 Created for POST success.Success for resource creation should be 201 (consistent with the Onboarding example below).
- - **Code:** 200 + - **Code:** 201
429-431: Mismatch between HTTP code (403) and payload statusCode (401).Make the payload’s
statusCodematch the HTTP code.- - **Code:** 403 - - **Content:** `{ "statusCode": 401, "error": "Forbidden", "message": "Only super users are allowed to acknowledge OOO requests" }` + - **Code:** 403 + - **Content:** `{ "statusCode": 403, "error": "Forbidden", "message": "Only super users are allowed to acknowledge OOO requests" }`
377-377: Fix typo.“exsisting” → “existing”.
- - This endpoint updates the details of any type of exsisting request (OOO, ONBOARDING etc.). + - This endpoint updates the details of any type of existing request (OOO, ONBOARDING, etc.).
92-93: Align empty/“not found” response semantics
File: requests/README.md Lines: 92-93- **Code:** 204 - **Content:** No contentEither rename this section to “No Content” for a 204 response, or switch to:
- 200 with an empty
data: [], or- 404 with an error object for “Not Found” semantics.
24-30: Renamestatefilter tostatus
- In GET /requests Query Parameters (requests/README.md lines 24–30), change
state→statusto align with the production response field.- Keep
stateas a deprecated alias (log a warning or note deprecation) for backward compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
requests/README.md(2 hunks)
| /* Additional request objects | ||
| //NOTE : In case of dev=true the response would be send as above but in case of dev=false : | ||
| message --> reason, | ||
| state --> status, | ||
| reason --> comment | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove invalid JSON comment and avoid schema changes behind a dev flag.
- The block comment inside the JSON sample makes the payload invalid.
- Changing field names based on
devtoggles is a breaking contract; pick a canonical schema (preferreason,comment,status) and document any legacy aliases separately.
Apply this diff to keep the sample valid; move any notes outside the JSON code block:
- /* Additional request objects
- //NOTE : In case of dev=true the response would be send as above but in case of dev=false :
- message --> reason,
- state --> status,
- reason --> comment
- */Follow-up (outside this range): add a Markdown “Notes” subsection clarifying the single, canonical response schema and any deprecated aliases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Additional request objects | |
| //NOTE : In case of dev=true the response would be send as above but in case of dev=false : | |
| message --> reason, | |
| state --> status, | |
| reason --> comment | |
| */ |
🤖 Prompt for AI Agents
In requests/README.md around lines 73 to 78, the JSON sample contains an inline
block comment which makes the payload invalid and documents conditional field
renames behind a dev flag; remove the comment from inside the JSON code block
and update the sample to use the single canonical schema (reason, comment,
status) so the example is valid JSON, then move the explanatory note about
legacy aliases and the dev flag into a separate "Notes" Markdown subsection
below the code block that documents the canonical field names and any deprecated
aliases (e.g., message→reason, state→status, reason→comment) rather than
changing the schema conditionally.
| - `reason`: Required string to specify the reason for the request. | ||
| - `status`: Required string to specify the status of the request (e.g., PENDING). | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not require client-supplied status on create; align POST body with canonical fields.
Creation should not accept status; the server sets it (typically PENDING). Keep reason only.
- - `reason`: Required string to specify the reason for the request.
- - `status`: Required string to specify the status of the request (e.g., PENDING).
+ - `reason`: Required string to specify the reason for the request.Follow-up: update ALL request examples to stop using message/state and consistently use reason/comment/status where applicable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `reason`: Required string to specify the reason for the request. | |
| - `status`: Required string to specify the status of the request (e.g., PENDING). | |
| - `reason`: Required string to specify the reason for the request. |
🤖 Prompt for AI Agents
In requests/README.md around lines 141 to 143, the docs currently list
client-supplied `status` as required for create; remove `status` from the
POST/create field list so only `reason` is required and note that the server
sets `status` (e.g., PENDING). Update all request creation examples in the
README to omit `status` (and stop using `message/state`) and instead use the
canonical fields `reason` (and `comment` where applicable) and show `status`
only in server responses or later PUT/PATCH examples.
Date: 01-09-2025
Developer Name: Rishi Chaubey
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Revise the API contract document for the Out-Of-Office (OOO) feature by updating the field names in the JSON response and request schema for clarity and consistency.
Why are these changes being made?
To ensure consistency and clarity when toggling between development (
dev=true) and production (dev=false) modes, the response and request field names have been standardized. This change also aligns the naming within the API to improve maintainability and reduce confusion among developers using the API.