Cassy and Sabine - DrivesMe #6
Conversation
… trip show pages.
This reverts commit 9ff8755.
Rideshare RailsWhat We're Looking For
|
| <td><%="#{@trip.date}" %></td> | ||
| <td><%="#{@trip.rating}" %></td> | ||
| <td><%="$#{@trip.convert_money(@trip.cost)}" %></td> | ||
| <td><%=link_to "#{@trip.driver_id}", driver_path(@trip.driver_id) %></td> |
There was a problem hiding this comment.
I'd recommend showing the passenger and driver's names instead of ids.
There was a problem hiding this comment.
Hi Chris, could you or Devin go over how to do this with me? I tried to set @driver and @Passenger in the trip show method so I could use it in this view as @driver.name/@passenger.name but was not successful.
| end | ||
|
|
||
| def update | ||
| passenger = Passenger.find_by(id: params[:id].to_i) |
There was a problem hiding this comment.
This isn't showing the user validation errors or asking them to fix the form. That's because you don't have an if statement checking to see if the update method returned true.
| <% else %> | ||
| <h1><%[email protected] %></h1> | ||
| <button> | ||
| <%= link_to "Add A Trip", new_trip_path %> |
There was a problem hiding this comment.
You should make this route be for the passenger_trip_path so the passenger can be pre-set.
|
|
||
| def amount_earned | ||
| sum = 0 | ||
| self.trips.each do |trip| |
There was a problem hiding this comment.
You need to consider when the trip's cost might be nil (incomplete trips).
There was a problem hiding this comment.
Could this be an if statement like this?
def amount_earned sum = 0 self.trips.each do |trip| # converting cents to dollars if trip.cost.nil? raise ArgumentError, "Please enter trip cost" end
Or would I do a method to only show the amount earned if the trip.cost is not nil?
There was a problem hiding this comment.
@cassyarchibald You could do something like this.
self.trips.select {|trip| trip.cost != nil }.each do |trip||
|
||
| def total_charged | ||
| charged = 0 | ||
| self.trips.each do |trip| |
There was a problem hiding this comment.
Consider incomplete trips. In that case the trip.cost would be nil.
| Rails.application.routes.draw do | ||
| root "trips#index" | ||
|
|
||
| resources :trips, :drivers, :passengers |
There was a problem hiding this comment.
Do you need a route for get '/trips'? What about /trips/new? Will you ever bring up a trip form without a passenger?
|
|
||
| # Nested routes for drivers and passengers to have access to associated trips | ||
| resources :drivers do | ||
| resources :trips, only: [:index, :new] |
There was a problem hiding this comment.
Do you ever show a trips index page in the context of a driver or passenger? Do you ever create a new trip in the context of a driver? Driver's don't normally initiate the rideshare process.
There was a problem hiding this comment.
Hmm I think i'm still a bit confused on nested routes. Would it have made more sense for these to be in reverse order where the nested routes are :drivers and :passengers, only [:index, :new]/trips being the outer resources? Would that mean that creating a new driver/passenger would be in the context of a trip/trips are initiating the rideshare process rather than vice versa?
There was a problem hiding this comment.
@cassyarchibald
I don't think you ever need a :new action for trips, as you don't really need a form for it. I'm not sure you even need an index page for trips, as we mostly just show trips in the driver and passenger show pages.
| end | ||
| return charged | ||
| end | ||
|
|
There was a problem hiding this comment.
It's good that you have some business logic here. You should also include a method to generate a trip for the current passenger assigning a driver.
Rideshare-Rails
Congratulations! You're submitting your assignment! These comprehension questions should be answered by both partners together, not by a single teammate.
My Teammate was out most of the week so this will be answered by a single teammate - Cassy *************************************
Comprehension Questions