Skip to content

Mariana & Tanisha | Pipes | Rideshare rails #8

Open
tanham wants to merge 93 commits into
Ada-C8:masterfrom
lmarianarp19:master
Open

Mariana & Tanisha | Pipes | Rideshare rails #8
tanham wants to merge 93 commits into
Ada-C8:masterfrom
lmarianarp19:master

Conversation

@tanham

@tanham tanham commented Oct 2, 2017

Copy link
Copy Markdown

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 3 entities are driver, passenger, and trip. Driver can have many trip and trip has only one driver. Passenger can have many trips and trip can only have one passenger.
Describe the role of model validations in your application We used model validations to make sure that all the parameters to create a new instance were included in creating a new trip, passenger, or driver.
How did your team break up the work to be done? For the problems that were the similar for example, setting up the drivers and passengers, we each worked separately so we each could have practice. For more difficult problems we worked 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 chose to prioritize that required functionality. We set aside styling and formatting money output. Also, if there are no drivers, an error occurs when the user tries to add a trip.
What was one thing that your team collectively gained more clarity on after completing this assignment? We both have a better understanding of routes and what how to use the model to add business logic to your project.
What is your Trello URL? https://trello.com/b/8akGgb0r/rideshare-rails
What is the Heroku URL of your deployed application? https://mar-tan-rideshare-app.herokuapp.com/

@kariabancroft

Copy link
Copy Markdown

Rideshare-Rails

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with no extraneous files checked in and both partners contributing Yes - looks good
Answered comprehension questions Yes
Uses named routes (like _path) Yes
RESTful routes utilized Yes - though by using resources for passenger by itself as well as with a nested route you have created some duplicate routes. I think you should be able to get rid of all of the individual passenger routes on line 4.
Rideshare Rails Specific Content
Table relationships The schema looks good with the foreign key columns on the trips table, and the ActiveRecord relationships look good within the model files.
Validation rules for Models Yes - you stuck mainly with the presence validations, so it will be good to practice with the other types in future projects.
Business logic is in the models Yes - you have the total amount and average rating methods in the Driver model.
Database is seeded from the CSV files Definitely didn't use the seeds on the Heroku deployment
Trello board is created and utilized in project management The Trello board is private - could you make it public and then @ me again so I can review?
Postgres database is used Yes
Heroku instance is online Yes - it does seem like there is an issue with the Driver show page. My guess that it is an issue with the linked trips or the model methods to display the total/average.
The app is styled to create an attractive user interface It is styled - and I see you've noted prioritizing features over styling.
Overall You did a nice job hitting the major learning goals on this assignment. In future assignments double check your rails routes to ensure you aren't creating unnecessary duplicate routes. Additionally, you should try out some different types of validations for your user input.

Comment thread app/models/driver.rb
end

if trips.length == 0
return "No trips yet"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note that it is fine to return two different data types from a method, but it is good practice to try to figure out how to return similar data rather than an error message. Instead of retuning this string you could return nil or 0 as a starting value.

Comment thread app/models/driver.rb
def average_rating
total = 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 an enumerable method to clean up this loop and simplify to a single line of code

@@ -0,0 +1,15 @@
<h1>Edit passenger</h1>

<%= render partial: 'layouts/error_messages',

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 like the way you used a partial for the error messages. It would be interesting to also use a partial for the rest of the form.

@tanham

tanham commented Oct 12, 2017

Copy link
Copy Markdown
Author

@kariabancroft I've added you to the trello board

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