From 08a376cbcbb2fa46c32c43937b77ddb081418af8 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 10:36:28 -0400 Subject: [PATCH 01/13] Fix `Style/CombinableLoops` cop (#27429) --- .rubocop_todo.yml | 6 ------ app/models/form/custom_emoji_batch.rb | 18 +++++++++++------- app/models/form/ip_block_batch.rb | 6 +++++- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 29d9f484c0..980339f496 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -517,12 +517,6 @@ Style/ClassVars: Exclude: - 'config/initializers/devise.rb' -# This cop supports unsafe autocorrection (--autocorrect-all). -Style/CombinableLoops: - Exclude: - - 'app/models/form/custom_emoji_batch.rb' - - 'app/models/form/ip_block_batch.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AllowedVars. Style/FetchEnvVar: diff --git a/app/models/form/custom_emoji_batch.rb b/app/models/form/custom_emoji_batch.rb index 484415f902..c63996e069 100644 --- a/app/models/form/custom_emoji_batch.rb +++ b/app/models/form/custom_emoji_batch.rb @@ -34,7 +34,7 @@ class Form::CustomEmojiBatch end def update! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + verify_authorization(:update?) category = if category_id.present? CustomEmojiCategory.find(category_id) @@ -49,7 +49,7 @@ class Form::CustomEmojiBatch end def list! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + verify_authorization(:update?) custom_emojis.each do |custom_emoji| custom_emoji.update(visible_in_picker: true) @@ -58,7 +58,7 @@ class Form::CustomEmojiBatch end def unlist! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + verify_authorization(:update?) custom_emojis.each do |custom_emoji| custom_emoji.update(visible_in_picker: false) @@ -67,7 +67,7 @@ class Form::CustomEmojiBatch end def enable! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :enable?) } + verify_authorization(:enable?) custom_emojis.each do |custom_emoji| custom_emoji.update(disabled: false) @@ -76,7 +76,7 @@ class Form::CustomEmojiBatch end def disable! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :disable?) } + verify_authorization(:disable?) custom_emojis.each do |custom_emoji| custom_emoji.update(disabled: true) @@ -85,7 +85,7 @@ class Form::CustomEmojiBatch end def copy! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :copy?) } + verify_authorization(:copy?) custom_emojis.each do |custom_emoji| copied_custom_emoji = custom_emoji.copy! @@ -94,11 +94,15 @@ class Form::CustomEmojiBatch end def delete! - custom_emojis.each { |custom_emoji| authorize(custom_emoji, :destroy?) } + verify_authorization(:destroy?) custom_emojis.each do |custom_emoji| custom_emoji.destroy log_action :destroy, custom_emoji end end + + def verify_authorization(permission) + custom_emojis.each { |custom_emoji| authorize(custom_emoji, permission) } + end end diff --git a/app/models/form/ip_block_batch.rb b/app/models/form/ip_block_batch.rb index f6fe9b5935..bdfeb91c8a 100644 --- a/app/models/form/ip_block_batch.rb +++ b/app/models/form/ip_block_batch.rb @@ -21,11 +21,15 @@ class Form::IpBlockBatch end def delete! - ip_blocks.each { |ip_block| authorize(ip_block, :destroy?) } + verify_authorization(:destroy?) ip_blocks.each do |ip_block| ip_block.destroy log_action :destroy, ip_block end end + + def verify_authorization(permission) + ip_blocks.each { |ip_block| authorize(ip_block, permission) } + end end From e0ed0f8c7c6d74534ba896ebe0d3a25495f27f0d Mon Sep 17 00:00:00 2001 From: Daniel M Brasil Date: Mon, 16 Oct 2023 12:15:24 -0300 Subject: [PATCH 02/13] Migrate to request specs in `/api/v1/notifications` (#25553) --- .../api/v1/notifications_controller_spec.rb | 137 ------------- spec/requests/api/v1/notifications_spec.rb | 183 ++++++++++++++++++ 2 files changed, 183 insertions(+), 137 deletions(-) delete mode 100644 spec/controllers/api/v1/notifications_controller_spec.rb create mode 100644 spec/requests/api/v1/notifications_spec.rb diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb deleted file mode 100644 index 6615848b83..0000000000 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ /dev/null @@ -1,137 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Api::V1::NotificationsController do - render_views - - let(:user) { Fabricate(:user, account_attributes: { username: 'alice' }) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } - let(:other) { Fabricate(:user) } - let(:third) { Fabricate(:user) } - - before do - allow(controller).to receive(:doorkeeper_token) { token } - end - - describe 'GET #show' do - let(:scopes) { 'read:notifications' } - - it 'returns http success' do - notification = Fabricate(:notification, account: user.account) - get :show, params: { id: notification.id } - - expect(response).to have_http_status(200) - end - end - - describe 'POST #dismiss' do - let(:scopes) { 'write:notifications' } - - it 'destroys the notification' do - notification = Fabricate(:notification, account: user.account) - post :dismiss, params: { id: notification.id } - - expect(response).to have_http_status(200) - expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - end - - describe 'POST #clear' do - let(:scopes) { 'write:notifications' } - - it 'clears notifications for the account' do - notification = Fabricate(:notification, account: user.account) - post :clear - - expect(notification.account.reload.notifications).to be_empty - expect(response).to have_http_status(200) - end - end - - describe 'GET #index' do - let(:scopes) { 'read:notifications' } - - before do - first_status = PostStatusService.new.call(user.account, text: 'Test') - @reblog_of_first_status = ReblogService.new.call(other.account, first_status) - mentioning_status = PostStatusService.new.call(other.account, text: 'Hello @alice') - @mention_from_status = mentioning_status.mentions.first - @favourite = FavouriteService.new.call(other.account, first_status) - @second_favourite = FavouriteService.new.call(third.account, first_status) - @follow = FollowService.new.call(other.account, user.account) - end - - describe 'with no options' do - before do - get :index - end - - it 'returns expected notification types', :aggregate_failures do - expect(response).to have_http_status(200) - - expect(body_json_types).to include 'reblog' - expect(body_json_types).to include 'mention' - expect(body_json_types).to include 'favourite' - expect(body_json_types).to include 'follow' - end - end - - describe 'with account_id param' do - before do - get :index, params: { account_id: third.account.id } - end - - it 'returns only notifications from specified user', :aggregate_failures do - expect(response).to have_http_status(200) - - expect(body_json_account_ids.uniq).to eq [third.account.id.to_s] - end - - def body_json_account_ids - body_as_json.map { |x| x[:account][:id] } - end - end - - describe 'with invalid account_id param' do - before do - get :index, params: { account_id: 'foo' } - end - - it 'returns nothing', :aggregate_failures do - expect(response).to have_http_status(200) - - expect(body_as_json.size).to eq 0 - end - end - - describe 'with exclude_types param' do - before do - get :index, params: { exclude_types: %w(mention) } - end - - it 'returns everything but excluded type', :aggregate_failures do - expect(response).to have_http_status(200) - - expect(body_as_json.size).to_not eq 0 - expect(body_json_types.uniq).to_not include 'mention' - end - end - - describe 'with types param' do - before do - get :index, params: { types: %w(mention) } - end - - it 'returns only requested type', :aggregate_failures do - expect(response).to have_http_status(200) - - expect(body_json_types.uniq).to eq ['mention'] - end - end - - def body_json_types - body_as_json.pluck(:type) - end - end -end diff --git a/spec/requests/api/v1/notifications_spec.rb b/spec/requests/api/v1/notifications_spec.rb new file mode 100644 index 0000000000..7a879c35b7 --- /dev/null +++ b/spec/requests/api/v1/notifications_spec.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Notifications' do + let(:user) { Fabricate(:user, account_attributes: { username: 'alice' }) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:scopes) { 'read:notifications write:notifications' } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + describe 'GET /api/v1/notifications' do + subject do + get '/api/v1/notifications', headers: headers, params: params + end + + let(:bob) { Fabricate(:user) } + let(:tom) { Fabricate(:user) } + let(:params) { {} } + + before do + first_status = PostStatusService.new.call(user.account, text: 'Test') + ReblogService.new.call(bob.account, first_status) + mentioning_status = PostStatusService.new.call(bob.account, text: 'Hello @alice') + mentioning_status.mentions.first + FavouriteService.new.call(bob.account, first_status) + FavouriteService.new.call(tom.account, first_status) + FollowService.new.call(bob.account, user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + context 'with no options' do + it 'returns expected notification types', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_json_types).to include 'reblog' + expect(body_json_types).to include 'mention' + expect(body_json_types).to include 'favourite' + expect(body_json_types).to include 'follow' + end + end + + context 'with account_id param' do + let(:params) { { account_id: tom.account.id } } + + it 'returns only notifications from specified user', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_json_account_ids.uniq).to eq [tom.account.id.to_s] + end + + def body_json_account_ids + body_as_json.map { |x| x[:account][:id] } + end + end + + context 'with invalid account_id param' do + let(:params) { { account_id: 'foo' } } + + it 'returns nothing', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json.size).to eq 0 + end + end + + context 'with exclude_types param' do + let(:params) { { exclude_types: %w(mention) } } + + it 'returns everything but excluded type', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json.size).to_not eq 0 + expect(body_json_types.uniq).to_not include 'mention' + end + end + + context 'with types param' do + let(:params) { { types: %w(mention) } } + + it 'returns only requested type', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_json_types.uniq).to eq ['mention'] + end + end + + context 'with limit param' do + let(:params) { { limit: 3 } } + + it 'returns the requested number of notifications paginated', :aggregate_failures do + subject + + notifications = user.account.notifications + + expect(body_as_json.size).to eq(params[:limit]) + expect(response.headers['Link'].find_link(%w(rel prev)).href).to eq(api_v1_notifications_url(limit: params[:limit], min_id: notifications.last.id.to_s)) + expect(response.headers['Link'].find_link(%w(rel next)).href).to eq(api_v1_notifications_url(limit: params[:limit], max_id: notifications[2].id.to_s)) + end + end + + def body_json_types + body_as_json.pluck(:type) + end + end + + describe 'GET /api/v1/notifications/:id' do + subject do + get "/api/v1/notifications/#{notification.id}", headers: headers + end + + let(:notification) { Fabricate(:notification, account: user.account) } + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + it 'returns http success' do + subject + + expect(response).to have_http_status(200) + end + + context 'when notification belongs to someone else' do + let(:notification) { Fabricate(:notification) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v1/notifications/:id/dismiss' do + subject do + post "/api/v1/notifications/#{notification.id}/dismiss", headers: headers + end + + let!(:notification) { Fabricate(:notification, account: user.account) } + + it_behaves_like 'forbidden for wrong scope', 'read read:notifications' + + it 'destroys the notification' do + subject + + expect(response).to have_http_status(200) + expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'when notification belongs to someone else' do + let(:notification) { Fabricate(:notification) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v1/notifications/clear' do + subject do + post '/api/v1/notifications/clear', headers: headers + end + + before do + Fabricate.times(3, :notification, account: user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'read read:notifications' + + it 'clears notifications for the account' do + subject + + expect(user.account.reload.notifications).to be_empty + expect(response).to have_http_status(200) + end + end +end From 708299bb0d17ec60f51226f43c26bd66e6cb8adc Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 11:20:28 -0400 Subject: [PATCH 03/13] Misc coverage improvements for `Admin::` and `Settings::` controllers (#25346) --- .../admin/disputes/appeals_controller_spec.rb | 10 ++++ .../admin/domain_blocks_controller_spec.rb | 11 ++++ .../export_domain_allows_controller_spec.rb | 8 +++ .../export_domain_blocks_controller_spec.rb | 8 +++ .../admin/instances_controller_spec.rb | 57 +++++++++++++++++++ .../admin/settings/about_controller_spec.rb | 8 +++ .../settings/appearance_controller_spec.rb | 8 +++ .../content_retention_controller_spec.rb | 8 +++ .../settings/discovery_controller_spec.rb | 8 +++ .../settings/registrations_controller_spec.rb | 8 +++ .../controllers/admin/tags_controller_spec.rb | 22 +++++++ .../admin/webhooks_controller_spec.rb | 18 ++++++ .../settings/imports_controller_spec.rb | 13 +++++ 13 files changed, 187 insertions(+) diff --git a/spec/controllers/admin/disputes/appeals_controller_spec.rb b/spec/controllers/admin/disputes/appeals_controller_spec.rb index 3c3f23f529..4afe074921 100644 --- a/spec/controllers/admin/disputes/appeals_controller_spec.rb +++ b/spec/controllers/admin/disputes/appeals_controller_spec.rb @@ -15,6 +15,16 @@ RSpec.describe Admin::Disputes::AppealsController do let(:strike) { Fabricate(:account_warning, target_account: target_account, action: :suspend) } let(:appeal) { Fabricate(:appeal, strike: strike, account: target_account) } + describe 'GET #index' do + let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } + + it 'lists appeals' do + get :index + + expect(response).to have_http_status(200) + end + end + describe 'POST #approve' do let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 9be55906ed..13826be366 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -165,6 +165,17 @@ RSpec.describe Admin::DomainBlocksController do end end + describe 'GET #edit' do + let(:domain_block) { Fabricate(:domain_block) } + + it 'returns http success' do + get :edit, params: { id: domain_block.id } + + expect(assigns(:domain_block)).to be_instance_of(DomainBlock) + expect(response).to have_http_status(200) + end + end + describe 'PUT #update' do subject do post :update, params: { :id => domain_block.id, :domain_block => { domain: 'example.com', severity: new_severity }, 'confirm' => '' } diff --git a/spec/controllers/admin/export_domain_allows_controller_spec.rb b/spec/controllers/admin/export_domain_allows_controller_spec.rb index 9d50c04aad..e1e5ecc1f0 100644 --- a/spec/controllers/admin/export_domain_allows_controller_spec.rb +++ b/spec/controllers/admin/export_domain_allows_controller_spec.rb @@ -9,6 +9,14 @@ RSpec.describe Admin::ExportDomainAllowsController do sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin')), scope: :user end + describe 'GET #new' do + it 'returns http success' do + get :new + + expect(response).to have_http_status(200) + end + end + describe 'GET #export' do it 'renders instances' do Fabricate(:domain_allow, domain: 'good.domain') diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb index 1a63077736..5a282c9572 100644 --- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -9,6 +9,14 @@ RSpec.describe Admin::ExportDomainBlocksController do sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin')), scope: :user end + describe 'GET #new' do + it 'returns http success' do + get :new + + expect(response).to have_http_status(200) + end + end + describe 'GET #export' do it 'renders instances' do Fabricate(:domain_block, domain: 'bad.domain', severity: 'silence', public_comment: 'bad server') diff --git a/spec/controllers/admin/instances_controller_spec.rb b/spec/controllers/admin/instances_controller_spec.rb index dd772d1036..5fed5d98d2 100644 --- a/spec/controllers/admin/instances_controller_spec.rb +++ b/spec/controllers/admin/instances_controller_spec.rb @@ -34,6 +34,63 @@ RSpec.describe Admin::InstancesController do end end + describe 'GET #show' do + it 'shows an instance page' do + get :show, params: { id: account_popular_main.domain } + + expect(response).to have_http_status(200) + end + end + + describe 'POST #clear_delivery_errors' do + let(:tracker) { instance_double(DeliveryFailureTracker, clear_failures!: true) } + + before { allow(DeliveryFailureTracker).to receive(:new).and_return(tracker) } + + it 'clears instance delivery errors' do + post :clear_delivery_errors, params: { id: account_popular_main.domain } + + expect(response).to redirect_to(admin_instance_path(account_popular_main.domain)) + expect(tracker).to have_received(:clear_failures!) + end + end + + describe 'POST #restart_delivery' do + let(:tracker) { instance_double(DeliveryFailureTracker, track_success!: true) } + + before { allow(DeliveryFailureTracker).to receive(:new).and_return(tracker) } + + context 'with an unavailable instance' do + before { Fabricate(:unavailable_domain, domain: account_popular_main.domain) } + + it 'tracks success on the instance' do + post :restart_delivery, params: { id: account_popular_main.domain } + + expect(response).to redirect_to(admin_instance_path(account_popular_main.domain)) + expect(tracker).to have_received(:track_success!) + end + end + + context 'with an available instance' do + it 'does not track success on the instance' do + post :restart_delivery, params: { id: account_popular_main.domain } + + expect(response).to redirect_to(admin_instance_path(account_popular_main.domain)) + expect(tracker).to_not have_received(:track_success!) + end + end + end + + describe 'POST #stop_delivery' do + it 'clears instance delivery errors' do + expect do + post :stop_delivery, params: { id: account_popular_main.domain } + end.to change(UnavailableDomain, :count).by(1) + + expect(response).to redirect_to(admin_instance_path(account_popular_main.domain)) + end + end + describe 'DELETE #destroy' do subject { delete :destroy, params: { id: Instance.first.id } } diff --git a/spec/controllers/admin/settings/about_controller_spec.rb b/spec/controllers/admin/settings/about_controller_spec.rb index 2ae26090b6..f322cb4434 100644 --- a/spec/controllers/admin/settings/about_controller_spec.rb +++ b/spec/controllers/admin/settings/about_controller_spec.rb @@ -18,4 +18,12 @@ describe Admin::Settings::AboutController do expect(response).to have_http_status(:success) end end + + describe 'PUT #update' do + it 'updates the settings' do + put :update, params: { form_admin_settings: { site_extended_description: 'new site description' } } + + expect(response).to redirect_to(admin_settings_about_path) + end + end end diff --git a/spec/controllers/admin/settings/appearance_controller_spec.rb b/spec/controllers/admin/settings/appearance_controller_spec.rb index 65b29acc3e..ea6f3b7833 100644 --- a/spec/controllers/admin/settings/appearance_controller_spec.rb +++ b/spec/controllers/admin/settings/appearance_controller_spec.rb @@ -18,4 +18,12 @@ describe Admin::Settings::AppearanceController do expect(response).to have_http_status(:success) end end + + describe 'PUT #update' do + it 'updates the settings' do + put :update, params: { form_admin_settings: { custom_css: 'html { display: inline; }' } } + + expect(response).to redirect_to(admin_settings_appearance_path) + end + end end diff --git a/spec/controllers/admin/settings/content_retention_controller_spec.rb b/spec/controllers/admin/settings/content_retention_controller_spec.rb index 53ce84d189..fb6a3d2848 100644 --- a/spec/controllers/admin/settings/content_retention_controller_spec.rb +++ b/spec/controllers/admin/settings/content_retention_controller_spec.rb @@ -18,4 +18,12 @@ describe Admin::Settings::ContentRetentionController do expect(response).to have_http_status(:success) end end + + describe 'PUT #update' do + it 'updates the settings' do + put :update, params: { form_admin_settings: { media_cache_retention_period: '2' } } + + expect(response).to redirect_to(admin_settings_content_retention_path) + end + end end diff --git a/spec/controllers/admin/settings/discovery_controller_spec.rb b/spec/controllers/admin/settings/discovery_controller_spec.rb index c7307ffc88..33109e3c01 100644 --- a/spec/controllers/admin/settings/discovery_controller_spec.rb +++ b/spec/controllers/admin/settings/discovery_controller_spec.rb @@ -18,4 +18,12 @@ describe Admin::Settings::DiscoveryController do expect(response).to have_http_status(:success) end end + + describe 'PUT #update' do + it 'updates the settings' do + put :update, params: { form_admin_settings: { trends: '1' } } + + expect(response).to redirect_to(admin_settings_discovery_path) + end + end end diff --git a/spec/controllers/admin/settings/registrations_controller_spec.rb b/spec/controllers/admin/settings/registrations_controller_spec.rb index 3fc1f9d132..e076544603 100644 --- a/spec/controllers/admin/settings/registrations_controller_spec.rb +++ b/spec/controllers/admin/settings/registrations_controller_spec.rb @@ -18,4 +18,12 @@ describe Admin::Settings::RegistrationsController do expect(response).to have_http_status(:success) end end + + describe 'PUT #update' do + it 'updates the settings' do + put :update, params: { form_admin_settings: { registrations_mode: 'open' } } + + expect(response).to redirect_to(admin_settings_registrations_path) + end + end end diff --git a/spec/controllers/admin/tags_controller_spec.rb b/spec/controllers/admin/tags_controller_spec.rb index 313298f14a..4e06adaca6 100644 --- a/spec/controllers/admin/tags_controller_spec.rb +++ b/spec/controllers/admin/tags_controller_spec.rb @@ -20,4 +20,26 @@ RSpec.describe Admin::TagsController do expect(response).to have_http_status(200) end end + + describe 'PUT #update' do + let!(:tag) { Fabricate(:tag, listable: false) } + + context 'with valid params' do + it 'updates the tag' do + put :update, params: { id: tag.id, tag: { listable: '1' } } + + expect(response).to redirect_to(admin_tag_path(tag.id)) + expect(tag.reload).to be_listable + end + end + + context 'with invalid params' do + it 'does not update the tag' do + put :update, params: { id: tag.id, tag: { name: 'cant-change-name' } } + + expect(response).to have_http_status(200) + expect(response).to render_template(:show) + end + end + end end diff --git a/spec/controllers/admin/webhooks_controller_spec.rb b/spec/controllers/admin/webhooks_controller_spec.rb index 0ccfbbcc6e..17d8506025 100644 --- a/spec/controllers/admin/webhooks_controller_spec.rb +++ b/spec/controllers/admin/webhooks_controller_spec.rb @@ -86,6 +86,24 @@ describe Admin::WebhooksController do end end + describe 'POST #enable' do + it 'enables the webhook' do + post :enable, params: { id: webhook.id } + + expect(webhook.reload).to be_enabled + expect(response).to redirect_to(admin_webhook_path(webhook)) + end + end + + describe 'POST #disable' do + it 'disables the webhook' do + post :disable, params: { id: webhook.id } + + expect(webhook.reload).to_not be_enabled + expect(response).to redirect_to(admin_webhook_path(webhook)) + end + end + describe 'DELETE #destroy' do it 'destroys the record' do expect do diff --git a/spec/controllers/settings/imports_controller_spec.rb b/spec/controllers/settings/imports_controller_spec.rb index 76e1e4ecb0..35d2f08193 100644 --- a/spec/controllers/settings/imports_controller_spec.rb +++ b/spec/controllers/settings/imports_controller_spec.rb @@ -252,6 +252,19 @@ RSpec.describe Settings::ImportsController do include_examples 'export failed rows', "https://foo.com/1\nhttps://foo.com/2\n" end + + context 'with lists' do + let(:import_type) { 'lists' } + + let!(:rows) do + [ + { 'list_name' => 'Amigos', 'acct' => 'user@example.com' }, + { 'list_name' => 'Frenemies', 'acct' => 'user@org.org' }, + ].map { |data| Fabricate(:bulk_import_row, bulk_import: bulk_import, data: data) } + end + + include_examples 'export failed rows', "Amigos,user@example.com\nFrenemies,user@org.org\n" + end end describe 'POST #create' do From 893b2f33fd92a320146e6abf172e21b1b340ae24 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 11:52:06 -0400 Subject: [PATCH 04/13] Extract shared example for cacheable response in specs (#25388) --- spec/controllers/accounts_controller_spec.rb | 25 +++---------------- .../collections_controller_spec.rb | 16 ------------ .../activitypub/outboxes_controller_spec.rb | 16 ------------ .../activitypub/replies_controller_spec.rb | 16 ------------ spec/controllers/statuses_controller_spec.rb | 23 ++--------------- spec/support/examples/cache.rb | 22 ++++++++++++++++ 6 files changed, 27 insertions(+), 91 deletions(-) create mode 100644 spec/support/examples/cache.rb diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index 5a8585b069..d46fdb108d 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -7,25 +7,6 @@ RSpec.describe AccountsController do let(:account) { Fabricate(:account) } - shared_examples 'cacheable response' do - it 'does not set cookies' do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - end - - it 'does not set sessions' do - expect(session).to be_empty - end - - it 'returns Vary header' do - expect(response.headers['Vary']).to include 'Accept' - end - - it 'returns public Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'public' - end - end - describe 'GET #show' do let(:format) { 'html' } @@ -186,7 +167,7 @@ RSpec.describe AccountsController do expect(response.media_type).to eq 'application/activity+json' end - it_behaves_like 'cacheable response' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'renders account' do json = body_as_json @@ -244,7 +225,7 @@ RSpec.describe AccountsController do expect(response.media_type).to eq 'application/activity+json' end - it_behaves_like 'cacheable response' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'renders account' do json = body_as_json @@ -311,7 +292,7 @@ RSpec.describe AccountsController do expect(response).to have_http_status(200) end - it_behaves_like 'cacheable response' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end context 'with a normal account in an RSS request' do diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb index e2802cf565..cf484ff5a4 100644 --- a/spec/controllers/activitypub/collections_controller_spec.rb +++ b/spec/controllers/activitypub/collections_controller_spec.rb @@ -7,22 +7,6 @@ RSpec.describe ActivityPub::CollectionsController do let!(:private_pinned) { Fabricate(:status, account: account, text: 'secret private stuff', visibility: :private) } let(:remote_account) { nil } - shared_examples 'cacheable response' do - it 'does not set cookies' do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - end - - it 'does not set sessions' do - response - expect(session).to be_empty - end - - it 'returns public Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'public' - end - end - before do allow(controller).to receive(:signed_request_actor).and_return(remote_account) diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index 6946fdfcff..53c4f0c09c 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -5,22 +5,6 @@ require 'rails_helper' RSpec.describe ActivityPub::OutboxesController do let!(:account) { Fabricate(:account) } - shared_examples 'cacheable response' do - it 'does not set cookies' do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - end - - it 'does not set sessions' do - response - expect(session).to be_empty - end - - it 'returns public Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'public' - end - end - before do Fabricate(:status, account: account, visibility: :public) Fabricate(:status, account: account, visibility: :unlisted) diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb index c7b65f004d..7e6e0ffb0d 100644 --- a/spec/controllers/activitypub/replies_controller_spec.rb +++ b/spec/controllers/activitypub/replies_controller_spec.rb @@ -8,22 +8,6 @@ RSpec.describe ActivityPub::RepliesController do let(:remote_reply_id) { 'https://foobar.com/statuses/1234' } let(:remote_querier) { nil } - shared_examples 'cacheable response' do - it 'does not set cookies' do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - end - - it 'does not set sessions' do - response - expect(session).to be_empty - end - - it 'returns public Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'public' - end - end - shared_examples 'common behavior' do context 'when status is private' do let(:parent_visibility) { :private } diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index bd98929c02..8b715824b8 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -5,25 +5,6 @@ require 'rails_helper' describe StatusesController do render_views - shared_examples 'cacheable response' do - it 'does not set cookies' do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - end - - it 'does not set sessions' do - expect(session).to be_empty - end - - it 'returns Vary header' do - expect(response.headers['Vary']).to include 'Accept, Accept-Language, Cookie' - end - - it 'returns public Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'public' - end - end - describe 'GET #show' do let(:account) { Fabricate(:account) } let(:status) { Fabricate(:status, account: account) } @@ -88,7 +69,7 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - it_behaves_like 'cacheable response' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'renders ActivityPub Note object successfully', :aggregate_failures do expect(response).to have_http_status(200) @@ -371,7 +352,7 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - it_behaves_like 'cacheable response' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'renders ActivityPub Note object successfully', :aggregate_failures do expect(response).to have_http_status(200) diff --git a/spec/support/examples/cache.rb b/spec/support/examples/cache.rb new file mode 100644 index 0000000000..43cfbade82 --- /dev/null +++ b/spec/support/examples/cache.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +shared_examples 'cacheable response' do |expects_vary: false| + it 'does not set cookies' do + expect(response.cookies).to be_empty + expect(response.headers['Set-Cookies']).to be_nil + end + + it 'does not set sessions' do + expect(session).to be_empty + end + + if expects_vary + it 'returns Vary header' do + expect(response.headers['Vary']).to include(expects_vary) + end + end + + it 'returns public Cache-Control header' do + expect(response.headers['Cache-Control']).to include('public') + end +end From 8d0f12f776bcf7b6e0f1caabc812c5f473094a6a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 12:02:15 -0400 Subject: [PATCH 05/13] Extract partials from `admin/accounts/show` view (#27428) --- app/views/admin/accounts/_buttons.html.haml | 41 ++++ app/views/admin/accounts/_counters.html.haml | 43 ++++ .../admin/accounts/_local_account.html.haml | 82 +++++++ .../admin/accounts/_remote_account.html.haml | 15 ++ app/views/admin/accounts/show.html.haml | 210 +----------------- 5 files changed, 185 insertions(+), 206 deletions(-) create mode 100644 app/views/admin/accounts/_buttons.html.haml create mode 100644 app/views/admin/accounts/_counters.html.haml create mode 100644 app/views/admin/accounts/_local_account.html.haml create mode 100644 app/views/admin/accounts/_remote_account.html.haml diff --git a/app/views/admin/accounts/_buttons.html.haml b/app/views/admin/accounts/_buttons.html.haml new file mode 100644 index 0000000000..6eb141abc9 --- /dev/null +++ b/app/views/admin/accounts/_buttons.html.haml @@ -0,0 +1,41 @@ +- if account.suspended? + %hr.spacer/ + - if account.suspension_origin_remote? + %p.muted-hint= deletion_request.present? ? t('admin.accounts.remote_suspension_reversible_hint_html', date: content_tag(:strong, l(deletion_request.due_at.to_date))) : t('admin.accounts.remote_suspension_irreversible') + - else + %p.muted-hint= deletion_request.present? ? t('admin.accounts.suspension_reversible_hint_html', date: content_tag(:strong, l(deletion_request.due_at.to_date))) : t('admin.accounts.suspension_irreversible') + = link_to t('admin.accounts.undo_suspension'), unsuspend_admin_account_path(account.id), method: :post, class: 'button' if can?(:unsuspend, account) + = link_to t('admin.accounts.redownload'), redownload_admin_account_path(account.id), method: :post, class: 'button' if can?(:redownload, account) && account.suspension_origin_remote? + - if deletion_request.present? + = link_to t('admin.accounts.delete'), admin_account_path(account.id), method: :delete, class: 'button button--destructive', data: { confirm: t('admin.accounts.are_you_sure') } if can?(:destroy, account) +- else + .action-buttons + %div + - if account.local? && account.user_approved? + = link_to t('admin.accounts.warn'), new_admin_account_action_path(account.id, type: 'none'), class: 'button' if can?(:warn, account) + - if account.user_disabled? + = link_to t('admin.accounts.enable'), enable_admin_account_path(account.id), method: :post, class: 'button' if can?(:enable, account.user) + - else + = link_to t('admin.accounts.disable'), new_admin_account_action_path(account.id, type: 'disable'), class: 'button' if can?(:disable, account.user) + - if account.sensitized? + = link_to t('admin.accounts.undo_sensitized'), unsensitive_admin_account_path(account.id), method: :post, class: 'button' if can?(:unsensitive, account) + - elsif !account.local? || account.user_approved? + = link_to t('admin.accounts.sensitive'), new_admin_account_action_path(account.id, type: 'sensitive'), class: 'button' if can?(:sensitive, account) + - if account.silenced? + = link_to t('admin.accounts.undo_silenced'), unsilence_admin_account_path(account.id), method: :post, class: 'button' if can?(:unsilence, account) + - elsif !account.local? || account.user_approved? + = link_to t('admin.accounts.silence'), new_admin_account_action_path(account.id, type: 'silence'), class: 'button' if can?(:silence, account) + - if account.local? + - if account.user_pending? + = link_to t('admin.accounts.approve'), approve_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' if can?(:approve, account.user) + = link_to t('admin.accounts.reject'), reject_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:reject, account.user) + - unless account.user_confirmed? + = link_to t('admin.accounts.confirm'), admin_account_confirmation_path(account.id), method: :post, class: 'button' if can?(:confirm, account.user) + - if !account.local? || account.user_approved? + = link_to t('admin.accounts.perform_full_suspension'), new_admin_account_action_path(account.id, type: 'suspend'), class: 'button' if can?(:suspend, account) + %div + - if account.local? + - if !account.memorial? && account.user_approved? + = link_to t('admin.accounts.memorialize'), memorialize_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:memorialize, account) + - else + = link_to t('admin.accounts.redownload'), redownload_admin_account_path(account.id), method: :post, class: 'button' if can?(:redownload, account) diff --git a/app/views/admin/accounts/_counters.html.haml b/app/views/admin/accounts/_counters.html.haml new file mode 100644 index 0000000000..00ab98d094 --- /dev/null +++ b/app/views/admin/accounts/_counters.html.haml @@ -0,0 +1,43 @@ +.dashboard__counters.admin-account-counters + %div + = link_to admin_account_statuses_path(account.id) do + .dashboard__counters__num= number_with_delimiter account.statuses_count + .dashboard__counters__label= t 'admin.accounts.statuses' + %div + = link_to admin_account_statuses_path(account.id, { media: true }) do + .dashboard__counters__num= number_to_human_size account.media_attachments.sum('file_file_size') + .dashboard__counters__label= t 'admin.accounts.media_attachments' + %div + = link_to admin_account_relationships_path(account.id, location: account.local? ? nil : 'local', relationship: 'followed_by') do + .dashboard__counters__num= number_with_delimiter account.local_followers_count + .dashboard__counters__label= t 'admin.accounts.followers' + %div + = link_to admin_reports_path(account_id: account.id) do + .dashboard__counters__num= number_with_delimiter account.reports.count + .dashboard__counters__label= t 'admin.accounts.show.created_reports' + %div + = link_to admin_reports_path(target_account_id: account.id) do + .dashboard__counters__num= number_with_delimiter account.targeted_reports.count + .dashboard__counters__label= t 'admin.accounts.show.targeted_reports' + %div + = link_to admin_action_logs_path(target_account_id: account.id) do + .dashboard__counters__text + - if account.local? && account.user.nil? + = t('admin.accounts.deleted') + - elsif account.memorial? + = t('admin.accounts.memorialized') + - elsif account.suspended? + = t('admin.accounts.suspended') + - elsif account.silenced? + = t('admin.accounts.silenced') + - elsif account.local? && account.user&.disabled? + = t('admin.accounts.disabled') + - elsif account.local? && !account.user&.confirmed? + = t('admin.accounts.confirming') + - elsif account.local? && !account.user_approved? + = t('admin.accounts.pending') + - elsif account.sensitized? + = t('admin.accounts.sensitive') + - else + = t('admin.accounts.no_limits_imposed') + .dashboard__counters__label= t 'admin.accounts.login_status' diff --git a/app/views/admin/accounts/_local_account.html.haml b/app/views/admin/accounts/_local_account.html.haml new file mode 100644 index 0000000000..4b361fc8d1 --- /dev/null +++ b/app/views/admin/accounts/_local_account.html.haml @@ -0,0 +1,82 @@ +- if account.avatar? + %tr + %th= t('admin.accounts.avatar') + %td= table_link_to 'trash', t('admin.accounts.remove_avatar'), remove_avatar_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:remove_avatar, account) + %td +- if account.header? + %tr + %th= t('admin.accounts.header') + %td= table_link_to 'trash', t('admin.accounts.remove_header'), remove_header_admin_account_path(account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:remove_header, account) + %td +%tr + %th= t('admin.accounts.role') + %td + - if account.user_role&.everyone? + = t('admin.accounts.no_role_assigned') + - else + = account.user_role&.name + %td + = table_link_to 'vcard', t('admin.accounts.change_role.label'), admin_user_role_path(account.user) if can?(:change_role, account.user) +%tr + %th{ rowspan: can?(:create, :email_domain_block) ? 3 : 2 }= t('admin.accounts.email') + %td{ rowspan: can?(:create, :email_domain_block) ? 3 : 2 }= account.user_email + %td= table_link_to 'edit', t('admin.accounts.change_email.label'), admin_account_change_email_path(account.id) if can?(:change_email, account.user) +%tr + %td= table_link_to 'search', t('admin.accounts.search_same_email_domain'), admin_accounts_path(email: "%@#{account.user_email.split('@').last}") +- if can?(:create, :email_domain_block) + %tr + %td= table_link_to 'ban', t('admin.accounts.add_email_domain_block'), new_admin_email_domain_block_path(_domain: account.user_email.split('@').last) +- if account.user_unconfirmed_email.present? + %tr + %th= t('admin.accounts.unconfirmed_email') + %td= account.user_unconfirmed_email + %td +%tr + %th= t('admin.accounts.email_status') + %td + - if account.user&.confirmed? + = t('admin.accounts.confirmed') + - else + = t('admin.accounts.confirming') + %td= table_link_to 'refresh', t('admin.accounts.resend_confirmation.send'), resend_admin_account_confirmation_path(account.id), method: :post if can?(:confirm, account.user) +%tr + %th{ rowspan: can?(:reset_password, account.user) ? 2 : 1 }= t('admin.accounts.security') + %td{ rowspan: can?(:reset_password, account.user) ? 2 : 1 } + - if account.user&.two_factor_enabled? + = t 'admin.accounts.security_measures.password_and_2fa' + - else + = t 'admin.accounts.security_measures.only_password' + %td + - if account.user&.two_factor_enabled? + = table_link_to 'unlock', t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(account.user.id), method: :delete if can?(:disable_2fa, account.user) +- if can?(:reset_password, account.user) + %tr + %td + = table_link_to 'key', t('admin.accounts.reset_password'), admin_account_reset_path(account.id), method: :create, data: { confirm: t('admin.accounts.are_you_sure') } +%tr + %th= t('simple_form.labels.defaults.locale') + %td= standard_locale_name(account.user_locale) + %td +%tr + %th= t('admin.accounts.joined') + %td + %time.formatted{ datetime: account.created_at.iso8601, title: l(account.created_at) }= l account.created_at + %td +- recent_ips = account.user.ips.order(used_at: :desc).to_a +- recent_ips.each_with_index do |recent_ip, i| + %tr + - if i.zero? + %th{ rowspan: recent_ips.size }= t('admin.accounts.most_recent_ip') + %td= recent_ip.ip + %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: recent_ip.ip) +%tr + %th= t('admin.accounts.most_recent_activity') + %td + - if account.user_current_sign_in_at + %time.formatted{ datetime: account.user_current_sign_in_at.iso8601, title: l(account.user_current_sign_in_at) }= l account.user_current_sign_in_at + %td +- if account.user&.invited? + %tr + %th= t('admin.accounts.invited_by') + %td= admin_account_link_to account.user.invite.user.account + %td diff --git a/app/views/admin/accounts/_remote_account.html.haml b/app/views/admin/accounts/_remote_account.html.haml new file mode 100644 index 0000000000..99996e1d46 --- /dev/null +++ b/app/views/admin/accounts/_remote_account.html.haml @@ -0,0 +1,15 @@ +%tr + %th= t('admin.accounts.inbox_url') + %td + = account.inbox_url + = fa_icon DeliveryFailureTracker.available?(account.inbox_url) ? 'check' : 'times' + %td + = table_link_to 'search', domain_block.present? ? t('admin.domain_blocks.view') : t('admin.accounts.view_domain'), admin_instance_path(account.domain) +%tr + %th= t('admin.accounts.shared_inbox_url') + %td + = account.shared_inbox_url + = fa_icon DeliveryFailureTracker.available?(account.shared_inbox_url) ? 'check' : 'times' + %td + - if domain_block.nil? + = table_link_to 'ban', t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: account.domain) diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 7801ef1913..80b8fc92c2 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -27,49 +27,7 @@ %div .account__header__content.emojify= prerender_custom_emojis(account_bio_format(account), account.emojis) -.dashboard__counters.admin-account-counters - %div - = link_to admin_account_statuses_path(@account.id) do - .dashboard__counters__num= number_with_delimiter @account.statuses_count - .dashboard__counters__label= t 'admin.accounts.statuses' - %div - = link_to admin_account_statuses_path(@account.id, { media: true }) do - .dashboard__counters__num= number_to_human_size @account.media_attachments.sum('file_file_size') - .dashboard__counters__label= t 'admin.accounts.media_attachments' - %div - = link_to admin_account_relationships_path(@account.id, location: @account.local? ? nil : 'local', relationship: 'followed_by') do - .dashboard__counters__num= number_with_delimiter @account.local_followers_count - .dashboard__counters__label= t 'admin.accounts.followers' - %div - = link_to admin_reports_path(account_id: @account.id) do - .dashboard__counters__num= number_with_delimiter @account.reports.count - .dashboard__counters__label= t '.created_reports' - %div - = link_to admin_reports_path(target_account_id: @account.id) do - .dashboard__counters__num= number_with_delimiter @account.targeted_reports.count - .dashboard__counters__label= t '.targeted_reports' - %div - = link_to admin_action_logs_path(target_account_id: @account.id) do - .dashboard__counters__text - - if @account.local? && @account.user.nil? - = t('admin.accounts.deleted') - - elsif @account.memorial? - = t('admin.accounts.memorialized') - - elsif @account.suspended? - = t('admin.accounts.suspended') - - elsif @account.silenced? - = t('admin.accounts.silenced') - - elsif @account.local? && @account.user&.disabled? - = t('admin.accounts.disabled') - - elsif @account.local? && !@account.user&.confirmed? - = t('admin.accounts.confirming') - - elsif @account.local? && !@account.user_approved? - = t('admin.accounts.pending') - - elsif @account.sensitized? - = t('admin.accounts.sensitive') - - else - = t('admin.accounts.no_limits_imposed') - .dashboard__counters__label= t 'admin.accounts.login_status' += render 'admin/accounts/counters', account: @account - if @account.local? && @account.user.nil? = link_to t('admin.accounts.unblock_email'), unblock_email_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unblock_email, @account) && CanonicalEmailBlock.exists?(reference_account_id: @account.id) @@ -78,171 +36,11 @@ %table.table.inline-table %tbody - if @account.local? - - if @account.avatar? - %tr - %th= t('admin.accounts.avatar') - %td= table_link_to 'trash', t('admin.accounts.remove_avatar'), remove_avatar_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:remove_avatar, @account) - %td - - - if @account.header? - %tr - %th= t('admin.accounts.header') - %td= table_link_to 'trash', t('admin.accounts.remove_header'), remove_header_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } if can?(:remove_header, @account) - %td - - %tr - %th= t('admin.accounts.role') - %td - - if @account.user_role&.everyone? - = t('admin.accounts.no_role_assigned') - - else - = @account.user_role&.name - %td - = table_link_to 'vcard', t('admin.accounts.change_role.label'), admin_user_role_path(@account.user) if can?(:change_role, @account.user) - - %tr - %th{ rowspan: can?(:create, :email_domain_block) ? 3 : 2 }= t('admin.accounts.email') - %td{ rowspan: can?(:create, :email_domain_block) ? 3 : 2 }= @account.user_email - %td= table_link_to 'edit', t('admin.accounts.change_email.label'), admin_account_change_email_path(@account.id) if can?(:change_email, @account.user) - - %tr - %td= table_link_to 'search', t('admin.accounts.search_same_email_domain'), admin_accounts_path(email: "%@#{@account.user_email.split('@').last}") - - - if can?(:create, :email_domain_block) - %tr - %td= table_link_to 'ban', t('admin.accounts.add_email_domain_block'), new_admin_email_domain_block_path(_domain: @account.user_email.split('@').last) - - - if @account.user_unconfirmed_email.present? - %tr - %th= t('admin.accounts.unconfirmed_email') - %td= @account.user_unconfirmed_email - %td - - %tr - %th= t('admin.accounts.email_status') - %td - - if @account.user&.confirmed? - = t('admin.accounts.confirmed') - - else - = t('admin.accounts.confirming') - %td= table_link_to 'refresh', t('admin.accounts.resend_confirmation.send'), resend_admin_account_confirmation_path(@account.id), method: :post if can?(:confirm, @account.user) - %tr - %th{ rowspan: can?(:reset_password, @account.user) ? 2 : 1 }= t('admin.accounts.security') - %td{ rowspan: can?(:reset_password, @account.user) ? 2 : 1 } - - if @account.user&.two_factor_enabled? - = t 'admin.accounts.security_measures.password_and_2fa' - - else - = t 'admin.accounts.security_measures.only_password' - %td - - if @account.user&.two_factor_enabled? - = table_link_to 'unlock', t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete if can?(:disable_2fa, @account.user) - - - if can?(:reset_password, @account.user) - %tr - %td - = table_link_to 'key', t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, data: { confirm: t('admin.accounts.are_you_sure') } - - %tr - %th= t('simple_form.labels.defaults.locale') - %td= standard_locale_name(@account.user_locale) - %td - - %tr - %th= t('admin.accounts.joined') - %td - %time.formatted{ datetime: @account.created_at.iso8601, title: l(@account.created_at) }= l @account.created_at - %td - - - recent_ips = @account.user.ips.order(used_at: :desc).to_a - - - recent_ips.each_with_index do |recent_ip, i| - %tr - - if i.zero? - %th{ rowspan: recent_ips.size }= t('admin.accounts.most_recent_ip') - %td= recent_ip.ip - %td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: recent_ip.ip) - - %tr - %th= t('admin.accounts.most_recent_activity') - %td - - if @account.user_current_sign_in_at - %time.formatted{ datetime: @account.user_current_sign_in_at.iso8601, title: l(@account.user_current_sign_in_at) }= l @account.user_current_sign_in_at - %td - - - if @account.user&.invited? - %tr - %th= t('admin.accounts.invited_by') - %td= admin_account_link_to @account.user.invite.user.account - %td - + = render 'admin/accounts/local_account', account: @account - else - %tr - %th= t('admin.accounts.inbox_url') - %td - = @account.inbox_url - = fa_icon DeliveryFailureTracker.available?(@account.inbox_url) ? 'check' : 'times' - %td - = table_link_to 'search', @domain_block.present? ? t('admin.domain_blocks.view') : t('admin.accounts.view_domain'), admin_instance_path(@account.domain) - %tr - %th= t('admin.accounts.shared_inbox_url') - %td - = @account.shared_inbox_url - = fa_icon DeliveryFailureTracker.available?(@account.shared_inbox_url) ? 'check' : 'times' - %td - - if @domain_block.nil? - = table_link_to 'ban', t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @account.domain) + = render 'admin/accounts/remote_account', account: @account, domain_block: @domain_block - - if @account.suspended? - %hr.spacer/ - - - if @account.suspension_origin_remote? - %p.muted-hint= @deletion_request.present? ? t('admin.accounts.remote_suspension_reversible_hint_html', date: content_tag(:strong, l(@deletion_request.due_at.to_date))) : t('admin.accounts.remote_suspension_irreversible') - - else - %p.muted-hint= @deletion_request.present? ? t('admin.accounts.suspension_reversible_hint_html', date: content_tag(:strong, l(@deletion_request.due_at.to_date))) : t('admin.accounts.suspension_irreversible') - - = link_to t('admin.accounts.undo_suspension'), unsuspend_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsuspend, @account) - = link_to t('admin.accounts.redownload'), redownload_admin_account_path(@account.id), method: :post, class: 'button' if can?(:redownload, @account) && @account.suspension_origin_remote? - - - if @deletion_request.present? - = link_to t('admin.accounts.delete'), admin_account_path(@account.id), method: :delete, class: 'button button--destructive', data: { confirm: t('admin.accounts.are_you_sure') } if can?(:destroy, @account) - - else - .action-buttons - %div - - if @account.local? && @account.user_approved? - = link_to t('admin.accounts.warn'), new_admin_account_action_path(@account.id, type: 'none'), class: 'button' if can?(:warn, @account) - - - if @account.user_disabled? - = link_to t('admin.accounts.enable'), enable_admin_account_path(@account.id), method: :post, class: 'button' if can?(:enable, @account.user) - - else - = link_to t('admin.accounts.disable'), new_admin_account_action_path(@account.id, type: 'disable'), class: 'button' if can?(:disable, @account.user) - - - if @account.sensitized? - = link_to t('admin.accounts.undo_sensitized'), unsensitive_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsensitive, @account) - - elsif !@account.local? || @account.user_approved? - = link_to t('admin.accounts.sensitive'), new_admin_account_action_path(@account.id, type: 'sensitive'), class: 'button' if can?(:sensitive, @account) - - - if @account.silenced? - = link_to t('admin.accounts.undo_silenced'), unsilence_admin_account_path(@account.id), method: :post, class: 'button' if can?(:unsilence, @account) - - elsif !@account.local? || @account.user_approved? - = link_to t('admin.accounts.silence'), new_admin_account_action_path(@account.id, type: 'silence'), class: 'button' if can?(:silence, @account) - - - if @account.local? - - if @account.user_pending? - = link_to t('admin.accounts.approve'), approve_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' if can?(:approve, @account.user) - = link_to t('admin.accounts.reject'), reject_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:reject, @account.user) - - - unless @account.user_confirmed? - = link_to t('admin.accounts.confirm'), admin_account_confirmation_path(@account.id), method: :post, class: 'button' if can?(:confirm, @account.user) - - - if !@account.local? || @account.user_approved? - = link_to t('admin.accounts.perform_full_suspension'), new_admin_account_action_path(@account.id, type: 'suspend'), class: 'button' if can?(:suspend, @account) - - %div - - if @account.local? - - if !@account.memorial? && @account.user_approved? - = link_to t('admin.accounts.memorialize'), memorialize_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:memorialize, @account) - - else - = link_to t('admin.accounts.redownload'), redownload_admin_account_path(@account.id), method: :post, class: 'button' if can?(:redownload, @account) + = render 'admin/accounts/buttons', account: @account, deletion_request: @deletion_request %hr.spacer/ From 33b073f77daa52c04d6615feaec0edf7651ad413 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 12:07:17 -0400 Subject: [PATCH 06/13] Extract partials from `admin/reports/show` view (#27427) --- app/views/admin/reports/_comment.html.haml | 24 ++++ .../admin/reports/_header_card.html.haml | 46 +++++++ .../admin/reports/_header_details.html.haml | 53 +++++++ app/views/admin/reports/show.html.haml | 130 +----------------- 4 files changed, 126 insertions(+), 127 deletions(-) create mode 100644 app/views/admin/reports/_comment.html.haml create mode 100644 app/views/admin/reports/_header_card.html.haml create mode 100644 app/views/admin/reports/_header_details.html.haml diff --git a/app/views/admin/reports/_comment.html.haml b/app/views/admin/reports/_comment.html.haml new file mode 100644 index 0000000000..8c07210af9 --- /dev/null +++ b/app/views/admin/reports/_comment.html.haml @@ -0,0 +1,24 @@ +- if report.account.instance_actor? + %p= t('admin.reports.comment_description_html', name: content_tag(:strong, site_hostname, class: 'username')) +- elsif report.account.local? + %p= t('admin.reports.comment_description_html', name: content_tag(:strong, report.account.username, class: 'username')) +- else + %p= t('admin.reports.comment_description_html', name: t('admin.reports.remote_user_placeholder', instance: report.account.domain)) +.report-notes + .report-notes__item + - if report.account.local? && !report.account.instance_actor? + = image_tag report.account.avatar.url, class: 'report-notes__item__avatar' + - else + = image_tag(full_asset_url('avatars/original/missing.png', skip_pipeline: true), class: 'report-notes__item__avatar') + .report-notes__item__header + %span.username + - if report.account.instance_actor? + = site_hostname + - elsif report.account.local? + = link_to report.account.username, admin_account_path(report.account_id) + - else + = link_to report.account.domain, admin_instance_path(report.account.domain) + %time.relative-formatted{ datetime: report.created_at.iso8601 } + = l report.created_at.to_date + .report-notes__item__content + = simple_format(h(report.comment)) diff --git a/app/views/admin/reports/_header_card.html.haml b/app/views/admin/reports/_header_card.html.haml new file mode 100644 index 0000000000..6fd8b4ecc8 --- /dev/null +++ b/app/views/admin/reports/_header_card.html.haml @@ -0,0 +1,46 @@ +.report-header__card + .account-card + .account-card__header + = image_tag report.target_account.header.url, alt: '' + .account-card__title + .account-card__title__avatar + = image_tag report.target_account.avatar.url, alt: '' + .display-name + %bdi + %strong.emojify.p-name= display_name(report.target_account, custom_emojify: true) + %span + = acct(report.target_account) + = fa_icon('lock') if report.target_account.locked? + - if report.target_account.note.present? + .account-card__bio.emojify + = prerender_custom_emojis(account_bio_format(report.target_account), report.target_account.emojis) + .account-card__actions + .account-card__counters + .account-card__counters__item + = friendly_number_to_human report.target_account.statuses_count + %small= t('accounts.posts', count: report.target_account.statuses_count).downcase + .account-card__counters__item + = friendly_number_to_human report.target_account.followers_count + %small= t('accounts.followers', count: report.target_account.followers_count).downcase + .account-card__counters__item + = friendly_number_to_human report.target_account.following_count + %small= t('accounts.following', count: report.target_account.following_count).downcase + .account-card__actions__button + = link_to t('admin.reports.view_profile'), admin_account_path(report.target_account_id), class: 'button' + .report-header__details.report-header__details--horizontal + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.accounts.joined') + .report-header__details__item__content + %time.time-ago{ datetime: report.target_account.created_at.iso8601, title: l(report.target_account.created_at) }= l report.target_account.created_at + .report-header__details__item + .report-header__details__item__header + %strong= t('accounts.last_active') + .report-header__details__item__content + - if report.target_account.last_status_at.present? + %time.time-ago{ datetime: report.target_account.last_status_at.to_date.iso8601, title: l(report.target_account.last_status_at.to_date) }= l report.target_account.last_status_at + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.accounts.strikes') + .report-header__details__item__content + = report.target_account.previous_strikes_count diff --git a/app/views/admin/reports/_header_details.html.haml b/app/views/admin/reports/_header_details.html.haml new file mode 100644 index 0000000000..5878cd2ff8 --- /dev/null +++ b/app/views/admin/reports/_header_details.html.haml @@ -0,0 +1,53 @@ +.report-header__details + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.created_at') + .report-header__details__item__content + %time.formatted{ datetime: report.created_at.iso8601 } + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.reported_by') + .report-header__details__item__content + - if report.account.instance_actor? + = site_hostname + - elsif report.account.local? + = admin_account_link_to report.account + - else + = report.account.domain + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.status') + .report-header__details__item__content + - if report.action_taken? + = t('admin.reports.resolved') + - else + = t('admin.reports.unresolved') + - unless report.target_account.local? + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.forwarded') + .report-header__details__item__content + - if report.forwarded? + = t('simple_form.yes') + - else + = t('simple_form.no') + - if report.action_taken_by_account.present? + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.action_taken_by') + .report-header__details__item__content + = admin_account_link_to report.action_taken_by_account + - else + .report-header__details__item + .report-header__details__item__header + %strong= t('admin.reports.assigned') + .report-header__details__item__content + - if report.assigned_account.nil? + = t 'admin.reports.no_one_assigned' + - else + = admin_account_link_to report.assigned_account + — + - if report.assigned_account != current_user.account + = table_link_to 'user', t('admin.reports.assign_to_self'), assign_to_self_admin_report_path(report), method: :post + - elsif !report.assigned_account.nil? + = table_link_to 'trash', t('admin.reports.unassign'), unassign_admin_report_path(report), method: :post diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 41ce73cfcf..13a4d48344 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -8,106 +8,8 @@ = link_to t('admin.reports.mark_as_unresolved'), reopen_admin_report_path(@report), method: :post, class: 'button' .report-header - .report-header__card - .account-card - .account-card__header - = image_tag @report.target_account.header.url, alt: '' - .account-card__title - .account-card__title__avatar - = image_tag @report.target_account.avatar.url, alt: '' - .display-name - %bdi - %strong.emojify.p-name= display_name(@report.target_account, custom_emojify: true) - %span - = acct(@report.target_account) - = fa_icon('lock') if @report.target_account.locked? - - if @report.target_account.note.present? - .account-card__bio.emojify - = prerender_custom_emojis(account_bio_format(@report.target_account), @report.target_account.emojis) - .account-card__actions - .account-card__counters - .account-card__counters__item - = friendly_number_to_human @report.target_account.statuses_count - %small= t('accounts.posts', count: @report.target_account.statuses_count).downcase - .account-card__counters__item - = friendly_number_to_human @report.target_account.followers_count - %small= t('accounts.followers', count: @report.target_account.followers_count).downcase - .account-card__counters__item - = friendly_number_to_human @report.target_account.following_count - %small= t('accounts.following', count: @report.target_account.following_count).downcase - .account-card__actions__button - = link_to t('admin.reports.view_profile'), admin_account_path(@report.target_account_id), class: 'button' - .report-header__details.report-header__details--horizontal - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.accounts.joined') - .report-header__details__item__content - %time.time-ago{ datetime: @report.target_account.created_at.iso8601, title: l(@report.target_account.created_at) }= l @report.target_account.created_at - .report-header__details__item - .report-header__details__item__header - %strong= t('accounts.last_active') - .report-header__details__item__content - - if @report.target_account.last_status_at.present? - %time.time-ago{ datetime: @report.target_account.last_status_at.to_date.iso8601, title: l(@report.target_account.last_status_at.to_date) }= l @report.target_account.last_status_at - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.accounts.strikes') - .report-header__details__item__content - = @report.target_account.previous_strikes_count - - .report-header__details - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.created_at') - .report-header__details__item__content - %time.formatted{ datetime: @report.created_at.iso8601 } - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.reported_by') - .report-header__details__item__content - - if @report.account.instance_actor? - = site_hostname - - elsif @report.account.local? - = admin_account_link_to @report.account - - else - = @report.account.domain - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.status') - .report-header__details__item__content - - if @report.action_taken? - = t('admin.reports.resolved') - - else - = t('admin.reports.unresolved') - - unless @report.target_account.local? - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.forwarded') - .report-header__details__item__content - - if @report.forwarded? - = t('simple_form.yes') - - else - = t('simple_form.no') - - if @report.action_taken_by_account.present? - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.action_taken_by') - .report-header__details__item__content - = admin_account_link_to @report.action_taken_by_account - - else - .report-header__details__item - .report-header__details__item__header - %strong= t('admin.reports.assigned') - .report-header__details__item__content - - if @report.assigned_account.nil? - = t 'admin.reports.no_one_assigned' - - else - = admin_account_link_to @report.assigned_account - — - - if @report.assigned_account != current_user.account - = table_link_to 'user', t('admin.reports.assign_to_self'), assign_to_self_admin_report_path(@report), method: :post - - elsif !@report.assigned_account.nil? - = table_link_to 'trash', t('admin.reports.unassign'), unassign_admin_report_path(@report), method: :post + = render 'admin/reports/header_card', report: @report + = render 'admin/reports/header_details', report: @report %hr.spacer @@ -118,33 +20,7 @@ = react_admin_component :report_reason_selector, id: @report.id, category: @report.category, rule_ids: @report.rule_ids&.map(&:to_s), disabled: @report.action_taken? - if @report.comment.present? - - if @report.account.instance_actor? - %p= t('admin.reports.comment_description_html', name: content_tag(:strong, site_hostname, class: 'username')) - - elsif @report.account.local? - %p= t('admin.reports.comment_description_html', name: content_tag(:strong, @report.account.username, class: 'username')) - - else - %p= t('admin.reports.comment_description_html', name: t('admin.reports.remote_user_placeholder', instance: @report.account.domain)) - - .report-notes - .report-notes__item - - if @report.account.local? && !@report.account.instance_actor? - = image_tag @report.account.avatar.url, class: 'report-notes__item__avatar' - - else - = image_tag(full_asset_url('avatars/original/missing.png', skip_pipeline: true), class: 'report-notes__item__avatar') - - .report-notes__item__header - %span.username - - if @report.account.instance_actor? - = site_hostname - - elsif @report.account.local? - = link_to @report.account.username, admin_account_path(@report.account_id) - - else - = link_to @report.account.domain, admin_instance_path(@report.account.domain) - %time.relative-formatted{ datetime: @report.created_at.iso8601 } - = l @report.created_at.to_date - - .report-notes__item__content - = simple_format(h(@report.comment)) + = render 'admin/reports/comment', report: @report %hr.spacer/ From fc9ab61448af94513524f14f5fd287d2a26ceeab Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 16 Oct 2023 12:20:58 -0400 Subject: [PATCH 07/13] Expand spec coverage of `CLI::Media` (#27437) --- spec/lib/mastodon/cli/media_spec.rb | 69 +++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/spec/lib/mastodon/cli/media_spec.rb b/spec/lib/mastodon/cli/media_spec.rb index 29f7d424a9..9543640e96 100644 --- a/spec/lib/mastodon/cli/media_spec.rb +++ b/spec/lib/mastodon/cli/media_spec.rb @@ -4,9 +4,78 @@ require 'rails_helper' require 'mastodon/cli/media' describe Mastodon::CLI::Media do + let(:cli) { described_class.new } + describe '.exit_on_failure?' do it 'returns true' do expect(described_class.exit_on_failure?).to be true end end + + describe '#remove' do + context 'with --prune-profiles and --remove-headers' do + let(:options) { { prune_profiles: true, remove_headers: true } } + + it 'warns about usage and exits' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('--prune-profiles and --remove-headers should not be specified simultaneously') + ).to_stdout.and raise_error(SystemExit) + end + end + + context 'with --include-follows but not including --prune-profiles and --remove-headers' do + let(:options) { { include_follows: true } } + + it 'warns about usage and exits' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('--include-follows can only be used with --prune-profiles or --remove-headers') + ).to_stdout.and raise_error(SystemExit) + end + end + + context 'with a relevant account' do + let!(:account) do + Fabricate(:account, domain: 'example.com', updated_at: 1.month.ago, last_webfingered_at: 1.month.ago, avatar: attachment_fixture('attachment.jpg'), header: attachment_fixture('attachment.jpg')) + end + + context 'with --prune-profiles' do + let(:options) { { prune_profiles: true } } + + it 'removes account avatars' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('Visited 1') + ).to_stdout + + expect(account.reload.avatar).to be_blank + end + end + + context 'with --remove-headers' do + let(:options) { { remove_headers: true } } + + it 'removes account header' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('Visited 1') + ).to_stdout + + expect(account.reload.header).to be_blank + end + end + end + + context 'with a relevant media attachment' do + let!(:media_attachment) { Fabricate(:media_attachment, remote_url: 'https://example.com/image.jpg', created_at: 1.month.ago) } + + context 'without options' do + it 'removes account avatars' do + expect { cli.invoke(:remove) }.to output( + a_string_including('Removed 1') + ).to_stdout + + expect(media_attachment.reload.file).to be_blank + expect(media_attachment.reload.thumbnail).to be_blank + end + end + end + end end From 108470341788befa4602f315798c8de55d170553 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 17 Oct 2023 13:03:54 +0200 Subject: [PATCH 08/13] Update changelog (#27440) --- CHANGELOG.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fab3104bac..f9303f0115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,46 @@ All notable changes to this project will be documented in this file. +## [4.2.1] - 2023-10-10 + +### Added + +- Add redirection on `/deck` URLs for logged-out users ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27128)) +- Add support for v4.2.0 migrations to `tootctl maintenance fix-duplicates` ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27147)) + +### Changed + +- Change some worker lock TTLs to be shorter-lived ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27246)) +- Change user archive export allowed period from 7 days to 6 days ([suddjian](https://github.com/mastodon/mastodon/pull/27200)) + +### Fixed + +- Fix duplicate reports being sent when reporting some remote posts ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27355)) +- Fix clicking on already-opened thread post scrolling to the top of the thread ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27331), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/27338), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/27350)) +- Fix some remote posts getting truncated ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27307)) +- Fix some cases of infinite scroll code trying to fetch inaccessible posts in a loop ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27286)) +- Fix `Vary` headers not being set on some redirects ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27272)) +- Fix mentions being matched in some URL query strings ([mjankowski](https://github.com/mastodon/mastodon/pull/25656)) +- Fix unexpected linebreak in version string in the Web UI ([vmstan](https://github.com/mastodon/mastodon/pull/26986)) +- Fix double scroll bars in some columns in advanced interface ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27187)) +- Fix boosts of local users being filtered in account timelines ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27204)) +- Fix multiple instances of the trend refresh scheduler sometimes running at once ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27253)) +- Fix importer returning negative row estimates ([jgillich](https://github.com/mastodon/mastodon/pull/27258)) +- Fix incorrectly keeping outdated update notices absent from the API endpoint ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27021)) +- Fix import progress not updating on certain failures ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27247)) +- Fix websocket connections being incorrectly decremented twice on errors ([ThisIsMissEm](https://github.com/mastodon/mastodon/pull/27238)) +- Fix explore prompt appearing because of posts being received out of order ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27211)) +- Fix explore prompt sometimes showing up when the home TL is loading ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27062)) +- Fix link handling of mentions in user profiles when logged out ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27185)) +- Fix filtering audit log for entries about disabling 2FA ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27186)) +- Fix notification toasts not respecting reduce-motion ([c960657](https://github.com/mastodon/mastodon/pull/27178)) +- Fix retention dashboard not displaying correct month ([vmstan](https://github.com/mastodon/mastodon/pull/27180)) +- Fix tIME chunk not being properly removed from PNG uploads ([TheEssem](https://github.com/mastodon/mastodon/pull/27111)) +- Fix division by zero in video in bitrate computation code ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27129)) +- Fix inefficient queries in “Follows and followers” as well as several admin pages ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27116), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/27306)) +- Fix ActiveRecord using two connection pools when no replica is defined ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/27061)) +- Fix the search documentation URL in system checks ([renchap](https://github.com/mastodon/mastodon/pull/27036)) + ## [4.2.0] - 2023-09-21 The following changelog entries focus on changes visible to users, administrators, client developers or federated software developers, but there has also been a lot of code modernization, refactoring, and tooling work, in particular by [@danielmbrasil](https://github.com/danielmbrasil), [@mjankowski](https://github.com/mjankowski), [@nschonni](https://github.com/nschonni), [@renchap](https://github.com/renchap), and [@takayamaki](https://github.com/takayamaki). From 19900f647e81cb7078b9d329c4b57477068c4064 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 17 Oct 2023 07:05:28 -0400 Subject: [PATCH 09/13] Add coverage for `UnreservedUsernameValidator` (#25590) --- .../unreserved_username_validator.rb | 29 ++++- .../unreserved_username_validator_spec.rb | 123 ++++++++++++++---- 2 files changed, 122 insertions(+), 30 deletions(-) diff --git a/app/validators/unreserved_username_validator.rb b/app/validators/unreserved_username_validator.rb index f82f4b91d0..55a8c835fa 100644 --- a/app/validators/unreserved_username_validator.rb +++ b/app/validators/unreserved_username_validator.rb @@ -11,16 +11,31 @@ class UnreservedUsernameValidator < ActiveModel::Validator private - def pam_controlled? - return false unless Devise.pam_authentication && Devise.pam_controlled_service - - Rpam2.account(Devise.pam_controlled_service, @username).present? + def reserved_username? + pam_username_reserved? || settings_username_reserved? end - def reserved_username? - return true if pam_controlled? - return false unless Setting.reserved_usernames + def pam_username_reserved? + pam_controlled? && pam_reserves_username? + end + def pam_controlled? + Devise.pam_authentication && Devise.pam_controlled_service + end + + def pam_reserves_username? + Rpam2.account(Devise.pam_controlled_service, @username) + end + + def settings_username_reserved? + settings_has_reserved_usernames? && settings_reserves_username? + end + + def settings_has_reserved_usernames? + Setting.reserved_usernames.present? + end + + def settings_reserves_username? Setting.reserved_usernames.include?(@username.downcase) end end diff --git a/spec/validators/unreserved_username_validator_spec.rb b/spec/validators/unreserved_username_validator_spec.rb index 6f353eeafd..0eb5f83683 100644 --- a/spec/validators/unreserved_username_validator_spec.rb +++ b/spec/validators/unreserved_username_validator_spec.rb @@ -2,41 +2,118 @@ require 'rails_helper' -RSpec.describe UnreservedUsernameValidator, type: :validator do - describe '#validate' do - before do - allow(validator).to receive(:reserved_username?) { reserved_username } - validator.validate(account) +describe UnreservedUsernameValidator do + let(:record_class) do + Class.new do + include ActiveModel::Validations + attr_accessor :username + + validates_with UnreservedUsernameValidator end + end + let(:record) { record_class.new } - let(:validator) { described_class.new } - let(:account) { instance_double(Account, username: username, errors: errors) } - let(:errors) { instance_double(ActiveModel::Errors, add: nil) } + describe '#validate' do + context 'when username is nil' do + it 'does not add errors' do + record.username = nil - context 'when @username is blank?' do - let(:username) { nil } - - it 'not calls errors.add' do - expect(errors).to_not have_received(:add).with(:username, any_args) + expect(record).to be_valid + expect(record.errors).to be_empty end end - context 'when @username is not blank?' do - let(:username) { 'f' } + context 'when PAM is enabled' do + before do + allow(Devise).to receive(:pam_authentication).and_return(true) + end - context 'with reserved_username?' do - let(:reserved_username) { true } + context 'with a pam service available' do + let(:service) { double } + let(:pam_class) do + Class.new do + def self.account(service, username); end + end + end - it 'calls errors.add' do - expect(errors).to have_received(:add).with(:username, :reserved) + before do + stub_const('Rpam2', pam_class) + allow(Devise).to receive(:pam_controlled_service).and_return(service) + end + + context 'when the account exists' do + before do + allow(Rpam2).to receive(:account).with(service, 'username').and_return(true) + end + + it 'adds errors to the record' do + record.username = 'username' + + expect(record).to_not be_valid + expect(record.errors.first.attribute).to eq(:username) + expect(record.errors.first.type).to eq(:reserved) + end + end + + context 'when the account does not exist' do + before do + allow(Rpam2).to receive(:account).with(service, 'username').and_return(false) + end + + it 'does not add errors to the record' do + record.username = 'username' + + expect(record).to be_valid + expect(record.errors).to be_empty + end end end - context 'when username is not reserved' do - let(:reserved_username) { false } + context 'without a pam service' do + before do + allow(Devise).to receive(:pam_controlled_service).and_return(false) + end - it 'not calls errors.add' do - expect(errors).to_not have_received(:add).with(:username, any_args) + context 'when there are not any reserved usernames' do + before do + stub_reserved_usernames(nil) + end + + it 'does not add errors to the record' do + record.username = 'username' + + expect(record).to be_valid + expect(record.errors).to be_empty + end + end + + context 'when there are reserved usernames' do + before do + stub_reserved_usernames(%w(alice bob)) + end + + context 'when the username is reserved' do + it 'adds errors to the record' do + record.username = 'alice' + + expect(record).to_not be_valid + expect(record.errors.first.attribute).to eq(:username) + expect(record.errors.first.type).to eq(:reserved) + end + end + + context 'when the username is not reserved' do + it 'does not add errors to the record' do + record.username = 'chris' + + expect(record).to be_valid + expect(record.errors).to be_empty + end + end + end + + def stub_reserved_usernames(value) + allow(Setting).to receive(:[]).with('reserved_usernames').and_return(value) end end end From d54fec24e5ab97c0529172ee30d45350453e5b3c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 17 Oct 2023 07:06:16 -0400 Subject: [PATCH 10/13] Add coverage for `CLI::PreviewCards#remove` command (#27441) --- spec/lib/mastodon/cli/preview_cards_spec.rb | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/spec/lib/mastodon/cli/preview_cards_spec.rb b/spec/lib/mastodon/cli/preview_cards_spec.rb index b4b018b3be..1e064ed58e 100644 --- a/spec/lib/mastodon/cli/preview_cards_spec.rb +++ b/spec/lib/mastodon/cli/preview_cards_spec.rb @@ -4,9 +4,52 @@ require 'rails_helper' require 'mastodon/cli/preview_cards' describe Mastodon::CLI::PreviewCards do + let(:cli) { described_class.new } + describe '.exit_on_failure?' do it 'returns true' do expect(described_class.exit_on_failure?).to be true end end + + describe '#remove' do + context 'with relevant preview cards' do + before do + Fabricate(:preview_card, updated_at: 10.years.ago, type: :link) + Fabricate(:preview_card, updated_at: 10.months.ago, type: :photo) + Fabricate(:preview_card, updated_at: 10.days.ago, type: :photo) + end + + context 'with no arguments' do + it 'deletes thumbnails for local preview cards' do + expect { cli.invoke(:remove) }.to output( + a_string_including('Removed 2 preview cards') + .and(a_string_including('approx. 119 KB')) + ).to_stdout + end + end + + context 'with the --link option' do + let(:options) { { link: true } } + + it 'deletes thumbnails for local preview cards' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('Removed 1 link-type preview cards') + .and(a_string_including('approx. 59.6 KB')) + ).to_stdout + end + end + + context 'with the --days option' do + let(:options) { { days: 365 } } + + it 'deletes thumbnails for local preview cards' do + expect { cli.invoke(:remove, [], options) }.to output( + a_string_including('Removed 1 preview cards') + .and(a_string_including('approx. 59.6 KB')) + ).to_stdout + end + end + end + end end From 12bb7be8b558122e44a66a4337e7db999c11e0b3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 17 Oct 2023 07:32:10 -0400 Subject: [PATCH 11/13] Spec speed ups on `AccountsController` spec (#25391) --- spec/controllers/accounts_controller_spec.rb | 638 ++++++++----------- 1 file changed, 254 insertions(+), 384 deletions(-) diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index d46fdb108d..cc9e3198b6 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -7,449 +7,319 @@ RSpec.describe AccountsController do let(:account) { Fabricate(:account) } - describe 'GET #show' do - let(:format) { 'html' } + shared_examples 'unapproved account check' do + before { account.user.update(approved: false) } - let!(:status) { Fabricate(:status, account: account) } - let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) } - let!(:status_self_reply) { Fabricate(:status, account: account, thread: status) } - let!(:status_media) { Fabricate(:status, account: account) } - let!(:status_pinned) { Fabricate(:status, account: account) } - let!(:status_private) { Fabricate(:status, account: account, visibility: :private) } - let!(:status_direct) { Fabricate(:status, account: account, visibility: :direct) } - let!(:status_reblog) { Fabricate(:status, account: account, reblog: Fabricate(:status)) } + it 'returns http not found' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(404) + end + end + + shared_examples 'permanently suspended account check' do before do - status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image) - account.pinned_statuses << status_pinned - account.pinned_statuses << status_private + account.suspend! + account.deletion_request.destroy end - shared_examples 'preliminary checks' do - context 'when account is not approved' do - before do - account.user.update(approved: false) - end + it 'returns http gone' do + get :show, params: { username: account.username, format: format } - it 'returns http not found' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(404) - end + expect(response).to have_http_status(410) + end + end + + shared_examples 'temporarily suspended account check' do |code: 403| + before { account.suspend! } + + it 'returns appropriate http response code' do + get :show, params: { username: account.username, format: format } + + expect(response).to have_http_status(code) + end + end + + describe 'GET #show' do + context 'with basic account status checks' do + context 'with HTML' do + let(:format) { 'html' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check' + end + + context 'with JSON' do + let(:format) { 'json' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check', code: 200 + end + + context 'with RSS' do + let(:format) { 'rss' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check' end end - context 'with HTML' do - let(:format) { 'html' } - - it_behaves_like 'preliminary checks' - - context 'when account is permanently suspended' do - before do - account.suspend! - account.deletion_request.destroy - end - - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) - end - end - - context 'when account is temporarily suspended' do - before do - account.suspend! - end - - it 'returns http forbidden' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(403) - end - end - - shared_examples 'common response characteristics' do - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns Link header' do - expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) - end - - it 'renders show template' do - expect(response).to render_template(:show) - end - end - - context 'with a normal account in an HTML request' do - before do - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with replies' do - before do - allow(controller).to receive(:replies_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with media' do - before do - allow(controller).to receive(:media_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with tag' do - let(:tag) { Fabricate(:tag) } - - let!(:status_tag) { Fabricate(:status, account: account) } - - before do - allow(controller).to receive(:tag_requested?).and_return(true) - status_tag.tags << tag - get :show, params: { username: account.username, format: format, tag: tag.to_param } - end - - it_behaves_like 'common response characteristics' - end - end - - context 'with JSON' do - let(:authorized_fetch_mode) { false } - let(:format) { 'json' } + context 'with existing statuses' do + let!(:status) { Fabricate(:status, account: account) } + let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) } + let!(:status_self_reply) { Fabricate(:status, account: account, thread: status) } + let!(:status_media) { Fabricate(:status, account: account) } + let!(:status_pinned) { Fabricate(:status, account: account) } + let!(:status_private) { Fabricate(:status, account: account, visibility: :private) } + let!(:status_direct) { Fabricate(:status, account: account, visibility: :direct) } + let!(:status_reblog) { Fabricate(:status, account: account, reblog: Fabricate(:status)) } before do - allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode) + status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image) + account.pinned_statuses << status_pinned + account.pinned_statuses << status_private end - it_behaves_like 'preliminary checks' + context 'with HTML' do + let(:format) { 'html' } - context 'when account is suspended permanently' do - before do - account.suspend! - account.deletion_request.destroy + shared_examples 'common HTML response' do + it 'returns a standard HTML response', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) + + expect(response).to render_template(:show) + end end - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) + context 'with a normal account in an HTML request' do + before do + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with replies' do + before do + allow(controller).to receive(:replies_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with media' do + before do + allow(controller).to receive(:media_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with tag' do + let(:tag) { Fabricate(:tag) } + + let!(:status_tag) { Fabricate(:status, account: account) } + + before do + allow(controller).to receive(:tag_requested?).and_return(true) + status_tag.tags << tag + get :show, params: { username: account.username, format: format, tag: tag.to_param } + end + + it_behaves_like 'common HTML response' end end - context 'when account is suspended temporarily' do + context 'with JSON' do + let(:authorized_fetch_mode) { false } + let(:format) { 'json' } + before do - account.suspend! + allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode) end - it 'returns http success' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(200) - end - end + context 'with a normal account in a JSON request' do + before do + get :show, params: { username: account.username, format: format } + end - context 'with a normal account in a JSON request' do - before do - get :show, params: { username: account.username, format: format } + it 'returns a JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.media_type).to eq 'application/activity+json' + + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end + + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + + context 'with authorized fetch mode' do + let(:authorized_fetch_mode) { true } + + it 'returns http unauthorized' do + expect(response).to have_http_status(401) + end + end end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'when signed in' do + let(:user) { Fabricate(:user) } + + before do + sign_in(user) + get :show, params: { username: account.username, format: format } + end + + it 'returns a private JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.media_type).to eq 'application/activity+json' + + expect(response.headers['Cache-Control']).to include 'private' + + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end end - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end + context 'with signature' do + let(:remote_account) { Fabricate(:account, domain: 'example.com') } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + before do + allow(controller).to receive(:signed_request_actor).and_return(remote_account) + get :show, params: { username: account.username, format: format } + end - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end + it 'returns a JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) - context 'with authorized fetch mode' do - let(:authorized_fetch_mode) { true } + expect(response.media_type).to eq 'application/activity+json' - it 'returns http unauthorized' do - expect(response).to have_http_status(401) + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end + + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + + context 'with authorized fetch mode' do + let(:authorized_fetch_mode) { true } + + it 'returns a private signature JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.media_type).to eq 'application/activity+json' + + expect(response.headers['Cache-Control']).to include 'private' + + expect(response.headers['Vary']).to include 'Signature' + + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end end end end - context 'when signed in' do - let(:user) { Fabricate(:user) } - - before do - sign_in(user) - get :show, params: { username: account.username, format: format } - end - - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end - - it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'private' - end - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end - end - - context 'with signature' do - let(:remote_account) { Fabricate(:account, domain: 'example.com') } - - before do - allow(controller).to receive(:signed_request_actor).and_return(remote_account) - get :show, params: { username: account.username, format: format } - end - - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end - - context 'with authorized fetch mode' do - let(:authorized_fetch_mode) { true } + context 'with RSS' do + let(:format) { 'rss' } + shared_examples 'common RSS response' do it 'returns http success' do expect(response).to have_http_status(200) end - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + end + + context 'with a normal account in an RSS request' do + before do + get :show, params: { username: account.username, format: format } end - it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'private' - end + it_behaves_like 'common RSS response' - it 'returns Vary header with Signature' do - expect(response.headers['Vary']).to include 'Signature' - end - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + it 'responds with correct statuses', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to include_status_tag(status_self_reply) + expect(response.body).to include_status_tag(status) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) end end - end - end - context 'with RSS' do - let(:format) { 'rss' } + context 'with replies' do + before do + allow(controller).to receive(:replies_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end - it_behaves_like 'preliminary checks' + it_behaves_like 'common RSS response' - context 'when account is permanently suspended' do - before do - account.suspend! - account.deletion_request.destroy + it 'responds with correct statuses with replies', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to include_status_tag(status_reply) + expect(response.body).to include_status_tag(status_self_reply) + expect(response.body).to include_status_tag(status) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + end end - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) - end - end + context 'with media' do + before do + allow(controller).to receive(:media_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end - context 'when account is temporarily suspended' do - before do - account.suspend! + it_behaves_like 'common RSS response' + + it 'responds with correct statuses with media', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) + expect(response.body).to_not include_status_tag(status_self_reply) + expect(response.body).to_not include_status_tag(status) + end end - it 'returns http forbidden' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(403) - end - end + context 'with tag' do + let(:tag) { Fabricate(:tag) } - shared_examples 'common response characteristics' do - it 'returns http success' do - expect(response).to have_http_status(200) - end + let!(:status_tag) { Fabricate(:status, account: account) } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - end + before do + allow(controller).to receive(:tag_requested?).and_return(true) + status_tag.tags << tag + get :show, params: { username: account.username, format: format, tag: tag.to_param } + end - context 'with a normal account in an RSS request' do - before do - get :show, params: { username: account.username, format: format } - end + it_behaves_like 'common RSS response' - it_behaves_like 'common response characteristics' - - it 'renders public status' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'renders self-reply' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with replies' do - before do - allow(controller).to receive(:replies_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - - it 'renders public status' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'renders self-reply' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'renders reply to someone else' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with media' do - before do - allow(controller).to receive(:media_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - - it 'does not render public status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'does not render self-reply' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with tag' do - let(:tag) { Fabricate(:tag) } - - let!(:status_tag) { Fabricate(:status, account: account) } - - before do - allow(controller).to receive(:tag_requested?).and_return(true) - status_tag.tags << tag - get :show, params: { username: account.username, format: format, tag: tag.to_param } - end - - it_behaves_like 'common response characteristics' - - it 'does not render public status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'does not render self-reply' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'does not render status with media' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - - it 'renders status with tag' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_tag)) + it 'responds with correct statuses with a tag', :aggregate_failures do + expect(response.body).to include_status_tag(status_tag) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_media) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) + expect(response.body).to_not include_status_tag(status_self_reply) + expect(response.body).to_not include_status_tag(status) + end end end end end + + def include_status_tag(status) + include ActivityPub::TagManager.instance.url_for(status) + end end From c4bddc9855886b4ba60316fbcbc04375ee720da7 Mon Sep 17 00:00:00 2001 From: Victor Lee <43049052+leevic31@users.noreply.github.com> Date: Tue, 17 Oct 2023 08:56:24 -0400 Subject: [PATCH 12/13] Add spec for poll model (#23399) Co-authored-by: Nick Schonning Co-authored-by: Claire --- spec/models/poll_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/models/poll_spec.rb b/spec/models/poll_spec.rb index 8ae04ca41f..5aa5548cc8 100644 --- a/spec/models/poll_spec.rb +++ b/spec/models/poll_spec.rb @@ -29,4 +29,23 @@ describe Poll do end end end + + describe 'validations' do + context 'when valid' do + let(:poll) { Fabricate.build(:poll) } + + it 'is valid with valid attributes' do + expect(poll).to be_valid + end + end + + context 'when not valid' do + let(:poll) { Fabricate.build(:poll, expires_at: nil) } + + it 'is invalid without an expire date' do + poll.valid? + expect(poll).to model_have_error_on_field(:expires_at) + end + end + end end From 935d54124e80e9fe5365c724e5c8827a2b3ed5b3 Mon Sep 17 00:00:00 2001 From: Stanislas Signoud Date: Tue, 17 Oct 2023 14:59:07 +0200 Subject: [PATCH 13/13] Fix missing redirections to make sure /home redirect to the advanced UI (#27378) --- app/javascript/mastodon/features/ui/index.jsx | 2 ++ app/javascript/mastodon/initial_state.js | 1 + 2 files changed, 3 insertions(+) diff --git a/app/javascript/mastodon/features/ui/index.jsx b/app/javascript/mastodon/features/ui/index.jsx index ac5e2d9361..2f39be24a0 100644 --- a/app/javascript/mastodon/features/ui/index.jsx +++ b/app/javascript/mastodon/features/ui/index.jsx @@ -184,7 +184,9 @@ class SwitchingColumnsArea extends PureComponent { {singleColumn ? : null} {singleColumn && pathName.startsWith('/deck/') ? : null} + {/* Redirect old bookmarks (without /deck) with home-like routes to the advanced interface */} {!singleColumn && pathName === '/getting-started' ? : null} + {!singleColumn && pathName === '/home' ? : null} diff --git a/app/javascript/mastodon/initial_state.js b/app/javascript/mastodon/initial_state.js index 11cd2a1673..bf5ce556e8 100644 --- a/app/javascript/mastodon/initial_state.js +++ b/app/javascript/mastodon/initial_state.js @@ -100,6 +100,7 @@ const initialPath = document.querySelector("head meta[name=initialPath]")?.getAt /** @type {boolean} */ export const hasMultiColumnPath = initialPath === '/' || initialPath === '/getting-started' + || initialPath === '/home' || initialPath.startsWith('/deck'); /**