chore(docs): fix example path parameter #625
Conversation
WalkthroughThe documentation and code examples were updated to mark the route parameter Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| <code>/tasks/byPriority/{priority?}</code> | ||
| where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime, | ||
| and the question mark is used to indicate that a parameter is optional. The query parameter can have | ||
| any name you like, but <code>priority</code> seems the obvious choice. |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix terminology and resolve implementation inconsistency.
There are several issues with this documentation update:
-
Incorrect terminology: The text refers to
{priority?}as a "query parameter", but this is actually a path parameter. Query parameters are key-value pairs that appear after?in URLs (e.g.,?key=value). -
Implementation mismatch: The documentation shows the route as
/tasks/byPriority/{priority?}(optional), but the actual implementation on line 404 defines it as/tasks/byPriority/{priority}(required). -
Logical inconsistency: If the parameter is truly optional, the implementation should handle missing parameters differently. Currently, the code returns
400 Bad Requestwhen the parameter is null (lines 406-409), which suggests it's required.
Apply this diff to fix the terminology:
-where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime,
-and the question mark is used to indicate that a parameter is optional. The query parameter can have
+where <code>{priority?}</code> represents a path parameter that you will need to extract at runtime,
+and the question mark is used to indicate that a parameter is optional. The path parameter can haveAdditional action needed: Either update the route implementation to match the optional syntax, or revert the documentation to show the required parameter syntax {priority} to match the current implementation.
Let me verify if there are other route definitions that need to be updated consistently:
🏁 Script executed:
#!/bin/bash
# Description: Check for route definitions and parameter handling patterns in the documentation
# Search for route patterns with optional parameters
rg -A 3 -B 3 '\{[^}]*\?\}'
# Search for route definitions in code blocks
rg -A 5 -B 5 'get\("/[^"]*\{[^}]*\}[^"]*"\)'Length of output: 39761
Fix terminology and align documentation with implementation
The update introduces two issues that must be addressed:
• Incorrect terminology:
The text calls {priority?} a “query parameter” when it’s actually a path parameter.
• Docs/implementation mismatch:
Documentation shows /tasks/byPriority/{priority?} (optional) but the sample code still defines and handles it as a required parameter (get("/tasks/byPriority/{priority}")), returning 400 on null.
Please update the docs or code so they match:
- In
topics/server-requests-and-responses.topic(lines 374–377), change “query parameter” → “path parameter” and decide on optional vs. required syntax. For example, if you keep it required, revert to{priority}:
- <code>/tasks/byPriority/{priority?}</code>
+ <code>/tasks/byPriority/{priority}</code>
- where <code>{priority?}</code> represents a query parameter …
- and the question mark is used to indicate that a parameter is optional.
+ where <code>{priority}</code> represents a path parameter that you will extract at runtime.- If you truly want an optional parameter (
{priority?}), update the Kotlin routing code (around line 404) to useget("/tasks/byPriority/{priority?}")and handle the missing case (e.g., treat null as “all priorities” instead of BadRequest).
📝 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.
| <code>/tasks/byPriority/{priority?}</code> | |
| where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime, | |
| and the question mark is used to indicate that a parameter is optional. The query parameter can have | |
| any name you like, but <code>priority</code> seems the obvious choice. | |
| <code>/tasks/byPriority/{priority?}</code> | |
| - where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime, | |
| - and the question mark is used to indicate that a parameter is optional. The query parameter can have | |
| + where <code>{priority?}</code> represents a path parameter that you will need to extract at runtime, | |
| + and the question mark is used to indicate that a parameter is optional. The path parameter can have | |
| any name you like, but <code>priority</code> seems the obvious choice. |
🤖 Prompt for AI Agents
In topics/server-requests-and-responses.topic around lines 374 to 377, the
documentation incorrectly calls `{priority?}` a query parameter when it is a
path parameter, and it shows the parameter as optional while the implementation
treats it as required. Fix this by changing "query parameter" to "path
parameter" and either revert the route in the docs to
`/tasks/byPriority/{priority}` to match the required parameter in the code or
update the Kotlin routing code near line 404 to define the route as
`/tasks/byPriority/{priority?}` and handle the null case gracefully instead of
returning 400.
previous example cannot match "paramater is absent" case