-
Notifications
You must be signed in to change notification settings - Fork 26
RIDE 🚗 SHARE 🚗 danielle 🚗 sammi-jo 🚗 #15
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
base: master
Are you sure you want to change the base?
Conversation
Rideshare RailsWhat We're Looking For
|
| <button><%= link_to "Delete", driver_path(@driver.id), method: :delete, data: {confirm: 'Are you sure you want to delete this driver?'} %></button> | ||
|
|
||
|
|
||
| <%= render partial: "layouts/personaltrips", |
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.
Good idea to use a partial in the show view!
app/views/layouts/_form.html.erb
Outdated
| <div id="form-div"> | ||
|
|
||
|
|
||
| <%= form_with model: model, url: url, method: method, class:"form" , id:"form1" do |f| %> |
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.
I think with the state of the model you don't need to specify the url field.
| resources :trips, except: [:create, :new] | ||
| resources :drivers | ||
| resources :passengers, except: [:show] do | ||
| resources :trips, only: [:index, :show, :create] |
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.
Do you need the index and show actions here?
You are using the trips index page to show the details on the passenger, rather than details on the passenger. It works, but kinda odd.
| @@ -0,0 +1,26 @@ | |||
| <div id="form-main"> | |||
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.
Do you need to have a separate screen to edit a trip? Could you instead rate the trip on the passenger's show page?
| @@ -0,0 +1,50 @@ | |||
| class Driver < ApplicationRecord | |||
| has_many :trips, dependent: :nullify | |||
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.
Interesting way to handle the deletion of Drivers. This will work, but leave trips without a driver.
app/views/trips/edit.html.erb
Outdated
| <% cost_placeholder = @trip.formatted_cost %> | ||
| <% value_placeholder = @trip.formatted_cost %> | ||
| <% end %> | ||
| <%= f.text_field :cost, value: cost_value, placeholder: cost_placeholder, pattern: '\d{1,3}.\d{2}', required: true, class: "feedback-input", id: "cost_image" %> |
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's good that you're using front-end validations, but you still need model validations!
|
|
||
| def total_earnings | ||
| #The driver gets 80% of the trip cost after a fee of $1.65 is subtracted | ||
| total_cost = self.trips.reduce(0) { |sum, 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.
Good use of reduce!
As a style comment, instead of using { } for multiline blocks use do ... end instead.
| end | ||
| end | ||
|
|
||
| def assign_random_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.
This should probably be a self method or actually change the cost field of the Trip.
|
|
||
| def status | ||
| if self.trips.any? { |trip| trip.cost == nil || trip.rating == nil } | ||
| return :in_progress |
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.
Semantically this would probably be better to return :on_trip
| belongs_to :passenger | ||
| validates :date, presence: true | ||
| validates :rating, presence: {message: "can't be blank. Trip won't end until you select a rating." }, unless: :passenger_is_requesting_trip | ||
| validates :cost, presence: {message: "can't be blank."}, unless: :passenger_is_requesting_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.
Neat validations, and custom messages.
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