Feature fix n+1 issues #8
Conversation
There was a problem hiding this comment.
Hi @IsmailChangezi,
While you made a great effort in this project, unfortunately, I cannot proceed to review your code.
Invalid Code Review Request
- You have submitted a pair-programming activity with only one contributor. Kindly ensure that your team members contribute to this project.
Your Code Review Request will be marked as invalid in your Dashboard, so please submit a new one once you are ready 🙏
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
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.
Invalid Code Review Request does not count into the code reviews limit.
ImportantMy Coding partner is going to repeat due to some issues. He won't be collaborating with me this week. I have contacted Student Success about it. That is why I have to submit this project on my own. |
There was a problem hiding this comment.
Hi @IsmailChangezi 🙋♀️,
Great work so far! 💯
You have done a great job adding the Blog app - integration specs for views. 🎊 🥳
There are some issues that you still need to work on to improve your project according to the requirements. You are almost there! 💪🏾
Highlights 🌞
✔️ Professional pull request
✔️ The app solves N+1 problem when fetching all posts and their comments for a user
✔️ Integration specs for the Users and Posts index and show page views are sufficient
Required Changes ♻️
Kindly look through the comments below 👇 for required changes
Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
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 @KDlamini 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.
spec/requests/users_spec.rb
Outdated
There was a problem hiding this comment.
[OPTIONAL] Kindly consider using the correct directories for your tests to avoid confusion. For this milestone, your tests were supposed to be in the spec/views directory and yet they are in the requests directory.
spec/requests: contains specs that test the application's responses to HTTP requests. Request specs are useful for testing controllers and routes.spec/views: contains view specs to test rendering and view logic. View specs test the content and behavior of view templates. They render the view template and allow assertions against the rendered content.
The separation helps organize these related but distinct types of tests.
[REQUIRED] About 3 of your tests failed. Kindly look into the error messages and try to debug this issue.
Failures:
There was a problem hiding this comment.
Hey @KDlamini ,
The integration tests are in the feature folder. These tests that you have highlighted are pre-built. I think you should know that and I think you should be more honest in your work.
There was a problem hiding this comment.
Hi @IsmailChangezi, Good job so far maybe the previous reviewer mixed up the folders.
All your tests are passing, you are missing this test: Tests that asserts you can see the user's profile picture in your posts_show_spec.rb, can you update it?
There was a problem hiding this comment.
Hey @prolajumokeoni ,
Working on it
There was a problem hiding this comment.
@prolajumokeoni done. Sorry I put the wrong commit message
There was a problem hiding this comment.
Hi @IsmailChangezi, you updated the test in the posts_show_spec.rb please can you move this test: Tests that asserts you can see the user's profile picture to the posts_index_spec.rb file, that is the correct location 👍🏿
There was a problem hiding this comment.
Your project is complete! There is nothing else to say other than... it's time to merge it 
Congratulations! 🎉
Highlights
- All tests are passing ✅
Optional suggestions
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
- Please move this test: Tests that asserts you can see the user's profile picture to the posts_index_spec.rb file, that is the correct location 👍🏿
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 @prolajumokeoni.
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.
Description