From 2319e85a8a681b135b80e12a2f32a5cdbcec1b6a Mon Sep 17 00:00:00 2001 From: abcang Date: Fri, 29 Jan 2021 02:24:22 +0900 Subject: Fix react/jsx-no-duplicate-props (#15636) --- app/javascript/mastodon/features/account/components/header.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 4647b98b2..2087fc3b8 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -330,7 +330,7 @@ class Header extends ImmutablePureComponent {
-
+
{pair.get('verified_at') && }
-- cgit From c8c764dd8b168ec876918d16b3f25280893d20dc Mon Sep 17 00:00:00 2001 From: abcang Date: Mon, 1 Feb 2021 05:24:17 +0900 Subject: Fix N+1 query when rendering with StatusSerializer (#15641) --- app/models/status.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app') diff --git a/app/models/status.rb b/app/models/status.rb index fcc7a026d..74e81f612 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -114,7 +114,7 @@ class Status < ApplicationRecord :tags, :preview_cards, :preloadable_poll, - account: :account_stat, + account: [:account_stat, :user], active_mentions: { account: :account_stat }, reblog: [ :application, @@ -124,7 +124,7 @@ class Status < ApplicationRecord :conversation, :status_stat, :preloadable_poll, - account: :account_stat, + account: [:account_stat, :user], active_mentions: { account: :account_stat }, ], thread: { account: :account_stat } @@ -301,7 +301,7 @@ class Status < ApplicationRecord return if account_ids.empty? - accounts = Account.where(id: account_ids).includes(:account_stat).index_by(&:id) + accounts = Account.where(id: account_ids).includes(:account_stat, :user).index_by(&:id) cached_items.each do |item| item.account = accounts[item.account_id] -- cgit From 7ab53f221a3d66a0bbc94b36d847b7bcf62fbd16 Mon Sep 17 00:00:00 2001 From: abcang Date: Mon, 1 Feb 2021 05:24:57 +0900 Subject: Improved performance of notification preloading (#15640) * Improved performance of notification preloading * Remove Cacheable from Notification * Fix test --- app/controllers/api/v1/notifications_controller.rb | 7 +- app/models/notification.rb | 56 +++++++--- spec/controllers/application_controller_spec.rb | 4 - spec/models/notification_spec.rb | 123 ++++++++++++++++----- 4 files changed, 138 insertions(+), 52 deletions(-) (limited to 'app') diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index 522c35ba5..f9d780839 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -31,12 +31,13 @@ class Api::V1::NotificationsController < Api::BaseController private def load_notifications - cache_collection_paginated_by_id( - browserable_account_notifications, - Notification, + notifications = browserable_account_notifications.includes(from_account: :account_stat).to_a_paginated_by_id( limit_param(DEFAULT_NOTIFICATIONS_LIMIT), params_slice(:max_id, :since_id, :min_id) ) + Notification.preload_cache_collection_target_statuses(notifications) do |target_statuses| + cache_collection(target_statuses, Status) + end end def browserable_account_notifications diff --git a/app/models/notification.rb b/app/models/notification.rb index 5e9ea62a0..98a6a618f 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -17,7 +17,6 @@ class Notification < ApplicationRecord self.inheritance_column = nil include Paginable - include Cacheable LEGACY_TYPE_CLASS_MAP = { 'Mention' => :mention, @@ -38,7 +37,13 @@ class Notification < ApplicationRecord poll ).freeze - STATUS_INCLUDES = [:account, :application, :preloadable_poll, :media_attachments, :tags, active_mentions: :account, reblog: [:account, :application, :preloadable_poll, :media_attachments, :tags, active_mentions: :account]].freeze + TARGET_STATUS_INCLUDES_BY_TYPE = { + status: :status, + reblog: [status: :reblog], + mention: [mention: :status], + favourite: [favourite: :status], + poll: [poll: :status], + }.freeze belongs_to :account, optional: true belongs_to :from_account, class_name: 'Account', optional: true @@ -65,8 +70,6 @@ class Notification < ApplicationRecord end } - cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account, follow_request: :account, poll: [status: STATUS_INCLUDES] - def type @type ||= (super || LEGACY_TYPE_CLASS_MAP[activity_type]).to_sym end @@ -87,21 +90,40 @@ class Notification < ApplicationRecord end class << self - def cache_ids - select(:id, :updated_at, :activity_type, :activity_id) - end - - def reload_stale_associations!(cached_items) - account_ids = (cached_items.map(&:from_account_id) + cached_items.filter_map { |item| item.target_status&.account_id }).uniq - - return if account_ids.empty? - - accounts = Account.where(id: account_ids).includes(:account_stat).index_by(&:id) + def preload_cache_collection_target_statuses(notifications, &_block) + notifications.group_by(&:type).each do |type, grouped_notifications| + associations = TARGET_STATUS_INCLUDES_BY_TYPE[type] + next unless associations + + # Instead of using the usual `includes`, manually preload each type. + # If polymorphic associations are loaded with the usual `includes`, other types of associations will be loaded more. + ActiveRecord::Associations::Preloader.new.preload(grouped_notifications, associations) + end - cached_items.each do |item| - item.from_account = accounts[item.from_account_id] - item.target_status.account = accounts[item.target_status.account_id] if item.target_status + unique_target_statuses = notifications.map(&:target_status).compact.uniq + # Call cache_collection in block + cached_statuses_by_id = yield(unique_target_statuses).index_by(&:id) + + notifications.each do |notification| + next if notification.target_status.nil? + + cached_status = cached_statuses_by_id[notification.target_status.id] + + case notification.type + when :status + notification.status = cached_status + when :reblog + notification.status.reblog = cached_status + when :favourite + notification.favourite.status = cached_status + when :mention + notification.mention.status = cached_status + when :poll + notification.poll.status = cached_status + end end + + notifications end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 63ae27a92..e73a08a0e 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -349,10 +349,6 @@ describe ApplicationController, type: :controller do expect(C.new.cache_collection(raw, Object)).to eq raw end - context 'Notification' do - include_examples 'cacheable', :notification, Notification - end - context 'Status' do include_examples 'cacheable', :status, Status end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 0a045904b..1e9e45d8d 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -56,47 +56,114 @@ RSpec.describe Notification, type: :model do end end - describe '.reload_stale_associations!' do - context 'account_ids are empty' do - let(:cached_items) { [] } + describe '.preload_cache_collection_target_statuses' do + subject do + described_class.preload_cache_collection_target_statuses(notifications) do |target_statuses| + # preload account for testing instead of using cache_collection + Status.preload(:account).where(id: target_statuses.map(&:id)) + end + end - subject { described_class.reload_stale_associations!(cached_items) } + context 'notifications are empty' do + let(:notifications) { [] } - it 'returns nil' do - is_expected.to be nil + it 'returns []' do + is_expected.to eq [] end end - context 'account_ids are present' do + context 'notifications are present' do before do - allow(accounts_with_ids).to receive(:[]).with(stale_account1.id).and_return(account1) - allow(accounts_with_ids).to receive(:[]).with(stale_account2.id).and_return(account2) - allow(Account).to receive_message_chain(:where, :includes, :index_by).and_return(accounts_with_ids) + notifications.each(&:reload) end - let(:cached_items) do + let(:mention) { Fabricate(:mention) } + let(:status) { Fabricate(:status) } + let(:reblog) { Fabricate(:status, reblog: Fabricate(:status)) } + let(:follow) { Fabricate(:follow) } + let(:follow_request) { Fabricate(:follow_request) } + let(:favourite) { Fabricate(:favourite) } + let(:poll) { Fabricate(:poll) } + + let(:notifications) do [ - Fabricate(:notification, activity: Fabricate(:status)), - Fabricate(:notification, activity: Fabricate(:follow)), + Fabricate(:notification, type: :mention, activity: mention), + Fabricate(:notification, type: :status, activity: status), + Fabricate(:notification, type: :reblog, activity: reblog), + Fabricate(:notification, type: :follow, activity: follow), + Fabricate(:notification, type: :follow_request, activity: follow_request), + Fabricate(:notification, type: :favourite, activity: favourite), + Fabricate(:notification, type: :poll, activity: poll), ] end - let(:stale_account1) { cached_items[0].from_account } - let(:stale_account2) { cached_items[1].from_account } - - let(:account1) { Fabricate(:account) } - let(:account2) { Fabricate(:account) } - - let(:accounts_with_ids) { { account1.id => account1, account2.id => account2 } } - - it 'reloads associations' do - expect(cached_items[0].from_account).to be stale_account1 - expect(cached_items[1].from_account).to be stale_account2 - - described_class.reload_stale_associations!(cached_items) + it 'preloads target status' do + # mention + expect(subject[0].type).to eq :mention + expect(subject[0].association(:mention)).to be_loaded + expect(subject[0].mention.association(:status)).to be_loaded + + # status + expect(subject[1].type).to eq :status + expect(subject[1].association(:status)).to be_loaded + + # reblog + expect(subject[2].type).to eq :reblog + expect(subject[2].association(:status)).to be_loaded + expect(subject[2].status.association(:reblog)).to be_loaded + + # follow: nothing + expect(subject[3].type).to eq :follow + expect(subject[3].target_status).to be_nil + + # follow_request: nothing + expect(subject[4].type).to eq :follow_request + expect(subject[4].target_status).to be_nil + + # favourite + expect(subject[5].type).to eq :favourite + expect(subject[5].association(:favourite)).to be_loaded + expect(subject[5].favourite.association(:status)).to be_loaded + + # poll + expect(subject[6].type).to eq :poll + expect(subject[6].association(:poll)).to be_loaded + expect(subject[6].poll.association(:status)).to be_loaded + end - expect(cached_items[0].from_account).to be account1 - expect(cached_items[1].from_account).to be account2 + it 'replaces to cached status' do + # mention + expect(subject[0].type).to eq :mention + expect(subject[0].target_status.association(:account)).to be_loaded + expect(subject[0].target_status).to eq mention.status + + # status + expect(subject[1].type).to eq :status + expect(subject[1].target_status.association(:account)).to be_loaded + expect(subject[1].target_status).to eq status + + # reblog + expect(subject[2].type).to eq :reblog + expect(subject[2].target_status.association(:account)).to be_loaded + expect(subject[2].target_status).to eq reblog.reblog + + # follow: nothing + expect(subject[3].type).to eq :follow + expect(subject[3].target_status).to be_nil + + # follow_request: nothing + expect(subject[4].type).to eq :follow_request + expect(subject[4].target_status).to be_nil + + # favourite + expect(subject[5].type).to eq :favourite + expect(subject[5].target_status.association(:account)).to be_loaded + expect(subject[5].target_status).to eq favourite.status + + # poll + expect(subject[6].type).to eq :poll + expect(subject[6].target_status.association(:account)).to be_loaded + expect(subject[6].target_status).to eq poll.status end end end -- cgit From 3efa0c54b6656fba189baab0857e60c0bc4f3c7d Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 31 Jan 2021 21:25:31 +0100 Subject: Change custom emoji to be animated when hovering container (#15637) Co-authored-by: Claire --- .../__snapshots__/display_name-test.js.snap | 2 + app/javascript/mastodon/components/display_name.js | 43 ++++++------------ .../mastodon/components/status_content.js | 43 ++++++++---------- .../mastodon/features/account/components/header.js | 43 ++++++------------ .../direct_timeline/components/conversation.js | 43 ++++++------------ .../features/directory/components/account_card.js | 45 ++++++------------ .../getting_started/components/announcements.js | 53 ++++++++++------------ 7 files changed, 102 insertions(+), 170 deletions(-) (limited to 'app') diff --git a/app/javascript/mastodon/components/__tests__/__snapshots__/display_name-test.js.snap b/app/javascript/mastodon/components/__tests__/__snapshots__/display_name-test.js.snap index 29fdc2412..0f27473af 100644 --- a/app/javascript/mastodon/components/__tests__/__snapshots__/display_name-test.js.snap +++ b/app/javascript/mastodon/components/__tests__/__snapshots__/display_name-test.js.snap @@ -3,6 +3,8 @@ exports[` renders display name + account name 1`] = ` { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } - - componentDidUpdate () { - this._updateEmojis(); - } - - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - setRef = (c) => { - this.node = c; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render () { @@ -81,7 +66,7 @@ export default class DisplayName extends React.PureComponent { } return ( - + {displayName} {suffix} ); diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js index 190ced1a8..35bd50514 100644 --- a/app/javascript/mastodon/components/status_content.js +++ b/app/javascript/mastodon/components/status_content.js @@ -75,35 +75,38 @@ export default class StatusContent extends React.PureComponent { } } - _updateStatusEmojis () { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); + emoji.src = emoji.getAttribute('data-original'); + } + } + + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); } } componentDidMount () { this._updateStatusLinks(); - this._updateStatusEmojis(); } componentDidUpdate () { this._updateStatusLinks(); - this._updateStatusEmojis(); } onMentionClick = (mention, e) => { @@ -122,14 +125,6 @@ export default class StatusContent extends React.PureComponent { } } - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } - - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } - handleMouseDown = (e) => { this.startXY = [e.clientX, e.clientY]; } @@ -219,7 +214,7 @@ export default class StatusContent extends React.PureComponent { } return ( -
+
+
{!!status.get('poll') && } @@ -253,7 +248,7 @@ export default class StatusContent extends React.PureComponent { return output; } else { return ( -
+
{!!status.get('poll') && } diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 2087fc3b8..8e49486bd 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -96,45 +96,30 @@ class Header extends ImmutablePureComponent { return !location.pathname.match(/\/(followers|following)\/?$/); } - _updateEmojis () { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } - - componentDidUpdate () { - this._updateEmojis(); - } - - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - setRef = (c) => { - this.node = c; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render () { @@ -276,7 +261,7 @@ class Header extends ImmutablePureComponent { } return ( -
+
{!suspended && info} diff --git a/app/javascript/mastodon/features/direct_timeline/components/conversation.js b/app/javascript/mastodon/features/direct_timeline/components/conversation.js index 6ecc27fac..43e1d77b9 100644 --- a/app/javascript/mastodon/features/direct_timeline/components/conversation.js +++ b/app/javascript/mastodon/features/direct_timeline/components/conversation.js @@ -44,41 +44,30 @@ class Conversation extends ImmutablePureComponent { intl: PropTypes.object.isRequired, }; - _updateEmojis () { - const node = this.namesNode; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - componentDidUpdate () { - this._updateEmojis(); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } - - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } handleClick = () => { @@ -123,10 +112,6 @@ class Conversation extends ImmutablePureComponent { this.props.onToggleHidden(this.props.lastStatus); } - setNamesRef = (c) => { - this.namesNode = c; - } - render () { const { accounts, lastStatus, unread, scrollKey, intl } = this.props; @@ -171,7 +156,7 @@ class Conversation extends ImmutablePureComponent { {unread && }
-
+
{names} }} />
diff --git a/app/javascript/mastodon/features/directory/components/account_card.js b/app/javascript/mastodon/features/directory/components/account_card.js index e37733828..8f0e8db4b 100644 --- a/app/javascript/mastodon/features/directory/components/account_card.js +++ b/app/javascript/mastodon/features/directory/components/account_card.js @@ -102,42 +102,31 @@ class AccountCard extends ImmutablePureComponent { onMute: PropTypes.func.isRequired, }; - _updateEmojis() { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount() { - this._updateEmojis(); - } - - componentDidUpdate() { - this._updateEmojis(); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - }; + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - }; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } + } handleFollow = () => { this.props.onFollow(this.props.account); @@ -151,10 +140,6 @@ class AccountCard extends ImmutablePureComponent { this.props.onMute(this.props.account); }; - setRef = (c) => { - this.node = c; - }; - render() { const { account, intl } = this.props; @@ -239,7 +224,7 @@ class AccountCard extends ImmutablePureComponent {
-
+
{ - target.src = target.getAttribute('data-original'); + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-original'); + } } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render () { @@ -148,6 +141,8 @@ class Content extends ImmutablePureComponent { className='announcements__item__content translate' ref={this.setRef} dangerouslySetInnerHTML={{ __html: announcement.get('contentHtml') }} + onMouseEnter={this.handleMouseEnter} + onMouseLeave={this.handleMouseLeave} /> ); } -- cgit From a044ddac5b3c0e2012c0e91bfbc07aa256a0684f Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 2 Feb 2021 14:49:57 +0100 Subject: Fix race conditions on account migration creation (#15597) * Atomically check for processing lock in Move handler * Prevent race condition when creating account migrations Fixes #15595 * Add tests Co-authored-by: Claire --- app/lib/activitypub/activity/move.rb | 11 +-- app/models/account_migration.rb | 14 ++- .../settings/migrations_controller_spec.rb | 37 +++++++- spec/lib/activitypub/activity/move_spec.rb | 99 +++++++++++++++++----- 4 files changed, 127 insertions(+), 34 deletions(-) (limited to 'app') diff --git a/app/lib/activitypub/activity/move.rb b/app/lib/activitypub/activity/move.rb index 7e073f64d..8576ceccd 100644 --- a/app/lib/activitypub/activity/move.rb +++ b/app/lib/activitypub/activity/move.rb @@ -4,9 +4,8 @@ class ActivityPub::Activity::Move < ActivityPub::Activity PROCESSING_COOLDOWN = 7.days.seconds def perform - return if origin_account.uri != object_uri || processed? - - mark_as_processing! + return if origin_account.uri != object_uri + return unless mark_as_processing! target_account = ActivityPub::FetchRemoteAccountService.new.call(target_uri) @@ -35,12 +34,8 @@ class ActivityPub::Activity::Move < ActivityPub::Activity value_or_id(@json['target']) end - def processed? - redis.exists?("move_in_progress:#{@account.id}") - end - def mark_as_processing! - redis.setex("move_in_progress:#{@account.id}", PROCESSING_COOLDOWN, true) + redis.set("move_in_progress:#{@account.id}", true, nx: true, ex: PROCESSING_COOLDOWN) end def unmark_as_processing! diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index 4fae98ed7..ded32c9c6 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -14,6 +14,8 @@ # class AccountMigration < ApplicationRecord + include Redisable + COOLDOWN_PERIOD = 30.days.freeze belongs_to :account @@ -39,7 +41,13 @@ class AccountMigration < ApplicationRecord return false unless errors.empty? - save + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + save + else + raise Mastodon::RaceConditionError + end + end end def cooldown_at @@ -75,4 +83,8 @@ class AccountMigration < ApplicationRecord def validate_migration_cooldown errors.add(:base, I18n.t('migrations.errors.on_cooldown')) if account.migrations.within_cooldown.exists? end + + def lock_options + { redis: redis, key: "account_migration:#{account.id}" } + end end diff --git a/spec/controllers/settings/migrations_controller_spec.rb b/spec/controllers/settings/migrations_controller_spec.rb index 36e4ba86e..048d9de8d 100644 --- a/spec/controllers/settings/migrations_controller_spec.rb +++ b/spec/controllers/settings/migrations_controller_spec.rb @@ -51,7 +51,7 @@ describe Settings::MigrationsController do it_behaves_like 'authenticate user' end - context 'when user is sign in' do + context 'when user is signed in' do subject { post :create, params: { account_migration: { acct: acct, current_password: '12345678' } } } let(:user) { Fabricate(:user, password: '12345678') } @@ -67,12 +67,45 @@ describe Settings::MigrationsController do end end - context 'when acct is a current account' do + context 'when acct is the current account' do let(:acct) { user.account } it 'renders show' do is_expected.to render_template :show end + + it 'does not update the moved account' do + expect(user.account.reload.moved_to_account_id).to be_nil + end + end + + context 'when target account does not reference the account being moved from' do + let(:acct) { Fabricate(:account, also_known_as: []) } + + it 'renders show' do + is_expected.to render_template :show + end + + it 'does not update the moved account' do + expect(user.account.reload.moved_to_account_id).to be_nil + end + end + + context 'when a recent migration already exists ' do + let(:acct) { Fabricate(:account, also_known_as: [ActivityPub::TagManager.instance.uri_for(user.account)]) } + + before do + moved_to = Fabricate(:account, also_known_as: [ActivityPub::TagManager.instance.uri_for(user.account)]) + user.account.migrations.create!(acct: moved_to.acct) + end + + it 'renders show' do + is_expected.to render_template :show + end + + it 'does not update the moved account' do + expect(user.account.reload.moved_to_account_id).to be_nil + end end end end diff --git a/spec/lib/activitypub/activity/move_spec.rb b/spec/lib/activitypub/activity/move_spec.rb index 3574f273a..2d1d276c5 100644 --- a/spec/lib/activitypub/activity/move_spec.rb +++ b/spec/lib/activitypub/activity/move_spec.rb @@ -1,23 +1,11 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Move do - let(:follower) { Fabricate(:account) } - let(:old_account) { Fabricate(:account) } - let(:new_account) { Fabricate(:account) } - - before do - follower.follow!(old_account) - - old_account.update!(uri: 'https://example.org/alice', domain: 'example.org', protocol: :activitypub, inbox_url: 'https://example.org/inbox') - new_account.update!(uri: 'https://example.com/alice', domain: 'example.com', protocol: :activitypub, inbox_url: 'https://example.com/inbox', also_known_as: [old_account.uri]) - - stub_request(:post, 'https://example.org/inbox').to_return(status: 200) - stub_request(:post, 'https://example.com/inbox').to_return(status: 200) - - service_stub = double - allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_stub) - allow(service_stub).to receive(:call).and_return(new_account) - end + let(:follower) { Fabricate(:account) } + let(:old_account) { Fabricate(:account, uri: 'https://example.org/alice', domain: 'example.org', protocol: :activitypub, inbox_url: 'https://example.org/inbox') } + let(:new_account) { Fabricate(:account, uri: 'https://example.com/alice', domain: 'example.com', protocol: :activitypub, inbox_url: 'https://example.com/inbox', also_known_as: also_known_as) } + let(:also_known_as) { [old_account.uri] } + let(:returned_account) { new_account } let(:json) do { @@ -30,6 +18,17 @@ RSpec.describe ActivityPub::Activity::Move do }.with_indifferent_access end + before do + follower.follow!(old_account) + + stub_request(:post, old_account.inbox_url).to_return(status: 200) + stub_request(:post, new_account.inbox_url).to_return(status: 200) + + service_stub = double + allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_stub) + allow(service_stub).to receive(:call).and_return(returned_account) + end + describe '#perform' do subject { described_class.new(json, old_account) } @@ -37,16 +36,70 @@ RSpec.describe ActivityPub::Activity::Move do subject.perform end - it 'sets moved account on old account' do - expect(old_account.reload.moved_to_account_id).to eq new_account.id + context 'when all conditions are met' do + it 'sets moved account on old account' do + expect(old_account.reload.moved_to_account_id).to eq new_account.id + end + + it 'makes followers unfollow old account' do + expect(follower.following?(old_account)).to be false + end + + it 'makes followers follow-request the new account' do + expect(follower.requested?(new_account)).to be true + end end - it 'makes followers unfollow old account' do - expect(follower.following?(old_account)).to be false + context "when the new account can't be resolved" do + let(:returned_account) { nil } + + it 'does not set moved account on old account' do + expect(old_account.reload.moved_to_account_id).to be_nil + end + + it 'does not make followers unfollow old account' do + expect(follower.following?(old_account)).to be true + end + + it 'does not make followers follow-request the new account' do + expect(follower.requested?(new_account)).to be false + end end - it 'makes followers follow-request the new account' do - expect(follower.requested?(new_account)).to be true + context 'when the new account does not references the old account' do + let(:also_known_as) { [] } + + it 'does not set moved account on old account' do + expect(old_account.reload.moved_to_account_id).to be_nil + end + + it 'does not make followers unfollow old account' do + expect(follower.following?(old_account)).to be true + end + + it 'does not make followers follow-request the new account' do + expect(follower.requested?(new_account)).to be false + end + end + + context 'when a Move has been recently processed' do + around do |example| + Redis.current.set("move_in_progress:#{old_account.id}", true, nx: true, ex: 7.days.seconds) + example.run + Redis.current.del("move_in_progress:#{old_account.id}") + end + + it 'does not set moved account on old account' do + expect(old_account.reload.moved_to_account_id).to be_nil + end + + it 'does not make followers unfollow old account' do + expect(follower.following?(old_account)).to be true + end + + it 'does not make followers follow-request the new account' do + expect(follower.requested?(new_account)).to be false + end end end end -- cgit From e1fa06c459a28a575d0da540432c61b702e99cdd Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 31 Jan 2021 21:25:31 +0100 Subject: [Glitch] Change custom emoji to be animated when hovering container Port 3efa0c54b6656fba189baab0857e60c0bc4f3c7d to glitch-soc Co-authored-by: Claire Signed-off-by: Claire --- .../flavours/glitch/components/display_name.js | 43 +++++----------- .../flavours/glitch/components/status_content.js | 59 +++++++++++----------- .../glitch/features/account/components/header.js | 43 +++++----------- .../direct_timeline/components/conversation.js | 43 +++++----------- .../features/directory/components/account_card.js | 45 ++++++----------- .../getting_started/components/announcements.js | 53 +++++++++---------- 6 files changed, 111 insertions(+), 175 deletions(-) (limited to 'app') diff --git a/app/javascript/flavours/glitch/components/display_name.js b/app/javascript/flavours/glitch/components/display_name.js index 44662a8b8..ad978a2c6 100644 --- a/app/javascript/flavours/glitch/components/display_name.js +++ b/app/javascript/flavours/glitch/components/display_name.js @@ -15,45 +15,30 @@ export default class DisplayName extends React.PureComponent { handleClick: PropTypes.func, }; - _updateEmojis () { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } - - componentDidUpdate () { - this._updateEmojis(); - } - - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - setRef = (c) => { - this.node = c; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render() { @@ -101,7 +86,7 @@ export default class DisplayName extends React.PureComponent { } return ( - + {displayName} {inline ? ' ' : null} {suffix} diff --git a/app/javascript/flavours/glitch/components/status_content.js b/app/javascript/flavours/glitch/components/status_content.js index 61a28e9a7..34ff97305 100644 --- a/app/javascript/flavours/glitch/components/status_content.js +++ b/app/javascript/flavours/glitch/components/status_content.js @@ -154,35 +154,38 @@ export default class StatusContent extends React.PureComponent { } } - _updateStatusEmojis () { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); + emoji.src = emoji.getAttribute('data-original'); + } + } + + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); } } componentDidMount () { this._updateStatusLinks(); - this._updateStatusEmojis(); } componentDidUpdate () { this._updateStatusLinks(); - this._updateStatusEmojis(); if (this.props.onUpdate) this.props.onUpdate(); } @@ -206,14 +209,6 @@ export default class StatusContent extends React.PureComponent { } } - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } - - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } - handleMouseDown = (e) => { this.startXY = [e.clientX, e.clientY]; } @@ -253,10 +248,6 @@ export default class StatusContent extends React.PureComponent { } } - setRef = (c) => { - this.node = c; - } - setContentsRef = (c) => { this.contentsNode = c; } @@ -323,7 +314,7 @@ export default class StatusContent extends React.PureComponent { } return ( -
+
@@ -356,7 +349,6 @@ export default class StatusContent extends React.PureComponent { onMouseDown={this.handleMouseDown} onMouseUp={this.handleMouseUp} tabIndex='0' - ref={this.setRef} >
{media}
@@ -373,9 +367,16 @@ export default class StatusContent extends React.PureComponent {
-
+
{media}
); diff --git a/app/javascript/flavours/glitch/features/account/components/header.js b/app/javascript/flavours/glitch/features/account/components/header.js index 6a572862d..c18520c00 100644 --- a/app/javascript/flavours/glitch/features/account/components/header.js +++ b/app/javascript/flavours/glitch/features/account/components/header.js @@ -88,45 +88,30 @@ class Header extends ImmutablePureComponent { window.open(profileLink, '_blank'); } - _updateEmojis () { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } - - componentDidUpdate () { - this._updateEmojis(); - } - - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - setRef = (c) => { - this.node = c; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render () { @@ -279,7 +264,7 @@ class Header extends ImmutablePureComponent { } return ( -
+
{info} diff --git a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js index 0af8b348f..98b48cd90 100644 --- a/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js +++ b/app/javascript/flavours/glitch/features/direct_timeline/components/conversation.js @@ -67,41 +67,30 @@ class Conversation extends ImmutablePureComponent { } } - _updateEmojis () { - const node = this.namesNode; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount () { - this._updateEmojis(); - } - - componentDidUpdate () { - this._updateEmojis(); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - } + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } handleClick = () => { @@ -152,10 +141,6 @@ class Conversation extends ImmutablePureComponent { this.setState({ isExpanded: value }); } - setNamesRef = (c) => { - this.namesNode = c; - } - render () { const { accounts, lastStatus, unread, scrollKey, intl } = this.props; const { isExpanded } = this.state; @@ -206,7 +191,7 @@ class Conversation extends ImmutablePureComponent { {unread && }
-
+
{names} }} />
diff --git a/app/javascript/flavours/glitch/features/directory/components/account_card.js b/app/javascript/flavours/glitch/features/directory/components/account_card.js index 5f952b382..9fe84c10b 100644 --- a/app/javascript/flavours/glitch/features/directory/components/account_card.js +++ b/app/javascript/flavours/glitch/features/directory/components/account_card.js @@ -102,42 +102,31 @@ class AccountCard extends ImmutablePureComponent { onMute: PropTypes.func.isRequired, }; - _updateEmojis() { - const node = this.node; - - if (!node || autoPlayGif) { + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { return; } - const emojis = node.querySelectorAll('.custom-emoji'); + const emojis = currentTarget.querySelectorAll('.custom-emoji'); for (var i = 0; i < emojis.length; i++) { let emoji = emojis[i]; - if (emoji.classList.contains('status-emoji')) { - continue; - } - emoji.classList.add('status-emoji'); - - emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); - emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + emoji.src = emoji.getAttribute('data-original'); } } - componentDidMount() { - this._updateEmojis(); - } - - componentDidUpdate() { - this._updateEmojis(); - } + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } - handleEmojiMouseEnter = ({ target }) => { - target.src = target.getAttribute('data-original'); - }; + const emojis = currentTarget.querySelectorAll('.custom-emoji'); - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); - }; + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } + } handleFollow = () => { this.props.onFollow(this.props.account); @@ -151,10 +140,6 @@ class AccountCard extends ImmutablePureComponent { this.props.onMute(this.props.account); }; - setRef = (c) => { - this.node = c; - }; - render() { const { account, intl } = this.props; @@ -239,7 +224,7 @@ class AccountCard extends ImmutablePureComponent {
-
+
{ - target.src = target.getAttribute('data-original'); + handleMouseEnter = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-original'); + } } - handleEmojiMouseLeave = ({ target }) => { - target.src = target.getAttribute('data-static'); + handleMouseLeave = ({ currentTarget }) => { + if (autoPlayGif) { + return; + } + + const emojis = currentTarget.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + emoji.src = emoji.getAttribute('data-static'); + } } render () { @@ -148,6 +141,8 @@ class Content extends ImmutablePureComponent { className='announcements__item__content translate' ref={this.setRef} dangerouslySetInnerHTML={{ __html: announcement.get('contentHtml') }} + onMouseEnter={this.handleMouseEnter} + onMouseLeave={this.handleMouseLeave} /> ); } -- cgit