From fdd4d725621e361b986572d89a1dbc9ee0ea1eed Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 14:14:29 -0800 Subject: [PATCH 1/7] added validation for unique external id --- app/models/video.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/video.rb b/app/models/video.rb index f47b7f0b..0df502cb 100644 --- a/app/models/video.rb +++ b/app/models/video.rb @@ -2,6 +2,7 @@ class Video < ApplicationRecord has_many :rentals has_many :customers, through: :rentals + validates :external_id, uniqueness: true def available_inventory self.inventory - self.rentals.where(returned: false).length end From c67c4de61dcc998475e3ee2d57923ffdea4a02d0 Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 14:28:21 -0800 Subject: [PATCH 2/7] added model validation for videos --- test/fixtures/videos.yml | 2 ++ test/models/video_test.rb | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/fixtures/videos.yml b/test/fixtures/videos.yml index f2ccff73..d1f02ede 100644 --- a/test/fixtures/videos.yml +++ b/test/fixtures/videos.yml @@ -5,9 +5,11 @@ one: overview: MyText release_date: 2017-01-11 inventory: 4 + external_id: 7 two: title: MuchFilm overview: MyText release_date: 2017-01-11 inventory: 7 + external_id: 8 diff --git a/test/models/video_test.rb b/test/models/video_test.rb index b440a490..60a5c479 100644 --- a/test/models/video_test.rb +++ b/test/models/video_test.rb @@ -6,7 +6,8 @@ class VideoTest < ActiveSupport::TestCase "title": "Hidden Figures", "overview": "Some text", "release_date": "1960-06-16", - "inventory": 8 + "inventory": 8, + "external_id": 9 } } @@ -78,4 +79,16 @@ class VideoTest < ActiveSupport::TestCase expect(after_ai).must_equal before_ai + 1 end end + + describe "model validations" do + it "cannot add a movie with a matching external id (aka twice from MovieDB)" do + video_data["external_id"] = 7 # match fixture external_id + + invalid_video = Video.create(video_data) + + expect(invalid_video.valid?).must_equal false + expect(invalid_video.errors[:external_id]).must_include "has already been taken" + + end + end end From 9320faeeb165ed3cceaee228990735a97eeb5b6d Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 14:31:48 -0800 Subject: [PATCH 3/7] added create video in video controller --- app/controllers/videos_controller.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/controllers/videos_controller.rb b/app/controllers/videos_controller.rb index c9a2bb08..a5c5d325 100644 --- a/app/controllers/videos_controller.rb +++ b/app/controllers/videos_controller.rb @@ -21,8 +21,23 @@ def show ) end + def create + video = Video.new(video_params) + + if video.save + render json: video.as_json, status: :created + return + else + render json: {errors: video.errors.messages}, status: :bad_request + end + end + private + def video_params + params.permit(:id, :title, :release_date, :overview, :image_url, :external_id, :inventory) + end + def require_video @video = Video.find_by(title: params[:title]) unless @video From 8f6c0c92e9cb205feb5a7a7edf1cd48e9580e09b Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 14:42:05 -0800 Subject: [PATCH 4/7] added tests and route to create video --- app/controllers/videos_controller.rb | 2 +- config/routes.rb | 2 +- test/controllers/videos_controller_test.rb | 40 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/app/controllers/videos_controller.rb b/app/controllers/videos_controller.rb index a5c5d325..b834a8dc 100644 --- a/app/controllers/videos_controller.rb +++ b/app/controllers/videos_controller.rb @@ -35,7 +35,7 @@ def create private def video_params - params.permit(:id, :title, :release_date, :overview, :image_url, :external_id, :inventory) + params.permit(:title, :release_date, :overview, :image_url, :external_id, :inventory) end def require_video diff --git a/config/routes.rb b/config/routes.rb index 16fc2214..1111b4bf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,7 +3,7 @@ resources :customers, only: [:index] - resources :videos, only: [:index, :show], param: :title + resources :videos, only: [:index, :show, :create], param: :title post "/rentals/:title/check-out", to: "rentals#check_out", as: "check_out" post "/rentals/:title/return", to: "rentals#check_in", as: "check_in" diff --git a/test/controllers/videos_controller_test.rb b/test/controllers/videos_controller_test.rb index 730915ed..ced46894 100644 --- a/test/controllers/videos_controller_test.rb +++ b/test/controllers/videos_controller_test.rb @@ -77,4 +77,44 @@ class VideosControllerTest < ActionDispatch::IntegrationTest expect(data["errors"]).must_include "title" end end + + describe "create" do + before do + # Arrange + @video_hash = { + title: "the l3go movie", + overview: "it IS getting a trequel, right?", + release_date: "2022-04-30", + inventory: 10, + external_id: 10, + image_url: "https://www.themoviedb.org/t/p/w1280/lbctonEnewCYZ4FYoTZhs8cidAl.jpg" + } + end + it "can create a valid video" do + # Assert + expect { + post videos_path, params: @video_hash + }.must_change "Video.count", 1 + + must_respond_with :created + end + + it "will respond with bad request and errors for an invalid movie" do + # Arrange + @video_hash[:external_id] = 7 # match external id of fixture + + # Assert + expect { + post videos_path, params: @video_hash + }.wont_change "Video.count" + body = JSON.parse(response.body) + + expect(body.keys).must_include "errors" + expect(body["errors"].keys).must_include "external_id" + expect(body["errors"]["external_id"]).must_include "has already been taken" + + must_respond_with :bad_request + end + + end end From ccd4d6074c577781175a85a4f36c6f00e8fb41b3 Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 20:57:02 -0800 Subject: [PATCH 5/7] updated model validations for video --- app/models/video.rb | 1 + test/models/video_test.rb | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/app/models/video.rb b/app/models/video.rb index 0df502cb..58e16760 100644 --- a/app/models/video.rb +++ b/app/models/video.rb @@ -3,6 +3,7 @@ class Video < ApplicationRecord has_many :customers, through: :rentals validates :external_id, uniqueness: true + validates :inventory, numericality: {only_integer: true, greater_than: 0} def available_inventory self.inventory - self.rentals.where(returned: false).length end diff --git a/test/models/video_test.rb b/test/models/video_test.rb index 60a5c479..3dbdd577 100644 --- a/test/models/video_test.rb +++ b/test/models/video_test.rb @@ -90,5 +90,25 @@ class VideoTest < ActiveSupport::TestCase expect(invalid_video.errors[:external_id]).must_include "has already been taken" end + + it "must have a total_inventory greater than 0" do + videos.each do |video| + video.inventory = 0 + expect(video.valid?).must_equal false + expect(video.errors[:inventory]).must_include "must be greater than 0" + end + end + + it "must have numerical, integer values for total/available inventory" do + error_message_hash = {"NaN" => "is not a number", 1.5 => "must be an integer"} + # ^ allows us to test for invalid number and integer input in one neat test :) + error_message_hash.each do |error, message| + videos(:one)[:inventory] = error + + expect(videos(:one).valid?).must_equal false + + expect(videos(:one).errors[:inventory]).must_include message + end + end end end From fa2923894efcbfe4f96586cb74d2247cf134afff Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 23:12:17 -0800 Subject: [PATCH 6/7] added validation for valid video and customer --- app/models/rental.rb | 1 + test/models/rental_test.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/app/models/rental.rb b/app/models/rental.rb index 11495264..d63f591f 100644 --- a/app/models/rental.rb +++ b/app/models/rental.rb @@ -5,6 +5,7 @@ class Rental < ApplicationRecord # validates :video, uniqueness: { scope: :customer } validates :due_date, presence: true validate :due_date_in_future, on: :create + validates_associated :customer, :video after_initialize :set_checkout_date after_initialize :set_returned diff --git a/test/models/rental_test.rb b/test/models/rental_test.rb index 2c3675ae..fb1d2ada 100644 --- a/test/models/rental_test.rb +++ b/test/models/rental_test.rb @@ -35,6 +35,15 @@ class RentalTest < ActiveSupport::TestCase expect(@rental).must_respond_to :video end + it "has a video with sufficient inventory" do + videos(:one)['inventory'] = 0 + rental_data['video'] = videos(:one) + bad_rental = Rental.new(rental_data) + expect(bad_rental.valid?).must_equal false + expect(bad_rental.errors.messages).must_include :video + expect(bad_rental.errors.messages[:video]).must_include "is invalid" + end + it "Cannot be created without a video" do data = rental_data.clone data.delete :video From df7c98e336fa397bd9a093d014c04a06932b1838 Mon Sep 17 00:00:00 2001 From: Pauline Chane Date: Wed, 20 Jan 2021 23:35:48 -0800 Subject: [PATCH 7/7] added tests for adding a video with no available inventory --- app/controllers/rentals_controller.rb | 5 +++++ test/controllers/rentals_controller_test.rb | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/app/controllers/rentals_controller.rb b/app/controllers/rentals_controller.rb index c36d8cd0..fb6b0ea7 100644 --- a/app/controllers/rentals_controller.rb +++ b/app/controllers/rentals_controller.rb @@ -6,6 +6,11 @@ class RentalsController < ApplicationController def check_out rental = Rental.new(video: @video, customer: @customer, due_date: params[:due_date]) + if rental.video.available_inventory < 1 + render status: :bad_request, json: { errors: { video: ["No copies available"] } } + return + end + if rental.save render status: :ok, json: {} else diff --git a/test/controllers/rentals_controller_test.rb b/test/controllers/rentals_controller_test.rb index aaa25dd3..f87e5a00 100644 --- a/test/controllers/rentals_controller_test.rb +++ b/test/controllers/rentals_controller_test.rb @@ -69,6 +69,22 @@ class RentalsControllerTest < ActionDispatch::IntegrationTest expect(data).must_include "errors" expect(data["errors"]).must_include "due_date" end + + it "requires sufficient inventory" do + videos(:one)['inventory'] = 1 + videos(:one).save # ensure new inventory number is saved to invalidate this test + customer = customers(:two) + post check_out_path(title: videos(:one).title), params: { + customer_id: customer.id, + due_date: Date.today + 1 + } + + must_respond_with :bad_request + data = JSON.parse @response.body + expect(data).must_include "errors" + expect(data["errors"]).must_include "video" + expect(data["errors"]["video"]).must_include "No copies available" + end end describe "check-in" do