From b20af17a2c7dec49bc7ad1ec6706c5b02bf4f498 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 30 Nov 2023 14:47:07 +0100 Subject: [PATCH 1/4] Fix onboarding step descriptions being truncated on narrow screens (#28021) --- app/javascript/styles/mastodon/components.scss | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 9c3d9dc2c5..2106b529d6 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -2732,22 +2732,16 @@ $ui-header-height: 55px; &__description { flex: 1 1 auto; line-height: 20px; - white-space: nowrap; - text-overflow: ellipsis; - overflow: hidden; h6 { color: $highlight-text-color; font-weight: 500; font-size: 14px; - overflow: hidden; - text-overflow: ellipsis; } p { color: $darker-text-color; overflow: hidden; - text-overflow: ellipsis; } } } From a8473f582d0d25ac0a2ad385cf3915e8012a3646 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 30 Nov 2023 08:55:20 -0500 Subject: [PATCH 2/4] Add zeitwerk inflector for cli->CLI (#27635) --- config/initializers/inflections.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 3d17509456..ba459e19f2 100644 --- a/config/initializers/inflections.rb +++ b/config/initializers/inflections.rb @@ -13,21 +13,22 @@ # end ActiveSupport::Inflector.inflections(:en) do |inflect| - inflect.acronym 'StatsD' - inflect.acronym 'OEmbed' - inflect.acronym 'OStatus' inflect.acronym 'ActivityPub' - inflect.acronym 'PubSubHubbub' inflect.acronym 'ActivityStreams' - inflect.acronym 'JsonLd' - inflect.acronym 'Ed25519' - inflect.acronym 'TOC' - inflect.acronym 'RSS' - inflect.acronym 'REST' - inflect.acronym 'URL' inflect.acronym 'ASCII' + inflect.acronym 'CLI' inflect.acronym 'DeepL' inflect.acronym 'DSL' + inflect.acronym 'Ed25519' + inflect.acronym 'JsonLd' + inflect.acronym 'OEmbed' + inflect.acronym 'OStatus' + inflect.acronym 'PubSubHubbub' + inflect.acronym 'REST' + inflect.acronym 'RSS' + inflect.acronym 'StatsD' + inflect.acronym 'TOC' + inflect.acronym 'URL' inflect.singular 'data', 'data' end From e7c340a63475713d7f55b92fb8c963478f4d622a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 30 Nov 2023 08:59:46 -0500 Subject: [PATCH 3/4] Spec coverage for missing `q` param and error conditions in `api/v2/search` controller (#27842) --- app/controllers/api/v2/search_controller.rb | 31 ++++++++++-- .../api/v2/search_controller_spec.rb | 48 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v2/search_controller.rb b/app/controllers/api/v2/search_controller.rb index 35be549305..4339bee21e 100644 --- a/app/controllers/api/v2/search_controller.rb +++ b/app/controllers/api/v2/search_controller.rb @@ -8,6 +8,11 @@ class Api::V2::SearchController < Api::BaseController before_action -> { authorize_if_got_token! :read, :'read:search' } before_action :validate_search_params! + with_options unless: :user_signed_in? do + before_action :query_pagination_error, if: :pagination_requested? + before_action :remote_resolve_error, if: :remote_resolve_requested? + end + def index @search = Search.new(search_results) render json: @search, serializer: REST::SearchSerializer @@ -21,12 +26,22 @@ class Api::V2::SearchController < Api::BaseController def validate_search_params! params.require(:q) + end - return if user_signed_in? + def query_pagination_error + render json: { error: 'Search queries pagination is not supported without authentication' }, status: 401 + end - return render json: { error: 'Search queries pagination is not supported without authentication' }, status: 401 if params[:offset].present? + def remote_resolve_error + render json: { error: 'Search queries that resolve remote resources are not supported without authentication' }, status: 401 + end - render json: { error: 'Search queries that resolve remote resources are not supported without authentication' }, status: 401 if truthy_param?(:resolve) + def remote_resolve_requested? + truthy_param?(:resolve) + end + + def pagination_requested? + params[:offset].present? end def search_results @@ -34,7 +49,15 @@ class Api::V2::SearchController < Api::BaseController params[:q], current_account, limit_param(RESULTS_LIMIT), - search_params.merge(resolve: truthy_param?(:resolve), exclude_unreviewed: truthy_param?(:exclude_unreviewed), following: truthy_param?(:following)) + combined_search_params + ) + end + + def combined_search_params + search_params.merge( + resolve: truthy_param?(:resolve), + exclude_unreviewed: truthy_param?(:exclude_unreviewed), + following: truthy_param?(:following) ) end diff --git a/spec/controllers/api/v2/search_controller_spec.rb b/spec/controllers/api/v2/search_controller_spec.rb index d3ff42d6a0..a16716a10c 100644 --- a/spec/controllers/api/v2/search_controller_spec.rb +++ b/spec/controllers/api/v2/search_controller_spec.rb @@ -34,6 +34,26 @@ RSpec.describe Api::V2::SearchController do expect(body_as_json[:accounts].pluck(:id)).to contain_exactly(bob.id.to_s, ana.id.to_s, tom.id.to_s) end + context 'with truthy `resolve`' do + let(:params) { { q: 'test1', resolve: '1' } } + + it 'returns http unauthorized' do + get :index, params: params + + expect(response).to have_http_status(200) + end + end + + context 'with `offset`' do + let(:params) { { q: 'test1', offset: 1 } } + + it 'returns http unauthorized' do + get :index, params: params + + expect(response).to have_http_status(200) + end + end + context 'with following=true' do let(:params) { { q: 'test', type: 'accounts', following: 'true' } } @@ -48,6 +68,26 @@ RSpec.describe Api::V2::SearchController do end end end + + context 'when search raises syntax error' do + before { allow(Search).to receive(:new).and_raise(Mastodon::SyntaxError) } + + it 'returns http unprocessable_entity' do + get :index, params: params + + expect(response).to have_http_status(422) + end + end + + context 'when search raises not found error' do + before { allow(Search).to receive(:new).and_raise(ActiveRecord::RecordNotFound) } + + it 'returns http not_found' do + get :index, params: params + + expect(response).to have_http_status(404) + end + end end end @@ -59,6 +99,12 @@ RSpec.describe Api::V2::SearchController do get :index, params: search_params end + context 'without a `q` param' do + it 'returns http bad_request' do + expect(response).to have_http_status(400) + end + end + context 'with a `q` shorter than 5 characters' do let(:search_params) { { q: 'test' } } @@ -79,6 +125,7 @@ RSpec.describe Api::V2::SearchController do it 'returns http unauthorized' do expect(response).to have_http_status(401) + expect(response.body).to match('resolve remote resources') end end @@ -87,6 +134,7 @@ RSpec.describe Api::V2::SearchController do it 'returns http unauthorized' do expect(response).to have_http_status(401) + expect(response.body).to match('pagination is not supported') end end end From 0530ce5e9575c09464847412f43852f438b0494e Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 30 Nov 2023 09:28:05 -0500 Subject: [PATCH 4/4] Convert accounts controller spec to request spec (#28126) --- .../accounts_spec.rb} | 158 +++++++++--------- 1 file changed, 80 insertions(+), 78 deletions(-) rename spec/{controllers/accounts_controller_spec.rb => requests/accounts_spec.rb} (56%) diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/requests/accounts_spec.rb similarity index 56% rename from spec/controllers/accounts_controller_spec.rb rename to spec/requests/accounts_spec.rb index 542a748784..bf067cdc38 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -2,23 +2,22 @@ require 'rails_helper' -RSpec.describe AccountsController do - render_views - +describe 'Accounts show response' do let(:account) { Fabricate(:account) } - describe 'unapproved account check' do + context 'with an unapproved account' do before { account.user.update(approved: false) } it 'returns http not found' do %w(html json rss).each do |format| - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username), as: format + expect(response).to have_http_status(404) end end end - describe 'permanently suspended account check' do + context 'with a permanently suspended account' do before do account.suspend! account.deletion_request.destroy @@ -26,25 +25,26 @@ RSpec.describe AccountsController do it 'returns http gone' do %w(html json rss).each do |format| - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username), as: format + expect(response).to have_http_status(410) end end end - describe 'temporarily suspended account check' do + context 'with a temporarily suspended account' do before { account.suspend! } it 'returns appropriate http response code' do { html: 403, json: 200, rss: 403 }.each do |format, code| - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username), as: format expect(response).to have_http_status(code) end end end - describe 'GET #show' do + describe 'GET to short username paths' do context 'with existing statuses' do let!(:status) { Fabricate(:status, account: account) } let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) } @@ -66,17 +66,17 @@ RSpec.describe AccountsController do shared_examples 'common HTML response' do it 'returns a standard HTML response', :aggregate_failures do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and render_template(:show) expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) - - 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 } + get short_account_path(username: account.username), as: format end it_behaves_like 'common HTML response' @@ -84,8 +84,7 @@ RSpec.describe AccountsController do context 'with replies' do before do - allow(controller).to receive(:replies_requested?).and_return(true) - get :show, params: { username: account.username, format: format } + get short_account_with_replies_path(username: account.username), as: format end it_behaves_like 'common HTML response' @@ -93,8 +92,7 @@ RSpec.describe AccountsController do context 'with media' do before do - allow(controller).to receive(:media_requested?).and_return(true) - get :show, params: { username: account.username, format: format } + get short_account_media_path(username: account.username), as: format end it_behaves_like 'common HTML response' @@ -106,9 +104,8 @@ RSpec.describe AccountsController do 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 } + get short_account_tag_path(username: account.username, tag: tag), as: format end it_behaves_like 'common HTML response' @@ -117,21 +114,25 @@ RSpec.describe AccountsController do context 'with JSON' do let(:authorized_fetch_mode) { false } - let(:format) { 'json' } + let(:headers) { { 'ACCEPT' => 'application/json' } } - before do - allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode) + around do |example| + ClimateControl.modify AUTHORIZED_FETCH: authorized_fetch_mode.to_s do + example.run + end end context 'with a normal account in a JSON request' do before do - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username), headers: headers end 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(response) + .to have_http_status(200) + .and have_attributes( + media_type: eq('application/activity+json') + ) expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) end @@ -152,13 +153,15 @@ RSpec.describe AccountsController do before do sign_in(user) - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username), headers: headers.merge({ 'Cookie' => '123' }) 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) + .to have_http_status(200) + .and have_attributes( + media_type: eq('application/activity+json') + ) expect(response.headers['Cache-Control']).to include 'private' @@ -170,14 +173,15 @@ RSpec.describe AccountsController 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 } + get short_account_path(username: account.username), headers: headers, sign_with: remote_account end 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(response) + .to have_http_status(200) + .and have_attributes( + media_type: eq('application/activity+json') + ) expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) end @@ -188,12 +192,13 @@ RSpec.describe AccountsController 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) + .to have_http_status(200) + .and have_attributes( + media_type: 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) @@ -207,60 +212,58 @@ RSpec.describe AccountsController do context 'with a normal account in an RSS request' do before do - get :show, params: { username: account.username, format: format } + get short_account_path(username: account.username, format: format) end it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'responds with correct statuses', :aggregate_failures do expect(response).to have_http_status(200) - 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) + expect(response.body).to include(status_tag_for(status_media)) + expect(response.body).to include(status_tag_for(status_self_reply)) + expect(response.body).to include(status_tag_for(status)) + expect(response.body).to_not include(status_tag_for(status_direct)) + expect(response.body).to_not include(status_tag_for(status_private)) + expect(response.body).to_not include(status_tag_for(status_reblog.reblog)) + expect(response.body).to_not include(status_tag_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 } + get short_account_with_replies_path(username: account.username, format: format) end it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'responds with correct statuses with replies', :aggregate_failures do expect(response).to have_http_status(200) - 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) + expect(response.body).to include(status_tag_for(status_media)) + expect(response.body).to include(status_tag_for(status_reply)) + expect(response.body).to include(status_tag_for(status_self_reply)) + expect(response.body).to include(status_tag_for(status)) + expect(response.body).to_not include(status_tag_for(status_direct)) + expect(response.body).to_not include(status_tag_for(status_private)) + expect(response.body).to_not include(status_tag_for(status_reblog.reblog)) 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 } + get short_account_media_path(username: account.username, format: format) end it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'responds with correct statuses with media', :aggregate_failures do expect(response).to have_http_status(200) - 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) + expect(response.body).to include(status_tag_for(status_media)) + expect(response.body).to_not include(status_tag_for(status_direct)) + expect(response.body).to_not include(status_tag_for(status_private)) + expect(response.body).to_not include(status_tag_for(status_reblog.reblog)) + expect(response.body).to_not include(status_tag_for(status_reply)) + expect(response.body).to_not include(status_tag_for(status_self_reply)) + expect(response.body).to_not include(status_tag_for(status)) end end @@ -270,30 +273,29 @@ RSpec.describe AccountsController do 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 } + get short_account_tag_path(username: account.username, tag: tag, format: format) end it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' it 'responds with correct statuses with a tag', :aggregate_failures do expect(response).to have_http_status(200) - 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) + expect(response.body).to include(status_tag_for(status_tag)) + expect(response.body).to_not include(status_tag_for(status_direct)) + expect(response.body).to_not include(status_tag_for(status_media)) + expect(response.body).to_not include(status_tag_for(status_private)) + expect(response.body).to_not include(status_tag_for(status_reblog.reblog)) + expect(response.body).to_not include(status_tag_for(status_reply)) + expect(response.body).to_not include(status_tag_for(status_self_reply)) + expect(response.body).to_not include(status_tag_for(status)) end end end end end - def include_status_tag(status) - include ActivityPub::TagManager.instance.url_for(status) + def status_tag_for(status) + ActivityPub::TagManager.instance.url_for(status) end end