Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job migrating the application state to Redux! The new structure with separate slices is well-organized and follows best practices. I'm requesting a few changes to address some critical issues before we can merge this.
Here are the key points to address:
-
Broken Comment Creation: In
src/components/PostDetails.tsx, the new comment object is created without anidand then asserted as aComment. This will cause a runtime error because thekeyprop in the rendered list will beundefined. To fix this, theaddCommentthunk should use the complete comment object returned from the API (which includes theid) when dispatching theaddaction. -
Missing Error Handling: The
removeCommentthunk insrc/app/comments.tslacks atry...catchblock. If thedeleteCommentAPI call fails, it will cause an unhandled promise rejection. Please add error handling here to ensure the app remains stable. -
UI Flicker: In
src/app/posts.ts, when a new author is selected, theloadedstate is set totruebefore the new posts are fetched. This causes a brief "No posts yet" message to flash on the screen. Consider settingloadedtofalsein this case to show the loading indicator immediately.
These fixes will make the application more robust and improve the user experience. You're doing great work!
✨ 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
src/app/posts.ts
Outdated
| extraReducers: builder => { | ||
| builder.addCase(set.type, state => { | ||
| state.items = []; | ||
| state.loaded = true; |
There was a problem hiding this comment.
When a new author is selected, their posts have not yet been loaded. Setting loaded to true here will cause the UI to briefly show a "No posts yet" message before the loader appears. Consider what the loaded state should be to indicate that a data fetch is about to begin.
| export function removeComment(commentId: Comment['id']) { | ||
| return async (dispatch: Dispatch) => { | ||
| await deleteComment(commentId); | ||
|
|
||
| dispatch(remove(commentId)); | ||
| }; |
There was a problem hiding this comment.
The removeComment function is missing error handling. If the deleteComment API call fails, it will result in an unhandled promise rejection. It's good practice to wrap asynchronous operations that can fail in a try...catch block, similar to how you've done it in the addComment function.
src/components/PostDetails.tsx
Outdated
| const newComment = { | ||
| name, | ||
| email, | ||
| body, | ||
| postId: post.id, | ||
| } as Comment; |
There was a problem hiding this comment.
The type assertion as Comment is incorrect here. The newComment object is missing the id property, which is required by the Comment type. This will cause an issue when the new comment is added to the list, as the key={comment.id} in the render method will be using an undefined id.
To fix this, the addComment thunk should wait for the createComment API response (which presumably returns the created comment with its new ID) and then dispatch the add action with the complete comment object returned from the server.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this pull request! I am approving your solution as you have successfully addressed all the feedback from the previous review. The migration to Redux is well-executed, and the fixes for comment creation, error handling, and the UI flicker have significantly improved the application's robustness. All requirements have been met, and the code is clean and well-structured. Great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
DEMO LINK