Workflow - Peer Review #388
Conversation
…tart' and updating master.yml to run correct steps
|
WalkthroughThis pull request introduces a comprehensive set of changes, primarily focusing on configuration and testing setup, code formatting, and addition of new test cases. It also includes updates to the project documentation and GitHub Actions workflows. The logic and functionality of the application remain largely unchanged. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (102)
- .eslintrc.json (1)
- .github/workflows/master.yml (1)
- .github/workflows/pages.yml (3)
- .gitignore (1)
- .husky/pre-commit (1)
- .prettierrc (1)
- 6 (1)
- READEME.md (1)
- babel.config.js (1)
- cypress.config.js (1)
- cypress/e2e/3-testing/README.md (1)
- cypress/e2e/3-testing/login.cy.js (1)
- cypress/e2e/3-testing/postinteraction.cy.js (1)
- cypress/e2e/3-testing/register.cy.js (1)
- cypress/support/commands.js (1)
- cypress/support/e2e.js (1)
- index.html (1)
- index.js (1)
- src/js/api/apiBase.js (1)
- src/js/api/auth/index.js (1)
- src/js/api/auth/login.js (1)
- src/js/api/auth/login.test.js (1)
- src/js/api/auth/logout.js (1)
- src/js/api/auth/logout.test.js (1)
- src/js/api/auth/register.js (1)
- src/js/api/auth/state.js (1)
- src/js/api/constants.js (1)
- src/js/api/headers.js (1)
- src/js/api/index.js (1)
- src/js/api/posts/comment.js (1)
- src/js/api/posts/create.js (1)
- src/js/api/posts/delete.js (1)
- src/js/api/posts/index.js (1)
- src/js/api/posts/react.js (1)
- src/js/api/posts/read.js (1)
- src/js/api/posts/update.js (1)
- src/js/api/profiles/delete.js (1)
- src/js/api/profiles/follow.js (1)
- src/js/api/profiles/index.js (1)
- src/js/api/profiles/read.js (1)
- src/js/api/profiles/unfollow.js (1)
- src/js/api/profiles/update.js (1)
- src/js/data/blank/post.js (1)
- src/js/index.js (1)
- src/js/listeners/auth/index.js (1)
- src/js/listeners/auth/login.js (1)
- src/js/listeners/auth/logout.js (1)
- src/js/listeners/auth/register.js (1)
- src/js/listeners/index.js (1)
- src/js/listeners/post/comment.js (2)
- src/js/listeners/post/index.js (1)
- src/js/listeners/post/reaction.js (2)
- src/js/listeners/profile/follow.js (2)
- src/js/listeners/profile/index.js (1)
- src/js/listeners/profile/unfollow.js (2)
- src/js/router/index.js (1)
- src/js/router/searchParams.js (1)
- src/js/storage/index.js (1)
- src/js/storage/load.js (1)
- src/js/storage/save.js (1)
- src/js/templates/comment/badge.js (1)
- src/js/templates/comment/comment.js (1)
- src/js/templates/comment/comment.test.js (1)
- src/js/templates/comment/form.js (1)
- src/js/templates/comment/index.js (1)
- src/js/templates/index.js (1)
- src/js/templates/instance.js (1)
- src/js/templates/post/actions.js (2)
- src/js/templates/post/comments.js (1)
- src/js/templates/post/footer.js (1)
- src/js/templates/post/form.js (1)
- src/js/templates/post/header.js (1)
- src/js/templates/post/index.js (1)
- src/js/templates/post/loader.js (1)
- src/js/templates/post/media.js (1)
- src/js/templates/post/page.js (1)
- src/js/templates/post/reactions.js (1)
- src/js/templates/post/tabs.js (1)
- src/js/templates/post/tags.js (1)
- src/js/templates/post/thumbnail.js (1)
- src/js/templates/profile/button.js (1)
- src/js/templates/profile/followers.js (1)
- src/js/templates/profile/following.js (1)
- src/js/templates/profile/follows.js (1)
- src/js/templates/profile/index.js (1)
- src/js/templates/profile/page.js (1)
- src/js/templates/profile/thumbnail.js (1)
- src/js/tools/clear.js (1)
- src/js/tools/emojiSubset.js (1)
- src/js/tools/index.js (1)
- src/js/ui/auth.js (1)
- src/js/ui/footer.js (1)
- src/js/ui/header.js (1)
- src/js/ui/index.js (1)
- src/js/ui/modals.js (1)
- src/js/ui/modals.test.js (1)
- src/js/ui/renderView.js (1)
- src/js/views/index.js (1)
- src/js/views/postList.js (1)
- src/js/views/postPage.js (2)
- src/js/views/profileList.js (1)
- src/js/views/profilePage.js (1)
Files ignored due to filter (6)
- cypress.env.json
- cypress/fixtures/example.json
- cypress/fixtures/profile.json
- cypress/fixtures/users.json
- package-lock.json
- package.json
Files not summarized due to errors (1)
- index.html (diff tokens exceeds limit)
Files skipped from review due to trivial changes (95)
- .eslintrc.json
- .github/workflows/master.yml
- .github/workflows/pages.yml
- .gitignore
- .husky/pre-commit
- .prettierrc
- READEME.md
- babel.config.js
- cypress.config.js
- cypress/e2e/3-testing/README.md
- cypress/e2e/3-testing/login.cy.js
- cypress/support/commands.js
- cypress/support/e2e.js
- index.js
- src/js/api/apiBase.js
- src/js/api/auth/index.js
- src/js/api/auth/login.js
- src/js/api/auth/login.test.js
- src/js/api/auth/logout.js
- src/js/api/auth/logout.test.js
- src/js/api/auth/register.js
- src/js/api/auth/state.js
- src/js/api/constants.js
- src/js/api/headers.js
- src/js/api/index.js
- src/js/api/posts/comment.js
- src/js/api/posts/create.js
- src/js/api/posts/delete.js
- src/js/api/posts/index.js
- src/js/api/posts/react.js
- src/js/api/posts/read.js
- src/js/api/posts/update.js
- src/js/api/profiles/delete.js
- src/js/api/profiles/follow.js
- src/js/api/profiles/index.js
- src/js/api/profiles/read.js
- src/js/api/profiles/unfollow.js
- src/js/data/blank/post.js
- src/js/index.js
- src/js/listeners/auth/index.js
- src/js/listeners/auth/logout.js
- src/js/listeners/auth/register.js
- src/js/listeners/index.js
- src/js/listeners/post/comment.js
- src/js/listeners/post/index.js
- src/js/listeners/post/reaction.js
- src/js/listeners/profile/follow.js
- src/js/listeners/profile/index.js
- src/js/listeners/profile/unfollow.js
- src/js/router/index.js
- src/js/router/searchParams.js
- src/js/storage/index.js
- src/js/storage/load.js
- src/js/storage/save.js
- src/js/templates/comment/comment.js
- src/js/templates/comment/comment.test.js
- src/js/templates/comment/form.js
- src/js/templates/comment/index.js
- src/js/templates/index.js
- src/js/templates/instance.js
- src/js/templates/post/actions.js
- src/js/templates/post/comments.js
- src/js/templates/post/footer.js
- src/js/templates/post/form.js
- src/js/templates/post/header.js
- src/js/templates/post/index.js
- src/js/templates/post/loader.js
- src/js/templates/post/media.js
- src/js/templates/post/page.js
- src/js/templates/post/reactions.js
- src/js/templates/post/tabs.js
- src/js/templates/post/tags.js
- src/js/templates/post/thumbnail.js
- src/js/templates/profile/button.js
- src/js/templates/profile/followers.js
- src/js/templates/profile/following.js
- src/js/templates/profile/follows.js
- src/js/templates/profile/index.js
- src/js/templates/profile/page.js
- src/js/templates/profile/thumbnail.js
- src/js/tools/clear.js
- src/js/tools/emojiSubset.js
- src/js/tools/index.js
- src/js/ui/auth.js
- src/js/ui/footer.js
- src/js/ui/header.js
- src/js/ui/index.js
- src/js/ui/modals.js
- src/js/ui/renderView.js
- src/js/views/index.js
- src/js/views/postList.js
- src/js/views/postPage.js
- src/js/views/profileList.js
- src/js/views/profilePage.js
- index.html (diff too large)
Review comments generated (17)
- Review: 12
- LGTM: 5
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
6
Outdated
|
|
||
| added 56 packages, and audited 385 packages in 4s | ||
|
|
||
| 60 packages are looking for funding | ||
| run `npm fund` for details | ||
|
|
||
| 3 high severity vulnerabilities | ||
|
|
||
| To address all issues (including breaking changes), run: | ||
| npm audit fix --force | ||
|
|
||
| Run `npm audit` for details. |
There was a problem hiding this comment.
The output of the npm install command shows that there are 3 high severity vulnerabilities in your dependencies. This is a security concern and should be addressed. You can use npm audit fix to automatically install any compatible updates that address these vulnerabilities. If automatic fixes are not available or fail, you may need to manually update or replace the affected packages.
- 3 high severity vulnerabilities
+ No vulnerabilities foundPlease note that the above diff is just an example of what the output should look like after addressing the vulnerabilities. The actual changes will depend on the specific packages and versions involved.
| import * as auth from '../../api/auth/index.js'; | ||
| import { updateLoginVisibility } from '../../ui/auth.js'; | ||
|
|
||
| export async function loginListener(event) { | ||
| event.preventDefault() | ||
| const form = event.target | ||
| const data = new FormData(form) | ||
| const email = data.get("email") | ||
| const password = data.get("password") | ||
| event.preventDefault(); | ||
| const form = event.target; | ||
| const data = new FormData(form); | ||
| const email = data.get('email'); | ||
| const password = data.get('password'); | ||
| try { | ||
| const { name } = await auth.login(email, password) | ||
| updateLoginVisibility() | ||
| location.href = `./?view=profile&name=${name}` | ||
| const { name } = await auth.login(email, password); | ||
| updateLoginVisibility(); | ||
| location.href = `./?view=profile&name=${name}`; | ||
| } catch { | ||
| return alert("Either your username was not found or your password is incorrect") | ||
| return alert( | ||
| 'Either your username was not found or your password is incorrect' | ||
| ); | ||
| } |
There was a problem hiding this comment.
The code looks good overall, but there's a potential security issue. The alert function is being used to display error messages to the user. While this isn't inherently bad, it can be problematic if the error message contains sensitive information or data that could be exploited by malicious users. In this case, the error message is hardcoded and doesn't contain any sensitive data, but it's a good practice to avoid using alert for error handling in production code. Consider using a custom modal or another method to safely display error messages to the user.
- return alert(
- 'Either your username was not found or your password is incorrect'
- );
+ // Replace with a safer method of displaying error messages
+ displayErrorMessage('Either your username was not found or your password is incorrect');Also, you should add a newline at the end of the file to comply with standard coding conventions.
- }
\ No newline at end of file
+ }
+| describe('Register Test', () => { | ||
| beforeEach(() => { | ||
| cy.visit('/'); | ||
| cy.wait(500); // waiting for the modal to fully initialize | ||
| }); | ||
|
|
||
| it('fails to register with invalid inputs', () => { | ||
| // Testing name input with invalid and valid data | ||
| cy.get('#registerName').should('be.visible').clear(); | ||
| cy.get('#registerName').type('InvalidName---{enter}'); | ||
| cy.wait(500); | ||
| cy.get('#registerName').clear(); | ||
| const registrationUsername = Cypress.env('REGISTRATION_USER_NAME'); | ||
| cy.get('#registerName').type(`${registrationUsername}{enter}`); | ||
| cy.log('Username input ok!'); | ||
| // Testing email input with invalid and valid data | ||
| cy.get('#registerEmail').clear(); | ||
| cy.get('#registerEmail').type('invalidemail@error{enter}'); | ||
| cy.wait(500); | ||
| cy.get('#registerEmail').clear(); | ||
| const registrationEmail = Cypress.env('TEST_EMAIL'); | ||
| cy.get('#registerEmail').type(`${registrationEmail}{enter}`); | ||
| cy.log('Email input ok!'); | ||
| // Testing password input with invalid and valid data | ||
| cy.get('#registerPassword').should('be.visible').clear(); | ||
| cy.get('#registerPassword').type('error{enter}'); | ||
| cy.wait(500); | ||
| cy.get('#registerPassword').clear(); | ||
| const registrationPsw = Cypress.env('TEST_PASSWORD'); | ||
| cy.get('#registerPassword').type(`${registrationPsw}{enter}`); | ||
| cy.log('Password input ok!'); | ||
| // Testing avatar URL input with invalid and valid data | ||
| cy.get('#registerAvatar').clear(); | ||
| cy.get('#registerAvatar').type('invalidurl{enter}'); | ||
| cy.wait(500); | ||
| cy.get('#registerAvatar').clear(); | ||
| const registrationIMG = Cypress.env('REGISTRATION_IMG'); | ||
| cy.get('#registerAvatar').type(`${registrationIMG}{enter}`); | ||
| cy.log('avatar/media input ok!'); | ||
| cy.log('FINISHED'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test case 'fails to register with invalid inputs' does not seem to assert any conditions. It's important to have assertions in your tests to verify that the application is behaving as expected. For example, after entering an invalid email, you should check if an error message is displayed.
Here's a suggestion on how you could add assertions:
// Testing email input with invalid and valid data
cy.get('#registerEmail').clear();
cy.get('#registerEmail').type('invalidemail@error{enter}');
cy.wait(500);
+ cy.get('.error-message').should('be.visible');
cy.get('#registerEmail').clear();
const registrationEmail = Cypress.env('TEST_EMAIL');
cy.get('#registerEmail').type(`${registrationEmail}{enter}`);
cy.log('Email input ok!');In this example, I'm assuming there's an element with class error-message that shows up when an error occurs. You would need to replace it with the actual selector for your application.
Also, consider removing the cy.log() statements or replacing them with more meaningful messages. These logs are only visible in the Cypress command log and do not provide much value in their current form. If you want to keep them, consider making them more descriptive, e.g., cy.log('Valid username entered successfully').
| export async function updateProfileImage(avatar) { | ||
| const me = profile() | ||
| const me = profile(); | ||
|
|
||
| const response = await fetch(`${apiPath}/social/profiles/${me.name}`, { | ||
| method: "put", | ||
| method: 'put', | ||
| body: JSON.stringify({ ...me, avatar }), | ||
| headers: headers("application/json") | ||
| }) | ||
| headers: headers('application/json'), | ||
| }); |
There was a problem hiding this comment.
The updateProfileImage function is missing error handling for the fetch request. If the fetch request fails due to network issues, it will throw an unhandled promise rejection. You should wrap the fetch call in a try-catch block to handle potential errors.
export async function updateProfileImage(avatar) {
const me = profile();
try {
const response = await fetch(`${apiPath}/social/profiles/${me.name}`, {
method: 'put',
body: JSON.stringify({ ...me, avatar }),
headers: headers('application/json'),
});
return response;
} catch (error) {
console.error(error);
throw error;
}
}|
|
||
| if (response.ok) { | ||
| return await response.json() | ||
| return await response.json(); | ||
| } |
There was a problem hiding this comment.
The response.ok check is good, but it's also important to handle cases where the response status is not ok. In such cases, you should throw an error with the status text or some other meaningful message.
if (response.ok) {
return await response.json();
}
+ else {
+ throw new Error(`Failed to update profile image: ${response.statusText}`);
+ }| const loginPsw = Cypress.env('TEST_PASSWORD'); | ||
| cy.get('#loginPassword').type(`${loginPsw}{enter}`); | ||
|
|
||
| cy.wait(500); |
There was a problem hiding this comment.
Again, avoid using cy.wait(500);. Use the built-in wait mechanism instead.
- cy.wait(500);
+ cy.get('a[href="./"]', { timeout: 5000 });| cy.wait(500); | ||
| cy.get('a[href="./"]').click(); | ||
|
|
||
| cy.wait(500); |
There was a problem hiding this comment.
Avoid using cy.wait(500);. Use the built-in wait mechanism instead.
- cy.wait(500);
+ cy.get('.post .dropdown-toggle', { timeout: 5000 });| .invoke('attr', 'data-post-id') | ||
| .then(postId => { | ||
| cy.get(`button[data-symbol="😀"][data-post-id="${postId}"]`).click(); | ||
| cy.wait(2000); |
There was a problem hiding this comment.
Avoid using cy.wait(2000);. Use the built-in wait mechanism instead.
- cy.wait(2000);
+ cy.get(`a[href="./?view=post&postId=${postId}"]`, { timeout: 5000 });| cy.log('Reaction test successful'); | ||
| cy.get(`a[href="./?view=post&postId=${postId}"]`).click(); | ||
|
|
||
| cy.wait(2000); |
There was a problem hiding this comment.
Avoid using cy.wait(2000);. Use the built-in wait mechanism instead.
- cy.wait(2000);
+ cy.get('textarea#commentBody', { timeout: 5000 });| cy.get('textarea#commentBody').type(testComment); | ||
| cy.get('form.comment-form button[type="submit"]').click(); | ||
|
|
||
| cy.wait(2000); |
There was a problem hiding this comment.
Avoid using cy.wait(2000);. Use the built-in wait mechanism instead.
- cy.wait(2000);
+ cy.get('.comment', { timeout: 5000 });…d som excess files
There was a problem hiding this comment.
Files selected (1)
- cypress/e2e/3-testing/postinteraction.cy.js (1)
Files ignored due to filter (2)
- package-lock.json
- package.json
Review comments generated (8)
- Review: 8
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| cy.wait(500); | ||
|
|
There was a problem hiding this comment.
It's not a good practice to use cy.wait() with hardcoded time values in Cypress tests. This can lead to flaky tests if the operation takes longer than expected. Instead, you should use Cypress's built-in ability to automatically wait for elements to exist in the DOM, commands to complete, or requests to finish.
- cy.wait(500);| cy.wait(300); | ||
| cy.get('#loginEmail').type(`${loginEmail}{enter}`); |
There was a problem hiding this comment.
Same issue here with cy.wait(). Also, it's better to use .type() method directly without clearing the input field first unless it's necessary.
- cy.wait(300);
- cy.get('#loginEmail').clear();
- cy.get('#loginEmail').type(`${loginEmail}{enter}`);
+ cy.get('#loginEmail').type(`${loginEmail}{enter}`, {force: true});| cy.get('#loginPassword').clear(); | ||
| const loginPsw = Cypress.env('TEST_PASSWORD'); | ||
| cy.get('#loginPassword').type(`${loginPsw}{enter}`); |
There was a problem hiding this comment.
Same issue here with cy.wait() and .clear().
- cy.get('#loginPassword').clear();
- cy.get('#loginPassword').type(`${loginPsw}{enter}`);
+ cy.get('#loginPassword').type(`${loginPsw}{enter}`, {force: true});| cy.wait(500); | ||
| cy.get('a[href="./"]').click(); |
There was a problem hiding this comment.
Again, avoid using cy.wait(). Use cy.click() directly.
- cy.wait(500);
- cy.get('a[href="./"]').click();
+ cy.get('a[href="./"]').click({force: true});| cy.wait(500); | ||
| cy.window().then(window => { |
There was a problem hiding this comment.
Avoid using cy.wait(). Use assertions directly.
- cy.wait(500);| cy.wait(2000); | ||
| cy.log('Reaction test successful'); |
There was a problem hiding this comment.
Avoid using cy.wait(). Use assertions directly.
- cy.wait(2000);| cy.wait(2000); | ||
|
|
There was a problem hiding this comment.
Avoid using cy.wait(). Use assertions directly.
- cy.wait(2000);| cy.wait(2000); | ||
|
|
There was a problem hiding this comment.
Avoid using cy.wait(). Use assertions directly.
- cy.wait(2000);
Summary by CodeRabbit
loginandlogoutfunctions.