Fix expired announcements being re-published (#12964)

app/models/announcement.rb

@@ -13,13 +13,14 @@
 #  ends_at      :datetime
 #  created_at   :datetime         not null
 #  updated_at   :datetime         not null
+#  published_at :datetime
 #
 
 class Announcement < ApplicationRecord
   scope :unpublished, -> { where(published: false) }
   scope :published, -> { where(published: true) }
   scope :without_muted, ->(account) { joins("LEFT OUTER JOIN announcement_mutes ON announcement_mutes.announcement_id = announcements.id AND announcement_mutes.account_id = #{account.id}").where('announcement_mutes.id IS NULL') }
-  scope :chronological, -> { order(Arel.sql('COALESCE(announcements.starts_at, announcements.scheduled_at, announcements.created_at) ASC')) }
+  scope :chronological, -> { order(Arel.sql('COALESCE(announcements.starts_at, announcements.scheduled_at, announcements.published_at, announcements.created_at) ASC')) }
 
   has_many :announcement_mutes, dependent: :destroy
   has_many :announcement_reactions, dependent: :destroy
@@ -31,10 +32,6 @@ class Announcement < ApplicationRecord
   before_validation :set_all_day
   before_validation :set_published, on: :create
 
-  def published_at
-    scheduled_at || created_at
-  end
-
   def time_range?
     starts_at.present? && ends_at.present?
   end
@@ -73,6 +70,9 @@ class Announcement < ApplicationRecord
   end
 
   def set_published
-    self.published = true if scheduled_at.blank? || scheduled_at.past?
+    return unless scheduled_at.blank? || scheduled_at.past?
+
+    self.published = true
+    self.published_at = Time.now.utc
   end
 end

app/workers/publish_scheduled_announcement_worker.rb

@@ -6,7 +6,8 @@ class PublishScheduledAnnouncementWorker
 
   def perform(announcement_id)
     announcement = Announcement.find(announcement_id)
-    announcement.update(published: true)
+
+    announcement.update(published: true, published_at: Time.now.utc, scheduled_at: nil) unless announcement.published?
 
     payload = InlineRenderer.render(announcement, nil, :announcement)
     payload = Oj.dump(event: :announcement, payload: payload)

app/workers/scheduler/scheduled_statuses_scheduler.rb

@@ -34,7 +34,7 @@ class Scheduler::ScheduledStatusesScheduler
   end
 
   def unpublish_expired_announcements!
-    expired_announcements.in_batches.update_all(published: false)
+    expired_announcements.in_batches.update_all(published: false, scheduled_at: nil)
   end
 
   def expired_announcements

db/migrate/20200126203551_add_published_at_to_announcements.rb

@@ -0,0 +1,5 @@
+class AddPublishedAtToAnnouncements < ActiveRecord::Migration[5.2]
+  def change
+    add_column :announcements, :published_at, :datetime
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_01_19_112504) do
+ActiveRecord::Schema.define(version: 2020_01_26_203551) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -228,6 +228,7 @@ ActiveRecord::Schema.define(version: 2020_01_19_112504) do
     t.datetime "ends_at"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "published_at"
   end
 
   create_table "backups", force: :cascade do |t|