Skip to content

task#419

Open
shelyaa wants to merge 3 commits intomate-academy:masterfrom
shelyaa:develop
Open

task#419
shelyaa wants to merge 3 commits intomate-academy:masterfrom
shelyaa:develop

Conversation

@shelyaa
Copy link
Copy Markdown

@shelyaa shelyaa commented Aug 21, 2025

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

Decision: REQUEST_CHANGES

Your server is almost perfect and meets nearly all requirements. However, there’s one critical blocker that violates requirement 3.6 and can break expected output for encoded URLs.

Critical issue to fix (must address):

  • Missing URL-decoding of path segments (requirement 3.6)
    • Location: src/createServer.js, in the logic where you split and filter url.pathname into parts (look around the lines where you do pathname.split('/') and .filter(Boolean)).
    • Why it’s wrong: Encoded characters in the path (e.g., /foo%20bar/) remain encoded, so parts becomes ["foo%20bar"] instead of ["foo bar"], violating the spec and likely failing tests that use encoded paths.
    • How to fix: After splitting and filtering, map each segment through decodeURIComponent, e.g., segments.map(s => decodeURIComponent(s)). Keep the filter to remove empty segments. Note: URLSearchParams already decodes query keys/values, so only the path needs this change.

Minor notes (non-blocking):

  • The rest of the implementation is clear, concise, and handles edge cases well. Consider adding a brief comment explaining why decoding is applied to path segments for maintainability.

Once you add decodeURIComponent to each path segment, this should be fully compliant. Great work overall—just one small fix to land this!


✨ 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

res.setHeader('Content-type', 'application/json');

const normalizedURL = req.url.split('?');
const parts = normalizedURL[0].split('/').filter(Boolean);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Path segments are split and filtered, but they are not URL-decoded. According to requirement 3.6, you must apply decodeURIComponent to each segment so that encoded characters (like %20 for space) are correctly represented in the output. Consider mapping each segment through decodeURIComponent after filtering.

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

Decision: REQUEST_CHANGES

Your server is very close to complete and handles most requirements well (JSON response shape, headers, query parsing, empty segments, robustness). However, there’s one critical blocker that violates the specification and will cause tests to fail.

Critical issue to fix (blocker):

  • Path segments are not URL-decoded.
    • Where: src/createServer.js, in the logic that builds parts from the pathname (look for where you do split('/') and filter(Boolean)).
    • Why: The requirements state that each path segment must be URL-decoded (e.g., %20 → space). Returning encoded strings breaks compliance and yields incorrect output for encoded URLs.
    • How to fix: After filtering empty segments, map each segment with decodeURIComponent, for example: const parts = normalizedURL[0].split('/').filter(Boolean).map(decodeURIComponent);
    • Tip: Consider wrapping the decode in a try/catch to avoid unexpected crashes on malformed encodings.

Minor (non-blocking) suggestions:

  • Add a short comment explaining why decoding is required for maintainability.

Once you apply the decoding, the implementation should fully meet the requirements. Nice work on everything else—this is a quick, focused fix to get you across the finish line!


✨ 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

res.setHeader('Content-type', 'application/json');

const normalizedURL = req.url.split('?');
const parts = normalizedURL[0].split('/').filter(Boolean);
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 parts array is created by splitting the path and filtering out empty segments, but it does not decode URL-encoded characters in each segment. According to the requirements, each path segment must be URL-decoded (e.g., %20 becomes a space). You should map each segment through decodeURIComponent after filtering. For example:

const parts = normalizedURL[0].split('/').filter(Boolean).map(decodeURIComponent);

This will ensure that encoded path segments are correctly handled, as required by the task.

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

Educational Approval: Great job! This solution meets the requirements for the HTTP server: it splits and URL-decodes path segments using decodeURIComponent, parses the query with URLSearchParams, and always returns a JSON object with parts (array) and query (object).

What you did well

  • Clear, concise logic in src/createServer.js with robust handling of multiple/leading/trailing slashes.
  • Correct URL-decoding of path segments and string-only query values.
  • Consistent response shape: { parts: [...], query: { ... } }.

Minor improvement opportunities (optional, for learning):

  • If you ever need deterministic behavior for duplicate query keys, note that URLSearchParams takes the last value by default; if you prefer first-value-wins, you can guard by checking if (!(key in obj)) obj[key] = value; when building the object.
  • Consider setting explicit headers like Content-Type: application/json; charset=utf-8 to be explicit and client-friendly.

You’re on the right track—clean, robust, and specification-accurate. Keep it up!


✨ 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