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

Branches - Kelsey and Brianna #11

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

Conversation

brikemp
Copy link

@brikemp brikemp commented Oct 11, 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 Relationships: trip belongs_to :driver, trip belongs_to :passenger, driver has_many trips, passenger has_many trips. We set them up this way because it was intuitive and manageable.
Describe the role of model validations in your application Validations: Driver (name, vin) Passenger(name, phone_num), Trips(date, driver, passenger). They ensure that our methods cannot be executed without these parameters being fulfilled. All of our validations are set to have a presence of true
How did your team break up the work to be done? Each of us would write methods for a specific class/object then the other person would write the tests. We utilized Trello to assign ourselves tasks.
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 working code over correct code and de-prioritized CSS features
What was one thing that your team collectively gained more clarity on after completing this assignment? Routes - Kelsey, Everything - both of us
What is your Trello board URL? https://trello.com/b/ri94OniC/rideshare
What is the Heroku URL of your deployed application? https://still-island-23738.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 discussed our confidence in our ability to successfully complete this project within the deadline and our ability to overcome obstacles.

brikemp and others added 30 commits October 7, 2019 15:40
Comment on lines +13 to +17
def convert_to_dollars(pennies)
if pennies.nil?
return 0
else
(pennies / 100.0).round(2)

Choose a reason for hiding this comment

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

Love that y'all made a separate method for this! That said, the fact that this is repeated in three different classes is a sign that it probably belongs in its own class. It could be put in a class (maybe a new class called Money, for instance) and made a class method so that an instance of money isn't needed to call it.

validates :driver, presence: true
validates :passenger, presence: true

def self.all_in_alpha_order

Choose a reason for hiding this comment

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

Love this method!

Comment on lines +38 to +40
@drivers = Driver.where(active: false)
@selected_driver = @drivers[rand([email protected])]
@trip.driver_id = @selected_driver.id

Choose a reason for hiding this comment

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

this is a good candidate for a model method

# Assert
expect(new_driver.valid?).must_equal false
expect(new_driver.errors.messages).must_include :vin
expect(new_driver.errors.messages[:vin]).must_equal ["can't be blank"]
end
end

# Tests for methods you create should go here
describe "custom methods" do

Choose a reason for hiding this comment

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

Great test coverage here!

@jmaddox19
Copy link

Rideshare Rails

What We're Looking For

Great job y'all! Love the video link. Haha. Also love that all the driver and passenger names are links everywhere, making for easy navigation.

I left some inline comments as well, so please do check those out.

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