From 389ed2600e56ba3a7a6d33ec9fcdae35e4872f6d Mon Sep 17 00:00:00 2001 From: Cory Streiff <90390502+coalest@users.noreply.github.com> Date: Sun, 13 Oct 2024 23:55:00 +0200 Subject: [PATCH] 1021 Refactor Pet instance method to use the Match model instead of the AdopterApplication model (#1055) * Refactor Pet#is_adopted? method to use Match model * Improve performance of staff pets index page * Remove 2 unneeded queries from rendering /staff/pets * Fix N+1 in AdoptablePetsController * Fix N+1 for pets in LikesController * Fix AdoptablePetsController#index action to expose only unadopted pets --- .../adoptable_pets_controller.rb | 5 ++--- .../adopter_fosterer/likes_controller.rb | 2 +- .../organizations/staff/pets_controller.rb | 5 ++++- app/models/concerns/pet_ransackable.rb | 2 +- app/models/pet.rb | 12 +++++++++-- .../adoptable_pets_controller_test.rb | 10 ++++++++++ test/factories/matches.rb | 6 ++++++ test/models/pet_test.rb | 20 ++++++++++++++----- 8 files changed, 49 insertions(+), 13 deletions(-) diff --git a/app/controllers/organizations/adoptable_pets_controller.rb b/app/controllers/organizations/adoptable_pets_controller.rb index f3273f089..f092462ff 100644 --- a/app/controllers/organizations/adoptable_pets_controller.rb +++ b/app/controllers/organizations/adoptable_pets_controller.rb @@ -8,10 +8,9 @@ class Organizations::AdoptablePetsController < Organizations::BaseController helper_method :get_animals def index - @q = authorized_scope(Pet.includes(:adopter_applications, images_attachments: :blob), - with: Organizations::AdoptablePetPolicy).ransack(params[:q]) + @q = authorized_scope(Pet.unadopted, with: Organizations::AdoptablePetPolicy).ransack(params[:q]) @pagy, paginated_adoptable_pets = pagy( - @q.result, + @q.result.includes(:adopter_applications, :matches, images_attachments: :blob), limit: 9 ) @pets = paginated_adoptable_pets diff --git a/app/controllers/organizations/adopter_fosterer/likes_controller.rb b/app/controllers/organizations/adopter_fosterer/likes_controller.rb index b17e73b82..f40d73cea 100644 --- a/app/controllers/organizations/adopter_fosterer/likes_controller.rb +++ b/app/controllers/organizations/adopter_fosterer/likes_controller.rb @@ -9,7 +9,7 @@ def index likes = current_user.person.likes @likes_by_id = likes.index_by(&:pet_id) - @pets = current_user.person.liked_pets + @pets = current_user.person.liked_pets.includes(:matches, :adopter_applications, images_attachments: :blob) end def create diff --git a/app/controllers/organizations/staff/pets_controller.rb b/app/controllers/organizations/staff/pets_controller.rb index 62931ae0f..e4c30138c 100644 --- a/app/controllers/organizations/staff/pets_controller.rb +++ b/app/controllers/organizations/staff/pets_controller.rb @@ -8,7 +8,10 @@ def index authorize! Pet, context: {organization: Current.organization} @q = Pet.ransack(params[:q]) - @pagy, @pets = pagy(authorized_scope(@q.result), limit: 10) + @pagy, @pets = pagy( + authorized_scope(@q.result.includes(:matches, :adopter_applications, images_attachments: :blob)), + limit: 10 + ) end def new diff --git a/app/models/concerns/pet_ransackable.rb b/app/models/concerns/pet_ransackable.rb index ae042bdce..f973b960f 100644 --- a/app/models/concerns/pet_ransackable.rb +++ b/app/models/concerns/pet_ransackable.rb @@ -7,7 +7,7 @@ def ransackable_attributes(auth_object = nil) end def ransackable_associations(auth_object = nil) - ["adopter_applications"] + ["adopter_applications", "matches"] end def ransackable_scopes(auth_object = nil) diff --git a/app/models/pet.rb b/app/models/pet.rb index 6719d9c48..eaa33253b 100644 --- a/app/models/pet.rb +++ b/app/models/pet.rb @@ -97,11 +97,19 @@ def has_adoption_pending? end def is_adopted? - adopter_applications.any? { |app| app.status == "adoption_made" } + if matches.loaded? + matches.any? { |m| m.match_type == "adoption" } + else + matches.adoption.any? + end end def in_foster? - matches.in_foster.exists? + if matches.loaded? + matches.any? { |m| m.match_type == "foster" && (m.start_date..m.end_date).cover?(Time.now.utc) } + else + matches.in_foster.any? + end end def open? diff --git a/test/controllers/organizations/adoptable_pets_controller_test.rb b/test/controllers/organizations/adoptable_pets_controller_test.rb index 09903f947..56af0984d 100644 --- a/test/controllers/organizations/adoptable_pets_controller_test.rb +++ b/test/controllers/organizations/adoptable_pets_controller_test.rb @@ -16,6 +16,16 @@ class Organizations::AdoptablePetsControllerTest < ActionDispatch::IntegrationTe get adoptable_pets_url end end + + should "assign only unadopted pets" do + adopted_pet = create(:pet, :adopted) + + get adoptable_pets_url + + assert_response :success + assert_equal 1, assigns[:pets].count + assert assigns[:pets].pluck(:id).exclude?(adopted_pet.id) + end end context "#show" do diff --git a/test/factories/matches.rb b/test/factories/matches.rb index 7d6cebf6c..cd83b923d 100644 --- a/test/factories/matches.rb +++ b/test/factories/matches.rb @@ -11,5 +11,11 @@ start_date { Faker::Time.between(from: 1.year.ago, to: 1.month.from_now) } end_date { start_date + rand(3..6).months } end + + trait :adoption do + match_type { :adoption } + start_date { Faker::Time.between(from: 1.year.ago, to: 1.day.ago) } + end_date { 1.day.from_now } + end end end diff --git a/test/models/pet_test.rb b/test/models/pet_test.rb index 90e1f7d47..9b6f4fa26 100644 --- a/test/models/pet_test.rb +++ b/test/models/pet_test.rb @@ -98,17 +98,27 @@ class PetTest < ActiveSupport::TestCase end context "#is_adopted?" do - should "return true if there is an adopter application with 'adoption_pending' status" do - pet = Pet.new - adopter_application = pet.adopter_applications.new - adopter_application.status = "adoption_made" + should "return true if there is a match with 'adoption' match_type" do + pet = create(:pet) + match = create(:match, :adoption, pet:) assert pet.is_adopted? - adopter_application.status = "awaiting_review" + match.update(match_type: "foster") assert_not pet.is_adopted? end end + context "#in_foster?" do + should "return true if there is a match with 'foster' match_type" do + pet = create(:pet) + match = create(:foster, pet:, start_date: 1.month.ago, end_date: 1.day.from_now) + assert pet.in_foster? + + match.update(match_type: "adoption") + assert_not pet.in_foster? + end + end + context "ransack_adopted" do setup do create(:pet)