Tor Shimizu - Wini Irarrazaval - Octos - Rideshare Rails #26
Conversation
… in the edit.html.erb and new.html.erb views. We are able to update and add drivers
…iew the info of each trip.
…ngs methods. Then I added them to the driver/show
…ant be black to be able to submit
…irect to the same page
… cases and updated driver#destroy to replace the driver in its trips with a placeholder driver before destroying the selected driver
…that has the longest time with no tripss
…uming that if there is a trip with no rating is still in trip
…able driver, added some styling to driver page
…er show page, will display a form to get rating
Rideshare-RailsWhat We're Looking For
|
| end | ||
|
|
||
| resources :trips, except: [:destroy] | ||
| resources :rideshare, only: [:index] |
There was a problem hiding this comment.
I like that you're excluding the destroy route on trips, but I think you could go even further. It doesn't look like new or the un-nested create routes are being used.
| def self.the_driver | ||
| drivers = Driver.where(disabled: false) | ||
| longest_driver_not_driving = drivers.first | ||
| drivers.each do |driver| |
There was a problem hiding this comment.
I'm not sure about the name of this method - if I hadn't seen this file and I saw a call to Driver.the_driver, I would not know what that method is doing. Something like select_driver might be more mnemonic.
|
|
||
| def driver_on_trip | ||
| self.trips.each do |trip| | ||
| return true if trip.rating == nil |
There was a problem hiding this comment.
In general, you don't need to include the name of the class in an instance method for that class. Also, since this method returns true or false, the Ruby convention is to end the name in a question mark. So this one should be on_trip?.
| <% if model = @passenger %> | ||
| <%= f.label :phone_num, class: 'label' %> | ||
| <%= f.text_field :phone_num, class: 'control input' %> | ||
| <%= f.submit action_name, class: 'control button is-link' %> |
There was a problem hiding this comment.
While I appreciate the enthusiasm, the fact that you've got a big if statement here indicates you may have gone too far trying to DRY this up. I would probably have made the errors and hero section their own partial, and written a separate form_for for driver and passenger.
| <h4> Trips: </h4> | ||
| <ul class='display-trips'> | ||
| <li>ID</li> | ||
| <li>Date</li> |
There was a problem hiding this comment.
Good work breaking this out as a separate view partial!
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