Katrina && Melissa - Edges - Ride Share #17
Conversation
add passenger view
Rideshare RailsWhat We're Looking For
|
| <!-- Trip details for passenger --> | ||
| <% elsif @trips.first.passenger == @user %> | ||
| <strong><%= link_to "Request Ride", passenger_trips_path(@user.id), method: :post %></strong> | ||
| <strong><%= link_to "Edit This Passenger", edit_passenger_path(@user.id) %></strong> |
There was a problem hiding this comment.
Because these buttons are inside of this conditional, if a user does not have any rides, they cannot request a ride!
| <!-- Trip details for driver --> | ||
| <% elsif @trips.first.driver == @user %> | ||
| <p><%= link_to "Edit This Driver", edit_driver_path(@user.id) %></p> | ||
| <p><%= link_to "Delete This Driver", driver_path(@user.id), method: :delete, |
There was a problem hiding this comment.
The fact that this page contains such a big if statement implies to me that it ought to be two different pages.
|
|
||
| sum = driver_trips.sum { |t| t.rating }.to_f | ||
|
|
||
| return sum / driver_trips.length |
There was a problem hiding this comment.
This will cause problems if the driver has no trips (trying to divide by zero)
| if driver.destroy | ||
| redirect_to drivers_path | ||
| else | ||
| render :show |
There was a problem hiding this comment.
I don't believe that driver.destroy can fail. If it did, this error handling code wouldn't work anyway - you don't have a show page for drivers.
| def create | ||
| @passenger = Passenger.find_by(id: params[:passenger_id]) | ||
| @driver = Driver.find_by(status: true) | ||
| @trip = @passenger.trips.new() |
There was a problem hiding this comment.
All of the work done in this method could be moved to a model method. A class method on Trip would probably work nicely, something like Trip.request_for(passenger_id).
Also, this action doesn't render a view, so you shouldn't need to use instance variables.
|
|
||
| @trip.save | ||
|
|
||
| redirect_to passenger_trips_path |
| def rating(trip) | ||
| if trip.rating.present? | ||
| trip.rating | ||
| else |
|
|
||
| <%= form_for @passenger do |f| %> | ||
| <%= f.label :name %> | ||
| <%= f.text_field :name %> |
There was a problem hiding this comment.
You have the same form for both edit and new. Could you DRY this up with a view partial?
| resources :trips, only: [:index, :assign_rating] | ||
| end | ||
|
|
||
| resources :drivers, except: [:show] |
There was a problem hiding this comment.
You don't need to include resources :drivers twice. The resources method can take both a block and an except keyword argument.
|
|
||
| root 'homepages#index' | ||
|
|
||
| resources :homepages, only: [:index] |
There was a problem hiding this comment.
Given how your site is designed, I might not even include the route for /homepages, and use the root route everywhere.
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