From f93575d886002e51c1ee322363ff799c96488c4e Mon Sep 17 00:00:00 2001
From: Matthew Rider <mattwr18@gmail.com>
Date: Tue, 19 Nov 2024 12:06:02 +0100
Subject: [PATCH] [fix] Scope tag counts to active contributors (#2070)

Fixes #2069
---
 .../organizations/contributors_controller.rb     |  2 +-
 app/models/organization.rb                       |  2 +-
 spec/models/organization_spec.rb                 | 15 +++++++++++++++
 spec/requests/contributors_spec.rb               | 16 +++++++++++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/app/controllers/organizations/contributors_controller.rb b/app/controllers/organizations/contributors_controller.rb
index c6befa1e6..5872aeffb 100644
--- a/app/controllers/organizations/contributors_controller.rb
+++ b/app/controllers/organizations/contributors_controller.rb
@@ -49,7 +49,7 @@ def destroy
     end
 
     def count
-      render json: { count: @organization.contributors.with_tags(params[:tag_list]).count }
+      render json: { count: @organization.contributors.active.with_tags(params[:tag_list]).count }
     end
 
     def conversations
diff --git a/app/models/organization.rb b/app/models/organization.rb
index a60d926bf..0a8ec781c 100644
--- a/app/models/organization.rb
+++ b/app/models/organization.rb
@@ -95,7 +95,7 @@ def contributors_tags_with_count
     ActsAsTaggableOn::Tag
       .for_tenant(id)
       .joins(:taggings)
-      .where(taggings: { taggable_type: Contributor.name })
+      .where(taggings: { taggable_type: Contributor.name, taggable_id: Contributor.active })
       .select('tags.id, tags.name, count(taggings.id) as taggings_count')
       .group('tags.id')
       .all
diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb
index adeb4a4ea..8f6c3b333 100644
--- a/spec/models/organization_spec.rb
+++ b/spec/models/organization_spec.rb
@@ -87,6 +87,10 @@
   describe '#contributors_tags_with_count' do
     subject { organization.contributors_tags_with_count.pluck(:name, :count) }
 
+    it 'makes one database query' do
+      expect { subject }.to make_database_queries(count: 1)
+    end
+
     context 'given a contributor with a tag' do
       let!(:contributor) { create(:contributor, tag_list: %w[Homeowner], organization: organization) }
       it { should eq([['Homeowner', 1]]) }
@@ -95,6 +99,17 @@
         let!(:request) { create(:request, tag_list: %w[Homeowner], organization: organization) }
         it { should eq([['Homeowner', 1]]) }
       end
+
+      context 'given non-active contributors with the same tag' do
+        before do
+          create(:contributor, tag_list: %w[Homeowner], deactivated_at: 1.day.ago, organization: organization)
+          create(:contributor, tag_list: 'teacher', unsubscribed_at: 1.day.ago, organization: organization)
+        end
+
+        it "does not count inactive contributor's tags" do
+          expect(subject).to eq([['Homeowner', 1]])
+        end
+      end
     end
   end
 end
diff --git a/spec/requests/contributors_spec.rb b/spec/requests/contributors_spec.rb
index ac0b37761..b9844713b 100644
--- a/spec/requests/contributors_spec.rb
+++ b/spec/requests/contributors_spec.rb
@@ -55,13 +55,27 @@
   end
 
   describe 'GET /count' do
+    subject { -> { get count_organization_contributors_path(organization, tag_list: ['teacher'], as: user) } }
+
     let!(:teachers) { create_list(:contributor, 2, tag_list: 'teacher', organization: organization) }
     let!(:other_teachers) { create_list(:contributor, 2, tag_list: 'teacher') }
 
     it 'returns count of contributors with a specific tag within the organization' do
-      get count_organization_contributors_path(organization, tag_list: ['teacher'], as: user)
+      subject.call
       expect(response.body).to eq({ count: 2 }.to_json)
     end
+
+    context 'given non-active contributors' do
+      before do
+        create(:contributor, tag_list: 'teacher', deactivated_at: 1.day.ago, organization: organization)
+        create(:contributor, tag_list: 'teacher', unsubscribed_at: 1.day.ago, organization: organization)
+        subject.call
+      end
+
+      it 'returns the count of contributors with a specific tag for active contributors' do
+        expect(response.body).to eq({ count: 2 }.to_json)
+      end
+    end
   end
 
   describe 'GET /conversations' do