Ampers - Abinnet + Lily

[lillers1122]

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.	We read the readme and the seed data and compiled the ERD from that information.
Describe a set of positive and negative test cases you implemented for a model.	For a movie, we tested if we could create one with valid and invalid data. We tested the methods more extensively with the controllers.
Describe a set of positive and negative test cases you implemented for a controller.	We tested a valid rental and an invalid rental (where the movie had no available inventory).
How does your API respond when bad data is sent to it?	Bad_request; not_found; and error messages.
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	Our method to increase movies_checked_out_count was wrapped in a method because it didn't need to be in the controller. It made testing and calling it easier.
Do you have any recommendations on how we could improve this project for the next cohort?	More clarity about Postman and smoketests at the start.
Link to Trello	https://trello.com/b/D6EkjxYU/videostoreapi
Link to ERD	https://trello.com/c/Bcy3iOvT/11-erd

[tildeee]

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	x
Comprehension questions	x
General
Business Logic in Models	x
All required endpoints return expected JSON data	x
Requests respond with appropriate HTTP Status Code	x
Errors are reported	x
Testing
Passes all Smoke Tests	x
Model Tests - all relations, validations, and custom functions test positive & negative cases
Controller Tests - URI parameters and data in the request body have positive & negative cases
Overall

Seeding doesn't seem to work for me? Running rails db:seed (or reset, which includes the seed command) fails and breaks. You all call Movie.create even though you need to set the available_inventory property (which you do in the next line.. and even .save after that......)

The methods on Customer take in as a parameter an instance of customer... and then they change a property on that instance of customer... Why are the methods on Customer all class methods/self methods? Why not just make them instance methods?

Same goes for the increment/decrement methods on Movie... they take in an instance of Movie and operate on it, yet you've written them as self/class methods. What is the difference between

  def self.increment(movie)
    if movie.available_inventory < movie.inventory
      movie.available_inventory += 1
      movie.save
    end
  end

and

  def increment
    if self.available_inventory < self.inventory
      self.available_inventory += 1
      self.save
    end
  end

Let me know if you can't figure out the answer to this question/the difference between self/class and instance methods. It's pretty important that you can distinguish why an instance method is more appropriate for this

In RentalsController#update you have the following line...

rental_movie = Movie.returnable_movie?(Movie.find_by(id: params[:movie_id]))

You're using what's in params[:movie_id], finding an instance of movie, and passing that in.

That method returnable_movie? takes in what appears to be an id in Movie...

def self.rentable_movie?(id)
      movie = Movie.find_by(id: id)
      if movie.available_inventory < 1
        movie = false
      end
    return movie
  end

You're taking in an "id" and then finding an instance of movie with that id

Try to stay consistent-- you're doing redundant work here

That being said, I'm pleased that the smoke tests all pass.

[tildeee]

you just checked for movie's truthiness a line above, maybe there's a way to refactor this logic?

[tildeee]

What is this line doing?