Skip to content

fear: implement a server for case transformer - #1179

Open
linska wants to merge 2 commits into
mate-academy:masterfrom
linska:dev
Open

linska wants to merge 2 commits into
mate-academy:masterfrom
linska:dev

Conversation

@linska

@linska linska commented Aug 15, 2026

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I’m requesting changes because there is a critical validation bug that can block correct behavior for specified inputs. In src/checkRequest.js, lines 28-29 perform an early return after adding the toCase-missing error; this prevents collecting multiple errors when both text and toCase are missing, which violates the requirement to return all validation failures. Additionally, the if (!toCase) check (around lines 25-29) conflates missing and invalid values and can interfere with proper validation flow; the function should distinguish between a truly missing toCase and an invalid toCase value so the invalidToCase check can run when appropriate. To fix this, remove the early return so both errors can be accumulated, and adjust the condition to check specifically for toCase === undefined (or similar) before proceeding to the invalid-case check. The rest of the server logic (parsing, invoking convertToCase, response shapes, headers, and status codes) looks solid and aligns with the requirements. Minor note: using the conventional status text 'Bad Request' (capital R) is stylistic and not a blocker. Keep up the good work—once the validation logic is corrected, this should be good to go.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/checkRequest.js
Comment on lines +8 to +10
},
noToCase: {
message:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checklist validation: The code correctly exports createServer (checklist items #1, #14) and returns an http server created via http.createServer (items #2, #15). It parses the request using parseRequest (line 10), validates using checkRequest (line 11), and calls convertToCase (line 19) when validation passes (items #7, #26, #27). The response objects and headers match the required JSON structure and Content-Type header (lines 28-29 and 14-16), and status codes/statusText are used as required (lines 14 and 28).

Comment thread src/checkRequest.js Outdated
Comment on lines +19 to +29
const checkRequest = (text, toCase) => {
const errors = [];

if (!text) {
errors.push(errorCases.noText);
}

if (!toCase) {
errors.push(errorCases.noToCase);

return errors;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validation logic is implemented in src/checkRequest.js and returns an array of error objects matching the required messages (lines 3, 4-17, 22-36). It correctly accumulates both missing text and missing toCase when appropriate and returns immediately after adding the toCase missing error (line 29) which matches intended behavior since further validation of toCase value isn't meaningful when it's missing (checklist items #19-#25).

Comment thread src/checkRequest.js
'"toCase" query param is required. Correct request is: "/<TEXT_TO_CONVERT>?toCase=<CASE_NAME>".',
},
invalidToCase: {
message:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion: In createServer.js you return the response with writeHead including status text strings 'Bad request' and 'OK' which are acceptable, but note Node's conventional status message for 400 is 'Bad Request' (capital R). Tests likely check numeric status and body, so this is not a functional failure—just a stylistic note.

@linska

linska commented Aug 15, 2026

Copy link
Copy Markdown
Author

@mateacademy-ai-mentor

I don't think the suggested change is necessary here.

The early return does not prevent collecting both errors when text and toCase are missing. The text validation runs first, so in that case the flow is:

errors.push(errorCases.noText);
errors.push(errorCases.noToCase);
return errors;

As a result, both required errors are returned.

The early return is intentional: once toCase is missing, there is no reason to additionally validate whether it is included in supportedCases. Without the early return, a missing toCase could also produce the invalidToCase error, which would be incorrect.

Also, toCase comes from URLSearchParams.get(), which returns null when the parameter is missing, not undefined. Therefore, checking specifically for toCase === undefined would not correctly handle the missing parameter in this implementation.

For these reasons, I believe the current validation flow satisfies the requirements.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because the validation in src/checkRequest.js conflates a missing toCase with an invalid value and returns early, which violates explicit requirements to distinguish these cases and to accumulate multiple errors. Specifically, the check uses a falsy condition (if (!toCase)) and returns immediately, preventing the invalidToCase check from running when toCase is present but invalid and risking incorrect handling of empty strings or null from URLSearchParams.get() [CHECKLIST ITEMS #18, #19, #22, #29, #30]. To fix this:

  • Detect missing with toCase == null (covers null and undefined) and push noToCase without returning.
  • Only if toCase != null, run !supportedCases.includes(toCase) and push invalidToCase.
  • Remove the early return so both noText and noToCase can be collected when both are absent.
    The rest of createServer and main.js looks solid and aligned with the requirements. Once the missing-vs-invalid logic is corrected, the implementation should meet the task standards.

✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/checkRequest.js Outdated
Comment on lines +26 to +29
if (!toCase) {
errors.push(errorCases.noToCase);

return errors;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkRequest currently returns early when toCase is falsy. This conflates "missing" and "invalid" states and prevents the invalid-case check from running when appropriate. According to the requirements you must: (a) report noToCase when toCase is truly missing, and (b) report invalidToCase when toCase is present but not one of the supported cases. Also avoid the early return so multiple errors can be collected when applicable. Consider specifically testing for toCase == null (which covers both null and undefined) to detect a missing param, and only run the supportedCases.includes check when toCase is not null. This aligns with checklist items #18, #19, #22, #29 and #30.

Comment thread src/checkRequest.js
errors.push(errorCases.noText);
}

if (!toCase) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because URLSearchParams.get() returns null when a parameter is missing, use a missing check like toCase == null rather than !toCase. That prevents treating empty-string or other falsy-but-present values the same as missing. Then, only if toCase != null run supportedCases.includes(toCase) to determine invalid-toCase.

Comment thread src/createServer.js
Comment on lines +25 to +29
convertedText: result.convertedText,
};

res.writeHead(200, 'OK', { 'Content-Type': 'application/json' });
res.end(JSON.stringify(response));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check here uses a falsy check and returns immediately after pushing the noToCase error. This conflates missing and invalid cases. According to the task you must distinguish a truly missing toCase from an invalid value and allow collecting multiple errors (e.g. when both text and toCase are missing). Change this check to explicitly detect a missing param (for example toCase == null which covers null and undefined), remove the early return so multiple errors can be accumulated, and only run the invalidToCase check when toCase is present.

Comment thread src/createServer.js
Comment on lines +32 to +33
return server;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The supported-cases membership check is fine, but it should only execute when toCase is present. After adjusting the missing check as suggested, this includes check will behave correctly.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Here is my approval with an educational override to help you progress while learning. You’ve done a great job overall: the server is correctly created and exported, parses requests, invokes the business logic, and responds with proper JSON and headers. The main issue to address is in src/checkRequest.js: at lines where you validate parameters, using if (!toCase) conflates missing vs. invalid values and can prevent accumulating multiple errors. Per requirements, separate the conditions as: if toCase == null then add the missing error; else if !supportedCases.includes(toCase) then add the invalid-value error. Similarly, use text == null instead of !text to detect a truly missing text value, and avoid early returns so both errors can be reported together [CHECKLIST ITEM #19, #20, #23]. Minor: keep error ordering consistent (e.g., text first, then toCase-related) for predictable outputs. Overall, strong work—these small validation tweaks will make your solution fully compliant and more robust.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/checkRequest.js
errors.push(errorCases.noText);
}

if (!toCase) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current check uses a falsy check if (!toCase) which conflates missing toCase (null) and other falsy values. Per requirements you must detect a missing parameter with toCase == null and only when toCase != null check if it's supported. Also avoid early returns so multiple errors (e.g., missing text and missing toCase) can be returned together.

Comment thread src/checkRequest.js

if (!toCase) {
errors.push(errorCases.noToCase);
} else if (!supportedCases.includes(toCase)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the falsy check above, an invalid value for toCase (e.g. toCase=UNKNOWN) will be correctly detected only when toCase is present; however the logic should explicitly separate the missing-vs-invalid cases as described in the task and previous review guidance. Change the condition to: if (toCase == null) { errors.push(...) } else if (!supportedCases.includes(toCase)) { errors.push(...) }.

Comment thread src/createServer.js
targetCase: toCase,
originalText: text,
convertedText: result.convertedText,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check treats any falsy toCase (including empty string) as missing and immediately skips the invalid-value check. Per requirements you must detect missing toCase using toCase == null (so only null/undefined are considered missing) and, when toCase is present but not supported, add the invalidToCase error. Also ensure you do not return early so errors for both missing text and missing/invalid toCase can be accumulated. Update the conditions accordingly (do not use if (!toCase)).

Comment thread src/createServer.js
convertedText: result.convertedText,
};

res.writeHead(200, 'OK', { 'Content-Type': 'application/json' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the if (!toCase) branch prevents executing the invalidToCase check when toCase is present but falsy, change the logic to: if (toCase == null) { push noToCase } else if (!supportedCases.includes(toCase)) { push invalidToCase }. This will satisfy checklist items #19, #20 and #23 (accumulating errors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants