Final Capstone Project #24
Conversation
…tion Authentication and authorization
Update mentors and users
…s approved,banned and removed
…oval Mentor controller admin panel functionality for backend
Rspec test
Revert "Rspec test"
There was a problem hiding this comment.
Hi dear student,
Good job so far!
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.
To see the improvement points for Front-end project, please check the Front-end PR
You can 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 you 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, 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.
| **`bundle install`** to install require tools for the project to run | ||
|
|
||
| **`rails server`** | ||
|
|
||
| **`http://localhost:3000/`** | ||
|
|
||
| **`Rspec spec`** to run all the test |
There was a problem hiding this comment.
- Talking about installation, good job adding security to your application with a secret key. I'm afraid for this exact reason I wasn't able to install the database using
rails db:setup. I invite you to add this information to the readme file or disable this option. As a code reviewer, I need to install the entire environment locally to check everything inside.
There was a problem hiding this comment.
Hey @vichuge, Thanks for your review. considering this error, the problem is with the master key that's why you're having the error. Below is the master key you can use to access the database.
You can use the master key by creating the master.key file in the config file.
dab637913393951c2dd60cdd446343be
There was a problem hiding this comment.
Thanks, now is a good idea to add this info to the readme file. I understand is for security reasons, but including the info this app is gonna be used only for educational purposes, I don't think as required to keep the master key. I invite you to add this information to your readme file.
config/routes.rb
Outdated
| post '/technologies', to: 'api/v1/technologies#create' | ||
| get '/mentors/list', to: 'api/v1/mentors/mentors#index' | ||
| put '/approve_mentor', to: 'api/v1/mentors/mentors#approve_mentor' | ||
| put '/ban_mentor', to: 'api/v1/mentors/mentors#ban_mentor' | ||
| delete '/remove_mentor/:id', to: 'api/v1/mentors/mentors#remove_mentor' |
There was a problem hiding this comment.
- I invite you to consider using
resoucesto group all the endpoints for the same controller. For example, you can consider grouping all the mentors' routes using this. The same for bookings, remember to use theonlystatement.
There was a problem hiding this comment.
Hey @vichuge, Thanks for the observation on the resources. We didn't think about that but we will implement it later as using the resources would warrant us to refactor all the codebase which will disturb the app and we do not have all the time to fix them now. Also, we had used the custom routes and integrated them into the front end.
Thanks
There was a problem hiding this comment.
Sure, no problem. Please, always remember:
In this project, code reviews are not the final evaluation of students's projects. Their purpose is to help students to improve their project BEFORE the final presentation. Please take that into account while communicating with students - you can (and you should) suggest as many improvements you can but students can use your tips or ignore them.
There was a problem hiding this comment.
Hi dear student,
Great work on making the changes 👏
You've done well implementing some of the requested changes, but there are still some which aren't addressed yet.
Suggested changes
Check the comments under the review.
There are some issues on FE PR too, please review it.
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.
| # Examples: | ||
| # | ||
| # movies = Movie.create([{ name: "Star Wars" }, { name: "Lord of the Rings" }]) | ||
| # Character.create(name: "Luke", movie: movies.first) |
There was a problem hiding this comment.
- I kindly invite you to add seeds to your app. This could help to work better on this application and having seed info makes it easier to see the front-end functionality.
Fix errors in mentors controller
In this Final Capstone Project Our team has done the following:
Basics
Features
- In the navigation panel, the user can see links to:
On the main page, the user can see a [list of Mentor/classes/items that you selected as a theme]
(https://www.behance.net/gallery/26425031/Vespa-Responsive-Redesign/modules/173005577).
When the user selects a specific item, they can see the details page with its full description (skip the "Rotate image" button).
In the details page, the user can click the "Reserve" button (in the design you can see the "Configure" button - please replace it with the "Reserve" button).
When the user clicks the "Add item" link in the navigation panel they can see a form for adding a new item.
Make the app responsive, creating both mobile and desktop versions.