dm-4202 edit practice scopes to use counter cache (#710)

* add specs for `DiffusionHistory` scopes

* remove extra scope from queries in `cached_json_practices` method in `Practice` model

edit `:get_with_categories_and_adoptions_ct`, fixes to use fewer joins and chained scopes, now returns array for aggregate `category_names`

* add spec for `:with_categories_and_adoptions_ct` scope`

* working commit

* update spec

* dm-4203 add factory bot and model scope specs (#708)

* add `faker` and `factory_bot` gems

* add `faker` and `factory_bot` to `RSpec.configure`

* add `:visn` and `:va_facility` factories

* add specs for `VaFacility` scopes

* add factories

* add specs for `DiffusionHistory` scopes

* spacing

* moves `faker` gem to `:development, :test` block in gemfile

* edits `va_facilities` factory

* updates github actions to drop and create db before loading schema to ensure no unique constraints violations

* updates `:visns` factory

* updates specs

* add spec for `:with_categories_and_adoptions_ct` scope`

* edit `:with_categories_and_adoptions_ct` scope to combine `:with_categories` scope

* spacing

* edit `counter_cache` spec to utilize factories

* working commit

* edits scopes specs to be more dry and robust

* typo

* spacing

* remove extra joins in `:with_categories_and_adoptions_ct` scope

remove errant commented line

* edit `:with_categories_and_adoptions_ct` scope to use left outer joins

* spacing

* edit `:with_categories_and_adoptions_ct` scope to use left outer joins

* Remove crh created practices endpoint (#709)

* add factories

* spacing

* remove `javascript_include_tag` for `clinical_resource_hubs/crh_show` js from crh show view

* remove `clinical_resource_hubs/crh_show.es6` file

* remove `#created-crh-practices` endpoint and route

* add specs for `:get_by_created_crh` and `:get_by_adopted_crh` scopes

* remove `get_crh_created_innovations` method from `ClinicalResourceHub` model

* remove `clinical_resource_hubs/crh_show.js` from rails assets precompile list

* spacing

* add specs for `Practice.get_by_created_crh` and `Practice.get_by_adopted_crh` scopes

* update `:visns` factory

* update `:va_facility` factory

* update `:diffusion_history_statuses` factory to have default status

* add note to `diffusion_histories` factory

* remove deleted action from `before_action` in `ClinicalResourceHubsController`

* remove errant pry

* typo fix

app/models/clinical_resource_hub.rb

@@ -49,4 +49,4 @@ def get_crh_created_practices(crh_id, options = { is_user_guest: true })
     options[:is_user_guest] ? Practice.public_facing.load_associations.get_by_created_crh(crh_id) :
         Practice.published_enabled_approved.load_associations.get_by_created_crh(crh_id)
   end
-end
+end

app/models/practice.rb

@@ -34,12 +34,12 @@ class Practice < ApplicationRecord
   def self.cached_json_practices(is_guest_user)
     if is_guest_user
       Rails.cache.fetch('searchable_public_practices_json', expires_in: 30.minutes) do
-        practices = Practice.published_enabled_approved.includes(:practice_origin_facilities).public_facing.sort_by_retired.get_with_categories_and_adoptions_ct
+        practices = Practice.published_enabled_approved.includes(:practice_origin_facilities).public_facing.sort_by_retired
         practices_json(practices)
       end
     else
       Rails.cache.fetch('searchable_practices_json', expires_in: 30.minutes) do
-        practices = Practice.published_enabled_approved.includes(:practice_origin_facilities).sort_by_retired.get_with_categories_and_adoptions_ct
+        practices = Practice.published_enabled_approved.includes(:practice_origin_facilities).sort_by_retired
         practices_json(practices)
       end
     end
@@ -182,12 +182,19 @@ def highlight_attachment_s3_presigned_url(style = nil)
   scope :published,   -> { where(published: true) }
   scope :unpublished,  -> { where(published: false) }
   scope :get_practice_owner_emails, -> {where.not(user_id: nil)}
-  scope :get_with_category_names, -> { left_outer_joins(:categories).select("practices.*, categories.id as category_ids, categories.name as category_names") }
-  scope :get_with_adoptions_ct, -> { left_outer_joins(:diffusion_histories).select("practices.*, COUNT(diffusion_histories.*) as adoption_count") }
-  scope :with_categories_and_adoptions_ct, -> { published_enabled_approved.get_with_adoptions_ct.get_with_category_names }
-  scope :get_with_categories_and_adoptions_ct, -> { with_categories_and_adoptions_ct.group("practices.id, categories.id").uniq }
+  scope :with_categories_and_adoptions_ct, -> {
+    published_enabled_approved
+      .left_outer_joins(:categories)
+      .left_outer_joins(:diffusion_histories)
+      .group("practices.id")
+      .select(
+      "practices.*, " +
+      "ARRAY_AGG(DISTINCT categories.name) category_names, " +
+      "practices.diffusion_histories_count as adoption_count"
+    )
+  }
   scope :sort_a_to_z, -> { order(Arel.sql("lower(practices.name) ASC")) }
-  scope :sort_adoptions_ct, -> { order(Arel.sql("COUNT(diffusion_histories) DESC, lower(practices.name) ASC")) }
+  scope :sort_adoptions_ct, -> { order(Arel.sql("diffusion_histories_count DESC, lower(practices.name) ASC")) }
   scope :sort_added, -> { order(Arel.sql("practices.created_at DESC")) }
   scope :filter_by_category_ids, -> (cat_ids) { where('category_practices.category_id IN (?)', cat_ids)} # cat_ids should be a id number or an array of id numbers
   scope :published_enabled_approved, -> { where(published: true, enabled: true, approved: true, hidden: false) }

spec/factories/categories.rb

@@ -0,0 +1,5 @@
+FactoryBot.define do
+  factory :category do
+    name { Faker::Company.unique.name }
+  end
+end

spec/factories/category_practices.rb

@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :category_practice do
+    association :practice
+    association :category
+  end
+end

spec/models/practice_spec.rb

@@ -54,43 +54,114 @@
   end
 
   describe 'counter_cache' do
-    before do
-      user = User.create!(
-        email: '[email protected]',
-        password: 'Password123',
-        password_confirmation: 'Password123',
-      )
-      @practice = Practice.create!(
-        name: 'A public practice',
-        slug: 'a-public-practice',
-        main_display_image: File.new(File.join(Rails.root, '/spec/assets/charmander.png')),
-        user: user
-      )
-      visn_1 = create(:visn)
-      @fac_1 = VaFacility.create!(
-        visn: visn_1,
-        station_number: "402GA",
-        official_station_name: "Caribou VA Clinic",
-        common_name: "Caribou",
-        latitude: "44.2802701",
-        longitude: "-69.70413586",
-        street_address_state: "ME",
-        station_phone_number: "207-623-2123 x",
-        fy17_parent_station_complexity_level: "1c-High Complexity"
-      )
-    end
-
+    let(:practice) { create(:practice) }
+    let(:fac_1) { create(:va_facility) }
+
     it "increments counter cache on create" do
       expect {
-        @practice.diffusion_histories.create!(practice: @practice, va_facility: @fac_1)
-      }.to change { @practice.reload.diffusion_histories_count }.by(1)
+        practice.diffusion_histories.create!(practice: practice, va_facility: fac_1)
+      }.to change { practice.reload.diffusion_histories_count }.by(1)
     end
 
     it "decrements counter cache on destroy" do
-      diffusion_history = @practice.diffusion_histories.create!(practice: @practice, va_facility: @fac_1)
+      diffusion_history = practice.diffusion_histories.create!(practice: practice, va_facility: fac_1)
       expect {
         diffusion_history.destroy
-      }.to change { @practice.reload.diffusion_histories_count }.by(-1)
+      }.to change { practice.reload.diffusion_histories_count }.by(-1)
+    end
+  end
+
+  describe 'scopes' do
+    let!(:clinical_resource_hub_1) { create(:clinical_resource_hub) }
+    let!(:clinical_resource_hub_2) { create(:clinical_resource_hub) }
+    let!(:clinical_resource_hub_3) { create(:clinical_resource_hub) }
+
+    let!(:category1) { create(:category, name: "Category 1") }
+    let!(:category2) { create(:category, name: "Category 2") }
+    let!(:category3) { create(:category, name: "Category 3") }
+    let!(:category4) { create(:category, name: "Category 4") }
+
+    let!(:practice1) { create(:practice, name: "A", published: true, enabled: true, approved: true, hidden: false) }
+    let!(:practice2) { create(:practice, name: "D", published: true, enabled: true, approved: true, hidden: false) }
+    let!(:practice3) { create(:practice, name: "C", published: false, enabled: false, approved: false, hidden: true) }
+    let!(:practice4) { create(:practice, name: "B", published: true, enabled: true, approved: true, hidden: false) }
+
+
+    before do
+      create_list(:diffusion_history, 3, :with_va_facility, practice: practice1)
+      create_list(:diffusion_history, 2, :with_va_facility, practice: practice2)
+      create_list(:diffusion_history, 2, :with_va_facility, practice: practice4)
+
+      create(:category_practice, practice: practice1, category: category1)
+      create(:category_practice, practice: practice1, category: category2)
+      create(:category_practice, practice: practice2, category: category3)
+      create(:category_practice, practice: practice3, category: category4)
+      create(:category_practice, practice: practice4, category: category1)
+      create(:category_practice, practice: practice4, category: category3)
+    end
+
+    describe '.get_by_created_crh' do
+      before do
+        create(:practice_origin_facility, practice: practice1, clinical_resource_hub: clinical_resource_hub_1)
+        create(:practice_origin_facility, practice: practice2, clinical_resource_hub: clinical_resource_hub_2)
+        create(:practice_origin_facility, practice: practice3, clinical_resource_hub: clinical_resource_hub_3)
+      end
+
+      it 'returns practices that are published, enabled, approved, associated with provided clinical_resource_hub_id, and have loaded associations' do
+
+        result = Practice.published_enabled_approved.load_associations.get_by_created_crh(clinical_resource_hub_1.id)
+
+        expect(result).to include(practice1)
+        expect(result).not_to include(practice2, practice3, practice4)
+      end
+    end
+
+    describe '.get_by_adopted_crh' do
+      before do
+        create(:diffusion_history, practice: practice1, clinical_resource_hub: clinical_resource_hub_1)
+        create(:diffusion_history, practice: practice2, clinical_resource_hub: clinical_resource_hub_2)
+        create(:diffusion_history, practice: practice3, clinical_resource_hub: clinical_resource_hub_3)
+      end
+
+      it 'returns practices that are published, enabled, approved, adopted by the given clinical_resource_hub_id, and have loaded associations' do
+        result = Practice.published_enabled_approved.load_associations.get_by_adopted_crh(clinical_resource_hub_1.id)
+
+        expect(result).to include(practice1)
+        expect(result).not_to include(practice2, practice3, practice4)
+      end
+    end
+
+    describe '.with_categories_and_adoptions_ct' do
+      it 'returns practices with associated category names and adoptions count' do
+        result = Practice.with_categories_and_adoptions_ct.to_a
+
+        expect(result.count).to eq(3)
+
+        practice_with_counts_1 = result.find { |p| p.id == practice1.id }
+        expect(practice_with_counts_1.adoption_count).to eq(3)
+        expect(practice_with_counts_1.category_names).to include('Category 1', 'Category 2')
+
+        practice_with_counts_4 = result.find { |p| p.id == practice4.id }
+        expect(practice_with_counts_4.adoption_count).to eq(2)
+        expect(practice_with_counts_4.category_names).to include('Category 1', 'Category 3')
+
+        practice_with_counts_2 = result.find { |p| p.id == practice2.id }
+        expect(practice_with_counts_2.adoption_count).to eq(2)
+        expect(practice_with_counts_2.category_names).to include('Category 3')
+      end
+    end
+
+    describe '.sort_adoptions_ct' do
+      it 'orders practices by the count of their adoptions and then by name' do
+        # mimicks the chaining of scopes applied in #search_practices()
+        sorted_practices = Practice.with_categories_and_adoptions_ct.left_outer_joins(:practice_origin_facilities).sort_adoptions_ct.group("practices.id, categories.id, practice_origin_facilities.id").uniq
+
+        # practice4 and 2 have equal dh counts but 4 is named "B" and 2 is named "D"
+        expect(sorted_practices[0]).to eq(practice1)
+        expect(sorted_practices[1]).to eq(practice4)
+        expect(sorted_practices[2]).to eq(practice2)
+        expect(sorted_practices).not_to include(practice3)
+      end
     end
   end