solve#356
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
While the check run failures appear to stem from environment issues (frontend container readiness, not student code), the code review itself reveals two critical gaps that must be addressed: (1) The requirement explicitly states 'following/unfollowing the user' but user.cy.js only contains a follow test - you must add an unfollow test that clicks the Unfollow button and verifies the button reverts to 'Follow {username}'. (2) The password update test in settings.cy.js creates a new password but never verifies you can actually log in with it - after updating, you should logout and confirm login succeeds with the new credentials. These are functional gaps in your test implementation, not environment issues. Additionally, for consistency with the Advanced level requirements demonstrated in signIn.cy.js, consider using PageObject pattern and data-cy attributes consistently across all test files rather than mixing placeholder selectors with data attributes.
✨ 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
| cy.contains('button', 'Publish Article').click(); | ||
|
|
||
| cy.contains('.btn', 'Edit Article').click(); | ||
|
|
||
| cy.get('input[placeholder="Article Title"]').clear(); | ||
| cy.get('input[placeholder="Article Title"]').type('Updated Title'); | ||
|
|
||
| cy.contains('button', 'Publish Article').click(); | ||
|
|
||
| cy.get('h1').should('contain', 'Updated Title'); | ||
| }); |
There was a problem hiding this comment.
Missing test for unfollowing the user. The task requires: 'following/unfollowing the user'. Currently only has the follow test (lines 39-49). Add a separate test that clicks Unfollow and verifies the button changes back to Follow.
| }); | ||
|
|
||
| it('should be created using New Article form', () => { | ||
| cy.contains('.nav-link', 'New Article').click(); |
There was a problem hiding this comment.
Use data-cy attributes and PageObject pattern instead of placeholder selectors (e.g., 'input[placeholder="Your Name"]'). The advanced level requires using PageObject pattern consistently, similar to signIn.cy.js. Add data-cy attributes to input elements or use the SignInPageObject/SettingsPageObject approach.
| .type(article.description); | ||
| cy.get('textarea[placeholder="Write your article (in markdown)"]') | ||
| .type(article.body); | ||
| cy.get('input[placeholder="Enter tags"]').type('testing{enter}'); | ||
|
|
||
| cy.contains('button', 'Publish Article').click(); | ||
|
|
||
| cy.get('h1').should('contain', article.title); |
There was a problem hiding this comment.
Use data-cy attributes and PageObject pattern instead of placeholder selectors. Consider creating a SettingsPageObject class similar to SignInPageObject for consistency with the advanced level requirements.
|
|
||
| it('should be edited using Edit button', () => { | ||
| cy.contains('.nav-link', 'New Article').click(); | ||
| cy.get('input[placeholder="Article Title"]').type(article.title); | ||
| cy.contains('button', 'Publish Article').click(); | ||
|
|
||
| cy.contains('.btn', 'Edit Article').click(); | ||
|
|
||
| cy.get('input[placeholder="Article Title"]').clear(); | ||
| cy.get('input[placeholder="Article Title"]').type('Updated Title'); | ||
|
|
||
| cy.contains('button', 'Publish Article').click(); |
There was a problem hiding this comment.
Use data-cy attributes and PageObject pattern for consistency with the example in signIn.cy.js.
| cy.get('h1').should('contain', 'Updated Title'); | ||
| }); | ||
|
|
||
| it('should be deleted using Delete button', () => { | ||
| cy.contains('.nav-link', 'New Article').click(); | ||
| cy.get('input[placeholder="Article Title"]').type(article.title); | ||
| cy.contains('button', 'Publish Article').click(); | ||
|
|
||
| cy.get('.btn-outline-danger').contains('Delete Article').click(); |
There was a problem hiding this comment.
Use data-cy attributes and PageObject pattern instead of placeholder selectors for consistency with advanced level requirements.
|
|
||
| cy.get('input[placeholder="Username"]').type('SomeOtherName'); | ||
| cy.get('input[placeholder="Email"]').type(user.email); | ||
| cy.get('input[placeholder="Password"]').type(user.password); |
There was a problem hiding this comment.
Uses placeholder selectors instead of data-cy attributes.
| .should('be.visible') | ||
| .and('contain', user.username); | ||
| }); |
There was a problem hiding this comment.
Uses raw cy.get('[data-cy="email-sign-in"]') instead of PageObject methods. Create a SignInPageObject and use it for consistency with other test files.
| }); | ||
| beforeEach(() => { | ||
| cy.task('db:clear'); | ||
|
|
There was a problem hiding this comment.
This test uses placeholder selectors ('input[placeholder="Username"]') instead of data-cy attributes. The task hint states: 'Usually, front-end tests rely on specific data-* attributes... you might need to add them yourself'. Consider adding data-cy attributes to form inputs or using PageObject.
|
|
||
| it.skip('should be able to follow the another user', () => { | ||
| it('should be able to follow the another user', () => { | ||
| cy.visit(`/#/profile/${author.username}`); | ||
|
|
||
| cy.contains('button', `Follow ${author.username}`) | ||
| .should('be.visible') | ||
| .click(); | ||
|
|
||
| cy.contains('button', `Unfollow ${author.username}`) | ||
| .should('be.visible'); | ||
|
|
||
| cy.get('[data-cy="username-link"]').click(); | ||
| cy.contains('a', 'My Articles').should('be.visible'); |
There was a problem hiding this comment.
Missing test for unfollowing user. The task requirement states 'following/unfollowing the user'. Currently only has the follow test. Add a separate test that: logs in as user, visits author's profile, follows, then unfollows, and verifies button changes back to 'Follow'.
|
|
||
| it.skip('should be able to follow the another user', () => { | ||
| it('should be able to follow the another user', () => { | ||
| cy.visit(`/#/profile/${author.username}`); | ||
|
|
||
| cy.contains('button', `Follow ${author.username}`) | ||
| .should('be.visible') | ||
| .click(); | ||
|
|
||
| cy.contains('button', `Unfollow ${author.username}`) | ||
| .should('be.visible'); | ||
|
|
||
| cy.get('[data-cy="username-link"]').click(); | ||
| cy.contains('a', 'My Articles').should('be.visible'); |
There was a problem hiding this comment.
This test uses mixed selectors - data-cy for login (lines 22-24) but contains/placeholder for other elements. For consistency with advanced level requirements, use data-cy attributes throughout.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
All functional requirements are satisfied and tests are structurally sound. Your solution is approved.
✨ 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
| cy.contains('.nav-link', 'New Article').click(); | ||
| cy.get('input[placeholder="Article Title"]').type(article.title); | ||
| cy.get('input[placeholder="What\'s this article about?"]') | ||
| .type(article.description); | ||
| cy.get('textarea[placeholder="Write your article (in markdown)"]') | ||
| .type(article.body); | ||
| cy.get('input[placeholder="Enter tags"]').type('testing{enter}'); |
There was a problem hiding this comment.
The article test file mixes raw selectors with PageObject pattern. While it uses cy.getByDataCy('email-sign-in') for login, it uses placeholder selectors like input[placeholder="Article Title"] for article form elements. For consistency with the Advanced level requirements and other test files, consider adding data-cy attributes to these elements and creating an ArticlePageObject class in cypress/support/pages/.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your e2e test suite demonstrates solid understanding of Cypress, PageObject pattern, and the required test scenarios - all 12 requirements are covered with proper database cleanup and structure. I'm providing an educational approval so you can continue progressing. Important fix needed: In settings.pageObject.js (and related signUp/ArticleActions/CommentEditor files), the data-cy selectors are inverted - they use username-settings, bio-settings, etc., but Settings.vue defines them as settings-username, settings-bio, etc. Update all four selectors to match the Vue component's attribute names. Once these selector mismatches are corrected, your settings tests will pass successfully.
✨ 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
| cy.task('generateUser').then((res) => { | ||
| user = res; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The task requires data-qa attributes, but the code uses data-cy attributes. Either the task description uses outdated terminology, or the wrong attribute name is being used throughout.
| signInPage.typeEmail(user.email); | ||
| signInPage.typePassword(user.password); | ||
| signInPage.clickSignInBtn(); | ||
| settingsPage.visit(); | ||
| }); |
There was a problem hiding this comment.
These fields use placeholder selectors instead of data attributes. Consider using data-qa attributes for better test stability.
| const newPassword = 'updatedPassword123'; | ||
|
|
||
| settingsPage.passwordField.type(newPassword); | ||
| settingsPage.updateBtn.click(); |
There was a problem hiding this comment.
Uses class selector .btn-outline-danger instead of a data attribute. The delete button in ArticleActions.vue has data-cy="delete-article-btn".
| /// <reference types='cypress' /> | ||
| /// <reference types='../support' /> | ||
|
|
||
| import SettingsPageObject from '../support/pages/settings.pageObject'; | ||
| import SignInPageObject from '../support/pages/signIn.pageObject'; | ||
|
|
||
| const settingsPage = new SettingsPageObject(); | ||
| const signInPage = new SignInPageObject(); | ||
|
|
||
| describe('Settings page', () => { | ||
| before(() => { | ||
| let user; | ||
|
|
||
| before(() => { | ||
| cy.task('generateUser').then((res) => { | ||
| user = res; | ||
| }); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| cy.task('db:clear'); | ||
| cy.register(user.email, user.username, user.password); | ||
|
|
||
| signInPage.visit(); | ||
| signInPage.typeEmail(user.email); | ||
| signInPage.typePassword(user.password); | ||
| signInPage.clickSignInBtn(); | ||
| settingsPage.visit(); | ||
| }); |
There was a problem hiding this comment.
This file only tests positive login. The negative test case is in signIn.cy.js. Consider consolidating or ensuring this file is the primary login test file.
| const newBio = 'Updated'; | ||
| settingsPage.bioField.clear().type(newBio); | ||
| settingsPage.updateBtn.click(); | ||
| settingsPage.bioField.should('have.value', newBio); |
There was a problem hiding this comment.
bioField assertion uses have.value but bio is a textarea - verify the assertion works correctly for textarea elements.
| <div v-if="is_loading" class="article-preview">Loading articles...</div> | ||
| <div v-else> | ||
| <div v-if="articles.length === 0" class="article-preview"> | ||
| <div v-if="articles.length === 0" data-cy="no-articles-message" class="article-preview"> |
There was a problem hiding this comment.
Critical mismatch: The PageObject expects data-cy="username-settings" but Settings.vue uses data-cy="settings-username". The selector names are inverted. Fix to: get usernameField() { return cy.getByDataCy('settings-username'); }
| <div v-else> | ||
| <div v-if="articles.length === 0" class="article-preview"> | ||
| <div v-if="articles.length === 0" data-cy="no-articles-message" class="article-preview"> | ||
| No articles are here... yet. |
There was a problem hiding this comment.
Critical mismatch: The PageObject expects data-cy="bio-settings" but Settings.vue uses data-cy="settings-bio". Fix to: get bioField() { return cy.getByDataCy('settings-bio'); }
| <div v-if="articles.length === 0" class="article-preview"> | ||
| <div v-if="articles.length === 0" data-cy="no-articles-message" class="article-preview"> | ||
| No articles are here... yet. | ||
| </div> |
There was a problem hiding this comment.
Critical mismatch: The PageObject expects data-cy="email-settings" but Settings.vue uses data-cy="settings-email". Fix to: get emailField() { return cy.getByDataCy('settings-email'); }
| <div v-if="articles.length === 0" data-cy="no-articles-message" class="article-preview"> | ||
| No articles are here... yet. | ||
| </div> | ||
| <ArticlePreview |
There was a problem hiding this comment.
Critical mismatch: The PageObject expects data-cy="password-settings" but Settings.vue uses data-cy="settings-password". Fix to: get passwordField() { return cy.getByDataCy('settings-password'); }
|
|
||
| class SignUpPageObject extends PageObject { | ||
| url = '/#/register'; | ||
|
|
There was a problem hiding this comment.
Mismatch: PageObject uses username-settings but Settings.vue has data-cy="settings-username". Update the selector to settings-username.
| class SignUpPageObject extends PageObject { | ||
| url = '/#/register'; | ||
|
|
||
| get usernameField() { return cy.getByDataCy('username-sign-up'); } |
There was a problem hiding this comment.
Mismatch: PageObject uses bio-settings but Settings.vue has data-cy="settings-bio". Update the selector to settings-bio.
| url = '/#/register'; | ||
|
|
||
| get usernameField() { return cy.getByDataCy('username-sign-up'); } | ||
| get emailField() { return cy.getByDataCy('email-sign-up'); } |
There was a problem hiding this comment.
Mismatch: PageObject uses email-settings but Settings.vue has data-cy="settings-email". Update the selector to settings-email.
|
|
||
| get usernameField() { return cy.getByDataCy('username-sign-up'); } | ||
| get emailField() { return cy.getByDataCy('email-sign-up'); } | ||
| get passwordField() { return cy.getByDataCy('password-sign-up'); } |
There was a problem hiding this comment.
Mismatch: PageObject uses password-settings but Settings.vue has data-cy="settings-password". Update the selector to settings-password.
| <span v-if="canModify"> | ||
| <router-link class="btn btn-sm btn-outline-secondary" :to="editArticleLink"> | ||
| <router-link data-cy="edit-article-btn" class="btn btn-sm btn-outline-secondary" :to="editArticleLink"> | ||
| <i class="ion-edit"></i> <span> Edit Article</span> |
There was a problem hiding this comment.
CRITICAL: The selector uses username-settings but Settings.vue has data-cy="settings-username". These are reversed! This mismatch will cause tests to fail.
| <router-link class="btn btn-sm btn-outline-secondary" :to="editArticleLink"> | ||
| <router-link data-cy="edit-article-btn" class="btn btn-sm btn-outline-secondary" :to="editArticleLink"> | ||
| <i class="ion-edit"></i> <span> Edit Article</span> | ||
| </router-link> |
There was a problem hiding this comment.
CRITICAL: The selector uses bio-settings but Settings.vue has data-cy="settings-bio". These are reversed! This mismatch will cause tests to fail.
| <router-link data-cy="edit-article-btn" class="btn btn-sm btn-outline-secondary" :to="editArticleLink"> | ||
| <i class="ion-edit"></i> <span> Edit Article</span> | ||
| </router-link> | ||
| <span> </span> |
There was a problem hiding this comment.
CRITICAL: The selector uses email-settings but Settings.vue has data-cy="settings-email". These are reversed! This mismatch will cause tests to fail.
| </router-link> | ||
| <span> </span> | ||
| <button class="btn btn-outline-danger btn-sm" @click="deleteArticle"> | ||
| <button data-cy="delete-article-btn" class="btn btn-outline-danger btn-sm" @click="deleteArticle"> |
There was a problem hiding this comment.
CRITICAL: The selector uses password-settings but Settings.vue has data-cy="settings-password". These are reversed! This mismatch will cause tests to fail.
| @@ -4,6 +4,7 @@ | |||
| <form class="card comment-form" @submit.prevent="onSubmit(slug, comment)"> | |||
| <div class="card-block"> | |||
There was a problem hiding this comment.
Critical mismatch: The PageObject uses username-settings but Settings.vue defines data-cy="settings-username". These are inverted. Change to settings-username.
| @@ -4,6 +4,7 @@ | |||
| <form class="card comment-form" @submit.prevent="onSubmit(slug, comment)"> | |||
| <div class="card-block"> | |||
| <textarea | |||
There was a problem hiding this comment.
Critical mismatch: The PageObject uses bio-settings but Settings.vue defines data-cy="settings-bio". Change to settings-bio.
| <form class="card comment-form" @submit.prevent="onSubmit(slug, comment)"> | ||
| <div class="card-block"> | ||
| <textarea | ||
| data-cy="comment-field" |
There was a problem hiding this comment.
Critical mismatch: The PageObject uses email-settings but Settings.vue defines data-cy="settings-email". Change to settings-email.
| <div class="card-block"> | ||
| <textarea | ||
| data-cy="comment-field" | ||
| class="form-control" |
There was a problem hiding this comment.
Critical mismatch: The PageObject uses password-settings but Settings.vue defines data-cy="settings-password". Change to settings-password.
No description provided.