From a1f5188c8c4c98149ce157f9a9a596874f2b46dd Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 10 Jul 2023 03:06:09 +0200 Subject: [PATCH 1/8] Change feed merge, unmerge and regeneration workers to use a replica (#25849) --- app/workers/merge_worker.rb | 9 ++++++++- app/workers/regeneration_worker.rb | 9 +++++++-- app/workers/unmerge_worker.rb | 9 ++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index e526d2887..50cfcc3f0 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -5,7 +5,14 @@ class MergeWorker include Redisable def perform(from_account_id, into_account_id) - FeedManager.instance.merge_into_home(Account.find(from_account_id), Account.find(into_account_id)) + ApplicationRecord.connected_to(role: :primary) do + @from_account = Account.find(from_account_id) + @into_account = Account.find(into_account_id) + end + + ApplicationRecord.connected_to(role: :read, prevent_writes: true) do + FeedManager.instance.merge_into_home(@from_account, @into_account) + end rescue ActiveRecord::RecordNotFound true ensure diff --git a/app/workers/regeneration_worker.rb b/app/workers/regeneration_worker.rb index 5c13c894f..5ac095e65 100644 --- a/app/workers/regeneration_worker.rb +++ b/app/workers/regeneration_worker.rb @@ -6,8 +6,13 @@ class RegenerationWorker sidekiq_options lock: :until_executed def perform(account_id, _ = :home) - account = Account.find(account_id) - PrecomputeFeedService.new.call(account) + ApplicationRecord.connected_to(role: :primary) do + @account = Account.find(account_id) + end + + ApplicationRecord.connected_to(role: :read, prevent_writes: true) do + PrecomputeFeedService.new.call(@account) + end rescue ActiveRecord::RecordNotFound true end diff --git a/app/workers/unmerge_worker.rb b/app/workers/unmerge_worker.rb index 1a23faae5..f911ea2f9 100644 --- a/app/workers/unmerge_worker.rb +++ b/app/workers/unmerge_worker.rb @@ -6,7 +6,14 @@ class UnmergeWorker sidekiq_options queue: 'pull' def perform(from_account_id, into_account_id) - FeedManager.instance.unmerge_from_home(Account.find(from_account_id), Account.find(into_account_id)) + ApplicationRecord.connected_to(role: :primary) do + @from_account = Account.find(from_account_id) + @into_account = Account.find(into_account_id) + end + + ApplicationRecord.connected_to(role: :read, prevent_writes: true) do + FeedManager.instance.unmerge_from_home(@from_account, @into_account) + end rescue ActiveRecord::RecordNotFound true end From f3fca78756c02fb19f75a2a712d80fa712d32229 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Sun, 9 Jul 2023 21:06:22 -0400 Subject: [PATCH 2/8] Refactor `NotificationMailer` to use parameterization (#25718) --- app/mailers/notification_mailer.rb | 71 +++++++++---------- app/services/notify_service.rb | 7 +- spec/mailers/notification_mailer_spec.rb | 21 ++++-- .../previews/notification_mailer_preview.rb | 29 +++++--- 4 files changed, 73 insertions(+), 55 deletions(-) diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 7cd3bab1a..277612366 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -1,83 +1,76 @@ # frozen_string_literal: true class NotificationMailer < ApplicationMailer - helper :accounts - helper :statuses + helper :accounts, + :statuses, + :routing - helper RoutingHelper + before_action :process_params + before_action :set_status, only: [:mention, :favourite, :reblog] + before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request] - def mention(recipient, notification) - @me = recipient - @user = recipient.user - @type = 'mention' - @status = notification.target_status + default to: -> { email_address_with_name(@user.email, @me.username) } + def mention return unless @user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct) + mail subject: default_i18n_subject(name: @status.account.acct) end end - def follow(recipient, notification) - @me = recipient - @user = recipient.user - @type = 'follow' - @account = notification.from_account - + def follow return unless @user.functional? locale_for_account(@me) do - mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.follow.subject', name: @account.acct) + mail subject: default_i18n_subject(name: @account.acct) end end - def favourite(recipient, notification) - @me = recipient - @user = recipient.user - @type = 'favourite' - @account = notification.from_account - @status = notification.target_status - + def favourite return unless @user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct) + mail subject: default_i18n_subject(name: @account.acct) end end - def reblog(recipient, notification) - @me = recipient - @user = recipient.user - @type = 'reblog' - @account = notification.from_account - @status = notification.target_status - + def reblog return unless @user.functional? && @status.present? locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct) + mail subject: default_i18n_subject(name: @account.acct) end end - def follow_request(recipient, notification) - @me = recipient - @user = recipient.user - @type = 'follow_request' - @account = notification.from_account - + def follow_request return unless @user.functional? locale_for_account(@me) do - mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct) + mail subject: default_i18n_subject(name: @account.acct) end end private + def process_params + @notification = params[:notification] + @me = params[:recipient] + @user = @me.user + @type = action_name + end + + def set_status + @status = @notification.target_status + end + + def set_account + @account = @notification.from_account + end + def thread_by_conversation(conversation) return if conversation.nil? diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index ad9e6e3d6..06b48d558 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -162,7 +162,12 @@ class NotifyService < BaseService end def send_email! - NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes) if NotificationMailer.respond_to?(@notification.type) + return unless NotificationMailer.respond_to?(@notification.type) + + NotificationMailer + .with(recipient: @recipient, notification: @notification) + .public_send(@notification.type) + .deliver_later(wait: 2.minutes) end def email_needed? diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index bf364b625..3efb97cb1 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -23,7 +23,8 @@ RSpec.describe NotificationMailer do describe 'mention' do let(:mention) { Mention.create!(account: receiver.account, status: foreign_status) } - let(:mail) { described_class.mention(receiver.account, Notification.create!(account: receiver.account, activity: mention)) } + let(:notification) { Notification.create!(account: receiver.account, activity: mention) } + let(:mail) { prepared_mailer_for(receiver.account).mention } include_examples 'localized subject', 'notification_mailer.mention.subject', name: 'bob' @@ -40,7 +41,8 @@ RSpec.describe NotificationMailer do describe 'follow' do let(:follow) { sender.follow!(receiver.account) } - let(:mail) { described_class.follow(receiver.account, Notification.create!(account: receiver.account, activity: follow)) } + let(:notification) { Notification.create!(account: receiver.account, activity: follow) } + let(:mail) { prepared_mailer_for(receiver.account).follow } include_examples 'localized subject', 'notification_mailer.follow.subject', name: 'bob' @@ -56,7 +58,8 @@ RSpec.describe NotificationMailer do describe 'favourite' do let(:favourite) { Favourite.create!(account: sender, status: own_status) } - let(:mail) { described_class.favourite(own_status.account, Notification.create!(account: receiver.account, activity: favourite)) } + let(:notification) { Notification.create!(account: receiver.account, activity: favourite) } + let(:mail) { prepared_mailer_for(own_status.account).favourite } include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob' @@ -73,7 +76,8 @@ RSpec.describe NotificationMailer do describe 'reblog' do let(:reblog) { Status.create!(account: sender, reblog: own_status) } - let(:mail) { described_class.reblog(own_status.account, Notification.create!(account: receiver.account, activity: reblog)) } + let(:notification) { Notification.create!(account: receiver.account, activity: reblog) } + let(:mail) { prepared_mailer_for(own_status.account).reblog } include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob' @@ -90,7 +94,8 @@ RSpec.describe NotificationMailer do describe 'follow_request' do let(:follow_request) { Fabricate(:follow_request, account: sender, target_account: receiver.account) } - let(:mail) { described_class.follow_request(receiver.account, Notification.create!(account: receiver.account, activity: follow_request)) } + let(:notification) { Notification.create!(account: receiver.account, activity: follow_request) } + let(:mail) { prepared_mailer_for(receiver.account).follow_request } include_examples 'localized subject', 'notification_mailer.follow_request.subject', name: 'bob' @@ -103,4 +108,10 @@ RSpec.describe NotificationMailer do expect(mail.body.encoded).to match('bob has requested to follow you') end end + + private + + def prepared_mailer_for(recipient) + described_class.with(recipient: recipient, notification: notification) + end end diff --git a/spec/mailers/previews/notification_mailer_preview.rb b/spec/mailers/previews/notification_mailer_preview.rb index 214161881..a63c20c27 100644 --- a/spec/mailers/previews/notification_mailer_preview.rb +++ b/spec/mailers/previews/notification_mailer_preview.rb @@ -5,31 +5,40 @@ class NotificationMailerPreview < ActionMailer::Preview # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/mention def mention - m = Mention.last - NotificationMailer.mention(m.account, Notification.find_by(activity: m)) + activity = Mention.last + mailer_for(activity.account, activity).mention end # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/follow def follow - f = Follow.last - NotificationMailer.follow(f.target_account, Notification.find_by(activity: f)) + activity = Follow.last + mailer_for(activity.target_account, activity).follow end # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/follow_request def follow_request - f = Follow.last - NotificationMailer.follow_request(f.target_account, Notification.find_by(activity: f)) + activity = Follow.last + mailer_for(activity.target_account, activity).follow_request end # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/favourite def favourite - f = Favourite.last - NotificationMailer.favourite(f.status.account, Notification.find_by(activity: f)) + activity = Favourite.last + mailer_for(activity.status.account, activity).favourite end # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/reblog def reblog - r = Status.where.not(reblog_of_id: nil).first - NotificationMailer.reblog(r.reblog.account, Notification.find_by(activity: r)) + activity = Status.where.not(reblog_of_id: nil).first + mailer_for(activity.reblog.account, activity).reblog + end + + private + + def mailer_for(account, activity) + NotificationMailer.with( + recipient: account, + notification: Notification.find_by(activity: activity) + ) end end From c27b82a43763b44b0b2a2929b9cde588260581b4 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 10 Jul 2023 18:26:56 +0200 Subject: [PATCH 3/8] Add `forward_to_domains` parameter to `POST /api/v1/reports` (#25866) --- app/controllers/api/v1/reports_controller.rb | 2 +- .../mastodon/features/report/comment.jsx | 150 +++++++++++------- .../features/ui/components/report_modal.jsx | 34 ++-- .../styles/mastodon/components.scss | 1 + app/services/report_service.rb | 22 ++- spec/services/report_service_spec.rb | 24 ++- 6 files changed, 153 insertions(+), 80 deletions(-) diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb index 8ff6c8fe5..300c9faa3 100644 --- a/app/controllers/api/v1/reports_controller.rb +++ b/app/controllers/api/v1/reports_controller.rb @@ -23,6 +23,6 @@ class Api::V1::ReportsController < Api::BaseController end def report_params - params.permit(:account_id, :comment, :category, :forward, status_ids: [], rule_ids: []) + params.permit(:account_id, :comment, :category, :forward, forward_to_domains: [], status_ids: [], rule_ids: []) end end diff --git a/app/javascript/mastodon/features/report/comment.jsx b/app/javascript/mastodon/features/report/comment.jsx index 4888b76bc..98ac4caa0 100644 --- a/app/javascript/mastodon/features/report/comment.jsx +++ b/app/javascript/mastodon/features/report/comment.jsx @@ -1,87 +1,121 @@ import PropTypes from 'prop-types'; -import { PureComponent } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; -import { injectIntl, defineMessages, FormattedMessage } from 'react-intl'; +import { useIntl, defineMessages, FormattedMessage } from 'react-intl'; + +import { OrderedSet, List as ImmutableList } from 'immutable'; +import ImmutablePropTypes from 'react-immutable-proptypes'; +import { shallowEqual } from 'react-redux'; +import { createSelector } from 'reselect'; import Toggle from 'react-toggle'; +import { fetchAccount } from 'mastodon/actions/accounts'; import Button from 'mastodon/components/button'; +import { useAppDispatch, useAppSelector } from 'mastodon/store'; const messages = defineMessages({ placeholder: { id: 'report.placeholder', defaultMessage: 'Type or paste additional comments' }, }); -class Comment extends PureComponent { +const selectRepliedToAccountIds = createSelector( + [ + (state) => state.get('statuses'), + (_, statusIds) => statusIds, + ], + (statusesMap, statusIds) => statusIds.map((statusId) => statusesMap.getIn([statusId, 'in_reply_to_account_id'])), + { + resultEqualityCheck: shallowEqual, + } +); - static propTypes = { - onSubmit: PropTypes.func.isRequired, - comment: PropTypes.string.isRequired, - onChangeComment: PropTypes.func.isRequired, - intl: PropTypes.object.isRequired, - isSubmitting: PropTypes.bool, - forward: PropTypes.bool, - isRemote: PropTypes.bool, - domain: PropTypes.string, - onChangeForward: PropTypes.func.isRequired, - }; +const Comment = ({ comment, domain, statusIds, isRemote, isSubmitting, selectedDomains, onSubmit, onChangeComment, onToggleDomain }) => { + const intl = useIntl(); - handleClick = () => { - const { onSubmit } = this.props; - onSubmit(); - }; + const dispatch = useAppDispatch(); + const loadedRef = useRef(false); - handleChange = e => { - const { onChangeComment } = this.props; - onChangeComment(e.target.value); - }; + const handleClick = useCallback(() => onSubmit(), [onSubmit]); + const handleChange = useCallback((e) => onChangeComment(e.target.value), [onChangeComment]); + const handleToggleDomain = useCallback(e => onToggleDomain(e.target.value, e.target.checked), [onToggleDomain]); - handleKeyDown = e => { + const handleKeyDown = useCallback((e) => { if (e.keyCode === 13 && (e.ctrlKey || e.metaKey)) { - this.handleClick(); + handleClick(); } - }; + }, [handleClick]); - handleForwardChange = e => { - const { onChangeForward } = this.props; - onChangeForward(e.target.checked); - }; + // Memoize accountIds since we don't want it to trigger `useEffect` on each render + const accountIds = useAppSelector((state) => domain ? selectRepliedToAccountIds(state, statusIds) : ImmutableList()); - render () { - const { comment, isRemote, forward, domain, isSubmitting, intl } = this.props; + // While we could memoize `availableDomains`, it is pretty inexpensive to recompute + const accountsMap = useAppSelector((state) => state.get('accounts')); + const availableDomains = domain ? OrderedSet([domain]).union(accountIds.map((accountId) => accountsMap.getIn([accountId, 'acct'], '').split('@')[1]).filter(domain => !!domain)) : OrderedSet(); - return ( - <> -

+ useEffect(() => { + if (loadedRef.current) { + return; + } -