Conversation
rotimiazeez
left a comment
rotimiazeez
left a comment
There was a problem hiding this comment.
Hi @MosDevx & @Talha-Hanif5666 👍🏽,
Good job so far!
To Highlight 👇🏽
- You did great solving the n+1 query 👍🏽
- Capybara gem is added in
Gemfilefor integration test ✔️ - Good PR description 👍🏽
- Good commit message(s) 😉
- Descriptive README 📖
You still need to work on some issues to prepare your project for the final evaluation, but you are almost there!
Suggested changes
Check the comments under the review.
Please feel free to use as many of my suggestions as you want. If there is anything you would like to skip - feel free to do that. However, I strongly recommend considering them as they can improve your code._
Cheers, and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Remember to tag (@rotimiazeez) me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
| require 'rails_helper' | ||
|
|
||
| RSpec.feature 'Users', type: :feature, feat: true do | ||
| before(:all) do | ||
| @user = User.create(name: 'Jann Doe', bio: 'I am John Doe', email: 'rand_email') | ||
| end | ||
|
|
||
| after(:all) do | ||
| @user.destroy | ||
| end | ||
|
|
||
| it 'displays all users' do | ||
| visit users_path | ||
| expect(page).to have_content('Users') | ||
| end | ||
|
|
||
| it 'displays a user' do | ||
| # user = User.create(name: 'John Doe', bio: 'I am John Doe') | ||
| # puts user.id | ||
| visit user_path(@user) | ||
| expect(page).to have_content('John Doe') | ||
| end | ||
|
|
||
| it 'displays a user edit page' do | ||
| # user = User.create(name: 'John Doe', bio: 'I am John Doe') | ||
| visit edit_user_path(@user) | ||
| expect(page).to have_content('Edit User') | ||
| end | ||
| end |
There was a problem hiding this comment.
Please ensure featured tests for User show page is implemented and includes the following tests to meet all the project requirements:
- Tests that assert you can see the user's profile picture.
- Tests that asserts you can see the user's username.
- Tests that asserts you can see the number of posts the user has written.
- Tests that assert you can see the user's bio.
- Tests that assert you can see the user's first 3 posts.
- Tests that assert you can see a button that lets me view all of a user's posts.
- Tests that redirect to the post's show page when you click on a user's post.
- Tests that redirect to the user's post's index page when you click on a see all posts.
There was a problem hiding this comment.
Please ensure featured tests for User index page is implemented and includes the following tests to meet all the project requirement:
- Tests that assert you can see the username of all other users.
- Tests that assert you can see the profile picture for each user.
- Tests that assert you can see the number of posts each user has written.
- Tests that redirect to the user's show page when you click on a user.
| require 'rails_helper' | ||
|
|
||
| RSpec.describe 'Posts', type: :feature, feat: true do | ||
| before(:all) do | ||
| @user = User.create(name: 'Jann Doe', bio: 'I am John Doe', email: 'rand_email') | ||
| @post = Post.create(user: @user, title: 'Hello', text: 'Teacher need to get trained') | ||
| end | ||
|
|
||
| after(:all) do | ||
| @user.destroy | ||
| @post.destroy | ||
| end | ||
|
|
||
| it 'displays all posts' do | ||
| visit user_posts_path(@user) | ||
| expect(page).to have_content('Post') | ||
| end | ||
|
|
||
| it 'displays a post' do | ||
| visit user_post_path(user_id: @user.id, id: @post.id) | ||
| expect(page).to have_content('Teacher') | ||
| end | ||
| end |
There was a problem hiding this comment.
Please ensure featured tests for Post show page is implemented and includes the following tests to meet all the project requirements:
- Tests that assert you can see the post's title.
- Tests that assert you can see who wrote the post.
- Tests that assert you can see how many comments it has.
- Tests that assert you can see how many likes it has.
- Tests that assert you can see the post body.
- Tests that assert you can see the username of each commentor.
- Tests that assert you can see the comment each commentor left.
There was a problem hiding this comment.
Please ensure featured tests for Post index page is implemented and includes the following tests to meet all the project requirements:
- Tests that assert you can see the user's profile picture.
- Tests that assert you can see the user's username.
- Tests that assert you can see the number of posts the user has written.
- Tests that assert you can see a post's title.
- Tests that assert you can see some of the post's body.
- Tests that assert you can see the first comments on a post.
- Tests that assert you can see how many comments a post has.
- Tests that assert you can see how many likes a post has.
- Tests that assert you can see a section for pagination if there are more posts than fit on the view.
- Tests that redirect to the post's show page when you click on a post.
There was a problem hiding this comment.
Done. Ive added the tests. Thanks
codecaiine
left a comment
codecaiine
left a comment
There was a problem hiding this comment.
Hi @MosDevx @Talha-Hanif5666 ,
Great work on making the changes requested by a previous reviewer 👏
You've done well implementing some of the requested changes, but there are still some which aren't addressed yet.
Highlights
- Good test setup ✔️
Required Changes ♻️
Check the comments under the review.
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me (@codecaiine) in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
| @@ -1,9 +1,10 @@ | |||
| require 'rails_helper' | |||
|
|
|||
| RSpec.describe 'Posts', type: :feature, feat: true do | |||
There was a problem hiding this comment.
- Good job so far 👏 But some features especially for posts are missing 😢 As the previous reviewer said Please make sure that all featured tests are implemented and include the all tests to meet all the project requirements 👍
|
HI @MosDevx I would like to recommend you make sure that your project meets the requirement before submitting it, always double-check the project requirement and also the reviewer's request change. Because when you submit the project to be reviewed there is a 99% chance that it is under review, someone is reviewing it. Please note that making changes while the reviewer is submitting the review can be confusing. 👍 |
OyePriscilla
left a comment
OyePriscilla
left a comment
There was a problem hiding this comment.
Hi @MosDevx and @Talha-Hanif5666 👏 👏,
Great work on making the changes requested by a previous reviewer 👏
You've done well implementing some of the requested changes, but there are still some which aren't addressed yet.
Highlights
Write integration tests with Capybara gem ✔️
All test passed ✔️
Required Changes ♻️
Check the comments under the review.
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
|
|
||
| # rubocop:disable Metrics/BlockLength | ||
| RSpec.feature 'User', type: :feature do |
There was a problem hiding this comment.
- I observed that this linter was not disabled as at last review, kindly remove it and make use of rubocop.yml file to achieve your aim. This will make your work neat. ℹ️ Note: Line 79 inclusive
Owusu-Desmond
left a comment
Owusu-Desmond
left a comment
There was a problem hiding this comment.
Hi @MosDevx ,
Your project is complete! There is nothing else to say other than... it's time to merge it 
Congratulations! 🎉
Highlights
📌 No Linters error ✅
📌 Descriptive pull request✅
📌 Good commit messages ✅
📌 Professional README file ✅
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
6 participants
Changes Implemented
Before
After