-
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
Updated PR from master #32
base: master
Are you sure you want to change the base?
Conversation
…views/drivers/show
…driver_status partial, updated driver#edit
…yed as a link if the driver has not been deleted
…passenger show view
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.
The feedback hasn't changed much from: #13 (comment)
However I've left some notes in the code here. You still, on MediaRanker, need to work on putting business logic in the model, rather than doing calculations in the controllers. Overall you did well however.
resources :drivers | ||
|
||
|
||
get 'user_interfaces', to: 'user_interfaces#index', as: 'user_interfaces' |
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 don't need this route here, as you're just using user_interfacesController
as the root path.
# Calculate driver total rating: | ||
number_of_ratings = 0 | ||
@rating = 0 | ||
Trip.where(driver_id: @driver.id).each do |trip| |
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 should totally be in the Driver
model.
@passenger = Passenger.find(id) | ||
|
||
@total_paid = 0 | ||
@passenger.trips.each {|trip| @total_paid += trip.cost} |
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.
Should be in the model.
def update | ||
@passenger = Passenger.find_by(id: params[:id]) | ||
if [email protected]? | ||
@passenger.update(passenger_params) ? (redirect_to passenger_path(@passenger.id)) : (render :edit) |
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 a lot of good uses of ternary operators.
|
||
<h2><%= page_title %></h2> | ||
<% if @driver.errors.any? %> | ||
<ul class="errors"> |
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.
👍
<%= f.text_field :phone_num %> | ||
|
||
<%= f.label :img, "Picture" %> | ||
<%= f.select :img, [['Bear', "https://placebear.com/100/100"], ['Beyonce', "http://placebeyonce.com/100-100"]] %> |
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.
Clever!
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