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

Water - Kareha & Hanh #17

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

Water - Kareha & Hanh #17

wants to merge 63 commits into from

Conversation

agesak
Copy link

@agesak agesak commented Nov 7, 2020

Assignment Submission: Rideshare Rails

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions. These questions should be answered by all team members together, not by a single teammate.

Reflection

Prompt Response
Describe the types of entity relationships you set up in your project and why you set up the relationships that way Trips "belong to" passengers and drivers. Both drivers and passengers "have many" trips.
Describe the role of model validations in your application Model validations ensure users can't input incorrect information into the database.
How did your team break up the work to be done? We kinda worked on stuff together mostly. But we did break up who was working on what tests 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 prioritized all the functional requirements and set aside any styling.
What was one thing that your team collectively gained more clarity on after completing this assignment? Nested routes
What are two discussion points that you and your team discussed when giving/receiving feedback from each other that you would be willing to share? We discussed when to take breaks and how to effectively use our time.
Optional: What is the URL of your deployed Heroku app?

@jmaddox19
Copy link

Rideshare Rails

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Practices collaborating with git, and all team members contribute git commits and best git practices ✔️ Plenty of commits from both of you! Many of your commits could use a more detailed message (eg. "working on tests", "Updated tests")
Demonstrates understanding of relationships by giving accurate answers to the reflection questions and appropriate code in the models ✔️
Logic to calculate a driver's average rating and total earnings is located in the model, and it has unit tests ✔️
There are reasonable tests to test the validation requirements for all models ✔️ Mostly! but no tests in Trip
There are reasonable tests to test the relationship requirements for all models ✔️ Mostly! but no tests in Trip
There are reasonable tests to test the controller actions for all controllers ✔️
The app has an attractive and usable user interface No styling
The app uses/is compatible with database seeds ✔️
Router code is clean: uses resources and RESTful routes ✔️

Functional Requirements

Functional Requirement yes/no
On the passenger's details page, I want to be able to see total charged, list of trips, a link to edit, and delete ✔️
When adding a new passenger, I want to see errors and validations that show that a passenger must be provided a name and a phone number, so that I cannot make a passenger without name or phone number ✔️
On the passenger's details page, I can create a new trip for this passenger, with an assigned driver and no rating ✔️
On the driver's details page, I want to be able to see total earnings, average rating, list of trips, a link to edit, and delete ✔️
When adding a new driver, I want to see errors and validations that show that a driver must be provided a name and VIN, so that I cannot make a driver without name or VIN ✔️
On the trip's detail page, I want to be able to view details, assign a rating, navigate to the trip's passenger, driver, a link to edit, and delete ✔️ Yes, one little note: It feels a little awkward that after I edit a trip to add a rating or update other info, I'm redirected to the homepage rather than to the trip details page.

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 7+ in Code Review && 5+ in Functional Requirements ✔️
Yellow (Approaches Standards) 5+ in Code Review && 4+ in Functional Requirements, or the instructor judges that this project needs special attention
Red (Not at Standard) 0-4 in Code Review or 0-3 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging, or the instructor judges that this project needs special attention

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

return 0
end

def change_status

Choose a reason for hiding this comment

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

Cool that y'all tried out making an additional custom model method!

describe "can go online" do
# Your code here
it "returns float" do
expect(@driver.total_revenue).must_be_kind_of Float

Choose a reason for hiding this comment

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

The tests in driver are quite thorough for the most part! This is an opportunity for improvement, where you could test the exact value of the revenue.

Suggested change
expect(@driver.total_revenue).must_be_kind_of Float
expect(@driver.total_revenue).must_be_kind_of Float
expect(@driver.total_revenue).must_equal 7568

Comment on lines +9 to +10
<%= f.label :available %>
<%= f.text_field :available%>

Choose a reason for hiding this comment

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

This is a little awkward as a text field. There's some implicit conversion the ruby code is doing when it sends it along to the model. If a user enters "no" into that field, it will get saved as true.

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