Skip to content

Develop#1454

Open
Vitaligna wants to merge 1 commit into
mate-academy:masterfrom
Vitaligna:develop
Open

Develop#1454
Vitaligna wants to merge 1 commit into
mate-academy:masterfrom
Vitaligna:develop

Conversation

@Vitaligna

Copy link
Copy Markdown

No description provided.

@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

The request meets the required criteria for approval.


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

Footnotes

  1. Rate AI review example

Comment on lines +84 to 89
removeError('name');
}}
/>

<span className="icon is-small is-left">
<i className="fas fa-user" />

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 #11 violation: The delete operation has no error handling. If the delete request fails after the UI already updated, the user won't know and can't retry. Consider adding error handling that restores the comment or shows a notification on failure.

Comment on lines +45 to +49
})
.then(() => {
setBody('');
setErrors({});
})

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 #11 violation: The catch block silently swallows errors without notifying the user. If adding a comment fails, the user loses their input and doesn't know it failed. Add error handling that allows the user to retry.

{isSelected ? 'Close' : 'Open'}
</button>
</td>
</tr>

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 #11: 'Handle Add and Delete errors so the user can retry'. The .catch(() => {}) silently ignores errors, so users cannot know when their comment failed to submit and cannot retry. Add proper error handling here.

Comment on lines +7 to +12
selectedPost: Post | null;
onSelect: (post: Post | null) => void;
};

export const PostsList: React.FC<Props> = ({
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.

This violates checklist item #3: 'make sure you described objects in propTypes'. The users and selectedUser props (both User objects) should have propTypes defined.

Comment on lines +6 to +10
posts: Post[];
selectedPost: Post | null;
onSelect: (post: Post | null) => void;
};

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 #3: 'make sure you described objects in propTypes'. The posts and selectedPost props (both Post objects) should have propTypes defined.

{canShowCommentsContent && comments.length === 0 && (
<p className="title is-4" data-cy="NoCommentsMessage">
No comments yet
</p>

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 #11 violation: The .catch(() => {}) block silently swallows errors. Users cannot retry if the comment deletion fails. Consider showing an error state to allow retry.

Comment on lines +83 to 89
)}

{canShowCommentsContent && !isFormVisible && (
<button
data-cy="WriteCommentButton"
type="button"
className="button is-link"

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 #11 violation: The delete operation doesn't handle errors. If the API call fails, the optimistic UI update has already removed the comment, but the user is not notified of the failure. Consider adding error handling to allow retry.

Comment thread src/App.tsx
Comment on lines +83 to +89

const deleteComment = (commentId: number) => {
setComments(currentComments =>
currentComments.filter(comment => comment.id !== commentId),
);

client.delete(`/comments/${commentId}`);

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 deleteComment function performs optimistic deletion but has no error handling. If the API call fails, the user cannot retry or recover the comment. According to checklist item #11, you should handle Delete errors so the user can retry. Consider wrapping the delete call in try/catch and adding error state.

Comment thread src/App.tsx
setPostsLoadingError(true);
})
.finally(() => {
setIsPostsLoading(false);

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 catch block for onSubmit is empty - errors are silently swallowed. If adding a comment fails, the user has no indication and cannot retry. Per checklist item #11, handle Add errors so the user can retry.

Comment on lines +5 to +9

type Props = {
users: User[];
selectedUser: User | null;
onSelect: (user: User) => void;

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 #3: 'make sure you described objects in propTypes'. While TypeScript types are used, the task specifically requires propTypes. Add propTypes definitions for users, selectedUser, and onSelect using the prop-types package.

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