Skip to content

Conversation

@alexandria7
Copy link

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

Question Answer
Describe the types of entity relationships you set up in your project and why you set up the relationships that way We had three entities, passenger, driver, and trip. Passengers and drivers had many trips, while trips belonged to each of them
Describe the role of model validations in your application We used validations to require a name, vin, and phone number for drivers and passengers. This prevents us from taking in bad data from the user that could break our code.
How did your team break up the work to be done? We split passenger and driver functionality, and then worked together on trips. Within trips we split functionality into chunks and worked separately on those.
What features did you choose to prioritize in your project, and what features, if any, did you have to set aside to meet the deadline? We prioritized basic functionality for creating, editing, deleting drivers, passengers, and trips. We stumbled with dealing with handling the errors that come with deleting a passenger that is part of a trip, which we are submitting without!
What was one thing that your team collectively gained more clarity on after completing this assignment? We gained clarity on RESTful routes - as the project continued, it became more natural to implement changes and introduce more functionality beyond CRUD.
What is your Trello board URL? https://trello.com/b/dzD1mTBC/rails-rideshare
What is the Heroku URL of your deployed application? https://ruby-on-roads.herokuapp.com/
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share? We both agreed that we effectively split the tasks to leverage our different styles of programming. Both of us were doing what we felt best at! It felt good. I think we also realized that being a little more organized at the start of the project would be beneficial when there aren't project waves!

alexandria7 and others added 30 commits April 15, 2019 14:46
@CheezItMan
Copy link

Rideshare Rails

What We're Looking For

Feature Feedback
Baseline
Appropriate git usage with no extraneous files checked in and all team members contributing See my inline notes on extra view files. Good number of commits and both members contributing.
Answered comprehension questions Check, glad you worked well together
Uses named routes (like _path) Check
RESTful routes utilized For the most part, see my notes in the routes.rb file
Project Requirements
Table relationships Check
Business logic is in the models Some, you could create more methods in the models.
Controller tests Check, see my notes on things not tested.
Database is seeded from the CSV files Check
Trello board is created and utilized in project management I can't see the trello board because you didn't make it public!
Heroku instance is online Check
The app is styled to create an attractive user interface Some styling in place
Overall You hit the major learning goals. However you did miss the functionality of requesting a ride and then being unable to request a ride until it's rated. You also don't have a direct way to rate a trip without editing the entire trip.

}

expect {
post trips_path, params: trip_hash

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have a route for the trips_path instead this should post to the passenger_trips_path

root to: "homepages#index"
resources :homepages, only: [:index]

resources :trips, :drivers, :passengers

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should limit the routes trips has, since there's no new trip form, or a trips show page.

resources :trips, :drivers, :passengers

resources :drivers do
resources :trips, only: [:index, :new]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trips shouldn't have a new form page, since the trip is created automatically without a form. You also don't have an index page under the driver's show page.

end

resources :passengers do
resources :trips, only: [:index, :new]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

resources :passengers do
  resources :trips, only: [:create]
end

resources :trips, only: [:index, :new]
end

post "/passengers/:passenger_id/trips/", to: "trips#create", as: "create_passenger_trip"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use resources like above to create this.

patch trip_path(trip_to_update.id), params: trip_hash
}.wont_change "Trip.count"

must_respond_with :redirect

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also verify that the trip's fields change.

describe "destroy" do
# Your tests go here
it "can delete a trip" do
trip1 = trip

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why assign trip1 to trip?


FEE = 165
PERCENTAGE = 0.80

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a go_online method wouldn't be bad.


validates :name, presence: true
validates :phone_num, presence: true, uniqueness: true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a good idea to have a request_trip method which generates a trip for an instance of Passenger

end

def create
@trip = Trip.new(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of this business logic could be moved to the Passenger model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants