FIX: properly delete expired event in calendar-based topics

approved

#1

FIX: properly delete expired event in calendar-based topics

The ‘EnsureExpiredEventDestruction’ is now only deleting events in topics with a [calendar].

REFACTOR: Remove all *_INDEX constants REFACTOR: UpdateHolidayUsernames job

PERF: Use already cooked version of the post instead of over-cooking it PERF: Only serialize ‘calendar_details’ on first post

diff --git a/jobs/scheduled/ensure_expired_event_destruction.rb b/jobs/scheduled/ensure_expired_event_destruction.rb
index feb90d0..dea1bdd 100644
--- a/jobs/scheduled/ensure_expired_event_destruction.rb
+++ b/jobs/scheduled/ensure_expired_event_destruction.rb
@@ -1,33 +1,38 @@
 module Jobs
   class ::DiscourseCalendar::EnsuredExpiredEventDestruction < Jobs::Scheduled
-    every 6.hours
+    every 1.hour
 
     def execute(args)
-      PostCustomField
-        .joins(:post)
-        .where("post_custom_fields.name = '#{::DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD}'")
-        .find_each do |pcf|
-          details = JSON.parse(pcf.value)
+      calendar_post_ids = PostCustomField
+        .where(name: ::DiscourseCalendar::CALENDAR_CUSTOM_FIELD)
+        .pluck(:post_id)
 
-          details.each do |post_number, detail|
-            return if detail[::DiscourseCalendar::RECURRING_INDEX]
+      calendar_topic_ids = Post
+        .joins(:topic)
+        .where(id: calendar_post_ids, post_number: 1)
+        .where("NOT topics.closed AND NOT topics.archived")
+        .pluck(:topic_id)
 
-            to = detail[::DiscourseCalendar::TO_INDEX] ||
-                 detail[::DiscourseCalendar::FROM_INDEX]
+      PostCustomField
+        .joins(post: :topic)
+        .includes(post: :topic)
+        .where("post_custom_fields.name = ?", ::DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD)
+        .where("topics.id IN (?)", calendar_topic_ids)
+        .find_each do |pcf|
 
-            to_time = Time.parse(to)
-            to_time += 24.hours unless detail[::DiscourseCalendar::TO_INDEX] # Add 24 hours if no explicit 'to' time
+        details = JSON.parse(pcf.value)
+        details.each do |post_number, (_, from, to, _, recurring)|
+          return if recurring
 
-            if (to_time + 1.hour) < Time.now.utc
-              topic = pcf.post.topic
-              post = topic.posts.find_by(post_number: post_number)
+          to_time = to ? Time.parse(to) : Time.parse(from) + 24.hours
 
-              if post
-                PostDestroyer.new(Discourse.system_user, post).destroy
-              end
+          if (to_time + 1.hour) < Time.zone.now
+            if post = pcf.post.topic.posts.find_by(post_number: post_number)
+              PostDestroyer.new(Discourse.system_user, post).destroy
             end
           end
         end
+      end
     end
   end
 end
diff --git a/jobs/scheduled/update_holiday_usernames.rb b/jobs/scheduled/update_holiday_usernames.rb
index e7a6c6e..4166a22 100644
--- a/jobs/scheduled/update_holiday_usernames.rb
+++ b/jobs/scheduled/update_holiday_usernames.rb
@@ -5,26 +5,18 @@ module Jobs
     PLUGIN_NAME ||= "calendar".freeze
 
     def execute(args)
-      topic_id = SiteSetting.holiday_calendar_topic_id
-      return if topic_id.blank?
+      return unless topic_id = SiteSetting.holiday_calendar_topic_id
+      return unless op = Post.find_by(topic_id: topic_id, post_number: 1)
+      return unless details = op.custom_fields[::DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD]
 
-      post_id = Post.find_by(topic_id: topic_id, post_number: 1)
-
-      # Build a list of discourse users currently on holiday
-      users_on_holiday = []
       user_ids = []
+      users_on_holiday = []
 
-      pcf = PostCustomField.find_by(name: ::DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD, post_id: post_id)
-      details = JSON.parse(pcf.value)
-      details.each do |post_number, detail|
-        from_time = Time.parse(detail[::DiscourseCalendar::FROM_INDEX])
-
-        to = detail[::DiscourseCalendar::TO_INDEX] || detail[::DiscourseCalendar::FROM_INDEX]
-        to_time = Time.parse(to)
-        to_time += 24.hours unless detail[::DiscourseCalendar::TO_INDEX] # Add 24 hours if no explicit 'to' time
+      details.values.each do |(_, from, to, username)|
+        from_time = Time.parse(from)
+        to_time   = to ? Time.parse(to) : from_time + 24.hours
 
-        if Time.zone.now > from_time && Time.zone.now < to_time
-          username = detail[::DiscourseCalendar::USERNAME_INDEX]
+        if from_time < Time.zone.now && Time.zone.now < to_time
           users_on_holiday << username
 
           user = User.find_by(username_lower: username)
@@ -36,8 +28,6 @@ module Jobs
 
       PluginStore.set(PLUGIN_NAME, DiscourseCalendar::USERS_ON_HOLIDAY_KEY, users_on_holiday)
       UserCustomField.where(name: ::DiscourseCalendar::HOLIDAY_CUSTOM_FIELD).where.not(user_id: user_ids).destroy_all
-
-      # puts "Users on holiday are #{users_on_holiday}"
     end
   end
 end
diff --git a/lib/calendar_validator.rb b/lib/calendar_validator.rb
index 4937933..69f13e3 100644
--- a/lib/calendar_validator.rb
+++ b/lib/calendar_validator.rb
@@ -7,9 +7,7 @@ module DiscourseCalendar
     def validate_calendar
       extracted_calendars = DiscourseCalendar::Calendar::extract(@post)
 
-      # if no calendar we do nothing
       return false if extracted_calendars.count == 0
-
       return false if more_than_one_calendar(extracted_calendars.count)
       return false if !calendar_in_first_post(@post.is_first_post?)
 
diff --git a/lib/event_updater.rb b/lib/event_updater.rb
index bbeca94..b5e455c 100644
--- a/lib/event_updater.rb
+++ b/lib/event_updater.rb
@@ -20,15 +20,16 @@ module DiscourseCalendar
 
       html = post.cooked
       doc = Nokogiri::HTML::fragment(html)
-      doc.css(".discourse-local-date").each { |span| span.remove }
+      doc.css(".discourse-local-date").each(&:remove)
       html = (doc.try(:to_html) || html).sub(' → ', '')
 
-      detail = []
-      detail[DiscourseCalendar::MESSAGE_INDEX] = PrettyText.excerpt(html, 30, strip_links: true, text_entities: true)
-      detail[DiscourseCalendar::USERNAME_INDEX] = post.user.username_lower
-      detail[DiscourseCalendar::FROM_INDEX] = from.iso8601.to_s
-      detail[DiscourseCalendar::TO_INDEX] = to.iso8601.to_s if to
-      detail[DiscourseCalendar::RECURRING_INDEX] = dates[0]['recurring'] if dates[0]['recurring']
+      detail = [
+        PrettyText.excerpt(html, 30, strip_links: true, text_entities: true),
+        post.user.username_lower,
+        from.iso8601.to_s,
+        to ? to.iso8601.to_s : nil,
+        dates[0]["recurring"].presence
+      ]
 
       op.set_calendar_event(post.post_number, detail)
       op.save_custom_fields(true)
diff --git a/plugin.rb b/plugin.rb
index f718561..1edfe7c 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -20,12 +20,6 @@ after_initialize do
     HOLIDAY_CUSTOM_FIELD ||= "on_holiday"
     USERS_ON_HOLIDAY_KEY ||= "users_on_holiday"
 
-    MESSAGE_INDEX = 0
-    FROM_INDEX = 1
-    TO_INDEX = 2
-    USERNAME_INDEX = 3
-    RECURRING_INDEX = 4
-
     def self.users_on_holiday
       PluginStore.get(PLUGIN_NAME, USERS_ON_HOLIDAY_KEY)
     end
@@ -50,9 +44,7 @@ after_initialize do
   class DiscourseCalendar::Calendar
     class << self
       def extract(post)
-        cooked = PrettyText.cook(post.raw, topic_id: post.topic_id, user_id: post.user_id)
-
-        Nokogiri::HTML(cooked).css('div.calendar').map do |cooked_calendar|
+        Nokogiri::HTML(post.cooked).css('div.calendar').map do |cooked_calendar|
           calendar = {}
 
           cooked_calendar.attributes.values.each do |attribute|
@@ -70,8 +62,7 @@ after_initialize do
   class DiscourseCalendar::Event
     class << self
       def count(post)
-        cooked = PrettyText.cook(post.raw, topic_id: post.topic_id, user_id: post.user_id)
-        Nokogiri::HTML(cooked).css('span.discourse-local-date').count
+        Nokogiri::HTML(post.cooked).css('span.discourse-local-date').count
       end
     end
   end
@@ -140,39 +131,25 @@ after_initialize do
 
   TopicView.default_post_custom_fields << DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD
 
-  require_dependency "post_serializer"
-  class ::PostSerializer
-    attributes :calendar_details
-
-    def calendar_details
-      return nil unless object.is_first_post?
-      details = post_custom_fields[DiscourseCalendar::CALENDAR_DETAILS_CUSTOM_FIELD]
-

[... diff too long, it was truncated ...]

GitHub sha: d3dbe130


#2

@jjaffeux can you review?


#3

Yes already looked the other day. We talked of it with regis. LGTM.


Approved #4