From c09b8a716473ff251ecd81fe6050a38133ddabb0 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 13 Mar 2024 10:11:23 -0400 Subject: [PATCH 1/4] Add `Account.without_internal` scope (#29559) Co-authored-by: Claire --- app/controllers/application_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/models/account.rb | 1 + spec/models/account_spec.rb | 8 ++++---- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a046ea19c9..8ba10d64c0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -129,7 +129,7 @@ class ApplicationController < ActionController::Base end def single_user_mode? - @single_user_mode ||= Rails.configuration.x.single_user_mode && Account.where('id > 0').exists? + @single_user_mode ||= Rails.configuration.x.single_user_mode && Account.without_internal.exists? end def use_seamless_external_login? diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4f7f66985d..a4f92743c5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -213,7 +213,7 @@ module ApplicationHelper state_params[:moved_to_account] = current_account.moved_to_account end - state_params[:owner] = Account.local.without_suspended.where('id > 0').first if single_user_mode? + state_params[:owner] = Account.local.without_suspended.without_internal.first if single_user_mode? json = ActiveModelSerializers::SerializableResource.new(InitialStatePresenter.new(state_params), serializer: InitialStateSerializer).to_json # rubocop:disable Rails/OutputSafety diff --git a/app/models/account.rb b/app/models/account.rb index d627fd6b64..0a4c0f3478 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -111,6 +111,7 @@ class Account < ApplicationRecord normalizes :username, with: ->(username) { username.squish } + scope :without_internal, -> { where(id: 1...) } scope :remote, -> { where.not(domain: nil) } scope :local, -> { where(domain: nil) } scope :partitioned, -> { order(Arel.sql('row_number() over (partition by domain)')) } diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index f6376eb36e..bdb33e53ce 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -888,7 +888,7 @@ RSpec.describe Account do { username: 'b', domain: 'b' }, ].map(&method(:Fabricate).curry(2).call(:account)) - expect(described_class.where('id > 0').alphabetic).to eq matches + expect(described_class.without_internal.alphabetic).to eq matches end end @@ -939,7 +939,7 @@ RSpec.describe Account do it 'returns an array of accounts who do not have a domain' do local_account = Fabricate(:account, domain: nil) _account_with_domain = Fabricate(:account, domain: 'example.com') - expect(described_class.where('id > 0').local).to contain_exactly(local_account) + expect(described_class.without_internal.local).to contain_exactly(local_account) end end @@ -950,14 +950,14 @@ RSpec.describe Account do matches[index] = Fabricate(:account, domain: matches[index]) end - expect(described_class.where('id > 0').partitioned).to match_array(matches) + expect(described_class.without_internal.partitioned).to match_array(matches) end end describe 'recent' do it 'returns a relation of accounts sorted by recent creation' do matches = Array.new(2) { Fabricate(:account) } - expect(described_class.where('id > 0').recent).to match_array(matches) + expect(described_class.without_internal.recent).to match_array(matches) end end From 6262ceeb704c9636f09c25e856638ee4441b58d9 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 13 Mar 2024 11:42:39 -0400 Subject: [PATCH 2/4] Fix `RSpec/DescribedClass` cop (#29472) --- spec/lib/feed_manager_spec.rb | 12 +++++----- spec/lib/sanitize/config_spec.rb | 2 +- spec/lib/signature_parser_spec.rb | 2 +- spec/lib/webfinger_resource_spec.rb | 4 ++-- spec/models/account_spec.rb | 2 +- spec/models/form/import_spec.rb | 2 +- spec/models/privacy_policy_spec.rb | 2 +- spec/models/tag_spec.rb | 2 +- spec/models/user_role_spec.rb | 22 +++++++++---------- spec/models/user_settings_spec.rb | 4 ++-- spec/services/post_status_service_spec.rb | 2 +- .../validators/follow_limit_validator_spec.rb | 2 +- 12 files changed, 29 insertions(+), 29 deletions(-) diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 618b6167bd..613bcb3045 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -11,7 +11,7 @@ RSpec.describe FeedManager do end it 'tracks at least as many statuses as reblogs', :skip_stub do - expect(FeedManager::REBLOG_FALLOFF).to be <= FeedManager::MAX_ITEMS + expect(described_class::REBLOG_FALLOFF).to be <= described_class::MAX_ITEMS end describe '#key' do @@ -225,12 +225,12 @@ RSpec.describe FeedManager do it 'trims timelines if they will have more than FeedManager::MAX_ITEMS' do account = Fabricate(:account) status = Fabricate(:status) - members = Array.new(FeedManager::MAX_ITEMS) { |count| [count, count] } + members = Array.new(described_class::MAX_ITEMS) { |count| [count, count] } redis.zadd("feed:home:#{account.id}", members) described_class.instance.push_to_home(account, status) - expect(redis.zcard("feed:home:#{account.id}")).to eq FeedManager::MAX_ITEMS + expect(redis.zcard("feed:home:#{account.id}")).to eq described_class::MAX_ITEMS end context 'with reblogs' do @@ -260,7 +260,7 @@ RSpec.describe FeedManager do described_class.instance.push_to_home(account, reblogged) # Fill the feed with intervening statuses - FeedManager::REBLOG_FALLOFF.times do + described_class::REBLOG_FALLOFF.times do described_class.instance.push_to_home(account, Fabricate(:status)) end @@ -321,7 +321,7 @@ RSpec.describe FeedManager do described_class.instance.push_to_home(account, reblogs.first) # Fill the feed with intervening statuses - FeedManager::REBLOG_FALLOFF.times do + described_class::REBLOG_FALLOFF.times do described_class.instance.push_to_home(account, Fabricate(:status)) end @@ -467,7 +467,7 @@ RSpec.describe FeedManager do status = Fabricate(:status, reblog: reblogged) described_class.instance.push_to_home(receiver, reblogged) - FeedManager::REBLOG_FALLOFF.times { described_class.instance.push_to_home(receiver, Fabricate(:status)) } + described_class::REBLOG_FALLOFF.times { described_class.instance.push_to_home(receiver, Fabricate(:status)) } described_class.instance.push_to_home(receiver, status) # The reblogging status should show up under normal conditions. diff --git a/spec/lib/sanitize/config_spec.rb b/spec/lib/sanitize/config_spec.rb index 550ad1c52b..2d8dc2f63b 100644 --- a/spec/lib/sanitize/config_spec.rb +++ b/spec/lib/sanitize/config_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' describe Sanitize::Config do describe '::MASTODON_STRICT' do - subject { Sanitize::Config::MASTODON_STRICT } + subject { described_class::MASTODON_STRICT } it 'converts h1 to p strong' do expect(Sanitize.fragment('

Foo

', subject)).to eq '

Foo

' diff --git a/spec/lib/signature_parser_spec.rb b/spec/lib/signature_parser_spec.rb index 08e9bea66c..3f398e8dd0 100644 --- a/spec/lib/signature_parser_spec.rb +++ b/spec/lib/signature_parser_spec.rb @@ -27,7 +27,7 @@ RSpec.describe SignatureParser do let(:header) { 'hello this is malformed!' } it 'raises an error' do - expect { subject }.to raise_error(SignatureParser::ParsingError) + expect { subject }.to raise_error(described_class::ParsingError) end end end diff --git a/spec/lib/webfinger_resource_spec.rb b/spec/lib/webfinger_resource_spec.rb index 0e2bdcb71a..442f91aad0 100644 --- a/spec/lib/webfinger_resource_spec.rb +++ b/spec/lib/webfinger_resource_spec.rb @@ -46,7 +46,7 @@ describe WebfingerResource do expect do described_class.new(resource).username - end.to raise_error(WebfingerResource::InvalidRequest) + end.to raise_error(described_class::InvalidRequest) end it 'finds the username in a valid https route' do @@ -137,7 +137,7 @@ describe WebfingerResource do expect do described_class.new(resource).username - end.to raise_error(WebfingerResource::InvalidRequest) + end.to raise_error(described_class::InvalidRequest) end end end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index bdb33e53ce..2c5df198d9 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -678,7 +678,7 @@ RSpec.describe Account do end describe 'MENTION_RE' do - subject { Account::MENTION_RE } + subject { described_class::MENTION_RE } it 'matches usernames in the middle of a sentence' do expect(subject.match('Hello to @alice from me')[1]).to eq 'alice' diff --git a/spec/models/form/import_spec.rb b/spec/models/form/import_spec.rb index 872697485e..c2f41d442c 100644 --- a/spec/models/form/import_spec.rb +++ b/spec/models/form/import_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Form::Import do it 'has errors' do subject.validate - expect(subject.errors[:data]).to include(I18n.t('imports.errors.over_rows_processing_limit', count: Form::Import::ROWS_PROCESSING_LIMIT)) + expect(subject.errors[:data]).to include(I18n.t('imports.errors.over_rows_processing_limit', count: described_class::ROWS_PROCESSING_LIMIT)) end end diff --git a/spec/models/privacy_policy_spec.rb b/spec/models/privacy_policy_spec.rb index 0d74713755..03bbe7264b 100644 --- a/spec/models/privacy_policy_spec.rb +++ b/spec/models/privacy_policy_spec.rb @@ -8,7 +8,7 @@ describe PrivacyPolicy do it 'has the privacy text' do policy = described_class.current - expect(policy.text).to eq(PrivacyPolicy::DEFAULT_PRIVACY_POLICY) + expect(policy.text).to eq(described_class::DEFAULT_PRIVACY_POLICY) end end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 69aaeed0af..5a1d620884 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Tag do end describe 'HASHTAG_RE' do - subject { Tag::HASHTAG_RE } + subject { described_class::HASHTAG_RE } it 'does not match URLs with anchors with non-hashtag characters' do expect(subject.match('Check this out https://medium.com/@alice/some-article#.abcdef123')).to be_nil diff --git a/spec/models/user_role_spec.rb b/spec/models/user_role_spec.rb index 96d12263ae..4ab66c3260 100644 --- a/spec/models/user_role_spec.rb +++ b/spec/models/user_role_spec.rb @@ -8,7 +8,7 @@ RSpec.describe UserRole do describe '#can?' do context 'with a single flag' do it 'returns true if any of them are present' do - subject.permissions = UserRole::FLAGS[:manage_reports] + subject.permissions = described_class::FLAGS[:manage_reports] expect(subject.can?(:manage_reports)).to be true end @@ -19,7 +19,7 @@ RSpec.describe UserRole do context 'with multiple flags' do it 'returns true if any of them are present' do - subject.permissions = UserRole::FLAGS[:manage_users] + subject.permissions = described_class::FLAGS[:manage_users] expect(subject.can?(:manage_reports, :manage_users)).to be true end @@ -51,7 +51,7 @@ RSpec.describe UserRole do describe '#permissions_as_keys' do before do - subject.permissions = UserRole::FLAGS[:invite_users] | UserRole::FLAGS[:view_dashboard] | UserRole::FLAGS[:manage_reports] + subject.permissions = described_class::FLAGS[:invite_users] | described_class::FLAGS[:view_dashboard] | described_class::FLAGS[:manage_reports] end it 'returns an array' do @@ -70,7 +70,7 @@ RSpec.describe UserRole do let(:input) { %w(manage_users) } it 'sets permission flags' do - expect(subject.permissions).to eq UserRole::FLAGS[:manage_users] + expect(subject.permissions).to eq described_class::FLAGS[:manage_users] end end @@ -78,7 +78,7 @@ RSpec.describe UserRole do let(:input) { %w(manage_users manage_reports) } it 'sets permission flags' do - expect(subject.permissions).to eq UserRole::FLAGS[:manage_users] | UserRole::FLAGS[:manage_reports] + expect(subject.permissions).to eq described_class::FLAGS[:manage_users] | described_class::FLAGS[:manage_reports] end end @@ -86,7 +86,7 @@ RSpec.describe UserRole do let(:input) { %w(foo) } it 'does not set permission flags' do - expect(subject.permissions).to eq UserRole::Flags::NONE + expect(subject.permissions).to eq described_class::Flags::NONE end end end @@ -96,7 +96,7 @@ RSpec.describe UserRole do subject { described_class.nobody } it 'returns none' do - expect(subject.computed_permissions).to eq UserRole::Flags::NONE + expect(subject.computed_permissions).to eq described_class::Flags::NONE end end @@ -110,11 +110,11 @@ RSpec.describe UserRole do context 'when role has the administrator flag' do before do - subject.permissions = UserRole::FLAGS[:administrator] + subject.permissions = described_class::FLAGS[:administrator] end it 'returns all permissions' do - expect(subject.computed_permissions).to eq UserRole::Flags::ALL + expect(subject.computed_permissions).to eq described_class::Flags::ALL end end @@ -135,7 +135,7 @@ RSpec.describe UserRole do end it 'has default permissions' do - expect(subject.permissions).to eq UserRole::FLAGS[:invite_users] + expect(subject.permissions).to eq described_class::FLAGS[:invite_users] end it 'has negative position' do @@ -155,7 +155,7 @@ RSpec.describe UserRole do end it 'has no permissions' do - expect(subject.permissions).to eq UserRole::Flags::NONE + expect(subject.permissions).to eq described_class::Flags::NONE end it 'has negative position' do diff --git a/spec/models/user_settings_spec.rb b/spec/models/user_settings_spec.rb index 653597c90d..dfc4910d6e 100644 --- a/spec/models/user_settings_spec.rb +++ b/spec/models/user_settings_spec.rb @@ -24,7 +24,7 @@ RSpec.describe UserSettings do context 'when setting was not defined' do it 'raises error' do - expect { subject[:foo] }.to raise_error UserSettings::KeyError + expect { subject[:foo] }.to raise_error described_class::KeyError end end end @@ -93,7 +93,7 @@ RSpec.describe UserSettings do describe '.definition_for' do context 'when key is defined' do it 'returns a setting' do - expect(described_class.definition_for(:always_send_emails)).to be_a UserSettings::Setting + expect(described_class.definition_for(:always_send_emails)).to be_a described_class::Setting end end diff --git a/spec/services/post_status_service_spec.rb b/spec/services/post_status_service_spec.rb index ee68092a36..3c2e4f3a79 100644 --- a/spec/services/post_status_service_spec.rb +++ b/spec/services/post_status_service_spec.rb @@ -150,7 +150,7 @@ RSpec.describe PostStatusService do expect do subject.call(account, text: '@alice hm, @bob is really annoying lately', allowed_mentions: [mentioned_account.id]) - end.to raise_error(an_instance_of(PostStatusService::UnexpectedMentionsError).and(having_attributes(accounts: [unexpected_mentioned_account]))) + end.to raise_error(an_instance_of(described_class::UnexpectedMentionsError).and(having_attributes(accounts: [unexpected_mentioned_account]))) end it 'processes duplicate mentions correctly' do diff --git a/spec/validators/follow_limit_validator_spec.rb b/spec/validators/follow_limit_validator_spec.rb index 51b0683d27..e069b0ed3a 100644 --- a/spec/validators/follow_limit_validator_spec.rb +++ b/spec/validators/follow_limit_validator_spec.rb @@ -56,7 +56,7 @@ RSpec.describe FollowLimitValidator do follow.valid? - expect(follow.errors[:base]).to include(I18n.t('users.follow_limit_reached', limit: FollowLimitValidator::LIMIT)) + expect(follow.errors[:base]).to include(I18n.t('users.follow_limit_reached', limit: described_class::LIMIT)) end end From 71e5f0f48c3bc95a894fa3ad2c5a34f05c584482 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 13 Mar 2024 11:43:40 -0400 Subject: [PATCH 3/4] Add coverage for suspended instance actor scenario (#29571) --- spec/controllers/instance_actors_controller_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/controllers/instance_actors_controller_spec.rb b/spec/controllers/instance_actors_controller_spec.rb index 70aaff9d65..42ffb67988 100644 --- a/spec/controllers/instance_actors_controller_spec.rb +++ b/spec/controllers/instance_actors_controller_spec.rb @@ -41,6 +41,14 @@ RSpec.describe InstanceActorsController do it_behaves_like 'shared behavior' end + + context 'with a suspended instance actor' do + let(:authorized_fetch_mode) { false } + + before { Account.representative.update(suspended_at: 10.days.ago) } + + it_behaves_like 'shared behavior' + end end end end From a32a126cac42c73236236b5a9bd660765b9c58ee Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 13 Mar 2024 17:47:48 +0100 Subject: [PATCH 4/4] Hide media by default in notification requests (#29572) --- app/javascript/mastodon/components/status.jsx | 25 ++++++----- .../features/notifications/request.jsx | 41 ++++++++++--------- .../ui/util/sensitive_media_context.tsx | 28 +++++++++++++ 3 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 app/javascript/mastodon/features/ui/util/sensitive_media_context.tsx diff --git a/app/javascript/mastodon/components/status.jsx b/app/javascript/mastodon/components/status.jsx index be9a1cec65..7b97e45766 100644 --- a/app/javascript/mastodon/components/status.jsx +++ b/app/javascript/mastodon/components/status.jsx @@ -22,6 +22,7 @@ import Card from '../features/status/components/card'; // to use the progress bar to show download progress import Bundle from '../features/ui/components/bundle'; import { MediaGallery, Video, Audio } from '../features/ui/util/async-components'; +import { SensitiveMediaContext } from '../features/ui/util/sensitive_media_context'; import { displayMedia } from '../initial_state'; import { Avatar } from './avatar'; @@ -78,6 +79,8 @@ const messages = defineMessages({ class Status extends ImmutablePureComponent { + static contextType = SensitiveMediaContext; + static propTypes = { status: ImmutablePropTypes.map, account: ImmutablePropTypes.record, @@ -133,19 +136,21 @@ class Status extends ImmutablePureComponent { ]; state = { - showMedia: defaultMediaVisibility(this.props.status), - statusId: undefined, + showMedia: defaultMediaVisibility(this.props.status) && !(this.context?.hideMediaByDefault), forceFilter: undefined, }; - static getDerivedStateFromProps(nextProps, prevState) { - if (nextProps.status && nextProps.status.get('id') !== prevState.statusId) { - return { - showMedia: defaultMediaVisibility(nextProps.status), - statusId: nextProps.status.get('id'), - }; - } else { - return null; + componentDidUpdate (prevProps) { + // This will potentially cause a wasteful redraw, but in most cases `Status` components are used + // with a `key` directly depending on their `id`, preventing re-use of the component across + // different IDs. + // But just in case this does change, reset the state on status change. + + if (this.props.status?.get('id') !== prevProps.status?.get('id')) { + this.setState({ + showMedia: defaultMediaVisibility(this.props.status) && !(this.context?.hideMediaByDefault), + forceFilter: undefined, + }); } } diff --git a/app/javascript/mastodon/features/notifications/request.jsx b/app/javascript/mastodon/features/notifications/request.jsx index 5977a6ce96..da9ed21e55 100644 --- a/app/javascript/mastodon/features/notifications/request.jsx +++ b/app/javascript/mastodon/features/notifications/request.jsx @@ -15,6 +15,7 @@ import Column from 'mastodon/components/column'; import ColumnHeader from 'mastodon/components/column_header'; import { IconButton } from 'mastodon/components/icon_button'; import ScrollableList from 'mastodon/components/scrollable_list'; +import { SensitiveMediaContextProvider } from 'mastodon/features/ui/util/sensitive_media_context'; import NotificationContainer from './containers/notification_container'; @@ -106,25 +107,27 @@ export const NotificationRequest = ({ multiColumn, params: { id } }) => { )} /> - - {notifications.map(item => ( - item && - ))} - + + + {notifications.map(item => ( + item && + ))} + + {columnTitle} diff --git a/app/javascript/mastodon/features/ui/util/sensitive_media_context.tsx b/app/javascript/mastodon/features/ui/util/sensitive_media_context.tsx new file mode 100644 index 0000000000..408154c31b --- /dev/null +++ b/app/javascript/mastodon/features/ui/util/sensitive_media_context.tsx @@ -0,0 +1,28 @@ +import { createContext, useContext, useMemo } from 'react'; + +export const SensitiveMediaContext = createContext<{ + hideMediaByDefault: boolean; +}>({ + hideMediaByDefault: false, +}); + +export function useSensitiveMediaContext() { + return useContext(SensitiveMediaContext); +} + +type ContextValue = React.ContextType; + +export const SensitiveMediaContextProvider: React.FC< + React.PropsWithChildren<{ hideMediaByDefault: boolean }> +> = ({ hideMediaByDefault, children }) => { + const contextValue = useMemo( + () => ({ hideMediaByDefault }), + [hideMediaByDefault], + ); + + return ( + + {children} + + ); +};