Closed Israel-Laguan closed 4 years ago
About the required changes:
* [x] authentication have has integration tests (Rspec + Capybara), see this [image](https://gitlab.com/microverse/guides/projects/requirements_screenshots/raw/master/images/rails/associations/integration_tests_requirement.png) * [x] event managment features have integration tests (Rspec + Capybara), see this [image](https://gitlab.com/microverse/guides/projects/requirements_screenshots/raw/master/images/rails/associations/integration_tests_requirement.png)
We added the test as required
Changes required
Hello @Israel-Laguan @raheebwa
Thank you for the changes did. Your code is now better than before. However, I think there are some changes to make
- It's possible to access to
http://localhost:3000/events/new
directly without being logged in please correct it. Read the comments below.- Add an index to
create_id
fromevent
tableKindly make the changes and submit a new code review request. Thanks
Mael FOSSO Github | Twitter | LinkedIn
Changes required
- Add an index to
create_id
fromevent
table
Thanks for the quick and thorough review Please note that the index is already on the table see below
Resume
We've followed the instructions as described in the project requirements, used TDD, added a nice documentation and came to the conclusion that we met all of the requirements.
Proposal
We want to have a review of the code to know if we need to make any changes. If no changes needed we want the project to be ACCEPTED so we can move to the next project with a happy smile :)
Comments
The most challenging part in this project was the lack of good documentation. Also in some parts we lacked good sources of information (like the many to many association or the scopes) but it was a great learning experience for the deep and quality of the work needed.