-
Notifications
You must be signed in to change notification settings - Fork 26
Ports- Ngoc-Myriam #17
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
…d, reset db to pull data from CSV into the database
…ay list of passengers in the navigation
…thod in passenger controller
…nd show in drivers_controller
…oller-create method
Rideshare RailsWhat We're Looking For
|
} | ||
|
||
expect { | ||
post passenger_trips_path, params: trip_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.
This test is failing because this path is wrong. You need a route param for the passenger id.
post passenger_trips_path(Passenger.first.id)
It also doesn't need a params hash, because there is no form being submitted to the controller action.
resources :drivers | ||
resources :trips, except: [:new, :create, :update] | ||
resources :passengers do | ||
resources :trips, only: [:new, :create, :update] |
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 new form action?
Does the update action need to be nested inside passengers?
It is really good that you are making sure not to produce unused routes.
end | ||
end | ||
|
||
def new |
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 a new action for trips?
The demo app has you create trips automatically without the form.
<%= f.label :driver_id %> | ||
<%= f.text_field :driver_id%> | ||
|
||
<%= f.label :passenger_id%> |
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 not be a text field, instead a select
or something, something like what we have in the Ada Books with authors. Of course you should also have created trips without a form.
belongs_to :driver | ||
belongs_to :passenger | ||
|
||
validates :driver_id, presence: :true |
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 the validations for the foreign keys.
|
||
describe "update" do | ||
# Your tests go here | ||
it "updates an existing trip" do |
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 have any test with invalid data and invalid trip id.
expect(driver_to_update.vin).must_equal update_input[:driver][:vin] | ||
end | ||
|
||
it "will return a bad_request when trying to update with invalid data" do |
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 should also have a test when the driver id is invalid.
def update | ||
driver = Driver.find_by(id: params[:id]) | ||
|
||
is_successful = driver.update(driver_params) |
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.
If the driver isn't found, you have a problem...
def update | ||
passenger = Passenger.find_by(id: params[:id]) | ||
|
||
is_successful = passenger.update(passenger_params) |
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.
What about if the passenger isn't found?
end | ||
|
||
def update | ||
trip = Trip.find_by(id: params[:id]) |
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.
Similar issue to other controllers.
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