-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Final Capstone Project: Rent-Eaze Booking System - Merge Request #55
Conversation
Initialize RoR
Feature/user model
Add 'Reservation' model
…odel Feature/create accommodation model
Feature/database migrations
Feature/api documentation
environment variable for database URL
Feature/deploy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lRebornsl,
Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!
To highlight:
- All tests but 1 are passing✔️
- Nice code organization ✔️
- API is working well✔️
- Good documentation✔️
You are really close to finishing the Microverse program!! Keep it up! 👍👍👍
After implementing the requested changes, please submit another review request. ♻️
Check the comments under the review.
Cheers and Happy coding!👏👏👏
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.
|
||
rspec | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!
Thanks for your feedback 🙌🏼
This is solved 😄
README.md
Outdated
|
||
rails db:create | ||
rails db:migrate | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You have included a functioning seeds.rb file. Kindly, include in your instructions the command rails db:seed.
That way, your users will enjoy a pre-seeded database. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!
Thanks for your feedback 🙌🏼
This is solved 😄
app/controllers/places_controller.rb
Outdated
def destroy | ||
place = @places.find(params[:id]) | ||
place.destroy | ||
head :no_content | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What happens here if the place is not successfully destroyed? Kindly, add some error handling like you did on the update method (line 32). 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!
Thanks for your feedback 🙌🏼
This is solved 😄
def destroy | ||
reservation = @reservations.find(params[:id]) | ||
reservation.destroy | ||
head :no_content | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What happens here if the reservation is not successfully destroyed? Kindly, add some error handling like you did on the update method (line 39). 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!
Thanks for your feedback 🙌🏼
This is solved 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lRebornsl,
Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!
You are really close to finishing the Microverse program!! Keep it up! 👍👍👍
After implementing the requested changes, please submit another review request. ♻️
Check the comments under the review.
Cheers and Happy coding!👏👏👏
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.
```sh | ||
|
||
rails db:create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- In this review iteration, when I try to create the database according to instructions, this happens:
Explanation
Problem 1:
The first problem is this little bang ("!"):
If I remove it, then the error becomes this:
Second problem
The new problem is that the JWT secret token is missing from rails credentials.
These are the steps to solve this second problem:
1. Remove config/master.key and config/credentials.yml.enc if they exist.
2. Run `rails secret`. Copy the key.
3. Run EDITOR="code --wait" bin/rails credentials:edit
4. In the editor that opens, add this: devise_jwt_secret_key: <the key you copied in step 2>
5. Save the file and close the editor. New master.key, credentials.yml.enc files will be generated, and the key will be stored in `Rails.application.credentials.devise_jwt_secret_key`.
Result:
Now it works.
This may be working for you on your end because you have the database and credentials already setup. But it won't for new users.
Therefore:
- Kindly fix the first problem in
config/initializers/devise.rb
(the bang "!"). - Next, please include instructions on your readme about how to add the devise_jwt_secret_key to the rails credentials, (these instructions have already been written for you above).
That way, the database will be able to be created, and all your tests will pass:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!
Thanks for your feedback 🙌🏼
This is solved 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lRebornsl,
Wow, you did it 🎉
Thank you for the changes implemented 💪 🥇 ㊗️
Unless you want to add more features, go ahead to your final presentation ⏩ ⏩ ⏩
You are about to finish the Microverse program. You have come a long way!!!
Good luck in the software industry!! I'll see you there. ✨
Congratulations!!!!!! 🎉
To highlight
- Readme instructions have been improved✔️
- All tests are now passing✔️
- Great job✔️
Cheers and Happy coding!👏👏👏
Overview
This pull request is a formal request to merge our development team's cumulative and concerted efforts from the
dev
branch into themain
branch. Our Final Capstone Project is a sophisticated rental house booking system, designed and tailored following the prototype provided by Murat Korkmaz on Behance, with our unique spin - instead of motorcycles, we facilitate the booking of rental houses.Our development team, composed of Javier Grau, Manuel Sanchez, and Anthony Vásquez, has meticulously adhered to coding best practices, proper Gitflow, and comprehensive professional documentation throughout the project lifecycle.
Pull Request from front-end
General Requirements Met
Project Implementation Details
Chosen Theme
Core Features Implemented
Additional Features (Beyond Basic Requirements)
Comprehensive Documentation
Useful Links
Merge Request Justification
With all required features meticulously implemented, we are confident that our project meets the specified criteria. We kindly request the review and approval for the merging of our
dev
branch intomain
.We eagerly await any feedback and are fully prepared to conduct further modifications if recommended by our esteemed reviewers.
Thank you for your consideration and time.
Warm regards,
@grauJavier
@Luffytaro22
@lRebornsl