solution #1184

k-shestakov · 2024-10-24T13:53:53Z

mateacademy-ai-mentor

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

Critical Functionality Issue: The removeComment function is using the post's id instead of the comment's id for deletion. This is a critical issue as it affects the core functionality of your application. Please update the function to use comment.id for the deletion process.
Typo in Prop Name: There is a consistent typo in the prop name choosenPost. It should be corrected to chosenPost to ensure consistency and avoid potential bugs.
Email Input Type: The type for the email input should be email instead of text. This change will provide built-in validation for email format, enhancing user experience.
Initialization of hasNoPosts: The hasNoPosts variable is not correctly initialized. Ensure it is assigned as a boolean expression to function correctly.

These issues are important to resolve to ensure the application functions correctly and maintains a high standard of code quality. Once these are addressed, your code will be in great shape! Keep up the good work, and feel free to reach out if you have any questions or need further guidance. 😊

✨ 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 · 2024-10-24T13:56:23Z

src/App.tsx

+  const hasNoPosts =
+    selectedUser && !postsIsLoading && !hasPostsError && !posts.length;


The hasNoPosts variable is not correctly initialized. It should be a boolean expression, but currently, it is missing the assignment operator =. It should be const hasNoPosts = selectedUser && !postsIsLoading && !hasPostsError && !posts.length;.

mateacademy-ai-mentor · 2024-10-24T13:56:23Z

src/App.tsx

+                {!postsIsLoading && posts.length > 0 && (
+                  <PostsList
+                    posts={posts}
+                    choosenPost={chosenPost}


There is a typo in the prop name choosenPost. It should be chosenPost to match the state variable name and maintain consistency.

mateacademy-ai-mentor · 2024-10-24T13:56:23Z

src/components/NewCommentForm.tsx

          <input
+            value={email}
+            onChange={handleEmailChange}
            type="text"


The type for the email input should be email instead of text. This will provide built-in validation for email format and improve user experience.

mateacademy-ai-mentor · 2024-10-24T13:56:23Z

src/components/PostDetails.tsx

+                        type="button"
+                        className="delete is-small"
+                        aria-label="delete"
+                        onClick={() => removeComment(id)}


The removeComment function is using the post's id instead of the comment's id for deletion. This will cause incorrect behavior as it attempts to delete the post instead of the comment. Change onClick={() => removeComment(id)} to onClick={() => removeComment(comment.id)} to fix this issue.

mateacademy-ai-mentor · 2024-10-24T13:56:23Z

src/components/PostsList.tsx

+
+type Props = {
+  posts: Post[];
+  choosenPost: Post | null;


There is a typo in the prop name choosenPost. It should be chosenPost to match the standard spelling and maintain consistency across the codebase.

mateacademy-ai-mentor · 2024-10-24T13:56:23Z

src/components/PostsList.tsx

+
+export const PostsList: React.FC<Props> = ({
+  posts,
+  choosenPost,


The prop choosenPost is misspelled. It should be chosenPost to align with the correct spelling and ensure consistency with other parts of the application.

mateacademy-ai-mentor · 2024-10-24T13:56:24Z

src/components/PostsList.tsx

+                  type="button"
+                  data-cy="PostButton"
+                  className={cn('button is-link', {
+                    'is-light': choosenPost?.id !== post.id,


The usage of choosenPost here should be corrected to chosenPost to match the corrected prop name.

mateacademy-ai-mentor · 2024-10-24T13:56:24Z

src/components/PostsList.tsx

+                    'is-light': choosenPost?.id !== post.id,
+                  })}
+                  onClick={() =>
+                    onSelect(choosenPost?.id === post.id ? null : post)


Ensure to update choosenPost to chosenPost in this line to reflect the corrected prop name.

mateacademy-ai-mentor · 2024-10-24T13:56:24Z

src/components/PostsList.tsx

+                    onSelect(choosenPost?.id === post.id ? null : post)
+                  }
+                >
+                  {choosenPost?.id === post.id ? 'Close' : 'Open'}


Update choosenPost to chosenPost in this line to maintain consistency with the corrected prop name.

mateacademy-ai-mentor · 2024-10-24T13:56:24Z

src/components/UserSelector.tsx

+    const currentUser = e.target as HTMLAnchorElement;
+
+    if (currentUser.tagName === 'A') {
+      const userId = +currentUser.href.split('-').slice(-1)[0];


Parsing the user ID from the href attribute using split('-').slice(-1)[0] is fragile and depends on the URL structure. Consider using a data attribute to store the user ID directly on the element, which would be more reliable and easier to maintain.

mateacademy-ai-mentor

Great work on implementing the code! 🎉 While there are a few areas for improvement, the overall structure and functionality are solid. Here are some key points to consider:

Type Safety: Ensure that variables like hasNoPosts and hasPostsError are initialized correctly as booleans or objects to enhance error handling flexibility.
Error Handling: It's important to handle errors in functions like addComment and removeComment to prevent unhandled promise rejections. Consider using try...catch blocks for better error management.
Form Validation: In the NewCommentForm.tsx, make sure the form is not reset if there are validation errors or if the addComment function fails.
Event Handling: In the UserSelector.tsx, ensure that the handleUserChange function checks if the event target is an anchor element to avoid runtime errors.
PropTypes: Consider adding propTypes to your components for better type-checking and to ensure that components receive the correct props.

These improvements will help you write more robust and maintainable code. Keep up the good work, and continue to learn and grow from these experiences! 🚀

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

mateacademy-ai-mentor · 2024-10-24T14:18:22Z

src/App.tsx

+  const [selectedUser, setSelectedUser] = useState<User | null>(null);
+  const [posts, setPosts] = useState<Post[]>([]);
+  const [postsIsLoading, setPostsIsLoading] = useState(false);
+  const [hasPostsError, setHasPostsError] = useState('');


The hasPostsError state is initialized as a string. Consider initializing it as a boolean or an object to allow for more flexible error handling in the future.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/App.tsx

+  const hasNoPosts =
+    selectedUser && !postsIsLoading && !hasPostsError && !posts.length;


The hasNoPosts variable should be explicitly initialized as a boolean. Consider using const hasNoPosts = Boolean(selectedUser && !postsIsLoading && !hasPostsError && !posts.length); to ensure it is always a boolean value.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/NewCommentForm.tsx

+      postId: post.id,
+    };
+
+    addComment(newComment).finally(() => setIsSending(false));


Consider handling errors in the addComment promise. Currently, errors from the addComment function are not caught, which might lead to unhandled promise rejections.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/NewCommentForm.tsx

+    };
+
+    addComment(newComment).finally(() => setIsSending(false));
+    reset();


The reset() function is called immediately after addComment. Ensure that the form is not reset if there are validation errors or if the addComment function fails.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/PostDetails.tsx

+export const PostDetails: React.FC<Props> = ({ post }) => {
+  const [comments, setComments] = useState<Comment[]>([]);
+  const [loading, setLoading] = useState(false);
+  const [hasError, setHasError] = useState('');


Consider initializing hasError as a boolean or an object to allow for more flexible error handling in the future.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/PostDetails.tsx

+  const addComment = (newComment: CommentData & { postId: number }) => {
+    return postComment(newComment).then(createdComment => {
+      setComments(curr => [...curr, createdComment]);
+    });


Consider adding error handling for the addComment function to manage potential errors from the postComment API call.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/PostDetails.tsx

+  const removeComment = (commentId: number) => {
+    return deleteComment(commentId).then(() =>
+      setComments(curr => curr.filter(comment => comment.id !== commentId)),
+    );


Consider adding error handling for the removeComment function to manage potential errors from the deleteComment API call.

mateacademy-ai-mentor · 2024-10-24T14:18:23Z

src/components/UserSelector.tsx

+  const handleUserChange = (
+    e: React.MouseEvent<HTMLDivElement, MouseEvent>,
+  ) => {
+    const currentUser = e.target as HTMLAnchorElement;


The handleUserChange function assumes that the event target is always an anchor element. Consider adding a check to ensure currentUser is an anchor element before accessing its attributes to prevent potential runtime errors.

solution

69421ae

mateacademy-ai-mentor suggested changes Oct 24, 2024

View reviewed changes

fix

c30b374

k-shestakov requested a review from mateacademy-ai-mentor October 24, 2024 14:16

mateacademy-ai-mentor approved these changes Oct 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

solution #1184

solution #1184

k-shestakov commented Oct 24, 2024

mateacademy-ai-mentor left a comment •

edited by sergii-nosachenko

Loading

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

mateacademy-ai-mentor Oct 24, 2024

		const hasNoPosts =
		selectedUser && !postsIsLoading && !hasPostsError && !posts.length;

solution #1184

Are you sure you want to change the base?

solution #1184

Conversation

k-shestakov commented Oct 24, 2024

mateacademy-ai-mentor left a comment • edited by sergii-nosachenko Loading

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

mateacademy-ai-mentor left a comment •

edited by sergii-nosachenko

Loading