Div-Hannah Rideshare

[Naltrexone]

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	The Driver class had many Trips and the Passenger class had mad Trips. Each Trip belongs to a Driver and a Passenger. Each Driver had a Passenger through Trips and each Passenger had a Driver through Trips.
Describe the role of model validations in your application	We used model validations to make sure that each of the textfields had to be present. We wrote these validations initially in the model and printed errors in the form page of drivers, passenger, and trips.
How did your team break up the work to be done?	Divya - worked on Passenger related requirements.- worked on Adding Trips and selecting rating.- worked on parts of setting up Trips- CSS for Passenger and Trip forms. Hannah - worked on Driver related requirements.- worked on setting up Trip - Styled homepage, index, show pages, and driver form.
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 created working backend of the website first - got methods working etc, before we decided to work on front end details. We didn't set aside anything in particular to meet the deadline.
What was one thing that your team collectively gained more clarity on after completing this assignment?	We gained more clarity on the general workings and flow of backend within ruby on rails and how to deploy to heroku.
What is your Trello board URL?	https://trello.com/b/xF2gNYvE/div-hannah-rideshare
What is the Heroku URL of your deployed application?	https://hannah-div-rideshare.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?	(1)Since we had divided up the work, we ended not working on many things together. In future, we would probably set aside specific tasks to work on together. (2)Both of us found the project to be not as stressful and the fluidity in which we achieved the project went well.

[CheezItMan]

Rideshare Rails

What We're Looking For

Feature	Feedback
Baseline
Appropriate git usage with no extraneous files checked in and both partners contributing	Good commit messages and good number of commits, both partners contributed.
Answered comprehension questions	Check, the passenger had mad trips. Hmm... must have been using Uber.
Uses named routes (like _path)	Check
RESTful routes utilized	Yes with a couple of issues, check my notes.
Project Requirements
Table relationships	Check
Validation rules for Models	You have validations, see my suggestions for more.
Business logic is in the models	Check
Database is seeded from the CSV files	Check
Trello board is created and utilized in project management	Check
Heroku instance is online	Check
The app is styled to create an attractive user interface	The site looks good, except for one broken image (home button)
Overall	Overall well done. Your requesting a trip is a bit awkward and done with a form, which it doesn't need to be. It also allows the user to edit their ID number, which can be problematic. Read my inline notes and let me know any questions you have. Overall however you hit all the major learning goals. Well done.

[CheezItMan]

This won't work if the driver has any trips!

[CheezItMan]

This won't work if the passenger has any trips.

[CheezItMan]

Search! Neat!

However it doesn't look like you have it hooked into the webpage

[CheezItMan]

You don't need resources :passengers in here twice.

[CheezItMan]

Do you need an :index action for trips in this context?

[CheezItMan]

You should also require that cost be numeric and positive.

[CheezItMan]

Rating should also have a validation. It should be 1-5.

[CheezItMan]

If the trip belongs to driver, this validation is unnecessary.

[CheezItMan]

Search! Neat! It doesn't look like it's integrated into the view/controller.

[CheezItMan]

Seems broken when I ran it.

[Naltrexone]

Thanks for the feedback Chris!☺️

…

On Sun, Oct 7, 2018, 6:24 PM Chris M ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/controllers/drivers_controller.rb <#11 (comment)> : > + @driver = Driver.find_by(id: params[:id].to_i) + end + end + + def update + @driver = Driver.find_by(id: params[:id].to_i) + if @driver.update(driver_params) + redirect_to driver_path + else + render :edit + end + end + + def destroy + driver = Driver.find_by(id: params[:id].to_i) + driver.destroy This won't work if the driver has any trips! ------------------------------ In app/controllers/passengers_controller.rb <#11 (comment)> : > + @Passenger = Passenger.find_by(id: params[:id].to_i) + end + + def update + @Passenger = Passenger.find_by(id: params[:id].to_i) + if @passenger.update(passenger_params) + redirect_to ***@***.***) + else + render :edit + end + end + + + def destroy + passenger = Passenger.find_by(id: params[:id].to_i) + passenger.destroy This won't work if the passenger has any trips. ------------------------------ In app/models/driver.rb <#11 (comment)> : > + self.trips.each do |trip| + trip_price = (trip.cost - 1.65) * 0.80 + total += trip_price + end + return total + end + + def average_rating + total = 0 + self.trips.each do |trip| + total += trip.rating + end + return (total.to_f / self.trips.length).round(2) + end + + def self.search(term, page) Search! Neat! However it doesn't look like you have it hooked into the webpage ------------------------------ In config/routes.rb <#11 (comment)> : > @@ -0,0 +1,18 @@ +Rails.application.routes.draw do + root 'welcome#index' + + get 'welcome/index' + # why are the paths different between resource trips and the trips I wrote? + resources :trips + + resources :drivers + + resources :passengers + + resources :passengers do You don't need resources :passengers in here twice. ------------------------------ In config/routes.rb <#11 (comment)> : > @@ -0,0 +1,18 @@ +Rails.application.routes.draw do + root 'welcome#index' + + get 'welcome/index' + # why are the paths different between resource trips and the trips I wrote? + resources :trips + + resources :drivers + + resources :passengers + + resources :passengers do + resources :trips, only: [:index, :new, :create] Do you need an :index action for trips in this context? ------------------------------ In app/views/trips/_form.html.erb <#11 (comment)> : > +<div class="passenger-form"> + <section class="form-contents form-section"> +<% if @trip.errors.any? %> + <ul class="errors"> + <% @trip.errors.each do |column, message| %> + <li> + <strong><%= column.capitalize %></strong> <%= message %> + </li> + <% end %> + </ul> +<% end %> + + <h1><%= page_title %></h1> + <%= form_with model: @trip do |f| %> + <p> + <%= f.label :passenger_id %> Should the passenger be able to edit their ID number? I would make this a hidden field, I would also do something similar for driver. Also you don't *have* to create the trip via a form. You could have a link which directly calls create, and the trip is created with the given passenger, and assigns a driver, todays date and a random price. ------------------------------ In app/models/trip.rb <#11 (comment)> : > @@ -0,0 +1,11 @@ +class Trip < ApplicationRecord + belongs_to :driver + belongs_to :passenger + #can you write these all in one line instead of individually? + validates :passenger_id, presence: true + validates :driver_id, presence: true + validates :date, presence: true + validates :cost, presence: true You should also require that cost be numeric and positive. ------------------------------ In app/models/trip.rb <#11 (comment)> : > @@ -0,0 +1,11 @@ +class Trip < ApplicationRecord + belongs_to :driver + belongs_to :passenger + #can you write these all in one line instead of individually? + validates :passenger_id, presence: true + validates :driver_id, presence: true + validates :date, presence: true + validates :cost, presence: true + Rating should also have a validation. It should be 1-5. ------------------------------ In app/models/trip.rb <#11 (comment)> : > @@ -0,0 +1,11 @@ +class Trip < ApplicationRecord + belongs_to :driver + belongs_to :passenger + #can you write these all in one line instead of individually? + validates :passenger_id, presence: true If the trip belongs to driver, this validation is unnecessary. ------------------------------ In app/models/passenger.rb <#11 (comment)> : > +class Passenger < ApplicationRecord + has_many :trips + + validates :name, presence: true + validates :phone_num, presence: true + + def total_charges + trips = self.trips + trips_total = 0 + trips.each do |trip| + trips_total += trip.cost + end + return trips_total + end + + def self.search(term, page) Search! Neat! It doesn't look like it's integrated into the view/controller. ------------------------------ In app/views/layouts/application.html.erb <#11 (comment)> : > + <div class="driver-dropdown dropdown"> + <button class="dropbtn">Driver</button> + <div class="dropdown-content"> + <p><%= link_to "All Drivers", drivers_path %></p> + <p><%= link_to "Add a New Driver", new_driver_path %></p> + </div> + </div> + <div class="dropdown"> + <button class="dropbtn">Passenger</button> + <div class="dropdown-content"> + <p><%= link_to "All Passengers", passengers_path %></p> + <p><%= link_to "Add a New Passenger", new_passenger_path %></p> + </div> + </div> + <article class="home-button"> + <%= link_to image_tag("https://www.iconsdb.com/icons/preview/white/home-xxl.png", alt: "home buttom"), root_path %> Seems broken when I ran it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AgAOSeE6qKCnCA6u-tlz6Gqt1gjOkFYjks5uiqk7gaJpZM4XLBzi> .