Solution #1000
Solution #1000
Conversation
There was a problem hiding this comment.
Review Summary
Decision: Request Changes — There is a critical blocker that breaks functionality.
- Critical bug: In
src/createServer.js, the call toconvertToCaseuses reversed arguments. Per requirement, it must beconvertToCase(toCase, text), but the code callsconvertToCase(text, toCase). This will produce incorrect conversions or failures. Please swap the arguments to match the required signature. - Requirement mismatch: The response’s
originalTextshould be the exact string from the URL path. Currently, you setoriginalTextto the decoded/trimmedtext. Instead, assignoriginalTextfrom the raw path segment (e.g.,rawText = path.slice(1)), and keep using a separately decoded/trimmed value for conversion if desired.
Once these are fixed, the rest of the implementation looks solid. Nice work on validation, consistent JSON responses, and proper status codes.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/createServer.js
Outdated
| return; | ||
| } | ||
|
|
||
| const result = convertToCase(text, toCase); |
There was a problem hiding this comment.
Critical: The call to convertToCase uses the wrong argument order. The task requires calling the business logic with (toCase, text) where the first param is the requested case name and the second is the text to convert. Your code calls convertToCase(text, toCase), which will pass the wrong values and break the conversion logic. Please swap the arguments so it becomes convertToCase(toCase, text). This violates the requirement “convertToCase must be invoked with two parameters: the requested case name and the text to convert.”
| const rawText = path.slice(1); | ||
| const text = decodeURIComponent(rawText).trim(); |
There was a problem hiding this comment.
Validation of the original input vs. what you return: you compute rawText = path.slice(1) then text = decodeURIComponent(rawText).trim(). The task states originalText should be the input text "exact as in URL path". Trimming and decoding can change the exact value. Consider keeping originalText equal to rawText (no trim/decoding) so the response matches the path literally, and use a separate decoded/trimmed value for conversion if decoding is desired. This ensures you meet the requirement that originalText reflects the URL path exactly.
src/createServer.js
Outdated
| { | ||
| originalCase: result.originalCase, | ||
| targetCase: toCase, | ||
| originalText: text, |
There was a problem hiding this comment.
Currently originalText in the success response is set to text (the decoded/trimmed value). Per the requirement, originalText should be the input text exactly as in the URL path. Use the raw path segment (e.g., rawText) for originalText so tests that assert the original value receive the exact string from the URL path.
There was a problem hiding this comment.
Review Summary
Decision: Request Changes
Your server implementation is solid overall and meets most requirements (exports createServer, uses Node's http, correct URL/query parsing, validation, headers, and response formats). However, there is a critical blocker that breaks functionality.
-
Critical blocker (must fix): In
src/createServer.js, the call to the conversion function passes arguments in the wrong order. On the success path you currently callconvertToCase(text, toCase). The requirement isconvertToCase(caseName, text), so this should beconvertToCase(toCase, text). This mismatch causes incorrect conversions and will fail tests. Please swap the arguments accordingly. If applicable, this likely occurs on the line where you buildconvertedText(look for the single call toconvertToCase). -
Minor improvement (optional): Consider guarding
decodeURIComponentto avoid a 500 on malformed encodings and return a 400 with a clear error message instead.
Once you swap the arguments, the implementation should satisfy the specification. Nice work on the rest of the requirements!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/createServer.js
Outdated
| return; | ||
| } | ||
|
|
||
| const result = convertToCase(text, toCase); |
There was a problem hiding this comment.
Critical: The call to the conversion function uses reversed arguments. The task requires calling convertToCase(caseName, text), but here you call convertToCase(text, toCase). Swap the arguments to convertToCase(toCase, text) so the conversion logic receives the case name first and the text second. This is causing incorrect conversions or failures and must be fixed to satisfy the requirement.
There was a problem hiding this comment.
Review Summary
Great job addressing the previous feedback on the convertToCase arguments! Your server's logic for parsing and validation is solid. I'm approving this submission as an educational override to help you progress, but there is one small but important fix needed to get the tests passing.
The originalText field in your success response is currently sending the raw, URL-encoded string instead of the clean, decoded version.
To fix this, you should use the text variable, which already holds the decoded string. In src/createServer.js, changing originalText: rawText to originalText: text will resolve the issue. This ensures the API returns clean, human-readable text as expected. Fantastic progress!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.