Develop #1247

mForkosh · 2025-02-26T15:29:54Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! While your code generally meets the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

Asynchronous Data Fetching: The use of useMemo for asynchronous data fetching is not appropriate. Please switch to using useEffect for fetching data to ensure proper side effect management.
Return Type Errors: The getUserById function has an incorrect return type. It should return Promise<User> instead of Promise<Post[]>. Additionally, the deletePostComment function should specify a return type for consistency.
Validation Logic: There are issues with the validation logic for commentTextIsValid, emailIsValid, and nameIsValid. These should be corrected to ensure proper validation.
Error Handling: Improve error handling by providing specific error messages or retry options to the user, rather than just setting a generic error state.
Typographical Errors: There are several typos in function names, variable names, and parameters (e.g., hendlClick, changeSlelectedPost, commemtId, etc.). Correcting these will improve readability and maintain consistency.

Please address these issues to ensure the code functions correctly and adheres to best practices. Once these changes are made, we can proceed with the approval. Keep up the great work! 🌟

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2025-02-26T15:42:51Z

src/App.tsx

-              >
-                Something went wrong!
-              </div>
+  useMemo(async () => {


Using useMemo for asynchronous data fetching is not appropriate. useMemo is intended for memoizing values, not for side effects like data fetching. Consider using useEffect instead to fetch users when the component mounts.

mateacademy-ai-mentor · 2025-02-26T15:42:51Z

src/App.tsx

-              </div>
+  useMemo(async () => {
+    try {
+      const loadedUsers: User[] = await getAllUsers();


The getAllUsers function is called inside useMemo, which is not the correct hook for side effects. Move this logic to a useEffect hook to ensure it runs correctly when the component mounts.

mateacademy-ai-mentor · 2025-02-26T15:42:51Z

src/App.tsx

+
+      setAllUsers(loadedUsers);
+    } catch {
+      setError(true);


Consider providing more detailed error feedback to the user. Currently, the error state is set to true, but no specific error message is displayed. This could be improved by setting an error message and displaying it in the UI.

mateacademy-ai-mentor · 2025-02-26T15:42:51Z

src/api/fetchComments.ts

+  return client.post('/comments', data);
+};
+
+export const deletePostComment = (id: number) => {


The deletePostComment function does not specify a return type. For consistency and clarity, consider specifying the return type, such as Promise<void> or Promise<Response>, depending on what the client.delete method returns.

mateacademy-ai-mentor · 2025-02-26T15:42:51Z

src/api/fetchUsers.ts

+import { User } from '../types/User';
+import { client } from '../utils/fetchClient';
+
+export const getUserById = (userId: number): Promise<Post[]> => {


The return type of getUserById is incorrectly specified as Promise<Post[]>. It should be Promise<User> since the function is fetching a single user by ID.

mateacademy-ai-mentor · 2025-02-26T15:42:52Z

src/components/PostDetails.tsx

+
+  async function addNewComment(comm: Omit<Comment, 'id' | 'postId'>) {
+    try {
+      const promis = await client.addNewComment({ ...comm, postId: id });


There is a typo in the variable name promis. It should be promise for consistency and to avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:42:52Z

src/components/PostDetails.tsx

+    }
+  }
+
+  useMemo(async () => {


Using useMemo for asynchronous data fetching is not appropriate. useMemo is intended for memoizing values, not for side effects like data fetching. Consider using useEffect instead to fetch comments when the post ID changes.

mateacademy-ai-mentor · 2025-02-26T15:42:52Z

src/components/PostDetails.tsx

+              {postComments.length > 0 && (
+                <CommentsList
+                  comments={postComments}
+                  onDeleteComment={commemtId => deleteComent(commemtId)}


There is a typo in the parameter name commemtId. It should be commentId for consistency and to avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:42:52Z

src/components/PostsList.tsx

+  selectedPost,
+  onChangeSelectedPost,
+}) => {
+  const changeSlelectedPost = (post: Post) => {


There is a typo in the function name changeSlelectedPost. It should be changeSelectedPost for consistency and to avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:42:52Z

src/components/UserSelector.tsx

+  }
+
+  useEffect(() => {
+    const hendlClick = (e: MouseEvent) => {


There is a typo in the function name hendlClick. It should be handleClick for consistency and to avoid confusion.

mateacademy-ai-mentor

Thank you for your hard work on this project! While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

Incorrect Use of useMemo: You've used useMemo for data fetching, which is not appropriate as useMemo is meant for memoizing values, not for side effects like fetching data. Please switch to using useEffect for fetching data when the component mounts or when dependencies change.
Error Handling: The current error handling is quite generic. It would be beneficial to provide more detailed error messages to the user or log them for debugging purposes, especially for actions like adding and deleting comments.
Typographical Errors: There are several typos in function names, variable names, and parameter names (e.g., hendlClick, changeSlelectedPost, commemtId, promis, deleteComent, formIsSHow, commentTextIsValidt, coment). These should be corrected to maintain consistency and avoid confusion.
Validation Logic: The validation logic for checking empty fields is inverted. Ensure that conditions check for non-empty values to determine validity.
Return Type Mismatch: The return type of getUserById is incorrectly specified as Promise<Post[]>. It should be Promise<User> to accurately reflect the data being fetched.

Please address these issues to ensure the code fully meets the task requirements. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you have any questions! 😊

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2025-02-26T15:43:41Z

src/App.tsx

-              >
-                Something went wrong!
-              </div>
+  useMemo(async () => {


Using useMemo for data fetching is not appropriate. useMemo is meant for memoizing values, not for side effects like fetching data. Consider using useEffect instead to fetch the users when the component mounts.

mateacademy-ai-mentor · 2025-02-26T15:43:41Z

src/App.tsx

+
+      setAllUsers(loadedUsers);
+    } catch {
+      setError(true);


The error handling here sets a generic error state. It might be beneficial to provide more detailed error messages to the user or log the error for debugging purposes.

mateacademy-ai-mentor · 2025-02-26T15:43:41Z

src/api/fetchUsers.ts

+import { User } from '../types/User';
+import { client } from '../utils/fetchClient';
+
+export const getUserById = (userId: number): Promise<Post[]> => {


The return type of getUserById is incorrectly specified as Promise<Post[]>. It should be Promise<User> to match the expected response of fetching a single user by ID.

mateacademy-ai-mentor · 2025-02-26T15:43:41Z

src/components/CommentsList.tsx

+  <>
+    <p className="title is-4">Comments:</p>
+
+    {comments.map(coment => (


There is a typo in the map function. The variable coment should be comment to match the type and improve code readability.

mateacademy-ai-mentor · 2025-02-26T15:43:41Z

src/components/NewCommentForm.tsx

+    const nameIsValid = authorName.trim().length === 0;
+    const emailIsValid = authorEmail.trim().length === 0;
+    const commentTextIsValidt = commentText.trim().length === 0;


The validation logic is inverted. The conditions should check for non-empty values to determine validity. For example, const nameIsValid = authorName.trim().length > 0; should be used to set nameIsValid to true when the name is valid.

mateacademy-ai-mentor · 2025-02-26T15:43:42Z

src/components/PostDetails.tsx

+    }
+  }
+
+  useMemo(async () => {


Using useMemo for data fetching is not appropriate. useMemo is meant for memoizing values, not for side effects like fetching data. Consider using useEffect instead to fetch comments when the post ID changes.

mateacademy-ai-mentor · 2025-02-26T15:43:42Z

src/components/PostDetails.tsx

+              {postComments.length > 0 && (
+                <CommentsList
+                  comments={postComments}
+                  onDeleteComment={commemtId => deleteComent(commemtId)}


There is a typo in the parameter name commemtId. It should be commentId to maintain consistency and avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:43:42Z

src/components/PostsList.tsx

+  selectedPost,
+  onChangeSelectedPost,
+}) => {
+  const changeSlelectedPost = (post: Post) => {


There is a typo in the function name changeSlelectedPost. It should be changeSelectedPost to maintain consistency and avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:43:42Z

src/components/UserSelector.tsx

+  }
+
+  useEffect(() => {
+    const hendlClick = (e: MouseEvent) => {


There is a typo in the function name hendlClick. It should be handleClick to maintain consistency and avoid confusion.

mateacademy-ai-mentor · 2025-02-26T15:43:42Z

src/components/UserSelector.tsx

+    document.addEventListener('click', hendlClick);
+
+    return () => {
+      document.removeEventListener('click', hendlClick);


Ensure that the event listener is properly cleaned up by using the same function reference for both addEventListener and removeEventListener. This is currently correct, but it's important to maintain this practice to avoid memory leaks.

mateacademy-ai-mentor

Great job on implementing the main application logic and structuring your components effectively! 🎉 While there are several areas for improvement, such as enhancing error handling, correcting typographical errors, and improving accessibility, your code is on the right track.

Here are some key points to focus on:

Error Handling: Ensure that all API functions include error handling to manage potential request failures. This will help in providing feedback or logging for debugging purposes. Consider making error messages more descriptive to aid in troubleshooting.
Typographical Errors: Correct variable names like formIsShow to formIsShown and loadingAddComm to loadingAddComment for clarity and consistency.
Accessibility: Enhance accessibility by adding tabIndex and aria attributes to dropdown items and accessible labels to buttons. This will improve usability for users relying on assistive technologies.
Validation: Implement robust email validation to ensure the correct format, not just checking for empty fields.
State Management: Initialize the error state as null instead of an empty string to avoid unnecessary rendering of error messages.

These improvements will enhance the robustness, clarity, and usability of your application. Keep up the good work, and continue learning from these experiences to refine your skills! 🚀

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

mateacademy-ai-mentor · 2025-02-27T08:30:19Z

src/App.tsx


-              <Loader />
+  const [loadingData, setLoadingData] = useState(false);
+  const [error, setError] = useState('');


Consider initializing the error state as null instead of an empty string. This will help in conditionally rendering error messages more effectively and avoid rendering an empty error message when there is no error.

mateacademy-ai-mentor · 2025-02-27T08:30:19Z

src/App.tsx

+
+        setAllUsers(loadedUsers);
+      } catch {
+        setError('Failed to get a list of users');


The error message 'Failed to get a list of users' is quite generic. Consider providing more detailed error information or logging the error for debugging purposes.

mateacademy-ai-mentor · 2025-02-27T08:30:19Z

src/App.tsx

+
+      setUserPosts(loadedUserPosts);
+    } catch {
+      setError('Failed to get a list of user posts');


The error message 'Failed to get a list of user posts' is quite generic. Consider providing more detailed error information or logging the error for debugging purposes.

mateacademy-ai-mentor · 2025-02-27T08:30:19Z

src/api/fetchComments.ts

+export const getPostComments = (postId: number): Promise<Comment[]> => {
+  return client.get(`/comments?postId=${postId}`);
+};


Consider adding error handling for the getPostComments function to manage potential request failures and provide feedback or logging.

mateacademy-ai-mentor · 2025-02-27T08:30:20Z

src/api/fetchComments.ts

+export const addNewComment = (data: Omit<Comment, 'id'>): Promise<Comment> => {
+  return client.post('/comments', data);
+};


Consider adding error handling for the addNewComment function to manage potential request failures and provide feedback or logging.

mateacademy-ai-mentor · 2025-02-27T08:30:21Z

src/components/PostDetails.tsx

+export const PostDetails: React.FC<Props> = ({ post }) => {
+  const [loadingComments, setLoadingComments] = useState(false);
+  const [postComments, setPostComments] = useState<Comment[] | null>(null);
+  const [formIsShow, setFormIsShow] = useState(false);


Consider renaming formIsShow to formIsShown for grammatical correctness.

mateacademy-ai-mentor · 2025-02-27T08:30:21Z

src/components/PostDetails.tsx

+  const [loadingComments, setLoadingComments] = useState(false);
+  const [postComments, setPostComments] = useState<Comment[] | null>(null);
+  const [formIsShow, setFormIsShow] = useState(false);
+  const [error, setError] = useState('');


Consider initializing the error state as null instead of an empty string. This will help in conditionally rendering error messages more effectively and avoid rendering an empty error message when there is no error.

mateacademy-ai-mentor · 2025-02-27T08:30:21Z

src/components/PostDetails.tsx

+  const { id, title, body } = post;
+
+  async function deleteComment(commId: number) {
+    setError('');


Ensure that the error state is consistently reset or cleared when appropriate to avoid displaying outdated error messages.

mateacademy-ai-mentor · 2025-02-27T08:30:21Z

src/components/PostsList.tsx

+          <tr className="has-background-link-light">
+            <th>#</th>
+            <th>Title</th>
+            {/* eslint-disable-next-line jsx-a11y/control-has-associated-label */}


Consider adding accessible labels to buttons to improve accessibility for screen readers. This will help users who rely on assistive technologies to understand the purpose of the buttons.

mateacademy-ai-mentor · 2025-02-27T08:30:21Z

src/components/UserSelector.tsx

+            <a
+              key={user.id}
+              href={`#user-${user.id}`}
+              className={classNames('dropdown-item', {
+                'is-active': user.id === activeUser?.id,
+              })}
+              onClick={() => clickOnUser(user)}
+            >
+              {user.name}
+            </a>


Ensure that the dropdown items are accessible by keyboard and screen readers. Consider adding tabIndex and aria attributes to improve accessibility.

mForkosh added 3 commits February 25, 2025 17:01

test

4a84669

test 2

15eea61

solution

384180d

mateacademy-ai-mentor suggested changes Feb 26, 2025

View reviewed changes

made made edition

2e6cb82

mForkosh requested a review from mateacademy-ai-mentor February 27, 2025 08:26

mateacademy-ai-mentor approved these changes Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop #1247

Develop #1247

mForkosh commented Feb 26, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor Feb 26, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

mateacademy-ai-mentor Feb 27, 2025

Develop #1247

Are you sure you want to change the base?

Develop #1247

Conversation

mForkosh commented Feb 26, 2025

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment