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

Updated PR from master #32

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

Updated PR from master #32

wants to merge 51 commits into from

Conversation

steffnay
Copy link

@steffnay steffnay commented Apr 9, 2018

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
Describe the role of model validations in your application
How did your team break up the work to be done?
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?
What was one thing that your team collectively gained more clarity on after completing this assignment?
What is your Trello URL?
What is the Heroku URL of your deployed application?
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?

steffnay and others added 30 commits April 3, 2018 11:26
…yed as a link if the driver has not been deleted
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.

The feedback hasn't changed much from: #13 (comment)

However I've left some notes in the code here. You still, on MediaRanker, need to work on putting business logic in the model, rather than doing calculations in the controllers. Overall you did well however.

resources :drivers


get 'user_interfaces', to: 'user_interfaces#index', as: 'user_interfaces'

Choose a reason for hiding this comment

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

You don't need this route here, as you're just using user_interfacesController as the root path.

# Calculate driver total rating:
number_of_ratings = 0
@rating = 0
Trip.where(driver_id: @driver.id).each do |trip|

Choose a reason for hiding this comment

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

This should totally be in the Driver model.

@passenger = Passenger.find(id)

@total_paid = 0
@passenger.trips.each {|trip| @total_paid += trip.cost}

Choose a reason for hiding this comment

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

Should be in the model.

def update
@passenger = Passenger.find_by(id: params[:id])
if [email protected]?
@passenger.update(passenger_params) ? (redirect_to passenger_path(@passenger.id)) : (render :edit)

Choose a reason for hiding this comment

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

You have a lot of good uses of ternary operators.


<h2><%= page_title %></h2>
<% if @driver.errors.any? %>
<ul class="errors">

Choose a reason for hiding this comment

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

👍

<%= f.text_field :phone_num %>

<%= f.label :img, "Picture" %>
<%= f.select :img, [['Bear', "https://placebear.com/100/100"], ['Beyonce', "http://placebeyonce.com/100-100"]] %>

Choose a reason for hiding this comment

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

Clever!

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