From b9d74d407673a6dbdc87c3310618b22c85358c85 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 26 Jan 2020 20:07:26 +0100 Subject: [PATCH] Add streaming API updates for announcements being modified or deleted (#12963) Change `all_day` to be a visual client-side cue only Publish immediately if `scheduled_at` is in the past Add `published_at` and `updated_at` to announcements JSON --- .../admin/announcements_controller.rb | 3 ++ .../mastodon/actions/announcements.js | 14 ++++-- app/javascript/mastodon/actions/streaming.js | 10 +++- .../mastodon/components/animated_number.js | 7 +-- .../components/announcements.js | 12 +++-- .../mastodon/reducers/announcements.js | 49 +++++++++++++++---- app/lib/feed_manager.rb | 4 ++ app/models/announcement.rb | 21 +++----- .../rest/announcement_serializer.rb | 3 +- .../publish_announcement_reaction_worker.rb | 2 +- .../publish_scheduled_announcement_worker.rb | 2 +- app/workers/unpublish_announcement_worker.rb | 14 ++++++ 12 files changed, 100 insertions(+), 41 deletions(-) create mode 100644 app/workers/unpublish_announcement_worker.rb diff --git a/app/controllers/admin/announcements_controller.rb b/app/controllers/admin/announcements_controller.rb index 02198f0b52..212a9f6933 100644 --- a/app/controllers/admin/announcements_controller.rb +++ b/app/controllers/admin/announcements_controller.rb @@ -20,6 +20,7 @@ class Admin::AnnouncementsController < Admin::BaseController @announcement = Announcement.new(resource_params) if @announcement.save + PublishScheduledAnnouncementWorker.perform_async(@announcement.id) if @announcement.published? log_action :create, @announcement redirect_to admin_announcements_path else @@ -35,6 +36,7 @@ class Admin::AnnouncementsController < Admin::BaseController authorize :announcement, :update? if @announcement.update(resource_params) + PublishScheduledAnnouncementWorker.perform_async(@announcement.id) if @announcement.published? log_action :update, @announcement redirect_to admin_announcements_path else @@ -45,6 +47,7 @@ class Admin::AnnouncementsController < Admin::BaseController def destroy authorize :announcement, :destroy? @announcement.destroy! + UnpublishAnnouncementWorker.perform_async(@announcement.id) if @announcement.published? log_action :destroy, @announcement redirect_to admin_announcements_path end diff --git a/app/javascript/mastodon/actions/announcements.js b/app/javascript/mastodon/actions/announcements.js index 53b19a6fda..f072a407f9 100644 --- a/app/javascript/mastodon/actions/announcements.js +++ b/app/javascript/mastodon/actions/announcements.js @@ -5,6 +5,7 @@ export const ANNOUNCEMENTS_FETCH_REQUEST = 'ANNOUNCEMENTS_FETCH_REQUEST'; export const ANNOUNCEMENTS_FETCH_SUCCESS = 'ANNOUNCEMENTS_FETCH_SUCCESS'; export const ANNOUNCEMENTS_FETCH_FAIL = 'ANNOUNCEMENTS_FETCH_FAIL'; export const ANNOUNCEMENTS_UPDATE = 'ANNOUNCEMENTS_UPDATE'; +export const ANNOUNCEMENTS_DELETE = 'ANNOUNCEMENTS_DELETE'; export const ANNOUNCEMENTS_REACTION_ADD_REQUEST = 'ANNOUNCEMENTS_REACTION_ADD_REQUEST'; export const ANNOUNCEMENTS_REACTION_ADD_SUCCESS = 'ANNOUNCEMENTS_REACTION_ADD_SUCCESS'; @@ -139,8 +140,11 @@ export const updateReaction = reaction => ({ reaction, }); -export function toggleShowAnnouncements() { - return dispatch => { - dispatch({ type: ANNOUNCEMENTS_TOGGLE_SHOW }); - }; -} +export const toggleShowAnnouncements = () => ({ + type: ANNOUNCEMENTS_TOGGLE_SHOW, +}); + +export const deleteAnnouncement = id => ({ + type: ANNOUNCEMENTS_DELETE, + id, +}); diff --git a/app/javascript/mastodon/actions/streaming.js b/app/javascript/mastodon/actions/streaming.js index ac325f74cd..79b08bddac 100644 --- a/app/javascript/mastodon/actions/streaming.js +++ b/app/javascript/mastodon/actions/streaming.js @@ -8,7 +8,12 @@ import { } from './timelines'; import { updateNotifications, expandNotifications } from './notifications'; import { updateConversations } from './conversations'; -import { fetchAnnouncements, updateAnnouncements, updateReaction as updateAnnouncementsReaction } from './announcements'; +import { + fetchAnnouncements, + updateAnnouncements, + updateReaction as updateAnnouncementsReaction, + deleteAnnouncement, +} from './announcements'; import { fetchFilters } from './filters'; import { getLocale } from '../locales'; @@ -51,6 +56,9 @@ export function connectTimelineStream (timelineId, path, pollingRefresh = null, case 'announcement.reaction': dispatch(updateAnnouncementsReaction(JSON.parse(data.payload))); break; + case 'announcement.delete': + dispatch(deleteAnnouncement(data.payload)); + break; } }, }; diff --git a/app/javascript/mastodon/components/animated_number.js b/app/javascript/mastodon/components/animated_number.js index 2d359fbc60..3bfe86aa6a 100644 --- a/app/javascript/mastodon/components/animated_number.js +++ b/app/javascript/mastodon/components/animated_number.js @@ -27,7 +27,8 @@ export default class AnimatedNumber extends React.PureComponent { } const styles = [{ - key: value, + key: `${value}`, + data: value, style: { y: spring(0, { damping: 35, stiffness: 400 }) }, }]; @@ -35,8 +36,8 @@ export default class AnimatedNumber extends React.PureComponent { {items => ( - {items.map(({ key, style }) => ( - 0 ? 'absolute' : 'static', transform: `translateY(${style.y * 100}%)` }}> + {items.map(({ key, data, style }) => ( + 0 ? 'absolute' : 'static', transform: `translateY(${style.y * 100}%)` }}> ))} )} diff --git a/app/javascript/mastodon/features/getting_started/components/announcements.js b/app/javascript/mastodon/features/getting_started/components/announcements.js index 8ff1b0b4e8..acaa552aa0 100644 --- a/app/javascript/mastodon/features/getting_started/components/announcements.js +++ b/app/javascript/mastodon/features/getting_started/components/announcements.js @@ -367,11 +367,13 @@ class Announcements extends ImmutablePureComponent { ))} -
- - {index + 1} / {announcements.size} - -
+ {announcements.size > 1 && ( +
+ + {index + 1} / {announcements.size} + +
+ )} ); diff --git a/app/javascript/mastodon/reducers/announcements.js b/app/javascript/mastodon/reducers/announcements.js index 1cfb598fb9..3215c1c2dc 100644 --- a/app/javascript/mastodon/reducers/announcements.js +++ b/app/javascript/mastodon/reducers/announcements.js @@ -9,6 +9,7 @@ import { ANNOUNCEMENTS_REACTION_REMOVE_REQUEST, ANNOUNCEMENTS_REACTION_REMOVE_FAIL, ANNOUNCEMENTS_TOGGLE_SHOW, + ANNOUNCEMENTS_DELETE, } from '../actions/announcements'; import { Map as ImmutableMap, List as ImmutableList, Set as ImmutableSet, fromJS } from 'immutable'; @@ -22,14 +23,10 @@ const initialState = ImmutableMap({ const updateReaction = (state, id, name, updater) => state.update('items', list => list.map(announcement => { if (announcement.get('id') === id) { return announcement.update('reactions', reactions => { - if (reactions.find(reaction => reaction.get('name') === name)) { - return reactions.map(reaction => { - if (reaction.get('name') === name) { - return updater(reaction); - } + const idx = reactions.findIndex(reaction => reaction.get('name') === name); - return reaction; - }); + if (idx > -1) { + return reactions.update(idx, reaction => updater(reaction)); } return reactions.push(updater(fromJS({ name, count: 0 }))); @@ -46,13 +43,33 @@ const addReaction = (state, id, name) => updateReaction(state, id, name, x => x. const removeReaction = (state, id, name) => updateReaction(state, id, name, x => x.set('me', false).update('count', y => y - 1)); const addUnread = (state, items) => { - if (state.get('show')) return state; + if (state.get('show')) { + return state; + } const newIds = ImmutableSet(items.map(x => x.get('id'))); const oldIds = ImmutableSet(state.get('items').map(x => x.get('id'))); + return state.update('unread', unread => unread.union(newIds.subtract(oldIds))); }; +const sortAnnouncements = list => list.sortBy(x => x.get('starts_at') || x.get('published_at')); + +const updateAnnouncement = (state, announcement) => { + const idx = state.get('items').findIndex(x => x.get('id') === announcement.get('id')); + + state = addUnread(state, [announcement]); + + if (idx > -1) { + // Deep merge is used because announcements from the streaming API do not contain + // personalized data about which reactions have been selected by the given user, + // and that is information we want to preserve + return state.update('items', list => sortAnnouncements(list.update(idx, x => x.mergeDeep(announcement)))); + } + + return state.update('items', list => sortAnnouncements(list.unshift(announcement))); +}; + export default function announcementsReducer(state = initialState, action) { switch(action.type) { case ANNOUNCEMENTS_TOGGLE_SHOW: @@ -65,15 +82,17 @@ export default function announcementsReducer(state = initialState, action) { case ANNOUNCEMENTS_FETCH_SUCCESS: return state.withMutations(map => { const items = fromJS(action.announcements); + map.set('unread', ImmutableSet()); - addUnread(map, items); map.set('items', items); map.set('isLoading', false); + + addUnread(map, items); }); case ANNOUNCEMENTS_FETCH_FAIL: return state.set('isLoading', false); case ANNOUNCEMENTS_UPDATE: - return addUnread(state, [fromJS(action.announcement)]).update('items', list => list.unshift(fromJS(action.announcement)).sortBy(announcement => announcement.get('starts_at'))); + return updateAnnouncement(state, fromJS(action.announcement)); case ANNOUNCEMENTS_REACTION_UPDATE: return updateReactionCount(state, action.reaction); case ANNOUNCEMENTS_REACTION_ADD_REQUEST: @@ -82,6 +101,16 @@ export default function announcementsReducer(state = initialState, action) { case ANNOUNCEMENTS_REACTION_REMOVE_REQUEST: case ANNOUNCEMENTS_REACTION_ADD_FAIL: return removeReaction(state, action.id, action.name); + case ANNOUNCEMENTS_DELETE: + return state.update('unread', set => set.delete(action.id)).update('items', list => { + const idx = list.findIndex(x => x.get('id') === action.id); + + if (idx > -1) { + return list.delete(idx); + } + + return list; + }); default: return state; } diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 3f167f0d88..13b7aafdff 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -11,6 +11,10 @@ class FeedManager # Must be <= MAX_ITEMS or the tracking sets will grow forever REBLOG_FALLOFF = 40 + def with_active_accounts(&block) + Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each(&block) + end + def key(type, id, subtype = nil) return "feed:#{type}:#{id}" unless subtype diff --git a/app/models/announcement.rb b/app/models/announcement.rb index 4da9f94d6d..c5cf08f7c7 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -16,8 +16,6 @@ # class Announcement < ApplicationRecord - after_commit :queue_publish, on: :create - 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') } @@ -31,8 +29,11 @@ class Announcement < ApplicationRecord validates :ends_at, presence: true, if: -> { starts_at.present? } before_validation :set_all_day - before_validation :set_starts_at, on: :create - before_validation :set_ends_at, on: :create + before_validation :set_published, on: :create + + def published_at + scheduled_at || created_at + end def time_range? starts_at.present? && ends_at.present? @@ -71,15 +72,7 @@ class Announcement < ApplicationRecord self.all_day = false if starts_at.blank? || ends_at.blank? end - def set_starts_at - self.starts_at = starts_at.change(hour: 0, min: 0, sec: 0) if all_day? && starts_at.present? - end - - def set_ends_at - self.ends_at = ends_at.change(hour: 23, min: 59, sec: 59) if all_day? && ends_at.present? - end - - def queue_publish - PublishScheduledAnnouncementWorker.perform_async(id) if scheduled_at.blank? + def set_published + self.published = true if scheduled_at.blank? || scheduled_at.past? end end diff --git a/app/serializers/rest/announcement_serializer.rb b/app/serializers/rest/announcement_serializer.rb index 924d87b341..ae962aa1de 100644 --- a/app/serializers/rest/announcement_serializer.rb +++ b/app/serializers/rest/announcement_serializer.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class REST::AnnouncementSerializer < ActiveModel::Serializer - attributes :id, :content, :starts_at, :ends_at, :all_day + attributes :id, :content, :starts_at, :ends_at, :all_day, + :published_at, :updated_at has_many :mentions has_many :tags, serializer: REST::StatusSerializer::TagSerializer diff --git a/app/workers/publish_announcement_reaction_worker.rb b/app/workers/publish_announcement_reaction_worker.rb index e01deb64dd..418dc71275 100644 --- a/app/workers/publish_announcement_reaction_worker.rb +++ b/app/workers/publish_announcement_reaction_worker.rb @@ -13,7 +13,7 @@ class PublishAnnouncementReactionWorker payload = InlineRenderer.render(reaction, nil, :reaction).tap { |h| h[:announcement_id] = announcement_id.to_s } payload = Oj.dump(event: :'announcement.reaction', payload: payload) - Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each do |account| + FeedManager.instance.with_active_accounts do |account| redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") end rescue ActiveRecord::RecordNotFound diff --git a/app/workers/publish_scheduled_announcement_worker.rb b/app/workers/publish_scheduled_announcement_worker.rb index 4b2014e34a..6b5898bf57 100644 --- a/app/workers/publish_scheduled_announcement_worker.rb +++ b/app/workers/publish_scheduled_announcement_worker.rb @@ -11,7 +11,7 @@ class PublishScheduledAnnouncementWorker payload = InlineRenderer.render(announcement, nil, :announcement) payload = Oj.dump(event: :announcement, payload: payload) - Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each do |account| + FeedManager.instance.with_active_accounts do |account| redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") end end diff --git a/app/workers/unpublish_announcement_worker.rb b/app/workers/unpublish_announcement_worker.rb new file mode 100644 index 0000000000..e99d70cf87 --- /dev/null +++ b/app/workers/unpublish_announcement_worker.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class UnpublishAnnouncementWorker + include Sidekiq::Worker + include Redisable + + def perform(announcement_id) + payload = Oj.dump(event: :'announcement.delete', payload: announcement_id.to_s) + + FeedManager.instance.with_active_accounts do |account| + redis.publish("timeline:#{account.id}", payload) if redis.exists("subscribed:timeline:#{account.id}") + end + end +end