FIX: Expired/non-expired events logic (#165)

FIX: Expired/non-expired events logic (#165)

Previously, an edited event could be returned in both expired and non-expired event queries.

diff --git a/app/models/discourse_post_event/event.rb b/app/models/discourse_post_event/event.rb
index d3976ca..17fcf7e 100644
--- a/app/models/discourse_post_event/event.rb
+++ b/app/models/discourse_post_event/event.rb
@@ -44,9 +44,12 @@ module DiscoursePostEvent
 
       event_dates.create!(
         starts_at: next_dates[:starts_at],
-        ends_at: next_dates[:ends_at],
-        finished_at: (next_dates[:starts_at] < Time.current) && next_dates[:starts_at]
-      )
+        ends_at: next_dates[:ends_at]
+      ) do |event_date|
+        if next_dates[:ends_at] && next_dates[:ends_at] < Time.current
+          event_date.finished_at = next_dates[:ends_at]
+        end
+      end
 
       publish_update!
       invitees.update_all(status: nil, notified: false)
diff --git a/lib/discourse_post_event/event_finder.rb b/lib/discourse_post_event/event_finder.rb
index 9240ecc..eb43b7f 100644
--- a/lib/discourse_post_event/event_finder.rb
+++ b/lib/discourse_post_event/event_finder.rb
@@ -7,23 +7,27 @@ module DiscoursePostEvent
       topics = listable_topics(guardian)
       pms = private_messages(user)
 
+      events = DiscoursePostEvent::Event
+        .select("discourse_post_event_events.*, dcped.starts_at")
+        .joins(post: :topic)
+        .merge(Post.secured(guardian))
+        .merge(topics.or(pms).distinct)
+        .joins("LEFT JOIN discourse_calendar_post_event_dates dcped ON dcped.event_id = discourse_post_event_events.id")
+        .order("dcped.starts_at ASC")
+
       if params[:expired]
-        event_ids = DiscoursePostEvent::EventDate.expired.order(starts_at: :asc).pluck(:event_id)
+        # The second part below makes the query ignore events that have non-expired event-dates
+        events = events
+          .where("dcped.finished_at IS NOT NULL AND (dcped.ends_at IS NOT NULL AND dcped.ends_at < ?)", Time.now)
+          .where("discourse_post_event_events.id NOT IN (SELECT DISTINCT event_id FROM discourse_calendar_post_event_dates WHERE event_id = discourse_post_event_events.id AND finished_at IS NULL)")
       else
-        event_ids = DiscoursePostEvent::EventDate.not_expired.order(starts_at: :asc).pluck(:event_id)
+        events = events.where("dcped.finished_at IS NULL AND (dcped.ends_at IS NULL OR dcped.ends_at > ?)", Time.now)
       end
 
-      events = DiscoursePostEvent::Event.where(id: event_ids)
-
       if params[:post_id]
         events = events.where(id: Array(params[:post_id]))
       end
 
-      events = events.joins(post: :topic)
-        .merge(Post.secured(guardian))
-        .merge(topics.or(pms).distinct)
-        .joins("LEFT JOIN discourse_calendar_post_event_dates dcped ON dcped.event_id = discourse_post_event_events.id")
-
       if params[:category_id].present?
         if params[:include_subcategories].present?
           events = events.where(topics: { category_id: Category.subcategory_ids(params[:category_id].to_i) })
diff --git a/spec/fabricators/event_fabricator.rb b/spec/fabricators/event_fabricator.rb
index 0e092f2..fab1d33 100644
--- a/spec/fabricators/event_fabricator.rb
+++ b/spec/fabricators/event_fabricator.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 Fabricator(:event, from: 'DiscoursePostEvent::Event') do
-  post { |attrs| attrs[:post] }
+  post { |attrs| attrs[:post] || Fabricate(:post) }
 
   id { |attrs| attrs[:post].id }
 
diff --git a/spec/lib/discourse_post_event/event_finder_spec.rb b/spec/lib/discourse_post_event/event_finder_spec.rb
index 3f7f3d2..16821b3 100644
--- a/spec/lib/discourse_post_event/event_finder_spec.rb
+++ b/spec/lib/discourse_post_event/event_finder_spec.rb
@@ -89,29 +89,32 @@ describe DiscoursePostEvent::EventFinder do
     end
 
     context 'by expiration status' do
-      let(:post1) {
-        PostCreator.create!(
-          user,
-          title: 'We should buy a boat',
-          raw: 'The boat market is quite active lately.'
-        )
-      }
-      let(:post2) {
-        PostCreator.create!(
-          user,
-          title: 'We should buy another boat',
-          raw: 'The boat market is very active lately.'
-        )
-      }
-      let!(:event1) { Fabricate(:event, post: post1, original_starts_at: 2.hours.ago, original_ends_at: 1.hour.ago) }
-      let!(:event2) { Fabricate(:event, post: post2, original_starts_at: 1.hour.from_now, original_ends_at: 2.hours.from_now) }
+      let!(:old_event) { Fabricate(:event, name: 'old_event', original_starts_at: 2.hours.ago, original_ends_at: 1.hour.ago) }
+      let!(:future_event) { Fabricate(:event, name: 'future_event', original_starts_at: 1.hour.from_now, original_ends_at: 2.hours.from_now) }
+      let!(:current_event) { Fabricate(:event, name: 'current_event', original_starts_at: 5.minutes.ago, original_ends_at: 5.minutes.from_now) }
+      let!(:older_event) { Fabricate(:event, name: 'older_event', original_starts_at: 4.hours.ago, original_ends_at: 3.hour.ago) }
 
-      it 'returns non-expired events when false' do
-        expect(subject.search(current_user, { expired: false })).to match_array([event2])
+      it 'returns correct events' do
+        expect(subject.search(current_user, { expired: false })).to eq([current_event, future_event])
+        expect(subject.search(current_user, { expired: true })).to eq([older_event, old_event])
       end
 
-      it 'returns expired events when true' do
-        expect(subject.search(current_user, { expired: true })).to match_array([event1])
+      context 'when a past event has been edited to be in the future' do
+        let!(:event_date) { Fabricate(:event_date, event: future_event, starts_at: 2.hours.ago, ends_at: 1.hour.ago, finished_at: 1.hour.ago) }
+
+        it 'returns correct events' do
+          expect(subject.search(current_user, { expired: false })).to eq([current_event, future_event])
+          expect(subject.search(current_user, { expired: true })).to eq([older_event, old_event])
+        end
+      end
+
+      context 'when a future event has been edited to be in the past' do
+        let!(:event_date) { Fabricate(:event_date, event: old_event, starts_at: 1.hour.from_now, ends_at: 2.hours.from_now, finished_at: 1.hour.ago) }
+
+        it 'returns correct events' do
+          expect(subject.search(current_user, { expired: false })).to eq([current_event, future_event])
+          expect(subject.search(current_user, { expired: true })).to eq([older_event, old_event])
+        end
       end
     end
   end

GitHub sha: 4cc79a2543c3c5a0db4c5c87e6c23667b8fb1c39

This commit appears in #165 which was approved by jjaffeux. It was merged by CvX.