Skip to content

Ports - Laneia && Mina#10

Open
laneia wants to merge 107 commits intoAda-C11:masterfrom
minams:master
Open

Ports - Laneia && Mina#10
laneia wants to merge 107 commits intoAda-C11:masterfrom
minams:master

Conversation

@laneia
Copy link
Copy Markdown

@laneia laneia commented Apr 19, 2019

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 Trip belongs to Passenger and Driver and both Passenger and Driver have many trips. This is because passengers and drivers will utilize the rideshare service potentially many times, but each trip should be its own unique instance with only one passenger and driver.
Describe the role of model validations in your application Our validations are ensuring that users enter required fields before saving or editing information. It's very helpful as some methods rely on that information, so this can keep them from failing.
How did your team break up the work to be done? We divided up work fairly and evenly based on project requirements and individual interests. Passengers and drivers work were similar so we split those duties up, came together on trips, and split styling/HTML and testing.
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 functionality but as far as I know we did not set any features aside. We included styling for additional readability.
What was one thing that your team collectively gained more clarity on after completing this assignment? The use of let in testing and how it can lead to unexpected test failures.
What is your Trello board URL? https://trello.com/b/Cb5koffQ/rideshare
What is the Heroku URL of your deployed application? https://raise-the-roof.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 felt like we balanced each other's strengths and weaknesses well and we also felt like we worked very well together overall.

@CheezItMan
Copy link
Copy Markdown

Rideshare Rails

What We're Looking For

Feature Feedback
Baseline
Appropriate git usage with no extraneous files checked in and all team members contributing
Answered comprehension questions Check, let can cause unexpected failures if you use the let variable after the controller action.
Uses named routes (like _path) Check
RESTful routes utilized Check, good work taking out unneeded routes.
Project Requirements
Table relationships Check
Business logic is in the models Check
Controller tests Mostly missing
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 Decent styling
Overall You have most of the requirements, except testing. You also don't prevent a passenger from requesting a trip, if they already have an unrated trip. You also have no way to take drivers online and offline. However, except for testing, those are minor. Let me know any questions you have about my comments.

Comment thread config/routes.rb

resources :drivers

resources :trips, except: [:index, :create]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

# Your tests go here
end
end
# require "test_helper"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmmm... so no tests here

@@ -1,31 +1,149 @@
require "test_helper"
# require "test_helper"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No tests here as well

cost: 1000,
},
}
it "can update an existing trip" do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about updating a trip with invalid arguments?


describe "destroy" do
# Your tests go here
it "will respond with not found if invalid id" do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about destroying a valid trip?

Comment thread app/models/driver.rb
total_money = 0

if (trips.length > 0)
trips.each do |trip|
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could use .sum

Comment thread app/models/driver.rb
return 0.0
end

trips.each do |trip|
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could use .where to get trips with a rating and .sum.

Comment thread app/models/trip.rb
class Trip < ApplicationRecord
belongs_to :driver
belongs_to :passenger
validates :date, :cost, presence: true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread app/models/passenger.rb
@@ -0,0 +1,12 @@
class Passenger < ApplicationRecord
has_many :trips, dependent: :destroy
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good use of dependent: destroy

Comment thread app/models/passenger.rb
class Passenger < ApplicationRecord
has_many :trips, dependent: :destroy
validates :name, :phone_num, presence: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suggest making a model method for a passenger to request a trip.

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