From 16b83d726a3f6ebd339ee6ac2fa100df3b39fbbb Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Wed, 30 Oct 2024 12:57:26 +0000
Subject: [PATCH] Fix upcoming event listing on community page

---
 app/controllers/communities_controller.rb |  7 +++--
 app/models/event.rb                       | 21 +++++++++++++
 test/models/event_test.rb                 | 37 +++++++++++++++++++++++
 3 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/app/controllers/communities_controller.rb b/app/controllers/communities_controller.rb
index 3a5464d72..db5182704 100644
--- a/app/controllers/communities_controller.rb
+++ b/app/controllers/communities_controller.rb
@@ -46,17 +46,18 @@ def set_latest_materials
   def set_upcoming_events
     n_events = 5
 
-    Event.search_and_filter(
+    events = Event.search_and_filter(
       nil,
       '',
       {
-        'include_expired' => 'true',
         'start' => "#{10.years.ago.beginning_of_day}/",
         **@community.filters
       },
       sort_by: 'early',
       per_page: 5 * n_events
-    ).results.group_by(&:content_provider_id).map { |_p_id, p_events| p_events.first }.first(n_events)
+    ).results
+
+    @events = Event.from_varied_providers(events, n_events).sort_by(&:created_at).reverse
   end
 
   def set_community
diff --git a/app/models/event.rb b/app/models/event.rb
index fe2a879a5..adaaae00d 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -464,6 +464,27 @@ def online=(value)
     self.presence = value
   end
 
+  # Method to ensure a good spread of events by picking one from each content provider until the limit hit,
+  # or no more events in set.
+  def self.from_varied_providers(events, count)
+    provider_events = events.group_by(&:content_provider_id)
+
+    events = []
+    events_left = true
+    while events_left && (events.length < count) do
+      events_left = false
+      provider_events.each_value do |p_events|
+        if p_events.any?
+          events << p_events.shift
+          break if events.length == count
+          events_left ||= p_events.any?
+        end
+      end
+    end
+
+    events
+  end
+
   private
 
   def allowed_url
diff --git a/test/models/event_test.rb b/test/models/event_test.rb
index f368737ec..8c3957b46 100644
--- a/test/models/event_test.rb
+++ b/test/models/event_test.rb
@@ -708,4 +708,41 @@ class EventTest < ActiveSupport::TestCase
       e.destroy!
     end
   end
+
+  test 'from_varied_providers' do
+    user = users(:regular_user)
+    provider_1 = content_providers(:goblet)
+    provider_2 = content_providers(:iann)
+    provider_3 = content_providers(:two)
+    e1a = provider_1.events.create!(title: 'Event1a', url: 'https://example.com/events/1a', user: user)
+    e1b = provider_1.events.create!(title: 'Event1b', url: 'https://example.com/events/1b', user: user)
+    e1c = provider_1.events.create!(title: 'Event1c', url: 'https://example.com/events/1c', user: user)
+    e2a = provider_2.events.create!(title: 'Event2a', url: 'https://example.com/events/2a', user: user)
+    e2b = provider_2.events.create!(title: 'Event2b', url: 'https://example.com/events/2b', user: user)
+    e2c = provider_2.events.create!(title: 'Event2c', url: 'https://example.com/events/2c', user: user)
+    e3a = provider_3.events.create!(title: 'Event3a', url: 'https://example.com/events/3a', user: user)
+    e3b = provider_3.events.create!(title: 'Event3b', url: 'https://example.com/events/3b', user: user)
+    e3c = provider_3.events.create!(title: 'Event3c', url: 'https://example.com/events/3c', user: user)
+
+    even_single_mix = Event.from_varied_providers([e1a, e1b, e1c, e2a, e2b, e2c, e3a, e3b, e3c], 3)
+    assert_equal 3, even_single_mix.length
+    assert_equal [e1a, e2a, e3a], even_single_mix, 'Should go through each provider and pick an event'
+
+    even_multi_mix = Event.from_varied_providers([e1a, e1b, e1c, e2a, e2b, e2c, e3a, e3b, e3c], 6)
+    assert_equal 6, even_multi_mix.length
+    assert_equal [e1a, e2a, e3a, e1b, e2b, e3b], even_multi_mix,
+                 'Should go through each provider and pick an event, several times until quota reached'
+
+    skewed_mix = Event.from_varied_providers([e1a, e1b, e1c, e2a], 3)
+    assert_equal 3, skewed_mix.length
+    assert_equal [e1a, e2a, e1b], skewed_mix, 'Should take multiple events from the same provider if needed'
+
+    best_effort_mix = Event.from_varied_providers([e1a, e1b, e1c], 10)
+    assert_equal 3, best_effort_mix.length
+    assert_equal [e1a, e1b, e1c], best_effort_mix,
+                 'Should take all the events from the one provider since that was all that was available'
+
+    empty_mix = Event.from_varied_providers([], 10)
+    assert_empty empty_mix
+  end
 end