Fix signup count descriptions incorrectly including withdrawn/waitlisted signups

.rubocop_todo.yml

@@ -49,8 +49,9 @@ Metrics/BlockLength:
   Exclude:
     - 'config/environments/production.rb'
     - 'config/initializers/doorkeeper.rb'
+    - 'test/presenters/signup_count_presenter_test.rb'
 
-# Offense count: 5
+# Offense count: 7
 # Configuration parameters: CountComments, Max, CountAsOne.
 Metrics/ClassLength:
   Exclude:
@@ -59,6 +60,26 @@ Metrics/ClassLength:
     - 'app/graphql/types/ability_type.rb'
     - 'app/graphql/types/mutation_type.rb'
     - 'app/graphql/types/query_type.rb'
+    - 'app/presenters/signup_count_presenter.rb'
+    - 'test/presenters/signup_count_presenter_test.rb'
+
+# Offense count: 1
+# Configuration parameters: Max.
+Metrics/ParameterLists:
+  Exclude:
+    - 'app/presenters/signup_count_presenter.rb'
+
+# Offense count: 1
+# Configuration parameters: ForbiddenPrefixes, AllowedMethods, MethodDefinitionMacros.
+# ForbiddenPrefixes: is_, has_, have_
+Naming/PredicateMethod:
+  Exclude:
+    - 'app/presenters/signup_count_presenter.rb'
+
+# Offense count: 1
+Naming/PredicatePrefix:
+  Exclude:
+    - 'app/presenters/signup_count_presenter.rb'
 
 # Offense count: 1
 # This cop supports safe autocorrection (--autocorrect).

app/presenters/signup_count_presenter.rb

@@ -3,7 +3,7 @@ class SignupCountPresenter
   include SortBuckets
   include ActionView::Helpers::TextHelper
 
-  DIMENSIONS = %i[state bucket_key requested_bucket_key counted team_member]
+  DIMENSIONS = %i[state bucket_key requested_bucket_key counted team_member].freeze
 
   class SignupCountRow
     attr_reader :count, :state, :bucket_key, :requested_bucket_key, :counted, :team_member
@@ -62,7 +62,7 @@ def next_dimension
 
     def final?
       return @final unless @final.nil?
-      @final = dimension == DIMENSIONS[DIMENSIONS.size - 1]
+      @final = dimension == DIMENSIONS[-1]
     end
 
     def count(**filters)
@@ -112,9 +112,7 @@ def self.signup_count_data_for_runs(runs)
 
   def self.for_runs(runs)
     data_by_run_id = signup_count_data_for_runs(runs)
-    runs.each_with_object({}) do |run, hash|
-      hash[run.id] = new(run, count_data: data_by_run_id[run.id] || SignupCountData.new(DIMENSIONS[0], []))
-    end
+    runs.to_h { |run| [run.id, new(run, count_data: data_by_run_id[run.id] || SignupCountData.new(DIMENSIONS[0], []))] }
   end
 
   def initialize(run, count_data: nil)
@@ -143,22 +141,10 @@ def bucket_descriptions_text(state)
   end
 
   def bucket_descriptions(state)
-    counted_key = counted_key_for_state(state)
-
     if buckets.size == 1
-      [count_data.count(bucket_key: buckets.first.key, counted: counted_key).to_s]
+      single_bucket_descriptions(state)
     else
-      bucket_texts =
-        buckets.map do |bucket|
-          bucket_counted_key = counted_key
-          bucket_counted_key = :not_counted if bucket.not_counted?
-          "#{bucket.name}: #{count_data.count(bucket_key: bucket.key, counted: bucket_counted_key)}"
-        end
-
-      no_pref_count = count_data.count(requested_bucket_key: nil, counted: false)
-      bucket_texts << "No preference: #{no_pref_count}" if state == "waitlisted" && no_pref_count.positive?
-
-      bucket_texts
+      multi_bucket_descriptions(state)
     end
   end
 
@@ -210,6 +196,31 @@ def buckets
 
   private
 
+  def single_bucket_descriptions(state)
+    if state == "waitlisted"
+      [count_data.count(state: state).to_s]
+    else
+      [count_data.count(state: state, bucket_key: buckets.first.key, counted: counted_key_for_state(state)).to_s]
+    end
+  end
+
+  def multi_bucket_descriptions(state)
+    counted_key = counted_key_for_state(state)
+    texts = buckets.map { |bucket| per_bucket_description(bucket, state, counted_key) }
+    no_pref_count = count_data.count(state: state, requested_bucket_key: nil, counted: false)
+    texts << "No preference: #{no_pref_count}" if state == "waitlisted" && no_pref_count.positive?
+    texts
+  end
+
+  def per_bucket_description(bucket, state, counted_key)
+    if state == "waitlisted"
+      "#{bucket.name}: #{count_data.count(state: state, requested_bucket_key: bucket.key, counted: false)}"
+    else
+      bucket_counted_key = bucket.not_counted? ? :not_counted : counted_key
+      "#{bucket.name}: #{count_data.count(state: state, bucket_key: bucket.key, counted: bucket_counted_key)}"
+    end
+  end
+
   def registration_policy
     @registration_policy ||= run.registration_policy
   end

test/presenters/signup_count_presenter_test.rb

@@ -0,0 +1,125 @@
+# frozen_string_literal: true
+require "test_helper"
+
+class SignupCountPresenterTest < ActiveSupport::TestCase
+  let(:convention) { create(:convention) }
+
+  describe "with a single-bucket limited event" do
+    let(:registration_policy) do
+      RegistrationPolicy.new(buckets: [{ key: "attendees", name: "Attendees", slots_limited: true, total_slots: 10 }])
+    end
+    let(:event) { create(:event, convention:, registration_policy:) }
+    let(:the_run) { create(:run, event:) }
+    let(:presenter) { SignupCountPresenter.new(the_run) }
+
+    describe "#confirmed_count" do
+      it "counts confirmed signups" do
+        create(:signup, run: the_run)
+        create(:signup, run: the_run)
+        assert_equal 2, presenter.confirmed_count
+      end
+
+      it "does not count withdrawn signups" do
+        create(:signup, run: the_run)
+        create(:signup, run: the_run, state: "withdrawn", bucket_key: nil, counted: false)
+        assert_equal 1, presenter.confirmed_count
+      end
+    end
+
+    describe "#waitlist_count" do
+      it "counts waitlisted signups" do
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        assert_equal 2, presenter.waitlist_count
+      end
+
+      it "does not count withdrawn signups" do
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        create(:signup, run: the_run, state: "withdrawn", counted: false, requested_bucket_key: "attendees")
+        assert_equal 1, presenter.waitlist_count
+      end
+    end
+
+    describe "#has_waitlist?" do
+      it "returns true when there are waitlisted signups" do
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        assert presenter.has_waitlist?
+      end
+
+      it "returns false when there are no waitlisted signups" do
+        create(:signup, run: the_run)
+        assert_not presenter.has_waitlist?
+      end
+    end
+
+    describe "#signups_description" do
+      it "reports confirmed count" do
+        create(:signup, run: the_run)
+        create(:signup, run: the_run)
+        assert_equal "Signed up: 2", presenter.signups_description
+      end
+
+      it "does not count withdrawn signups as confirmed" do
+        create(:signup, run: the_run)
+        create(:signup, run: the_run, state: "withdrawn", bucket_key: nil, counted: false)
+        assert_equal "Signed up: 1", presenter.signups_description
+      end
+
+      it "reports waitlisted count when the waitlist is non-empty" do
+        create(:signup, run: the_run)
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "attendees")
+        assert_equal "Signed up: 1\nWaitlisted: 2", presenter.signups_description
+      end
+    end
+  end
+
+  describe "with a multi-bucket event" do
+    let(:registration_policy) do
+      RegistrationPolicy.new(
+        buckets: [
+          { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 5 },
+          { key: "cats", name: "Cats", slots_limited: true, total_slots: 5 },
+          { key: "flex", name: "Flex", slots_limited: true, total_slots: 5, anything: true }
+        ]
+      )
+    end
+    let(:event) { create(:event, convention:, registration_policy:) }
+    let(:the_run) { create(:run, event:) }
+    let(:presenter) { SignupCountPresenter.new(the_run) }
+
+    describe "#signups_description" do
+      it "shows per-bucket confirmed counts" do
+        create(:signup, run: the_run, bucket_key: "dogs", requested_bucket_key: "dogs")
+        create(:signup, run: the_run, bucket_key: "dogs", requested_bucket_key: "dogs")
+        create(:signup, run: the_run, bucket_key: "cats", requested_bucket_key: "cats")
+        description = presenter.signups_description
+        assert_includes description, "Dogs: 2"
+        assert_includes description, "Cats: 1"
+      end
+
+      it "does not count withdrawn signups in per-bucket confirmed counts" do
+        create(:signup, run: the_run, bucket_key: "dogs", requested_bucket_key: "dogs")
+        create(:signup, run: the_run, state: "withdrawn", bucket_key: nil, counted: false, requested_bucket_key: "dogs")
+        assert_includes presenter.signups_description, "Dogs: 1"
+      end
+
+      it "shows waitlisted count broken down by requested bucket" do
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "dogs")
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "dogs")
+        create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: "cats")
+        description = presenter.signups_description
+        assert_includes description, "Dogs: 2"
+        assert_includes description, "Cats: 1"
+      end
+
+      it "does not count withdrawn signups in the no-preference waitlist count" do
+        # 3 genuine no-preference waitlisted signups
+        3.times { create(:signup, run: the_run, state: "waitlisted", counted: false, requested_bucket_key: nil) }
+        # 1 withdrawn signup whose requested_bucket_key is nil (e.g. was confirmed via the flex bucket)
+        create(:signup, run: the_run, state: "withdrawn", counted: false, requested_bucket_key: nil)
+        assert_includes presenter.signups_description, "No preference: 3"
+      end
+    end
+  end
+end