Added tests for comment #4
Conversation
There was a problem hiding this comment.
Review Summary
I’m requesting changes because there are critical blockers preventing full compliance with the task requirements.
Critical issues to fix:
-
Missing list comments endpoint implementation
- Where: src/api/resources/CommentsApi.js (no
getComments) and src/api/ApiComposite.js (no wrapper) - Why: The task requires GET
/api/articles/:slug/commentsto list comments for an article. Without it, required functionality is incomplete. - How: Implement
getComments(slug, token = null)inCommentsApithat performs a GET to/api/articles/:slug/comments, and expose it fromApiCompositeasgetComments(slug, authToken)delegating toCommentsApi.getComments.
- Where: src/api/resources/CommentsApi.js (no
-
Authorization header handling for unauthenticated requests
- Where: src/api/resources/CommentsApi.js, in both
createCommentanddeleteComment(exact lines where headers are built) - Why: The Authorization header is always set even when
tokenis null. This breaks unauthenticated scenarios and doesn’t reflect real API behavior. - How: Only include
Authorization: Token ${token}whentokenis provided; otherwise omit the header entirely. For example, build headers with a conditional spread:{ 'Content-Type': 'application/json', ...(token ? { Authorization: \ Token ${token} } : {}) }.
- Where: src/api/resources/CommentsApi.js, in both
-
Align method signatures and parameter names
- Where: src/api/ApiComposite.js (wrappers) and optionally src/api/resources/CommentsApi.js
- Why: The requirements specify explicit method signatures for clarity and correctness.
- How: Rename and reorder parameters to match the spec:
createComment(slug, body, authToken)(notarticle, comment, token), and pass{ comment: { body } }in the request body.deleteComment(slug, commentId, authToken)(notarticle, commentId, token). Ensure the same ordering inCommentsApior adapt the wrapper accordingly.
Minor suggestions (non-blockers):
- Consider renaming
ROUTES.comments(slug).createto a more generic key (e.g.,listorindex) if it’s also used for GET, to avoid confusion. - If you keep assertion helpers in ApiComposite, verify they’re correctly implemented in
CommentsApiand used consistently.
You’re very close—create/delete flows and tests look solid. Addressing the GET method and header handling will bring this fully in line with the requirements. Keep up the good 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/api/ApiComposite.js
Outdated
| return await this.articles.getArticleBySlug(slug, token); | ||
| } | ||
|
|
||
| async createComment(article, comment = null, token = null) { |
There was a problem hiding this comment.
The method signature for createComment should match the requirements: it should take slug, body, and authToken (not article, comment, token). Please align the parameter names and order with the requirements for clarity and correctness.
src/api/ApiComposite.js
Outdated
| return await this.comments.createComment(article, comment, token); | ||
| } | ||
|
|
||
| async deleteComment(article, commentId, token = null) { |
There was a problem hiding this comment.
The method signature for deleteComment should match the requirements: it should take slug, commentId, and authToken (not article, commentId, token). Please align the parameter names and order with the requirements for clarity and correctness.
| async deleteComment(article, commentId, token = null) { | ||
| return await this.comments.deleteComment(article, commentId, token); | ||
| } | ||
|
|
There was a problem hiding this comment.
You are missing a method to list comments for a specific article (GET /api/articles/:slug/comments), which is required by the task. Please add a method such as getComments(slug, authToken) that delegates to the corresponding method in CommentsApi.
| expect(body.comment.author.username).toBe(username); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
You are missing the required getComments(slug, token = null) method, which should perform a GET request to /api/articles/:slug/comments. This is necessary to fulfill the task requirements for listing comments.
src/api/resources/CommentsApi.js
Outdated
| ...(comment ? { comment: { body: comment } } : {}), | ||
| }, | ||
| headers: { | ||
| authorization: `Token ${token}`, |
There was a problem hiding this comment.
The Authorization header is always set, even if token is null. For unauthenticated requests, you should only include the Authorization header if token is provided. Otherwise, omit it to accurately simulate unauthenticated scenarios.
src/api/resources/CommentsApi.js
Outdated
| return await this.step(`Delete comment with id ${commentId}`, async () => { | ||
| return await this.client.delete(ROUTES.comments(slug, commentId).delete, { | ||
| headers: { | ||
| authorization: `Token ${token}`, |
There was a problem hiding this comment.
The Authorization header is always set, even if token is null. For unauthenticated requests, you should only include the Authorization header if token is provided. Otherwise, omit it to accurately simulate unauthenticated scenarios.
There was a problem hiding this comment.
Review Summary
Decision: APPROVE
Your implementation fully meets the requirements for the Comments API feature set and its tests.
- The
CommentsApiclass correctly implements create, list, and delete operations with proper endpoints, payloads, and conditional Authorization headers. - The composite API (
ApiComposite.js) cleanly exposescreateComment,getComments, anddeleteCommentwith correct signatures. - Routes and status code constants are accurate and consistent.
- Playwright fixtures integrate the Comments API cleanly, and the test suite robustly covers all required positive and negative scenarios (unauthorized, missing body, cross-user delete, own delete), asserting both status codes and response structures.
No critical blockers or functional gaps were found. Nice work on keeping the code clean, consistent with existing patterns, and well-tested. Keep it up!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
No description provided.