Carly and Goeun - Edges - Rail Rideshare #13

CarlyReiter · 2018-10-05T22:54:42Z

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 a driver, passenger, and trip models. One driver was related to many trips, one passenger was related to many trips, but Trips is not a joined table! It’s a indirect model relation between driver and passenger. Trip has detailed columns including date, rating, and cost.
Describe the role of model validations in your application	Model validations ensures that the data that we submit is appropriate. We had to adjust our model validations as we progressed, such as allowing ‘nil’ when passengers request a new trip without rating and cost.
How did your team break up the work to be done?	We started off looking at the project and basic component together, split up some of the CRUD and compared. We paired on the routing and nesting, as well as the difficult business logic and controller methods towards the end. The arrangement worked out really well.
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 the requirements and getting it completely functional, and set aside much of the CSS (even though we really wanted to play with it).
What was one thing that your team collectively gained more clarity on after completing this assignment?	RAILS! Status methods in the controller, nested routes - for sure, comfortable with database and migrations.
What is your Trello board URL?	https://trello.com/invite/b/jzPpZnOs/4d4b61baae6c09a6f31a51d3346c3343/goeun-carly-rideshare
What is the Heroku URL of your deployed application?	http://wild-ride-share.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?	At the start, we discussed what went well and what didn’t in past partnerships, we discussed communication styles, did a good job of checking in with each other at start and end, and throughout each day.

…lers.

…plates for Driver.

Gp/passenger

Cr/driver

…ot Home.

into gp/trip

…ing them

… nested routesgit add app/controllers/passengers_controller.rb

tildeee · 2018-10-11T21:05:22Z

Rideshare Rails

What We're Looking For

Feature	Feedback
Baseline
Appropriate git usage with no extraneous files checked in and both partners contributing	x
Answered comprehension questions	x
Uses named routes (like `_path`)	x
RESTful routes utilized	x
Project Requirements
Table relationships	x
Validation rules for Models	x
Business logic is in the models	x
Database is seeded from the CSV files	x
Trello board is created and utilized in project management	x
Heroku instance is online	x
The app is styled to create an attractive user interface	x
Overall

Great work Carly and Goeun! The project looks great-- I'm a huge fan of the code and how it looks. It also hits the requirements, particularly the ones we had on models.

In particular, the code you two wrote for the routes and controllers are fantastic!

good job on routes! This was a challenge for many pairs but I'm happy with where you all landed up!

Here's one more bug that I want to call out:
http://wild-ride-share.herokuapp.com/drivers/101
What happens if there are no trips associated with a driver? Then the rating would be 0.0 and trip count is 0. Ruby evaluates 0.0 / 0 as NaN or "Not A Number." When it gets to the driver/show.html.erb view, NaN.to_s evaluates to just "NaN" so it shows up on the driver page as "NaN" on the page. It'll show up like this:

"Minnie Dach
Rating is NaN"

I have a few other comments on the code, but otherwise, great work overall.

PS: i dont know who mistr toad is BUT I LOVE MR TOADS WILD RIDESHARE AND HIS FACE ON THE PAGE!!!!!!!!!!!!!!! IT MAKES ME SO HAPY!!!!!!!!!!!!!

tildeee · 2018-10-11T21:06:03Z

app/controllers/trips_controller.rb

+
+  def create
+    passenger = Passenger.find_by(id: params[:passenger_id])
+    driver = Driver.find_by(status: true)


I think this is a really great way to assign drivers!

tildeee · 2018-10-11T21:07:13Z

app/models/driver.rb

+    num_trips = self.trips.length
+    money = 0
+
+    num_trips.times do |i|


Instead of using a times loop, it may be better to think of this problem as iterating through the collection of self.trips, so I would prefer maybe to use an each loop instead.

tildeee · 2018-10-11T21:07:40Z

app/models/driver.rb

+      end
+    end
+
+    rating = rating.to_f/(num_trips-ongoing_trips)


you end up needing the ongoing_trips variable to keep track of the length of this collection... Maybe there's a different way to filter or select ( https://ruby-doc.org/core-2.5.1/Enumerable.html#method-i-select ), and use that collection...

tildeee · 2018-10-11T21:08:08Z

app/models/driver.rb

+    ongoing_trips = 0
+    rating = 0
+
+    num_trips.times do |i|


Why not use an each loop here?

tildeee · 2018-10-11T21:08:29Z

app/models/passenger.rb

+  validates :name, presence: true
+  validates :phone_num, presence:true
+
+  def spent


This is subtle, but I like the name of this method :c)

tildeee · 2018-10-11T21:08:44Z

app/views/passengers/show.html.erb

+      <li><strong>Trip: <%=trip.id %></strong></li>
+      <li>Date: <%=trip.date %></li>
+      <% if trip.cost == nil %>
+        <li>Cost: Ride in progress</li>


Good choice! There wasn't a requirement about what to do with the trip's cost, so we asked you all to make a decision on it... I like this decision!

tildeee · 2018-10-11T21:09:35Z

app/views/passengers/show.html.erb

+      </div>
+
+      <% if trip.driver_id %>
+        <%=link_to "Driver", driver_trips_path(trip.driver_id) %>


This link just says "Driver". Why not use the driver's name?

tildeee · 2018-10-11T21:09:46Z

app/views/passengers/show.html.erb

+      </div>
+
+      <% if trip.driver_id %>
+        <%=link_to "Driver", driver_trips_path(trip.driver_id) %>


Should this actually go to this path? When I click here, I actually can't tell who the driver is still.

CarlyReiter and others added 30 commits October 1, 2018 15:34

Set up initial project and database.

0c8a206

Modified routes, and generated Drivers, Trips, and Passengers control…

47cc2e4

…lers.

Created models for Driver, Trip, and Passenger

6c9f03f

Added model relationships to Trip, Passenger, and Driver.

6387192

Added Dan's validation fix file.

b6370e0

add a index view template for passenger and trips

fdefc6d

Modified the driver controller to meet CRUD, and created all view tem…

55236ee

…plates for Driver.

add show for each passenger

8a996e3

add link to show page for each passenger name

0b07665

add show method in PassengerController

8abe0fc

updated all of the views with CRUD and forms, including partial forms.

3fa5ce5

create and update of CRUD of passenger

09d9085

minimal styling on index

f7bf334

Added validations to driver controller and edit and new views.

2b70bdd

validation view fiddling.

2970a1a

add comment to add root to routes

733b86d

Merge pull request #1 from CarlyReiter/gp/passenger

0b4f0ef

Gp/passenger

Merge pull request #2 from CarlyReiter/cr/driver

d7d7172

Cr/driver

add edit or delete to trip view

580dfba

created a partial for form errors.

7ab01e2

modified forms for validation error partial.

8eb67ca

add show page for trips view

6e4a040

modified routes to accept Homes.

ce78716

Created Homes controller and then had to fix all files to Homes and n…

7713409

…ot Home.

add error message and form in trip edit view

cf46bda

add CRUD for trips

1c4943c

Merge branch 'master' of https://github.com/CarlyReiter/ride-share-rails

9171c1d

into gp/trip

added homepage and navigation, with nav links on all views.

bd21ddb

small modifications to home links.

0c2da99

Merge branch 'master' into cr/ridesharehomepage

d389d39

CarlyReiter and others added 21 commits October 4, 2018 16:14

not sure what I did here..

18266a1

dan did wizard magic idk

0f96c66

add conditional if no passenger in view for trips

db270b0

took out resources :passeners and :drivers because we had been repeat…

11e1d60

…ing them

changed up the views??

bd77d66

added @trip = Trip.new to show method in passenger controller because…

9bffda9

… nested routesgit add app/controllers/passengers_controller.rb

hardcoded attributes in create method

a9b1ffa

Added CSS and made pages consistent.

57f2eaa

merged changes.

b87ea85

Playing aroudn with sorting name of passengers on index.

5eadefa

add css, adjust html, and adjust create method in trips controller

eb55912

adjust rating and earned models to take in nil values

9d8bf2a

when creating new trip, cost is nil, date is current date

fa7838f

Added a no driver available message page.

5a2b802

Figuring out the choosing a rating portion.

0eb1546

add update to nested trip route??

cee8a56

add drop down menu to update ratings in passenger show

fc310e4

update driver status to unavailable when assigned a trip

ff4aa39

sorted trips, passenger, and driver lists so the most recent is first.

6065dd3

last minute styling

19f718e

stylig

8717261

tildeee reviewed Oct 11, 2018

View reviewed changes

app/models/driver.rb

ongoing_trips = 0

rating = 0

num_trips.times do |i|

Copy link

tildeee Oct 11, 2018

Choose a reason for hiding this comment

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

Why not use an each loop here?

tildeee reviewed Oct 11, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Carly and Goeun - Edges - Rail Rideshare #13

Carly and Goeun - Edges - Rail Rideshare #13

CarlyReiter commented Oct 5, 2018

tildeee commented Oct 11, 2018 •

edited

Loading

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

tildeee Oct 11, 2018

Carly and Goeun - Edges - Rail Rideshare #13

Are you sure you want to change the base?

Carly and Goeun - Edges - Rail Rideshare #13

Conversation

CarlyReiter commented Oct 5, 2018

Comprehension Questions

tildeee commented Oct 11, 2018 • edited Loading

Rideshare Rails

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tildeee commented Oct 11, 2018 •

edited

Loading