Skip to content

Solution#1107

Open
mykytalandar wants to merge 2 commits into
mate-academy:masterfrom
mykytalandar:develop
Open

Solution#1107
mykytalandar wants to merge 2 commits into
mate-academy:masterfrom
mykytalandar:develop

Conversation

@mykytalandar
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

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

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 there are a couple of functional issues that will cause the server to fail the required tests and not behave as specified. In src/checkErrors.js, the three validation error messages do not exactly match the required strings: they are missing the final period in the messages for missing text, missing toCase, and unsupported toCase values; these must be updated to end exactly with "...toCase=<CASE_NAME>." and Available cases: SNAKE, KEBAB, CAMEL, PASCAL, UPPER. including the trailing . so string comparisons pass. In both src/checkErrors.js and src/createServer.js, new URLSearchParams(normalizedUrl) is not correctly extracting the query string, so toCase will be null even when provided; you should instead use normalizedUrl.searchParams.get('toCase') or new URLSearchParams(normalizedUrl.search).get('toCase') to correctly read the query parameter. These two issues are critical because they break the expected behavior for valid requests and make error handling fail the specification, so they must be fixed before the solution can be considered complete. As minor improvements, you might also rename isErrors in src/createServer.js to errors (since it holds an array, not a boolean) and ensure all similar error messages are kept consistent by centralizing the text in one place. Once you adjust the error message strings and fix the query-parameter parsing, your overall structure (using createServer, returning JSON with the correct fields, and delegating conversion to convertToCase) looks solid and should meet the task 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/checkErrors.js Outdated
if (!textToConvert) {
errors.push({
message:
'Text to convert is required. Correct request is: "/<TEXT_TO_CONVERT>?toCase=<CASE_NAME>"',
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 error message for missing text must match the requirement exactly (including the final period). According to the spec it should end with: "...toCase=<CASE_NAME>". — add the trailing period inside the string so it matches exactly.

Comment thread src/checkErrors.js Outdated
if (!caseName) {
errors.push({
message:
'"toCase" query param is required. Correct request is: "/<TEXT_TO_CONVERT>?toCase=<CASE_NAME>"',
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 error message for a missing toCase param must match the required exact string, including the trailing period after the quoted example. Update the string to end with "...toCase=<CASE_NAME>". so tests expecting the exact message pass.

Comment thread src/checkErrors.js Outdated
if (!availableCases.includes(caseName)) {
errors.push({
message:
'This case is not supported. Available cases: SNAKE, KEBAB, CAMEL, PASCAL, UPPER',
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 unsupported-case error message must exactly match the required message, including the final period. Add a trailing . at the end of the string: "...UPPER.

Comment thread src/checkErrors.js
});
}

if (!caseName) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrect usage of URLSearchParams: new URLSearchParams(normalizedUrl) will not extract the query parameters and params.get('toCase') will be null. Use the URL object's searchParams property (normalizedUrl.searchParams.get('toCase')) or pass normalizedUrl.search to new URLSearchParams(...).

Comment thread src/createServer.js

function createServer() {
const server = http.createServer((req, res) => {
res.setHeader('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.

The string is missing the final period required by the spec. Error messages must match exactly. Update this message to end with a dot: ...toCase=<CASE_NAME>. (note the trailing period after the closing quote).

Comment thread src/createServer.js
const params = new URLSearchParams(normalizedUrl);
const caseName = params.get('toCase');

const isErrors = checkErrors(textToConvert, caseName);
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 string is missing the final period required by the spec. Update this message to exactly: "toCase" query param is required. Correct request is: "/<TEXT_TO_CONVERT>?toCase=<CASE_NAME>". (ensure the trailing period).

Comment thread src/createServer.js
res.statusCode(400);
res.statusMessage = 'Bad request';

res.end(
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 message is missing the final period. Per requirements it should end with a dot: This case is not supported. Available cases: SNAKE, KEBAB, CAMEL, PASCAL, UPPER.

Comment thread src/createServer.js Outdated

const normalizedUrl = new URL(req.url, `http://${req.headers.host}`);
const textToConvert = normalizedUrl.pathname.slice(1);
const params = new URLSearchParams(normalizedUrl);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You are creating URLSearchParams incorrectly. new URLSearchParams(normalizedUrl) will not extract the query from the URL object. Use normalizedUrl.searchParams.get('toCase') or new URLSearchParams(normalizedUrl.search) to obtain the toCase value.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it correctly implements all the required server behavior: createServer is exported and returns an http server, the URL and query parameters are parsed properly using the URL API, validations are performed via checkErrors, and responses are always JSON with the correct status codes and exact required error/success payloads. The validation messages for missing text, missing toCase, and unsupported toCase match the specification precisely, including punctuation, and valid requests correctly delegate to convertToCase(text, toCase) and return the expected response shape. There are no critical or blocking issues found that would prevent the task from being functionally complete or from passing the described checks.

For minor improvements, you could rename the isErrors array in src/createServer.js to something like errors to better reflect that it holds a list rather than a boolean, which makes the code easier to read. You might also consider making the URL base a bit more robust by defaulting req.headers.host to 'localhost' if it is missing, and optionally handling or clearly ignoring non-GET methods to make the server’s behavior more explicit. Overall, this is a solid, requirement-compliant implementation—nice work.


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

Footnotes

  1. Rate AI review example

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