Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carets -- Laura & Victoria #20

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open

Carets -- Laura & Victoria #20

wants to merge 41 commits into from

Conversation

vsawchuk
Copy link

@vsawchuk vsawchuk commented Oct 6, 2017

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 have a table each for drivers, passengers, and trips. Drivers and passengers have many trips, and trips belong to drivers and passengers. Drivers and passengers can have more than one trip, but each trip can only have one driver and one passenger.
Describe the role of model validations in your application They prevent improper user input, and ensure presence of all necessary variables. This helps to prevent db errors caused by changes to the controllers.
How did your team break up the work to be done? We paired on everything!...almost everything. We paired for all the required items, and broke out some smaller optional work at the end.
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 didn't finalize all styling or additional driver/rider/trip interaction, but we met all required features.
What was one thing that your team collectively gained more clarity on after completing this assignment? Validations, rendering partial forms with local variables, and styling.
What is your Trello URL? https://trello.com/b/9BQclyMP/rideshare
What is the Heroku URL of your deployed application? https://l-v-rideshare.herokuapp.com/

vsawchuk and others added 30 commits October 2, 2017 14:31
* Update routes to convention over configuration
@CheezItMan
Copy link

Rideshare-Rails

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with no extraneous files checked in and both partners contributing Good number of commits and good participation by both team members.
Answered comprehension questions Check
Uses named routes (like _path) Check
RESTful routes utilized Check, nice work with status and using resources.
Rideshare Rails Specific Content
Table relationships Check Trips related to both Drivers and Passengers.
Validation rules for Models Check, good messages and validations for presence and score values.
Business logic is in the models There is No business logic in the models. You should put things like finding an available driver as a method in the Driver class and a method to calculate the average rating in Trips. Do not put that in the controllers.
Database is seeded from the CSV files Good use of Rails.root.join, also good work reseting the ID field sequence in the DB.
Trello board is created and utilized in project management Check, nice work
Postgres database is used Check
Heroku instance is online Check, nicely styled
The app is styled to create an attractive user interface I like the styling simple and elegant.
Overall Nice work, you hit all the requirements. As noted, think about putting more business logic in the model files.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Some in-code comments.


def new
driver_id = find_driver()
@trip = Trip.new(date: Date.today, cost: rand(100..2000), driver_id: driver_id, passenger_id: params[:passenger_id], rating: nil)

Choose a reason for hiding this comment

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

This makes a good place to use strong params.

@passenger = Passenger.find_by(id: params[:id].to_i)
redirect_to passenger_path unless @passenger

if @passenger.update_attributes passenger_params

Choose a reason for hiding this comment

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

Good use of strong params.

return params.require(:driver).permit(:name, :vin)
end

def rating(trips)

Choose a reason for hiding this comment

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

Shouldn't this be in the model.

end

def total(trips)
return trips.sum {|trip| trip.cost}

Choose a reason for hiding this comment

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

Another good thing to move to the model

<section class="user_list">
<h1>Drivers</h1>
<p>
Our drivers are the best. Everyday carry enamel pin vape beard jean shorts artisan. Knausgaard flexitarian wolf keytar affogato, kale chips helvetica subway tile craft beer YOLO cold-pressed cliche. Sustainable +1 ugh, air plant yuccie cold-pressed humblebrag chicharrones seitan chartreuse cray jianbing.

Choose a reason for hiding this comment

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

huh?

<p>Please rate your last trip:</p>
<%= form_tag(rate_trip_path(id: @needs_rating.id), method: :patch, class: 'rating') do %>
<%= label_tag('Rating') %>
<div class='rating-labels'>

Choose a reason for hiding this comment

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

Good use-case for a div element.

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