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

Ashton & Addie's VideoStoreAPI #17

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

Conversation

add2point71dots
Copy link

Video Store API

Congratulations! You're submitting your assignment!
If you didn't get to the functionality the question is asking about, reply with what you would have done if you had completed it.

Comprehension Questions

Question Answer
Explain how you came up with the design of your ERD, based on the seed data. The seeds were just movies and customers, which are not connected directly, so we just used the fields from the seeds as our table columns. When we got to the optionals, we connected these models through a Rental model. A customer has many rentals, and a movie has many rentals.
List all the completed endpoints of your application. GET /customers, GET /movies, GET/movies/:title, POST /rentals/:title/check-out, POST /rentals/:title/check-in, GET /rentals/overdue
Describe a set of positive and negative test cases you implemented for a model. In the rental model, we tested that we can create a new Rental instance with valid inputs, and we tested that it wouldn't create a rental if the required fields weren't present. For custom method tests, we tested that our overdue method does not return rental objects that are not overdue.
Describe a set of positive and negative test cases you implemented for a controller. In the rentals controller, we tested that checkout out a movie (that's available) responds with :success. We also tested that if the checkout_path is given the title of a movie that doesn't exist, we get a :bad_request response and an error message.
How does your API respond when bad data is sent to it? It responds with :bad_request and a a customized json error message.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. In the Movie model, we have an available_inventory method that subtracts the number of movies currently checked out from that movie's inventory and returns that number. It was business logic, so we stuck it in a model method.
Do you have any recommendations on how we could improve this project for the next cohort? This was a fun project! Yay!
Link to Trello https://trello.com/b/JNkEfed7/videostoreapi
Link to ERD https://www.lucidchart.com/documents/edit/0a9e24ac-a81c-4d2e-9d1b-28ccd41088bc?shared=true&

add2point71dots and others added 30 commits May 9, 2017 13:08
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Both partners contributed a decent number of commits. Looks like you paired on this project.
Comprehension questions Check, glad you had fun with the project
General
Business Logic in Models Nice work!
All 3 required endpoints return expected JSON data Check
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check
Testing
Passes all Smoke Tests Check
Model Tests - all relations, validations, and custom functions test positive & negative cases Check, nice work with the loop of it blocks!
Controller Tests - URI parameters and data in the request body have positive & negative cases Good tests on content, positive tests and negative tests.
Optionals
POST routes use URI parameter and request body to add a new record to the database Check, well done
GET /customers shows how many movies are checked out by a customer Check
GET /movies/:title shows updated inventory Check, well done
Overall Very nicely done, you had the business logic in the models and all the required endpoints plus optionals. Great work!

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.

Very nicely done.

end

REQUIRED_FIELDS.each do |field|
it "#{field} is required" do

Choose a reason for hiding this comment

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

Very neat use of a loop to generate it blocks!

@alyssahursh
Copy link

Hey there! I don't remember APIs or Ruby well enough to give a good code review on this, so I'm going to give a non-code review focused on git practices instead.

One thing that jumps out to me in this project is the commit history. In my internship, I've been exposed to great git practices that I'd like to see you start practicing now. Here's the general overview:

  • Do you work on a branch (I'm sure you're already doing this). Let it be messy! Try to stay on topic while you're working. Commit whenever you want to, and don't worry about your commit messages too much; they're just for you.
  • When you've done a "logical unit of work" it's time to commit to master. Here's how that works:
  1. git checkout master
  2. git merge --squash
  3. git commit -m "Really great commit message"

Really great commit messages are an art. And they're WAY longer than I thought when I was in school! Here's the guide my coworkers made me read

What this process does is take all of your personal tiny commits (i.e. "Deleted a space") and squashes the down into one really solid commit, so that the only commits that show up on master are logical units of work.

This is how we handle every single commit on my team at Amazon, and I think it's a very common practice. I'd recommend that you start practicing this now so that it feels good when you start your internship!

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.

5 participants