Michelle & Barbara - Edges - Rideshare-Rails #24

kangazoom · 2018-10-06T01:11:28Z

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 said that the Driver has many Trips and that the Passenger has many Trips. We also said that Trips belongs to Driver as well as to Passenger. We set up the relationships this way since trips have one driver and one passenger, but drivers and passengers can each have many trips.
Describe the role of model validations in your application	Validations ensure that we are passing good data into the database. In this case, our validations ensure that entries like name, phone number, vin, etc. are present (not empty). Our validations only check for presence--not for any other validation specifications.
How did your team break up the work to be done?	We did the initial set up and generations (model, controllers, db, etc.) together. We also did some planning before working on the project. We divvied up the work by Passenger, Driver, Trips, and design work. We then worked on lingering issues together. We utilized branches.
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 foundational requirements first before delving deeper into more complex functionalities (such as those that required business logic). We did not get to the “On the trip details page I can: Assign a rating (1-5) to the trip, if it does not already have one” feature, and we wanted to do a bit more clean up / add more detail (ex: forms, deletion redirection for trips, more design, pagination, etc.)
What was one thing that your team collectively gained more clarity on after completing this assignment?	Nested routes and how to access methods in the model.
What is your Trello board URL?	https://trello.com/b/Izp4Kpuc/michelle-barbara
What is the Heroku URL of your deployed application?	https://barchelle-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?	We worked hard on organizing the tasks, and we communicated a lot about the branches, when to merge to master, etc.

…nd trips

deleted everything in passenger viewer index file

getting master up to date

adding passenger_work commits to master

Merge origin master with trip_work branch

…roller

…nd trips

…passenger

Merge master branch with trip_work_create_trip

Merge master to trip_work_create_trip

Merge master with trip_work_create_trip

droberts-sea · 2018-10-12T06:22:36Z

Rideshare Rails

What We're Looking For

Feature	Feedback
Baseline
Appropriate git usage with no extraneous files checked in and both partners contributing	yes
Answered comprehension questions	yes
Uses named routes (like `_path`)	yes
RESTful routes utilized	some extra / unused routes
Project Requirements
Table relationships	These are set up correctly, but in several places you're repeating work that Rails does for you. It would be worth reviewing all the things you get when you set up a model relation.
Validation rules for Models	yes
Business logic is in the models	mostly - good work
Database is seeded from the CSV files	yes
Trello board is created and utilized in project management	yes
Heroku instance is online	yes
The app is styled to create an attractive user interface	yes
Overall

Good work overall. This site's code is well-written and clear. You make good use of validations and model methods, and it's clear to me that the learning goals for this week were met. Aside from the many nitpicks that are inevitable on a project of this size, the two things that I would like you to review are model relations and routing with resources. However, in general I am happy with this submission. Keep up the hard work!

droberts-sea · 2018-10-12T05:59:11Z

app/models/driver.rb

+
+    driver_trips = self.trips.where(driver_id: id)
+    num_of_trips = driver_trips.count
+


Line 10 is redundant - self.trips will only return trips for this driver, which means all of them have the correct driver ID.

droberts-sea · 2018-10-12T06:00:36Z

app/models/trip.rb

+  validates :driver_id, presence: true
+  validates :passenger_id, presence: true
+
+


The belongs_to relationship should already validate the driver ID and passenger ID, so you do not need to do so explicitly.

droberts-sea · 2018-10-12T06:01:52Z

app/controllers/trips_controller.rb

+  def new
+    @trip = Trip.new
+  end
+


This controller action is not used. The link to request a trip goes directly to the create action, bypassing the new form.

droberts-sea · 2018-10-12T06:03:27Z

app/controllers/trips_controller.rb

+    first_avail_driver = all_avail_drivers.first
+
+    passenger_id = params[:passenger_id]
+    @trip = Trip.new(


All of the work done in this method could be moved to a model method. An instance method on Passenger would probably work well, something like Passenger#request_trip.

Note that you can call methods of one model from another model, so Passenger#request_trip could call Driver.where(available: true).

droberts-sea · 2018-10-12T06:05:55Z

app/views/drivers/_driver.html.erb

@@ -0,0 +1,21 @@
+<%= action_title %>
+
+<%= form_with model: @driver do |f| %>


I'm glad you're using a view partial here, but I don't know that I agree with the name of this one. The file name should probably include the word form.

droberts-sea · 2018-10-12T06:06:27Z

app/views/drivers/show.html.erb

+<!-- QUESTION: render partial to DRY code?? -->
+<h3>Driver Trips</h3>
+<table>
+  <thead>


droberts-sea · 2018-10-12T06:07:41Z

app/views/trips/_trip_list.html.erb

@@ -0,0 +1,44 @@
+<table>
+  <thead>
+   <tr>


It doesn't look like this partial is being used anywhere.

droberts-sea · 2018-10-12T06:10:17Z

config/routes.rb

+  # NOTE: added nested routes for passengers+trips
+  resources :passengers do
+    resources :trips, include: [:index, :show, :new, :delete]
+  end


You shouldn't need to nest the :delete route, since :delete already has the trip ID, which means you can figure out the passenger ID.

Also, you include :new here but it's not used in your application, and you exclude :create but do use it. I'll be honest, I'm not quite sure how the link to request a trip from the passenger show page works.

droberts-sea · 2018-10-12T06:17:15Z

app/controllers/drivers_controller.rb

+  def availability
+    @driver = Driver.find_by(id: params[:driver_id])
+    is_available = @driver.available
+


You have a route and controller action to set the availability, but as far as I can tell it is not used in your application. Nowhere is there a link to the availability_path.

kangazoom and others added 30 commits October 1, 2018 16:14

created new rails project

77f7ae0

created models+dbs

19cfb27

created controllers

7d40a17

added root to routes; fixed some errors

542608a

added some links in welcome viewer

feaca8a

did some passengers work

66c69e5

viewer folders weren't showing ugh added index.html.erb for drivers a…

cf2bb6f

…nd trips

ugh passengers

49fb57a

Merge branch 'passenger_work'

9f5967e

deleted everything in passenger viewer index file

Created Drivers#index

b53f2b1

added very basic CRUD functionality to passengers

876a896

Created migration for car_make and car_model

e69e37d

Using partial for drivers#new

26ef5ea

Created Drivers#Update

cff1043

Adding to Drivers#show, table for trips

bb68182

Convert date instance to string for Drivers#show, table of trips

233a9ec

Calculating driver avg rating in driver/show.html.erb

f5aa55d

added edit/delete links to show passenger page

5ef0c57

Errors for edit driver

9591d87

added validations

a66eaad

Replacing @trip loop in partial to a Trip controller

0e2f97f

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

741e513

getting master up to date

Merge branch 'passenger_work'

934d110

adding passenger_work commits to master

Using nested route /drivers/:id/trips

f93589a

Handling Trips#index regardless if params[:driver_id] present

46ae8dd

Refactor trips_controller.index.html.erb

5ae27b8

added some basic styling to start

b3eac96

Completing trips_controller.index.html.erb trips table

d8cb47c

Adding column to trips table displayed on view template

7f5d052

Create Trip#show

95d1cc7

BarbaraWidjono and others added 28 commits October 4, 2018 12:49

Validations for class Trip and error messaging

1b76646

Create Trip#destroy

c9fa8e3

Added form_with select option for rating

d7b8e81

Using partials for errors and forms

f6ebc61

Changing resources: drivers

64f6f5b

Change cost formatting

88e61bb

Moving business logic from controller to model

7e7f015

Using partial for trip edit

b9b5a3a

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

bc7346c

Merge origin master with trip_work branch

added destroy for trips from passengers + moved routing stuff to cont…

568178c

…roller

viewer folders weren't showing ugh added index.html.erb for drivers a…

068fb18

…nd trips

added more verbs to nested routes

5718243

add trip/passenger functionality - almost done w create new trip via …

bec2dbf

…passenger

Added migration for availability

f13df3d

Deleted unnecessary migrations

bd79f70

Created delete driver method

1db4290

Changed redirect_to drivers_path for Driver#update

2af76b7

added names to trips table for pass and driver

3f5e8cc

Changing Driver#availability

b28e803

Merge branch 'trip_work_create_trip'

8e565dd

Merge master branch with trip_work_create_trip

Changing a driver's availability method

f7460b8

cleaned up as much i could (the driver/trips debacle)

923e15e

Created Trip#create

1caae49

Merge branch 'trip_work_create_trip'

425c320

Merge master to trip_work_create_trip

changed dependency deletion

833d812

Changed path link

31eccbd

Merge branch 'trip_work_create_trip'

6d0a15f

Merge master with trip_work_create_trip

Changed local variable for form partial

fed6f83

droberts-sea reviewed Oct 12, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Michelle & Barbara - Edges - Rideshare-Rails #24

Michelle & Barbara - Edges - Rideshare-Rails #24

Uh oh!

kangazoom commented Oct 6, 2018

Uh oh!

droberts-sea commented Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

droberts-sea Oct 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		driver_trips = self.trips.where(driver_id: id)
		num_of_trips = driver_trips.count

		validates :driver_id, presence: true
		validates :passenger_id, presence: true

		@@ -0,0 +1,21 @@
		<%= action_title %>

		<%= form_with model: @driver do \|f\| %>

@@ @@ -0,0 +1,44 @@ @@
+              <table>
+                <thead>
+                 <tr>

Michelle & Barbara - Edges - Rideshare-Rails #24

Are you sure you want to change the base?

Michelle & Barbara - Edges - Rideshare-Rails #24

Uh oh!

Conversation

kangazoom commented Oct 6, 2018

Rideshare-Rails

Comprehension Questions

Uh oh!

droberts-sea commented Oct 12, 2018

Rideshare Rails

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants