Ports - Margaret & Steph by smarchante1 · Pull Request #1 · Ada-C11/ride-share-rails

smarchante1 · 2019-04-19T00:57:26Z

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	Our entities are: driver, trip and passenger. Drivers and Passengers both have trips. And a trip has a driver and a passenger.
Describe the role of model validations in your application	They validate certain form fields that are required and prevent the creation of a new trip or passenger etc, without required fields.
How did your team break up the work to be done?	We worked in tandem for the three main MVC's. Then we split tasks with Margaret working on tests for driver and passenger while Steph worked on HTML and CSS. Then we swapped places. Finally, we deployed together.
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 showing and creating data (i.e. new trip, edit). Some features such as toggling for driver availability are still on our low priority Trello board.
What was one thing that your team collectively gained more clarity on after completing this assignment?	Nested routes.
What is your Trello board URL?	https://trello.com/b/nWxpMPfM/ride-share-rails
What is the Heroku URL of your deployed application?	https://marganie-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?	We made decisions together and talked about any changes we wanted to make as the project went along. Any time we worked individually we made a plan of who would do what first and we made use of our Trello board to organize tasks.

…w and index for driver, created show and index html.erb

…tml.erb files and have new driver form

… on the show page

…n user that all trips will also be deleted

…ew in show

… routes in driver and passenger

…edit and delete trips

…action

…age tests

CheezItMan · 2019-04-26T19:54:08Z

Rideshare Rails

What We're Looking For

Feature	Feedback
Baseline
Appropriate git usage with no extraneous files checked in and all team members contributing	I do notice that you have a `.DS_Store` file checked into git. Good number of commits and good commit messages
Answered comprehension questions	Check
Uses named routes (like `_path`)	Check
RESTful routes utilized	You have a lot of unused routes defined in your `routes.rb` file.
Project Requirements
Table relationships	Check
Business logic is in the models	Check, see one method that I suggested.
Controller tests	See my inline comments, you have a number of issues in the tests
Database is seeded from the CSV files	Check
Trello board is created and utilized in project management	Check
Heroku instance is online	Check, I noticed I get an error when I try to request a trip.
The app is styled to create an attractive user interface	Well done
Overall	You hit most of the major learning goals, see my inline comments, there are quite a few. As you noted you can't have a passenger request a trip or a driver go offline or online. I would also suggest making the business logic of having a passenger request a ride part of the Passenger model. Let me know if you have questions. Also look at your routes.rb file. You have a lot of unused routes listed.

CheezItMan · 2019-04-24T17:57:39Z

+      trip: {
+        date: "April 19, 2019",
+        cost: 1445,
+        passenger_id: Passenger.last.id,


A couple of things:

This isn't in an it block so you don't have a Passenger or Driver at this stage.

Unless you have a fixture or a before block that creates a passenger this test will have an error! :(

CheezItMan · 2019-04-24T18:03:19Z

@@ -0,0 +1,15 @@
+# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
+


Nice try at fixtures, as you learned this week you can't set the foreign key fields like driver_id in this way. Instead do something like:

driver: one
passenger: two

CheezItMan · 2019-04-24T18:05:13Z

+  end
+
+  describe "create" do
+    it "should create a new trip" do


There really isn't a form being submitted for the trips controller, Instead the trip data is being set automatically

CheezItMan · 2019-04-24T18:07:01Z

+      must_respond_with :redirect
+    end
+
+    it "returns an error for and invalid trip" do


Again, you don't have a form submitting to the create action, instead a link is used. The only thing you need to test is when the create action is given an invalid passenger id.

CheezItMan · 2019-04-24T18:07:28Z

+        trip: {
+          date: date,
+          cost: cost,
+          passenger_id: 0,


At least you are checking with an invalid passenger id.

CheezItMan · 2019-04-24T18:15:00Z

@@ -0,0 +1 @@
+<%= link_to "All Drivers", drivers_path %>


Is this page really used? Should it be?

Also you have a trips/new.html.erb view, do you need one?

CheezItMan · 2019-04-24T18:16:55Z

@@ -0,0 +1,4 @@
+class Trip < ApplicationRecord
+  belongs_to :driver


You could also add validations to prevent a trip with an invalid cost or rating.

CheezItMan · 2019-04-24T18:17:30Z

+  validates :phone_number, presence: true
+
+  def total_charges
+    return (self.trips.sum { |trip| trip.cost }) / 100


If you want to show this with decimal numbers you should divide by 100.0 or convert it to a float before the division.

CheezItMan · 2019-04-24T18:18:35Z

+
+  validates :name, presence: true
+  validates :phone_number, presence: true
+


Another nice business logic method would be to make a request_trip method which would generate a trip for a specific instance of passenger. That would help you DRY up your controller.

CheezItMan · 2019-04-26T19:50:44Z

+  end
+
+  def create
+    passenger = Passenger.find_by(id: params[:trip][:passenger_id])


passenger_id is a route parameter, not a field submitted by a form. Instead this should be:

passenger = Passenger.find_by(id: params[:passenger_id])

smarchante1 and others added 30 commits April 15, 2019 15:16

create rails app in ride share fork folder

df98986

added model migrations for trip passenger and driver

25f5347

ran db migrate to create schema files

cf85ab6

Created actions for show and index for driver, created routes for sho…

8660219

…w and index for driver, created show and index html.erb

updated .gitignore file to match original

32b5ea2

added erb driver index page

3a1303b

added foreign keys

5d810ef

new commit from margaret on branch-a

dddc3e8

I have a new commit originally on branch named branch-a

bec3955

Created basic show page for drivers

5535c0d

created new and create methods for driver and created _form and new h…

3d2a5a7

…tml.erb files and have new driver form

new driver form is working

2f21b23

added edit and update task for driver and have working edit form

bee8416

did migration to add cascade on delete, working delete action in driver

28cd678

added link to edit driver to show page

4473622

added validation to new and edit

6a2ca56

added total earnings method to driver.rb and displayed earnings on show

e7bb9c7

added average rating method to driver.rb and displayed average rating…

d4ab410

… on the show page

added warning to confirmation message when you delete a driver to war…

f1d6793

…n user that all trips will also be deleted

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

f0fe094

added routes for passenger actions

63c6b8a

added show html for passenger data

e9c7576

added form to create new passenger

efc84a3

added the form to edit and did some troubleshooting

87ad6c9

added destroy action and associated link in show

dff9902

added total charges method in passenger model and added associated vi…

d0ade66

…ew in show

Added trips model, view, controller, made trip routes and nested trip…

394dcd0

… routes in driver and passenger

Created show, new, and create methods and params for trips

179db70

updated method to generate random cost for trip

b3932a4

created working delete method for passenger's trip

a5ad3ec

msfinnan and others added 28 commits April 18, 2019 13:21

driver and passenger tests

95fd4a6

fighting with navigation menu

af7c863

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

f9f1e0f

finished designing navigation menu

be5b048

finished passenger and driver tests and created homepage controller

4b75738

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

2d24a75

added homepage controller and index.html.erb, added missing links to …

7d44c44

…edit and delete trips

can switch between request ride and rate ride

2c6405c

adding tests for trips controller

3ec2913

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

e4f494f

can link to a trip and see details from drivers and trips shows

92eb3d4

added table to passenger show page

43e48cc

added table to driver show page

84f2128

finished update and edit tests. working on destroy method

d1a1cb7

finished trip controller tests. need to revisted one test in update …

e1b247c

…action

formatting tables for show pages

e8dbea4

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

9bf610f

small css change to table and fixed rounding on driver average rating

4a019e4

removed pry

d2a0f8f

fixed phone number issue in seed file

58cf143

made css changes for all pages

0329767

added band to line 17 in the show action for trips controller

643edbd

changed head not found to redirect in the show action

f9381cd

general css styling

1be7e52

made adjustments to indentations for html.erb files

1043320

removed spurious comments from from controller files

1b83d37

changed logic for show action and updated paths for show valid trip p…

97e2d20

…age tests

started working on homepages test

cc0fb01

CheezItMan reviewed Apr 26, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ports - Margaret & Steph#1

Ports - Margaret & Steph#1
smarchante1 wants to merge 68 commits intoAda-C11:masterfrom
smarchante1:master

smarchante1 commented Apr 19, 2019

Uh oh!

CheezItMan commented Apr 26, 2019 •

edited

Loading

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 24, 2019

Uh oh!

CheezItMan Apr 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,15 @@
		# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html

		@@ -0,0 +1 @@
		<%= link_to "All Drivers", drivers_path %> No newline at end of file

		@@ -0,0 +1,4 @@
		class Trip < ApplicationRecord
		belongs_to :driver


		validates :name, presence: true
		validates :phone_number, presence: true

Conversation

smarchante1 commented Apr 19, 2019

Rideshare-Rails

Comprehension Questions

Uh oh!

CheezItMan commented Apr 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

CheezItMan commented Apr 26, 2019 •

edited

Loading