-
Notifications
You must be signed in to change notification settings - Fork 24
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
Dikla & Wenjie - Rideshare Rails - Octos #27
base: master
Are you sure you want to change the base?
Conversation
Drew ERD diagram Created trello board
Added RESTFUL routes
Added new column in trip to have foreign keys
Added validation
Fixed passenger and driver id in trips data
Added trips info to passenger's show page Added links to trip, driver and passenger
Added a link to add a review in passenger's show page
Added business logic to calculate ave rating and total cost for driver and passenger
…heir trips Added condition to passenger's show page to access the link request a trip
|
||
<%= render partial: "layouts/errors_messages", locals: {model: @driver } %> | ||
|
||
<div class="new_page"> |
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.
To follow CSS naming conventions, we separate classes with the - not the _ so instead, this wold be new-page
<ul class="new_info"> | ||
<li> | ||
<%= f.label :car_make %> | ||
<%= f.select "car_make",raw("<option>Toyota</option><option>Honda</option><option>Tesla</option><option>Lamborghini</option><option>Ford</option><option>Hyundai</option><option>Subaru</option><option>Volkswagen</option><option>BMW</option><option>Mercedes</option><option>Aston Martin</option><option>Ferrari</option><option>Jaguar</option>") %> |
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.
It might make sense to move these options into the model or somewhere other than the view (assuming they'd be accessed/utilized elsewhere)
end | ||
|
||
resources :passengers do | ||
resources :trips, only: [:index, :new, :create, :show, :edit, :destroy] |
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 could probably clean this up by using except
rather than only
to list fewer actions
@@ -0,0 +1,13 @@ | |||
Rails.application.routes.draw do | |||
# For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html | |||
root 'trips#new' |
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.
This seems like an odd choice for the default home page based on what this route normally is responsible for.
</strong> | ||
|
||
<% @passengers.each do |passenger| %> | ||
<ul class="index_list"> |
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.
Watch your indentation in the views - when you utilize the code tags and HTML within, you still want to indent to make sure that it is as readable as possible.
Rideshare-RailsWhat We're Looking For
|
Rideshare-Rails
Congratulations! You're submitting your assignment! These comprehension questions should be answered by both partners together, not by a single teammate.
Comprehension Questions