Skip to content

Solution added#2034

Open
DDefty wants to merge 4 commits into
mate-academy:masterfrom
DDefty:master
Open

Solution added#2034
DDefty wants to merge 4 commits into
mate-academy:masterfrom
DDefty:master

Conversation

@DDefty

@DDefty DDefty commented Sep 27, 2025

Copy link
Copy Markdown

…lement PostList and CommentList components

@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 a few core requirements and explicit checklist items are not met, which blocks acceptance.

Critical issues to fix:

  • App must prepare posts with related data before rendering. In src/App.tsx you pass postsFromServer directly to PostList (see the line with <PostList posts={postsFromServer} />). The requirement says: “prepare posts by adding a user and comments to each post (each comment has a postId)”. Implement a helper like getPostsWithUsersAndComments (or preparePostsWithRelations) that maps posts to include user (matched by userId) and comments (matched by postId), then pass that prepared array to PostList.
  • Move relation logic out of PostInfo. In src/components/PostInfo/PostInfo.tsx you import commentsFromServer and usersFromServer and compute related data as a fallback. This contradicts the requirement to prepare posts upstream. After preparing in App, PostInfo should rely on post.user and post.comments directly.
  • Remove unnecessary React fragments around a single root element (checklist item #4). Files to fix: src/components/CommentInfo/CommentInfo.tsx, src/components/CommentList/CommentList.tsx, src/components/PostInfo/PostInfo.tsx, src/components/PostList/PostList.tsx, src/components/UserInfo/UserInfo.tsx. Return the single root element directly without <>...</>.
  • Use centralized types. In src/components/UserInfo/UserInfo.tsx you declare a local interface User. Replace it by importing Users from src/types/Users and use that in props.

Minor but recommended:

  • Ensure styles are moved into component-level SCSS (e.g., add import './PostList.scss'; in PostList and move styles from App.scss as required by the task).

Once you prepare posts in App, remove redundant fragments, and use the shared Users type, the submission should meet the requirements. Good job on keys and preserving class names—finish these fixes and you’ll be set.


✨ 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/App.tsx
import { PostList } from './components/PostList';

// import postsFromServer from './api/posts';
import postsFromServer from './api/posts';

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 project requirement says: "prepare posts by adding a user and comments to each post (each comment has a postId);". Here you import the raw posts array from the API but do not prepare/enrich it before passing to PostList. Prepare the posts first (map postsFromServer -> add matching user and comments) and pass the prepared array to PostList. Consider naming the helper getPostsWithUsersAndComments or preparePostsWithRelations to follow the checklist guidance on naming filter functions.

Comment thread src/App.tsx Outdated
</div>
</div>
</div>
<PostList posts={postsFromServer} />

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 passing the unmodified posts array to the list here: <PostList posts={postsFromServer} />. Replace postsFromServer with the prepared/enriched posts variable (for example preparedPosts returned by getPostsWithUsersAndComments) so the app meets the task requirement to add user and comments to each post before rendering.

Comment on lines +9 to +23
<>
<div className="CommentInfo">
<div className="CommentInfo__title">
<strong className="CommentInfo__name">{comment.name}</strong>

{' by '}

<a className="CommentInfo__email" href={`mailto:${comment.email}`}>
{comment.email}
</a>
</div>

<div className="CommentInfo__body">{comment.body}</div>
</div>
</>

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 component is wrapped in a React fragment even though there is already a single root <div>. That violates checklist item #4: "4. [CODE KNOWLEDGE] - Don't use React fragment, if you already have a wrapper". Remove the fragment and return the <div className="CommentInfo"> directly (no fragment needed).

Comment on lines +10 to +16
<>
<div className="CommentList">
{comments.map(comment => (
<CommentInfo key={comment.id} comment={comment} />
))}
</div>
</>

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 component wraps the single root <div className="CommentList"> inside a React fragment (<> / </>). That violates checklist item #4: "4. [CODE KNOWLEDGE] - Don't use React fragment, if you already have a wrapper". Remove the fragment and return the <div className="CommentList"> directly.

Comment thread src/components/PostInfo/PostInfo.tsx Outdated
Comment on lines +3 to +4
import commentsFromServer from '../../api/comments';
import usersFromServer from '../../api/users';

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 component imports API data directly: commentsFromServer and usersFromServer and then performs filtering/finding here. That contradicts the task requirement: "- prepare posts by adding a user and comments to each post (each comment has a postId);". Move relation preparation to App (or a dedicated helper) so PostInfo only receives an already-enriched post (with user and comments fields). See description: "prepare posts by adding a user and comments to each post (each comment has a postId);" .

Comment thread src/components/PostInfo/PostInfo.tsx Outdated
Comment on lines +20 to +42
<>
<div className="PostInfo">
<div className="PostInfo__header">
<h3 className="PostInfo__title">{post.title}</h3>

<p>
{' Posted by '}

{user ? <UserInfo user={user} /> : null}
</p>
</div>

<p className="PostInfo__body">{post.body}</p>

<hr />

{commentsArr.length === 0 ? (
<b data-cy="NoCommentsMessage">No comments yet</b>
) : (
<CommentList comments={commentsArr} />
)}
</div>
</>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A React fragment wraps the single root <div> returned by the component. This violates the checklist: "4. [CODE KNOWLEDGE] - Don't use React fragment, if you already have a wrapper". Remove the fragment and return the <div className="PostInfo"> directly.

Comment thread src/components/PostList/PostList.tsx Outdated
Comment on lines +9 to +15
<>
<div className="PostList">
{posts.map(post => (
<PostInfo key={post.id} post={post} />
))}
</div>
</>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove the unnecessary React fragment wrapping the single root <div>. This violates checklist item #4: "4. [CODE KNOWLEDGE] - Don't use React fragment, if you already have a wrapper". Return the <div className="PostList"> directly instead of <> … </> so the component has a single root element without a redundant fragment. Example fix: replace the fragment and the wrapped <div> with just the <div className="PostList"> block.

Comment thread src/components/PostList/PostList.tsx
Comment thread src/components/UserInfo/UserInfo.tsx Outdated
Comment on lines +3 to +8
interface User {
id: number;
name: string;
username: string;
email: string;
}

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 file defines a local interface User (lines 3–8), but the task requires creating and using types from ./src/types/: "create all required types in ./src/types/;". Instead of redeclaring the interface here, import the shared type from src/types/Users.ts (e.g., import { Users } from '../../types/Users';) and use Users for the prop type. The repo already contains src/types/Users.ts with the Users interface.

Comment thread src/components/UserInfo/UserInfo.tsx Outdated
Comment on lines +15 to +19
<>
<a className="UserInfo" href={`mailto:${user.email}`}>
{user.name}
</a>
</>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A React fragment wraps the single root <a> element. This violates checklist item #4: "4. [CODE KNOWLEDGE] - Don't use React fragment, if you already have a wrapper". Remove the fragment (<> / </>) and return the <a className="UserInfo" ...> directly.

…reparePostsWithRelations function for post-user-comment relationships
@DDefty DDefty changed the title Refactor components and add types for posts, comments, and users; imp… Solution added Sep 27, 2025

@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

Decision: REQUEST_CHANGES — there are critical blockers preventing task completion and CI success.

Blocking issues to fix:

  1. ESLint lint failure (CI blocker): In src/types/Posts.ts, line with import { Comments } from './Comments.ts'; violates the import/extensions rule. Change it to import { Comments } from './Comments'; so CI can pass.
  2. Required stylesheet refactor not done: src/App.tsx still imports ./App.scss (top of file). The task requires moving styles into per-component SCSS files and importing them from components. Create and import:
    • src/components/PostList/PostList.scss and add import './PostList.scss'; in PostList.tsx.
    • src/components/PostInfo/PostInfo.scss and add import './PostInfo.scss'; in PostInfo.tsx.
    • src/components/CommentList/CommentList.scss and add import './CommentList.scss'; in CommentList.tsx.
    • src/components/CommentInfo/CommentInfo.scss and add import './CommentInfo.scss'; in CommentInfo.tsx.
    • src/components/UserInfo/UserInfo.scss and add import './UserInfo.scss'; in UserInfo.tsx.
      Then remove or minimize App.scss to only truly global styles (if any).
  3. Checklist item (#5) string interpolation: In src/components/PostInfo/PostInfo.tsx, replace the literal chunk {' Posted by '} with interpolation, e.g. <p>{Posted by }{user && <UserInfo user={user} />}</p> to follow the style guideline.

Minor (non-blocking) note: The rest of the component structure, typing, and keys usage look solid. After fixing the above, re-run npm run lint and CI should proceed. Good job on data preparation and component separation; address these items to meet all 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/App.tsx
import React from 'react';

import './App.scss';
import { PostList } from './components/PostList';

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 project still imports a global stylesheet from App (see earlier import of ./App.scss). The task requires moving styles from App.scss into component-specific scss files and importing them inside their components (e.g., PostList.scss imported by PostList.tsx). Please split styles into per-component .scss files and add imports from the corresponding component files so styling is colocated with components.

Comment thread src/components/CommentList/CommentList.tsx
<h3 className="PostInfo__title">{post.title}</h3>

<p>
{' Posted by '}

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 uses a separate JSX string expression for the literal text: {' Posted by '}. The checklist requires: "Use string interpolation inside tag content" (checklist item #5). Consider replacing the literal chunk with a template string for the text and render the UserInfo component next to it, e.g.

{Posted by }{user && }

. This preserves behavior and follows the style guideline.

Comment thread src/components/PostList/PostList.tsx
Comment thread src/components/UserInfo/UserInfo.tsx
…nents; add missing styles for PostInfo and CommentList

@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

Educational approval: you’ve implemented the core functionality well — posts are enriched with users and comments, list/info components are split correctly, keys are set properly, and types are used consistently. A few improvements remain to fully align with the task/checklist:

  • Move component-specific styles out of App.scss (e.g., .App__title) into a component-scoped file and remove the ./App.scss import from src/App.tsx if only global rules remain. This is required by the task description.
  • In src/App.tsx (preparePostsWithRelations), replace inline find/filter with helpers like getUserById(userId) and getCommentsByPostId(postId) to satisfy checklist item #6 and clarify intent.
  • Fix string composition per checklist item #5: in src/components/CommentInfo/CommentInfo.tsx line with {' by '} → use { by }; in src/components/PostInfo/PostInfo.tsx line with {' Posted by '} → use {Posted by }.
  • In src/api/comments.ts, remove /* eslint-disable prettier/prettier */ so Prettier can format the file, or add it to .prettierignore with a short note in the PR.

Overall, nice progress and solid structure — addressing these small style/organization items will bring the solution fully in line with the requirements.


✨ 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/App.tsx
import React from 'react';

import './App.scss';
import { PostList } from './components/PostList';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Task description requires moving styles out of App.scss: "- styles from App.scss should be moved to separate files, as well. E.g.: PostList.scss;" You still import './App.scss' in App.tsx and App.scss contains component-specific styles (e.g., .App__title). Move component-level styles (like .App__title) into a component-scoped SCSS file (or remove the App.scss import if it will only contain global rules). See description: "- styles from App.scss should be moved to separate files, as well. E.g.: PostList.scss;".

Comment thread src/App.tsx
import commentsFromServer from './api/comments';
import usersFromServer from './api/users';

const preparePostsWithRelations = () => {

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 item: "6. [CODE STYLE] - If you are searching some entity by some filter, specify it in function/method name". preparePostsWithRelations currently uses inline find/filter calls. Extract and use named helpers (for example getUserById(userId) and getCommentsByPostId(postId)) so lookups reflect their filtering intent and match checklist guidance.

Comment thread src/App.tsx
const preparePostsWithRelations = () => {
return postsFromServer.map(post => ({
...post,
user: usersFromServer.find(u => u.id === post.userId),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inline user lookup here uses find directly: consider replacing with a helper getUserById(uId) per checklist item #6. This keeps preparePostsWithRelations concise and follows the rule: "If you are searching some entity by some filter, specify it in function/method name".

Comment thread src/App.tsx
return postsFromServer.map(post => ({
...post,
user: usersFromServer.find(u => u.id === post.userId),
comments: commentsFromServer.filter(c => c.postId === post.id),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inline comments grouping here uses filter directly: consider replacing with a helper getCommentsByPostId(postId) per checklist item #6. That makes the intent explicit and satisfies the naming guideline.

Comment thread src/api/comments.ts
@@ -1,3 +1,4 @@
/* eslint-disable prettier/prettier */

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 file disables Prettier enforcement with /* eslint-disable prettier/prettier */. The task instructions request using Prettier formatting (format on save per VSCode settings). Consider removing this directive so Prettier can format the file, or add the file to .prettierignore with a brief justification in the PR if you intentionally skip formatting this large data file.

<div className="CommentInfo__title">
<strong className="CommentInfo__name">{comment.name}</strong>

{' by '}

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 violates checklist item #5: "5. [CODE STYLE] - Use string interpolation inside tag content". The component uses the literal {' by '} at this location which should be replaced with a template-literal expression for readability and to match the style guideline. Suggested replacement:

// replace this line:
{' by '}

// with this:
{` by `}

This keeps the output identical but follows the checklist requirement.

<h3 className="PostInfo__title">{post.title}</h3>

<p>
{' Posted by '}

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 violates checklist item #5: "5. [CODE STYLE] - Use string interpolation inside tag content". The component uses the literal {' Posted by '} at this location; replace it with a template literal such as {Posted by } to comply with the guideline and improve readability. See the checklist examples for context.

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