Refactor matches (#524)

* Refactor matches routes to Rails convention

* Refactor MatchesController to use Rails naming conventions such as destroy instead of delete

* Move some business logic from MatchesController into AdopterApplication model

* Add tests for new withdrawing and retiring application methods

* Add safe navigation in case a match is created where no paired AdopterApplication exists

* Adjust strong params to require match

* Fix params reference in get_pet_id

app/controllers/matches_controller.rb

@@ -1,28 +1,27 @@
 class MatchesController < ApplicationController
   before_action :active_staff, :same_organization?
 
+  before_action :set_pet, only: %i[create]
+  before_action :set_match, only: %i[destroy]
+
   def create
-    @pet = Pet.find(adoption_params[:pet_id])
-    @match = Match.new(adoption_params.merge(organization_id: @pet.organization_id))
+    @match = Match.new(match_params.merge(
+      organization_id: @pet.organization_id
+    ))
 
     if @match.save
-      set_statuses_to_adoption_made
+      @match.retire_applications
+
       redirect_back_or_to dashboard_index_path, notice: "Pet successfully adopted."
     else
       redirect_back_or_to dashboard_index_path, alert: "Error."
-
     end
   end
 
-  def delete
-    @match = Match.find(params[:match_id])
-
-    @successful_application = @match.adopter_account
-      .adopter_applications
-      .where(pet_id: @match.pet_id)[0]
-    AdopterApplication.set_status_to_withdrawn(@successful_application)
-
+  def destroy
     if @match.destroy
+      @match.withdraw_application
+
       redirect_to pets_path, notice: "Adoption reverted & application set to 'Withdrawn'"
     else
       redirect_to pets_path, alert: "Failed to revert adoption"
@@ -31,25 +30,22 @@ def delete
 
   private
 
-  def adoption_params
-    params.permit(:pet_id, :adopter_account_id, :match_id)
+  def match_params
+    params.require(:match).permit(:pet_id, :adopter_account_id)
   end
 
-  # set status on all applications for a pet
-  def set_statuses_to_adoption_made
-    @applications = Pet.find(params[:pet_id]).adopter_applications
-    @applications.each do |app|
-      unless app.status == "withdrawn"
-        app.status = "adoption_made"
-        app.save
-      end
-    end
+  def set_pet
+    @pet = Pet.find(match_params[:pet_id])
+  end
+
+  def set_match
+    @match = Match.find(params[:id])
   end
 
   def get_pet_id
-    return params[:pet_id] if params[:pet_id]
+    return match_params[:pet_id] if match_params[:pet_id]
 
-    Match.find(params[:match_id]).pet_id
+    Match.find(params[:id]).pet_id
   end
 
   # staff and pet in the same org?

app/models/adopter_application.rb

@@ -53,16 +53,20 @@ def self.any_applications_profile_show_true?(adopter_account_id)
     applications.any? { |app| app.profile_show == true }
   end
 
-  # set application status to withdrawn e.g. if reverting an adoption
-  def self.set_status_to_withdrawn(adopter_application)
-    adopter_application.status = :withdrawn
-    adopter_application.save
+  def self.retire_applications(pet_id:)
+    where(pet_id:).each do |adopter_application|
+      adopter_application.update!(status: :adoption_made)
+    end
   end
 
   def applicant_name
     "#{adopter_account.user.last_name}, #{adopter_account.user.first_name}"
   end
 
+  def withdraw
+    update!(status: :withdrawn)
+  end
+
   ransacker :applicant_name do
     Arel.sql("CONCAT(users.last_name, ', ', users.first_name)")
   end

app/models/match.rb

@@ -40,11 +40,23 @@ def assign_checklist_template(checklist_template)
     end
   end
 
+  def withdraw_application
+    adopter_application&.withdraw
+  end
+
+  def retire_applications(application_class: AdopterApplication)
+    application_class.retire_applications(pet_id: pet_id)
+  end
+
   private
 
   def belongs_to_same_organization_as_pet
     if organization_id != pet.organization_id
       errors.add(:organization_id, :different_organization)
     end
   end
+
+  def adopter_application
+    AdopterApplication.find_by(pet:, adopter_account:)
+  end
 end

app/views/organizations/pets/tabs/partials/_application_status.html.erb

@@ -28,11 +28,15 @@
   <td class="align-middle text-center col-3">
     <% if app.status == 'successful_applicant' %>
       <!--create adoption button-->
-      <%= button_to "New Adoption", create_adoption_path(pet_id: app.pet.id, 
-                              dopter_account_id: app.adopter_account.id),
-                              class: "btn btn-outline-primary",
-                              data: { turbo_method: :post, 
-                                      turbo_confirm: "Click OK to finalize this adoption." } %>
+      <%= button_to "New Adoption", matches_path, method: :post,
+        params: {match:
+          {pet_id: app.pet_id, adopter_account_id: app.adopter_account_id}
+        },
+        class: "btn btn-outline-primary",
+        data: { turbo_method: :post, 
+                turbo_confirm: "Click OK to finalize this adoption." 
+        }
+      %>
     <% else %>
       <span class="d-inline-block" tabindex="0"
                 data-bs-toggle="tooltip" title="Finalize adoption">

config/routes.rb

@@ -54,8 +54,7 @@
   resources :adopter_applications, path: "applications",
     only: %i[index create update]
 
-  post "create_adoption", to: "matches#create"
-  delete "revoke_adoption", to: "matches#delete"
+  resources :matches, only: %i[create destroy]
 
   get "/contacts", to: "contacts#create"
   get "/contacts/new", to: "contacts#new", as: "new_contact"

test/integration/adopter_application_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class AdopterApplicationTest < ActionDispatch::IntegrationTest
+class AdopterApplicationIntegrationTest < ActionDispatch::IntegrationTest
   setup do
     @organization = create(:organization)
     @pet_id = create(:pet, organization: @organization).id

test/models/adopter_application_test.rb

@@ -0,0 +1,48 @@
+require "test_helper"
+
+class AdopterApplicationTest < ActiveSupport::TestCase
+  setup do
+    @application = create(:adopter_application)
+  end
+
+  context "self.retire_applications" do
+    context "when some applications match pet_id and some do not" do
+      setup do
+        @selected_applications = Array.new(3) {
+          create(:adopter_application, pet_id: @application.pet_id)
+        }
+        @unselected_applications = Array.new(2) {
+          create(:adopter_application)
+        }
+      end
+
+      should "update status of matching applications to :adoption_made" do
+        AdopterApplication.retire_applications(pet_id: @application.pet_id)
+
+        @selected_applications.each do |application|
+          assert_equal "adoption_made", application.reload.status
+        end
+      end
+
+      should "not update status of mismatching applications" do
+        cached_statuses = @unselected_applications.map(&:status)
+
+        AdopterApplication.retire_applications(pet_id: @application.pet_id)
+
+        current_statuses = @unselected_applications.map do |application|
+          application.reload.status
+        end
+
+        assert_equal cached_statuses, current_statuses
+      end
+    end
+  end
+
+  context "#withdraw" do
+    should "update status to :withdrawn" do
+      @application.withdraw
+
+      assert "withdrawn", @application.status
+    end
+  end
+end

test/models/match_test.rb

@@ -7,4 +7,33 @@ class MatchTest < ActiveSupport::TestCase
     should belong_to(:pet)
     should belong_to(:adopter_account)
   end
+
+  setup do
+    @match = build_stubbed(:match)
+  end
+
+  context "#withdraw_application" do
+    setup do
+      @application = mock("adopter_application")
+    end
+
+    should "send #withdraw to application" do
+      @match.expects(:adopter_application).returns(@application)
+      @application.expects(:withdraw)
+
+      @match.withdraw_application
+    end
+  end
+
+  context "#retire_applications" do
+    setup do
+      @application_class = mock("AdopterApplication")
+    end
+
+    should "send #retire_applications with pet_id to application_class" do
+      @application_class.expects(:retire_applications).with(pet_id: @match.pet_id)
+
+      @match.retire_applications(application_class: @application_class)
+    end
+  end
 end