solution by Bohdan259 · Pull Request #3571 · mate-academy/react_add-todo-form

Bohdan259 · 2026-03-08T17:00:17Z

mateacademy-ai-mentor

Review Summary

Great work on the form UI and validation logic! However, I'm requesting a few changes to address some critical issues with data handling and component responsibilities.

The most important issue is that new todos are always created with a hardcoded id of 0 in PostForm.tsx. This logic should be moved to the App component, which owns the todos state. App should be responsible for calculating the new id (as the largest existing ID + 1) and creating the complete newTodo object.

Additionally, PostForm.tsx currently imports user data directly. This violates a core requirement to prepare data in the root component (App) and pass it down via props [CHECKLIST ITEM #5].

Lastly, in TodoInfo.tsx, the <UserInfo> component is rendered without checking if todo.user is defined. Please add a conditional check to avoid rendering a component with undefined props [CHECKLIST ITEM #4].

Once these issues are fixed, your solution will be in excellent shape!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-08T17:02:38Z

src/App.tsx

+  const addTodo = (newTodo: Todos) => {
+    setTodos(currentTodos => [...currentTodos, newTodo]);
+  };


The logic for creating a new todo should reside here, inside the component that owns the todos state. This addTodo function should receive the necessary data from the form (e.g., title and userId), calculate the new id (as the largest existing ID + 1), and then construct the complete newTodo object before updating the state. Currently, the PostForm is creating the object with a hardcoded id of 0, which violates the requirements.

src/components/PostForm/PostForm.tsx

mateacademy-ai-mentor · 2026-03-08T17:02:38Z

src/components/PostForm/PostForm.tsx

+    setChooseUser(0);
+
+    onSubmit({
+      id: 0,


The id for the new todo is hardcoded. According to the requirements, the new id should be the largest id in the current todos array plus one. This logic for generating a new ID needs to be implemented in the parent App component.

mateacademy-ai-mentor · 2026-03-08T17:02:38Z

src/components/PostForm/PostForm.tsx

+          <option value="0" disabled>
+            Choose a user
+          </option>
+          {[...usersFromServer].map(user => (


This violates checklist item #3. The .map() method is non-mutating and already returns a new array, so creating a copy of usersFromServer with the spread operator ... is not necessary here.

mateacademy-ai-mentor · 2026-03-08T17:02:38Z

src/components/TodoInfo/TodoInfo.tsx

+    >
+      <h2 className="TodoInfo__title">{todo.title}</h2>
+
+      <UserInfo user={todo.user} />


The todo.user object can be undefined according to its type definition (User | undefined). This violates checklist item #4, which states that you should not render a component if a property passed to it is null or undefined. You should add a conditional check here to ensure UserInfo is only rendered when todo.user is available.

mateacademy-ai-mentor · 2026-03-08T17:02:38Z

src/index.tsx


-createRoot(document.getElementById('root') as HTMLElement).render(<App />);
+createRoot(document.getElementById('root') as HTMLElement).render(
+  <App onSubmit={() => {}} />,


The App component doesn't accept any props. Passing an onSubmit prop here has no effect and can be confusing. This line should be removed.

mateacademy-ai-mentor

Review Summary

Great job moving the id generation logic into the App component! However, a couple of architectural changes are needed before this can be approved.

First, the data flow needs to be adjusted to align with the project guidelines. The App component should be the single source of truth for data.

In App.tsx, please import usersFromServer and pass it down to PostForm as a prop.
The PostForm component should then be simplified. It should only send the raw form data (e.g., { title, userId }) to the addTodo function.
Finally, the addTodo function in App.tsx should be responsible for creating the complete newTodo object, including finding the correct user. This addresses [CHECKLIST ITEM #5].

Second, the conditional rendering logic for UserInfo is in the wrong place. The check for todo.user should be in the parent component, TodoInfo.tsx, before it renders <UserInfo>, not inside UserInfo.tsx itself [CHECKLIST ITEM #4].

Once these changes are made, your solution will be perfect!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/components/PostForm/PostForm.tsx

+import './PostForm.scss';
+import { useState } from 'react';
+import { getUser } from '../../services/user.service';
+import usersFromServer from '../../api/users';


This violates checklist item #5: "Prepare data in one place (App component) and pass it to child components. Don't use import keyword in non-root components". The usersFromServer array should be passed into this component as a prop from App.tsx instead of being imported directly.

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/components/PostForm/PostForm.tsx

+    onSubmit({
+      id: 0,
+      title: title,
+      userId: chooseUser,
+      completed: false,
+      user: getUser(chooseUser),
+    });


This component's responsibility should be to collect user input, not to construct the final todo object. It should call onSubmit with just the raw data from the form, for example: onSubmit({ title, userId: chooseUser }).

The App component should be responsible for creating the complete todo object, which includes calculating the id, setting completed: false, and finding the associated user object. This change would also mean the getUser import on line 3 is no longer needed in this file.

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/App.tsx

+  const addTodo = ({ id, ...data }: Todos) => {
+    const newTodo = {
+      id: getNewTodoId(todos),
+      ...data,
+    };

-        <article data-id="2" className="TodoInfo">
-          <h2 className="TodoInfo__title">
-            quis ut nam facilis et officia qui
-          </h2>
+    setTodos(currentTodos => [...currentTodos, newTodo]);
+  };


This function is a great improvement! To fully align with checklist item #5, the App component should be responsible for creating the entire newTodo object.

Currently, PostForm still prepares the user property. Instead, PostForm should only be concerned with form state and pass up raw data.

Please adjust the logic:

PostForm should pass up only the essential data, like { title, userId }.

This addTodo function should accept that simpler data object.

Here in App, you should find the corresponding user and construct the complete newTodo object before adding it to the state.

This also means usersFromServer should be imported here and passed as a prop to PostForm so it can render the user list.

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/App.tsx

+  return (
+    <div className="App">
+      <h1>Add todo form</h1>
+      <PostForm onSubmit={addTodo} />


To implement the changes suggested above, you'll need to pass the list of users to PostForm. You should import usersFromServer in this file and pass it as a users prop here.

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/components/TodoInfo/TodoInfo.tsx

+    >
+      <h2 className="TodoInfo__title">{todo.title}</h2>
+
+      <UserInfo todo={todo} />


This violates checklist item #4: "Don't render the component if the property that you pass to the component has null or undefined value."

While you've correctly added a check inside the UserInfo component, the guideline requires this check to happen in the parent component. You should conditionally render UserInfo only if todo.user exists.

For example:

{todo.user && <UserInfo todo={todo} />}

mateacademy-ai-mentor · 2026-03-08T18:34:37Z

src/components/UserInfo/UserInfo.tsx

+  if (!todo.user) {
+    return null;
+  }


This conditional check is a good safeguard, but it violates the guideline in checklist item #4. According to the rule, this check should be handled by the parent component (TodoInfo.tsx) before it decides to render UserInfo.

Please remove this logic from this component and ensure the parent component performs the check instead.

mateacademy-ai-mentor

Review Summary

Excellent work on this submission! You've done a fantastic job refactoring the application architecture, and the data flow is much cleaner now. I am approving this solution as an educational override so you can move forward, but please note one critical fix is needed. In src/components/PostForm/PostForm.tsx, the props destructuring is incorrect: ({ onSubmit }, { users }). A React component receives a single props object, so you should destructure everything from it like this: ({ onSubmit, users }). This will fix the error where the users prop was not being passed to the form. After that small change, your solution will be perfect. Keep up the great work!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-03-08T19:09:56Z

src/components/PostForm/PostForm.tsx

+  users: User[];
+};
+
+export const PostForm: React.FC<Props> = ({ onSubmit }, { users }) => {


The props destructuring here is incorrect for a functional component. It receives a single props object as its first argument. You should destructure all properties from that single object, like this: ({ onSubmit, users }).

solution

b954caf

mateacademy-ai-mentor suggested changes Mar 8, 2026

View reviewed changes

Bohdan259 added 2 commits March 8, 2026 18:31

solution comments

d3caa80

solution task

0c25fc4

Bohdan259 requested a review from mateacademy-ai-mentor March 8, 2026 18:32

mateacademy-ai-mentor suggested changes Mar 8, 2026

View reviewed changes

Bohdan259 added 2 commits March 8, 2026 19:07

solution tasks

38a2852

sol comments

614cb62

Bohdan259 requested a review from mateacademy-ai-mentor March 8, 2026 19:07

mateacademy-ai-mentor approved these changes Mar 8, 2026

View reviewed changes

Conversation

Bohdan259 commented Mar 8, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants