From 0437dd9e770c293ec4f16d0513d1d5dd65209406 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 3 Sep 2024 13:37:09 +0200 Subject: [PATCH 01/24] Fix radio buttons styling in web UI (#31723) --- app/javascript/styles/mastodon/components.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 208cdd6768..c1e5075702 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -7580,7 +7580,7 @@ a.status-card { } } -.radio-button.checked::before { +.radio-button__input.checked::before { position: absolute; left: 2px; top: 2px; From 219458d7d4c9bb9ee0ed3c89c72438ee4edc2e1c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 09:18:53 -0400 Subject: [PATCH 02/24] Convert `tags` controller spec to system and request specs (#31708) --- spec/controllers/tags_controller_spec.rb | 45 ------------------ spec/requests/tags_spec.rb | 59 ++++++++++++++++++++++++ spec/system/tags_spec.rb | 16 +++++++ 3 files changed, 75 insertions(+), 45 deletions(-) delete mode 100644 spec/controllers/tags_controller_spec.rb create mode 100644 spec/requests/tags_spec.rb create mode 100644 spec/system/tags_spec.rb diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb deleted file mode 100644 index 2bb0c8de3b..0000000000 --- a/spec/controllers/tags_controller_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe TagsController do - render_views - - describe 'GET #show' do - let(:format) { 'html' } - let(:tag) { Fabricate(:tag, name: 'test') } - let(:tag_name) { tag&.name } - - before do - get :show, params: { id: tag_name, format: format } - end - - context 'when tag exists' do - context 'when requested as HTML' do - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - end - - context 'when requested as JSON' do - let(:format) { 'json' } - - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - end - end - - context 'when tag does not exist' do - let(:tag_name) { 'hoge' } - - it 'returns http not found' do - expect(response).to have_http_status(404) - end - end - end -end diff --git a/spec/requests/tags_spec.rb b/spec/requests/tags_spec.rb new file mode 100644 index 0000000000..c037bbef5a --- /dev/null +++ b/spec/requests/tags_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Tags' do + describe 'GET /tags/:id' do + context 'when tag exists' do + let(:tag) { Fabricate :tag } + + context 'with HTML format' do + # TODO: Convert the cacheable response shared example into a matcher, + # remove this example, rely on system spec (which should use matcher) + before { get tag_path(tag) } + + it 'returns http success' do + expect(response) + .to have_http_status(200) + end + + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + end + + context 'with JSON format' do + before { get tag_path(tag, format: :json) } + + it 'returns http success' do + expect(response) + .to have_http_status(200) + expect(response.content_type) + .to start_with('application/activity+json') + end + + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + end + + context 'with RSS format' do + before { get tag_path(tag, format: :rss) } + + it 'returns http success' do + expect(response) + .to have_http_status(200) + expect(response.content_type) + .to start_with('application/rss+xml') + end + + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + end + end + + context 'when tag does not exist' do + before { get tag_path('missing') } + + it 'returns http not found' do + expect(response) + .to have_http_status(404) + end + end + end +end diff --git a/spec/system/tags_spec.rb b/spec/system/tags_spec.rb new file mode 100644 index 0000000000..e9ad970a54 --- /dev/null +++ b/spec/system/tags_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Tags' do + describe 'Viewing a tag' do + let(:tag) { Fabricate(:tag, name: 'test') } + + it 'visits the tag page and renders the web app' do + visit tag_path(tag) + + expect(page) + .to have_css('noscript', text: /Mastodon/) + end + end +end From c9641c8070c07caa2da1e2875367a6bad8f0ecca Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 09:19:48 -0400 Subject: [PATCH 03/24] Remove un-needed edge case sort condition in languages helper (#31724) --- app/helpers/languages_helper.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/helpers/languages_helper.rb b/app/helpers/languages_helper.rb index 9e1c0a7db1..b6c09b7314 100644 --- a/app/helpers/languages_helper.rb +++ b/app/helpers/languages_helper.rb @@ -238,9 +238,7 @@ module LanguagesHelper # Helper for self.sorted_locale_keys private_class_method def self.locale_name_for_sorting(locale) - if locale.blank? || locale == 'und' - '000' - elsif (supported_locale = SUPPORTED_LOCALES[locale.to_sym]) + if (supported_locale = SUPPORTED_LOCALES[locale.to_sym]) ASCIIFolding.new.fold(supported_locale[1]).downcase elsif (regional_locale = REGIONAL_LOCALE_NAMES[locale.to_sym]) ASCIIFolding.new.fold(regional_locale).downcase From ea0d691e196753b1a5b6747b014bac5b7da50e97 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 3 Sep 2024 16:32:26 +0200 Subject: [PATCH 04/24] Add `GET /api/v2_alpha/notifications/:group_key/accounts` (#31725) --- .../notifications/accounts_controller.rb | 50 ++++++++++++ .../api/v2_alpha/notifications_controller.rb | 4 +- config/routes/api.rb | 4 +- .../v2_alpha/notifications/accounts_spec.rb | 80 +++++++++++++++++++ 4 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 app/controllers/api/v2_alpha/notifications/accounts_controller.rb create mode 100644 spec/requests/api/v2_alpha/notifications/accounts_spec.rb diff --git a/app/controllers/api/v2_alpha/notifications/accounts_controller.rb b/app/controllers/api/v2_alpha/notifications/accounts_controller.rb new file mode 100644 index 0000000000..9933b63373 --- /dev/null +++ b/app/controllers/api/v2_alpha/notifications/accounts_controller.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +class Api::V2Alpha::Notifications::AccountsController < Api::BaseController + before_action -> { doorkeeper_authorize! :read, :'read:notifications' } + before_action :require_user! + before_action :set_notifications! + after_action :insert_pagination_headers, only: :index + + def index + @accounts = load_accounts + render json: @accounts, each_serializer: REST::AccountSerializer + end + + private + + def load_accounts + @paginated_notifications.map(&:from_account) + end + + def set_notifications! + @paginated_notifications = begin + current_account + .notifications + .without_suspended + .where(group_key: params[:notification_group_key]) + .includes(from_account: [:account_stat, :user]) + .paginate_by_max_id( + limit_param(DEFAULT_ACCOUNTS_LIMIT), + params[:max_id], + params[:since_id] + ) + end + end + + def next_path + api_v2_alpha_notification_accounts_url pagination_params(max_id: pagination_max_id) if records_continue? + end + + def prev_path + api_v2_alpha_notification_accounts_url pagination_params(min_id: pagination_since_id) unless @paginated_notifications.empty? + end + + def pagination_collection + @paginated_notifications + end + + def records_continue? + @paginated_notifications.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) + end +end diff --git a/app/controllers/api/v2_alpha/notifications_controller.rb b/app/controllers/api/v2_alpha/notifications_controller.rb index 13a016aeb7..bd6979955a 100644 --- a/app/controllers/api/v2_alpha/notifications_controller.rb +++ b/app/controllers/api/v2_alpha/notifications_controller.rb @@ -46,7 +46,7 @@ class Api::V2Alpha::NotificationsController < Api::BaseController end def show - @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:id]) + @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:group_key]) presenter = GroupedNotificationsPresenter.new(NotificationGroup.from_notifications([@notification])) render json: presenter, serializer: REST::DedupNotificationGroupSerializer end @@ -57,7 +57,7 @@ class Api::V2Alpha::NotificationsController < Api::BaseController end def dismiss - current_account.notifications.where(group_key: params[:id]).destroy_all + current_account.notifications.where(group_key: params[:group_key]).destroy_all render_empty end diff --git a/config/routes/api.rb b/config/routes/api.rb index c5addd3385..df975065bd 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -344,7 +344,7 @@ namespace :api, format: false do end namespace :v2_alpha do - resources :notifications, only: [:index, :show] do + resources :notifications, param: :group_key, only: [:index, :show] do collection do post :clear get :unread_count @@ -353,6 +353,8 @@ namespace :api, format: false do member do post :dismiss end + + resources :accounts, only: [:index], module: :notifications end end diff --git a/spec/requests/api/v2_alpha/notifications/accounts_spec.rb b/spec/requests/api/v2_alpha/notifications/accounts_spec.rb new file mode 100644 index 0000000000..6a6ce043d3 --- /dev/null +++ b/spec/requests/api/v2_alpha/notifications/accounts_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Accounts in grouped 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/v2_alpha/notifications/:group_key/accounts', :inline_jobs do + subject do + get "/api/v2_alpha/notifications/#{user.account.notifications.first.group_key}/accounts", headers: headers, params: params + end + + let(:params) { {} } + + before do + first_status = PostStatusService.new.call(user.account, text: 'Test') + FavouriteService.new.call(Fabricate(:account), first_status) + FavouriteService.new.call(Fabricate(:account), first_status) + ReblogService.new.call(Fabricate(:account), first_status) + FollowService.new.call(Fabricate(:account), user.account) + FavouriteService.new.call(Fabricate(:account), first_status) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + it 'returns a list of accounts' do + subject + + expect(response).to have_http_status(200) + + # The group we are interested in is only favorites + notifications = user.account.notifications.where(type: 'favourite').reorder(id: :desc) + expect(body_as_json).to match( + [ + a_hash_including( + id: notifications.first.from_account_id.to_s + ), + a_hash_including( + id: notifications.second.from_account_id.to_s + ), + a_hash_including( + id: notifications.third.from_account_id.to_s + ), + ] + ) + end + + context 'with limit param' do + let(:params) { { limit: 2 } } + + it 'returns the requested number of accounts, with pagination headers' do + subject + + expect(response).to have_http_status(200) + + # The group we are interested in is only favorites + notifications = user.account.notifications.where(type: 'favourite').reorder(id: :desc) + expect(body_as_json).to match( + [ + a_hash_including( + id: notifications.first.from_account_id.to_s + ), + a_hash_including( + id: notifications.second.from_account_id.to_s + ), + ] + ) + + expect(response) + .to include_pagination_headers( + prev: api_v2_alpha_notification_accounts_url(limit: params[:limit], min_id: notifications.first.id), + next: api_v2_alpha_notification_accounts_url(limit: params[:limit], max_id: notifications.second.id) + ) + end + end + end +end From 97bb8df1c1ba33631c71df13210aa4b7a9ce06f1 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:32:59 +0200 Subject: [PATCH 05/24] Update dependency rspec-rails to v7.0.1 (#31695) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6d13efdb66..192142fc5f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -714,7 +714,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.0) + rspec-core (3.13.1) rspec-support (~> 3.13.0) rspec-expectations (3.13.2) diff-lcs (>= 1.2.0, < 2.0) @@ -724,7 +724,7 @@ GEM rspec-mocks (3.13.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-rails (7.0.0) + rspec-rails (7.0.1) actionpack (>= 7.0) activesupport (>= 7.0) railties (>= 7.0) From ae363f05555115503ab4679e826c6658416c38b5 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 3 Sep 2024 16:43:34 +0200 Subject: [PATCH 06/24] Fix spacing between icons and labels in settings/admin interface (#31728) --- app/helpers/application_helper.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f1c77d40eb..7c91df8d4f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -106,11 +106,16 @@ module ApplicationHelper end def material_symbol(icon, attributes = {}) - inline_svg_tag( - "400-24px/#{icon}.svg", - class: ['icon', "material-#{icon}"].concat(attributes[:class].to_s.split), - role: :img, - data: attributes[:data] + safe_join( + [ + inline_svg_tag( + "400-24px/#{icon}.svg", + class: ['icon', "material-#{icon}"].concat(attributes[:class].to_s.split), + role: :img, + data: attributes[:data] + ), + ' ', + ] ) end From 5b595b8a5a0872a5c4eaaca83a60ba7b3652e847 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:23:16 -0400 Subject: [PATCH 07/24] Remove usage of `assigns` in controller specs (#30195) --- app/views/settings/imports/index.html.haml | 2 +- .../admin/accounts_controller_spec.rb | 11 ++++---- .../admin/domain_allows_controller_spec.rb | 1 - .../admin/domain_blocks_controller_spec.rb | 2 -- .../export_domain_blocks_controller_spec.rb | 22 +++++++++------- .../admin/instances_controller_spec.rb | 9 ++++--- .../admin/invites_controller_spec.rb | 3 ++- .../admin/reports_controller_spec.rb | 26 +++++++++---------- .../authorize_interactions_controller_spec.rb | 10 ++++--- .../account_controller_concern_spec.rb | 7 ++--- .../settings/imports_controller_spec.rb | 8 +++--- .../confirmations_controller_spec.rb | 13 +++++++--- .../recovery_codes_controller_spec.rb | 3 ++- 13 files changed, 66 insertions(+), 51 deletions(-) diff --git a/app/views/settings/imports/index.html.haml b/app/views/settings/imports/index.html.haml index ca815720fd..bfddd45460 100644 --- a/app/views/settings/imports/index.html.haml +++ b/app/views/settings/imports/index.html.haml @@ -46,7 +46,7 @@ %th= t('imports.failures') %tbody - @recent_imports.each do |import| - %tr + %tr{ id: dom_id(import) } %td= t("imports.types.#{import.type}") %td - if import.state_unconfirmed? diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index f241d261b1..89a7239f53 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -40,15 +40,16 @@ RSpec.describe Admin::AccountsController do expect(response) .to have_http_status(200) - expect(assigns(:accounts)) - .to have_attributes( - count: eq(1), - klass: be(Account) - ) + expect(accounts_table_rows.size) + .to eq(1) expect(AccountFilter) .to have_received(:new) .with(hash_including(params)) end + + def accounts_table_rows + Nokogiri::Slop(response.body).css('table.accounts-table tr') + end end describe 'GET #show' do diff --git a/spec/controllers/admin/domain_allows_controller_spec.rb b/spec/controllers/admin/domain_allows_controller_spec.rb index 6f82f322b5..036d229091 100644 --- a/spec/controllers/admin/domain_allows_controller_spec.rb +++ b/spec/controllers/admin/domain_allows_controller_spec.rb @@ -13,7 +13,6 @@ RSpec.describe Admin::DomainAllowsController do it 'assigns a new domain allow' do get :new - expect(assigns(:domain_allow)).to be_instance_of(DomainAllow) expect(response).to have_http_status(200) end end diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index eb2c6265d1..a99ca6c641 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -13,7 +13,6 @@ RSpec.describe Admin::DomainBlocksController do it 'assigns a new domain block' do get :new - expect(assigns(:domain_block)).to be_instance_of(DomainBlock) expect(response).to have_http_status(200) end end @@ -171,7 +170,6 @@ RSpec.describe Admin::DomainBlocksController do 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 diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb index bfcccfa06c..39195716c5 100644 --- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -42,11 +42,8 @@ RSpec.describe Admin::ExportDomainBlocksController do post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } } end - it 'renders page with expected domain blocks' do - expect(assigns(:domain_blocks).map { |block| [block.domain, block.severity.to_sym] }).to contain_exactly(['bad.domain', :silence], ['worse.domain', :suspend], ['reject.media', :noop]) - end - - it 'returns http success' do + it 'renders page with expected domain blocks and returns http success' do + expect(mapped_batch_table_rows).to contain_exactly(['bad.domain', :silence], ['worse.domain', :suspend], ['reject.media', :noop]) expect(response).to have_http_status(200) end end @@ -56,14 +53,19 @@ RSpec.describe Admin::ExportDomainBlocksController do post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks_list.txt') } } end - it 'renders page with expected domain blocks' do - expect(assigns(:domain_blocks).map { |block| [block.domain, block.severity.to_sym] }).to contain_exactly(['bad.domain', :suspend], ['worse.domain', :suspend], ['reject.media', :suspend]) - end - - it 'returns http success' do + it 'renders page with expected domain blocks and returns http success' do + expect(mapped_batch_table_rows).to contain_exactly(['bad.domain', :suspend], ['worse.domain', :suspend], ['reject.media', :suspend]) expect(response).to have_http_status(200) end end + + def mapped_batch_table_rows + batch_table_rows.map { |row| [row.at_css('[id$=_domain]')['value'], row.at_css('[id$=_severity]')['value'].to_sym] } + end + + def batch_table_rows + Nokogiri::Slop(response.body).css('body div.batch-table__row') + end end it 'displays error on no file selected' do diff --git a/spec/controllers/admin/instances_controller_spec.rb b/spec/controllers/admin/instances_controller_spec.rb index ca64dd90a0..a64bbb2c9f 100644 --- a/spec/controllers/admin/instances_controller_spec.rb +++ b/spec/controllers/admin/instances_controller_spec.rb @@ -28,12 +28,15 @@ RSpec.describe Admin::InstancesController do it 'renders instances' do get :index, params: { page: 2 } - instances = assigns(:instances).to_a - expect(instances.size).to eq 1 - expect(instances[0].domain).to eq 'less.popular' + expect(instance_directory_links.size).to eq(1) + expect(instance_directory_links.first.text.strip).to match('less.popular') expect(response).to have_http_status(200) end + + def instance_directory_links + Nokogiri::Slop(response.body).css('div.directory__tag a') + end end describe 'GET #show' do diff --git a/spec/controllers/admin/invites_controller_spec.rb b/spec/controllers/admin/invites_controller_spec.rb index 71748cbbec..8638f8e214 100644 --- a/spec/controllers/admin/invites_controller_spec.rb +++ b/spec/controllers/admin/invites_controller_spec.rb @@ -18,7 +18,8 @@ describe Admin::InvitesController do it 'renders index page' do expect(subject).to render_template :index - expect(assigns(:invites)).to include invite + expect(response.body) + .to include(invite.code) end end diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 5849163b5f..67fb28e7a5 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -13,39 +13,39 @@ describe Admin::ReportsController do describe 'GET #index' do it 'returns http success with no filters' do - specified = Fabricate(:report, action_taken_at: nil) - Fabricate(:report, action_taken_at: Time.now.utc) + specified = Fabricate(:report, action_taken_at: nil, comment: 'First report') + other = Fabricate(:report, action_taken_at: Time.now.utc, comment: 'Second report') get :index - reports = assigns(:reports).to_a - expect(reports.size).to eq 1 - expect(reports[0]).to eq specified expect(response).to have_http_status(200) + expect(response.body) + .to include(specified.comment) + .and not_include(other.comment) end it 'returns http success with resolved filter' do - specified = Fabricate(:report, action_taken_at: Time.now.utc) - Fabricate(:report, action_taken_at: nil) + specified = Fabricate(:report, action_taken_at: Time.now.utc, comment: 'First report') + other = Fabricate(:report, action_taken_at: nil, comment: 'Second report') get :index, params: { resolved: '1' } - reports = assigns(:reports).to_a - expect(reports.size).to eq 1 - expect(reports[0]).to eq specified - expect(response).to have_http_status(200) + expect(response.body) + .to include(specified.comment) + .and not_include(other.comment) end end describe 'GET #show' do it 'renders report' do - report = Fabricate(:report) + report = Fabricate(:report, comment: 'A big problem') get :show, params: { id: report } - expect(assigns(:report)).to eq report expect(response).to have_http_status(200) + expect(response.body) + .to include(report.comment) end end diff --git a/spec/controllers/authorize_interactions_controller_spec.rb b/spec/controllers/authorize_interactions_controller_spec.rb index 5282a196a6..ed55df08d9 100644 --- a/spec/controllers/authorize_interactions_controller_spec.rb +++ b/spec/controllers/authorize_interactions_controller_spec.rb @@ -46,8 +46,9 @@ describe AuthorizeInteractionsController do get :show, params: { acct: 'http://example.com' } - expect(response).to have_http_status(302) - expect(assigns(:resource)).to eq account + expect(response) + .to have_http_status(302) + .and redirect_to(web_url("@#{account.pretty_acct}")) end it 'sets resource from acct uri' do @@ -58,8 +59,9 @@ describe AuthorizeInteractionsController do get :show, params: { acct: 'acct:found@hostname' } - expect(response).to have_http_status(302) - expect(assigns(:resource)).to eq account + expect(response) + .to have_http_status(302) + .and redirect_to(web_url("@#{account.pretty_acct}")) end end end diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index 6eb970dedb..122ef21e93 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -7,7 +7,7 @@ describe AccountControllerConcern do include AccountControllerConcern def success - head 200 + render plain: @account.username # rubocop:disable RSpec/InstanceVariable end end @@ -51,12 +51,13 @@ describe AccountControllerConcern do context 'when account is not suspended' do let(:account) { Fabricate(:account, username: 'username') } - it 'assigns @account, returns success, and sets link headers' do + it 'Prepares the account, returns success, and sets link headers' do get 'success', params: { account_username: account.username } - expect(assigns(:account)).to eq account expect(response).to have_http_status(200) expect(response.headers['Link'].to_s).to eq(expected_link_headers) + expect(response.body) + .to include(account.username) end def expected_link_headers diff --git a/spec/controllers/settings/imports_controller_spec.rb b/spec/controllers/settings/imports_controller_spec.rb index 89ec39e54d..219b882e6d 100644 --- a/spec/controllers/settings/imports_controller_spec.rb +++ b/spec/controllers/settings/imports_controller_spec.rb @@ -21,9 +21,10 @@ RSpec.describe Settings::ImportsController do it 'assigns the expected imports', :aggregate_failures do expect(response).to have_http_status(200) - expect(assigns(:recent_imports)).to eq [import] - expect(assigns(:recent_imports)).to_not include(other_import) expect(response.headers['Cache-Control']).to include('private, no-store') + expect(response.body) + .to include("bulk_import_#{import.id}") + .and not_include("bulk_import_#{other_import.id}") end end @@ -261,7 +262,8 @@ RSpec.describe Settings::ImportsController do it 'does not creates an unconfirmed bulk_import', :aggregate_failures do expect { subject }.to_not(change { user.account.bulk_imports.count }) - expect(assigns(:import).errors).to_not be_empty + expect(response.body) + .to include('field_with_errors') end end diff --git a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb index 1b3b0cb0ae..1c8b483a0a 100644 --- a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -9,11 +9,16 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do it 'renders the new view' do subject - expect(assigns(:confirmation)).to be_instance_of Form::TwoFactorConfirmation - expect(assigns(:provision_url)).to eq 'otpauth://totp/cb6e6126.ngrok.io:local-part%40domain?secret=thisisasecretforthespecofnewview&issuer=cb6e6126.ngrok.io' - expect(assigns(:qrcode)).to be_instance_of RQRCode::QRCode expect(response).to have_http_status(200) expect(response).to render_template(:new) + expect(response.body) + .to include(qr_code_markup) + end + + def qr_code_markup + RQRCode::QRCode.new( + 'otpauth://totp/cb6e6126.ngrok.io:local-part%40domain?secret=thisisasecretforthespecofnewview&issuer=cb6e6126.ngrok.io' + ).as_svg(padding: 0, module_size: 4) end end @@ -61,10 +66,10 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do expect { post_create_with_options } .to change { user.reload.otp_secret }.to 'thisisasecretforthespecofnewview' - expect(assigns(:recovery_codes)).to eq otp_backup_codes expect(flash[:notice]).to eq 'Two-factor authentication successfully enabled' expect(response).to have_http_status(200) expect(response).to render_template('settings/two_factor_authentication/recovery_codes/index') + expect(response.body).to include(*otp_backup_codes) end end diff --git a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb index 28a40e138c..dbc2e3059c 100644 --- a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb @@ -15,10 +15,11 @@ describe Settings::TwoFactorAuthentication::RecoveryCodesController do sign_in user, scope: :user post :create, session: { challenge_passed_at: Time.now.utc } - expect(assigns(:recovery_codes)).to eq otp_backup_codes expect(flash[:notice]).to eq 'Recovery codes successfully regenerated' expect(response).to have_http_status(200) expect(response).to render_template(:index) + expect(response.body) + .to include(*otp_backup_codes) end it 'redirects when not signed in' do From 79b1841805f47cd3931bfdb7d51e1791529c3845 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:23:57 -0400 Subject: [PATCH 08/24] Disable `without_verify_partial_doubles` in statuses/show view spec (#29132) --- spec/spec_helper.rb | 6 ------ spec/views/statuses/show.html.haml_spec.rb | 21 +++++++++++++++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 248c2c4105..2d20239b27 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,12 +8,6 @@ RSpec.configure do |config| config.mock_with :rspec do |mocks| mocks.verify_partial_doubles = true - - config.around(:example, :without_verify_partial_doubles) do |example| - mocks.verify_partial_doubles = false - example.call - mocks.verify_partial_doubles = true - end end config.before :suite do diff --git a/spec/views/statuses/show.html.haml_spec.rb b/spec/views/statuses/show.html.haml_spec.rb index 1c408db6c5..fd08f2772d 100644 --- a/spec/views/statuses/show.html.haml_spec.rb +++ b/spec/views/statuses/show.html.haml_spec.rb @@ -2,14 +2,13 @@ require 'rails_helper' -describe 'statuses/show.html.haml', :without_verify_partial_doubles do +describe 'statuses/show.html.haml' do let(:alice) { Fabricate(:account, username: 'alice', display_name: 'Alice') } let(:status) { Fabricate(:status, account: alice, text: 'Hello World') } before do - allow(view).to receive_messages(api_oembed_url: '', site_title: 'example site', site_hostname: 'example.com', full_asset_url: '//asset.host/image.svg', current_account: nil, single_user_mode?: false) - allow(view).to receive(:local_time) - allow(view).to receive(:local_time_ago) + view.extend view_helpers + assign(:instance_presenter, InstancePresenter.new) Fabricate(:media_attachment, account: alice, status: status, type: :video) @@ -40,4 +39,18 @@ describe 'statuses/show.html.haml', :without_verify_partial_doubles do def header_tags view.content_for(:header_tags) end + + def view_helpers + Module.new do + def api_oembed_url(_) = '' + def show_landing_strip? = true + def site_title = 'example site' + def site_hostname = 'example.com' + def full_asset_url(_) = '//asset.host/image.svg' + def current_account = nil + def single_user_mode? = false + def local_time = nil + def local_time_ago = nil + end + end end From dc2f67f69bfd0536564268eb73a79f1eec80cb03 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:24:31 -0400 Subject: [PATCH 09/24] Remove `fuubar` gem and custom rspec `--format` setting (#30594) --- .gitignore | 3 +++ .rspec | 1 - Gemfile | 3 --- Gemfile.lock | 4 ---- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index a70f30f952..a74317bd7d 100644 --- a/.gitignore +++ b/.gitignore @@ -71,3 +71,6 @@ docker-compose.override.yml # Ignore dotenv .local files .env*.local + +# Ignore local-only rspec configuration +.rspec-local diff --git a/.rspec b/.rspec index 9a8e706d09..83e16f8044 100644 --- a/.rspec +++ b/.rspec @@ -1,3 +1,2 @@ --color --require spec_helper ---format Fuubar diff --git a/Gemfile b/Gemfile index af05ec8402..6538067cff 100644 --- a/Gemfile +++ b/Gemfile @@ -126,9 +126,6 @@ group :test do # Adds RSpec Error/Warning annotations to GitHub PRs on the Files tab gem 'rspec-github', '~> 2.4', require: false - # RSpec progress bar formatter - gem 'fuubar', '~> 2.5' - # RSpec helpers for email specs gem 'email_spec' diff --git a/Gemfile.lock b/Gemfile.lock index 192142fc5f..2e9037d8a6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -288,9 +288,6 @@ GEM fugit (1.11.1) et-orbi (~> 1, >= 1.2.11) raabro (~> 1.4) - fuubar (2.5.1) - rspec-core (~> 3.0) - ruby-progressbar (~> 1.4) globalid (1.2.1) activesupport (>= 6.1) google-protobuf (3.25.4) @@ -949,7 +946,6 @@ DEPENDENCIES flatware-rspec fog-core (<= 2.5.0) fog-openstack (~> 1.0) - fuubar (~> 2.5) haml-rails (~> 2.0) haml_lint hcaptcha (~> 7.1) From ef4920c6c92b4191b12f0de820d694e8abf14d4a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:28:57 -0400 Subject: [PATCH 10/24] Pull out https/hostname setup for request specs to shared config (#31622) --- spec/rails_helper.rb | 6 ++++++ spec/requests/account_show_page_spec.rb | 2 +- spec/requests/api/v1/streaming_spec.rb | 5 ++--- spec/requests/link_headers_spec.rb | 2 +- spec/requests/media_proxy_spec.rb | 17 +++++++++++------ 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ba712c08f9..0c3d01c785 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -142,6 +142,12 @@ RSpec.configure do |config| end end + config.before :each, type: :request do + # Use https and configured hostname in request spec requests + integration_session.https! + host! Rails.configuration.x.local_domain + end + config.after do Rails.cache.clear redis.del(redis.keys) diff --git a/spec/requests/account_show_page_spec.rb b/spec/requests/account_show_page_spec.rb index 830d778608..bdcec12fdb 100644 --- a/spec/requests/account_show_page_spec.rb +++ b/spec/requests/account_show_page_spec.rb @@ -14,7 +14,7 @@ describe 'The account show page' do expect(head_meta_content('og:title')).to match alice.display_name expect(head_meta_content('og:type')).to eq 'profile' expect(head_meta_content('og:image')).to match '.+' - expect(head_meta_content('og:url')).to match 'http://.+' + expect(head_meta_content('og:url')).to eq short_account_url(username: alice.username) end def head_link_icons diff --git a/spec/requests/api/v1/streaming_spec.rb b/spec/requests/api/v1/streaming_spec.rb index 6b550dfa60..6ce35c2fe6 100644 --- a/spec/requests/api/v1/streaming_spec.rb +++ b/spec/requests/api/v1/streaming_spec.rb @@ -10,12 +10,11 @@ describe 'API V1 Streaming' do Rails.configuration.x.streaming_api_base_url = before end - let(:headers) { { 'Host' => Rails.configuration.x.web_domain } } - context 'with streaming api on same host' do describe 'GET /api/v1/streaming' do it 'raises ActiveRecord::RecordNotFound' do - get '/api/v1/streaming', headers: headers + integration_session.https!(false) + get '/api/v1/streaming' expect(response).to have_http_status(404) end diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb index b822adbfb8..522cff4642 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -13,7 +13,7 @@ describe 'Link headers' do it 'contains webfinger url in link header' do link_header = link_header_with_type('application/jrd+json') - expect(link_header.href).to eq 'http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io' + expect(link_header.href).to eq 'https://cb6e6126.ngrok.io/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io' expect(link_header.attr_pairs.first).to eq %w(rel lrdd) end diff --git a/spec/requests/media_proxy_spec.rb b/spec/requests/media_proxy_spec.rb index 0524105d90..814d4c1166 100644 --- a/spec/requests/media_proxy_spec.rb +++ b/spec/requests/media_proxy_spec.rb @@ -4,12 +4,7 @@ require 'rails_helper' describe 'Media Proxy' do describe 'GET /media_proxy/:id' do - before do - integration_session.https! # TODO: Move to global rails_helper for all request specs? - host! Rails.configuration.x.local_domain # TODO: Move to global rails_helper for all request specs? - - stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) - end + before { stub_attachment_request } context 'when attached to a status' do let(:status) { Fabricate(:status) } @@ -63,5 +58,15 @@ describe 'Media Proxy' do .to have_http_status(404) end end + + def stub_attachment_request + stub_request( + :get, + 'http://example.com/attachment.png' + ) + .to_return( + request_fixture('avatar.txt') + ) + end end end From 928390c2ba4e8c6f433025f3eb3ecd89337a2660 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:29:32 -0400 Subject: [PATCH 11/24] Convert `admin/settings` controller specs to system specs (#31548) --- .../admin/settings/about_controller_spec.rb | 29 --------------- .../settings/appearance_controller_spec.rb | 29 --------------- .../settings/branding_controller_spec.rb | 17 --------- .../content_retention_controller_spec.rb | 29 --------------- .../settings/discovery_controller_spec.rb | 29 --------------- .../settings/registrations_controller_spec.rb | 29 --------------- spec/rails_helper.rb | 1 + spec/support/system_helpers.rb | 19 ++++++++++ spec/system/admin/settings/about_spec.rb | 22 +++++++++++ spec/system/admin/settings/appearance_spec.rb | 22 +++++++++++ spec/system/admin/settings/branding_spec.rb | 37 +++++++++++++++++++ .../admin/settings/content_retention_spec.rb | 22 +++++++++++ spec/system/admin/settings/discovery_spec.rb | 21 +++++++++++ .../admin/settings/registrations_spec.rb | 26 +++++++++++++ 14 files changed, 170 insertions(+), 162 deletions(-) delete mode 100644 spec/controllers/admin/settings/about_controller_spec.rb delete mode 100644 spec/controllers/admin/settings/appearance_controller_spec.rb delete mode 100644 spec/controllers/admin/settings/content_retention_controller_spec.rb delete mode 100644 spec/controllers/admin/settings/discovery_controller_spec.rb delete mode 100644 spec/controllers/admin/settings/registrations_controller_spec.rb create mode 100644 spec/support/system_helpers.rb create mode 100644 spec/system/admin/settings/about_spec.rb create mode 100644 spec/system/admin/settings/appearance_spec.rb create mode 100644 spec/system/admin/settings/branding_spec.rb create mode 100644 spec/system/admin/settings/content_retention_spec.rb create mode 100644 spec/system/admin/settings/discovery_spec.rb create mode 100644 spec/system/admin/settings/registrations_spec.rb diff --git a/spec/controllers/admin/settings/about_controller_spec.rb b/spec/controllers/admin/settings/about_controller_spec.rb deleted file mode 100644 index f322cb4434..0000000000 --- a/spec/controllers/admin/settings/about_controller_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Admin::Settings::AboutController do - render_views - - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - - before do - sign_in user, scope: :user - end - - describe 'GET #show' do - it 'returns http success' do - get :show - - 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 deleted file mode 100644 index ea6f3b7833..0000000000 --- a/spec/controllers/admin/settings/appearance_controller_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Admin::Settings::AppearanceController do - render_views - - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - - before do - sign_in user, scope: :user - end - - describe 'GET #show' do - it 'returns http success' do - get :show - - 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/branding_controller_spec.rb b/spec/controllers/admin/settings/branding_controller_spec.rb index e30300b4e4..5e46910cc6 100644 --- a/spec/controllers/admin/settings/branding_controller_spec.rb +++ b/spec/controllers/admin/settings/branding_controller_spec.rb @@ -10,14 +10,6 @@ RSpec.describe Admin::Settings::BrandingController do sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin')), scope: :user end - describe 'GET #show' do - it 'returns http success' do - get :show - - expect(response).to have_http_status(200) - end - end - describe 'PUT #update' do it 'cannot create a setting value for a non-admin key' do expect(Setting.new_setting_key).to be_blank @@ -27,15 +19,6 @@ RSpec.describe Admin::Settings::BrandingController do expect(response).to redirect_to(admin_settings_branding_path) expect(Setting.new_setting_key).to be_nil end - - it 'creates a settings value that didnt exist before for eligible key' do - expect(Setting.site_short_description).to be_blank - - patch :update, params: { form_admin_settings: { site_short_description: 'New key value' } } - - expect(response).to redirect_to(admin_settings_branding_path) - expect(Setting.site_short_description).to eq 'New key value' - end 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 deleted file mode 100644 index fb6a3d2848..0000000000 --- a/spec/controllers/admin/settings/content_retention_controller_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Admin::Settings::ContentRetentionController do - render_views - - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - - before do - sign_in user, scope: :user - end - - describe 'GET #show' do - it 'returns http success' do - get :show - - 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 deleted file mode 100644 index 33109e3c01..0000000000 --- a/spec/controllers/admin/settings/discovery_controller_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Admin::Settings::DiscoveryController do - render_views - - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - - before do - sign_in user, scope: :user - end - - describe 'GET #show' do - it 'returns http success' do - get :show - - 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 deleted file mode 100644 index e076544603..0000000000 --- a/spec/controllers/admin/settings/registrations_controller_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Admin::Settings::RegistrationsController do - render_views - - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - - before do - sign_in user, scope: :user - end - - describe 'GET #show' do - it 'returns http success' do - get :show - - 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/rails_helper.rb b/spec/rails_helper.rb index 0c3d01c785..2a602d113a 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -112,6 +112,7 @@ RSpec.configure do |config| config.include ThreadingHelpers config.include SignedRequestHelpers, type: :request config.include CommandLineHelpers, type: :cli + config.include SystemHelpers, type: :system config.around(:each, use_transactional_tests: false) do |example| self.use_transactional_tests = false diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb new file mode 100644 index 0000000000..05c9d3b125 --- /dev/null +++ b/spec/support/system_helpers.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module SystemHelpers + def admin_user + Fabricate(:user, role: UserRole.find_by(name: 'Admin')) + end + + def submit_button + I18n.t('generic.save_changes') + end + + def success_message + I18n.t('generic.changes_saved_msg') + end + + def form_label(key) + I18n.t key, scope: 'simple_form.labels' + end +end diff --git a/spec/system/admin/settings/about_spec.rb b/spec/system/admin/settings/about_spec.rb new file mode 100644 index 0000000000..0f8ae5605c --- /dev/null +++ b/spec/system/admin/settings/about_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::About' do + it 'Saves changes to about settings' do + sign_in admin_user + visit admin_settings_about_path + + fill_in extended_description_field, + with: 'new site description' + + click_on submit_button + + expect(page) + .to have_content(success_message) + end + + def extended_description_field + form_label 'form_admin_settings.site_extended_description' + end +end diff --git a/spec/system/admin/settings/appearance_spec.rb b/spec/system/admin/settings/appearance_spec.rb new file mode 100644 index 0000000000..99e97ea4d1 --- /dev/null +++ b/spec/system/admin/settings/appearance_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::Appearance' do + it 'Saves changes to appearance settings' do + sign_in admin_user + visit admin_settings_appearance_path + + fill_in custom_css_field, + with: 'html { display: inline; }' + + click_on submit_button + + expect(page) + .to have_content(success_message) + end + + def custom_css_field + form_label 'form_admin_settings.custom_css' + end +end diff --git a/spec/system/admin/settings/branding_spec.rb b/spec/system/admin/settings/branding_spec.rb new file mode 100644 index 0000000000..ac47e04d53 --- /dev/null +++ b/spec/system/admin/settings/branding_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::Branding' do + it 'Saves changes to branding settings' do + sign_in admin_user + visit admin_settings_branding_path + + fill_in short_description_field, + with: 'new key value' + + fill_in site_contact_email_field, + with: User.last.email + + fill_in site_contact_username_field, + with: Account.last.username + + expect { click_on submit_button } + .to change(Setting, :site_short_description).to('new key value') + + expect(page) + .to have_content(success_message) + end + + def short_description_field + form_label 'form_admin_settings.site_short_description' + end + + def site_contact_email_field + form_label 'form_admin_settings.site_contact_email' + end + + def site_contact_username_field + form_label 'form_admin_settings.site_contact_username' + end +end diff --git a/spec/system/admin/settings/content_retention_spec.rb b/spec/system/admin/settings/content_retention_spec.rb new file mode 100644 index 0000000000..9867122675 --- /dev/null +++ b/spec/system/admin/settings/content_retention_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::ContentRetention' do + it 'Saves changes to content retention settings' do + sign_in admin_user + visit admin_settings_content_retention_path + + fill_in media_cache_retention_period_field, + with: '2' + + click_on submit_button + + expect(page) + .to have_content(success_message) + end + + def media_cache_retention_period_field + form_label 'form_admin_settings.media_cache_retention_period' + end +end diff --git a/spec/system/admin/settings/discovery_spec.rb b/spec/system/admin/settings/discovery_spec.rb new file mode 100644 index 0000000000..bdab91107d --- /dev/null +++ b/spec/system/admin/settings/discovery_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::Discovery' do + it 'Saves changes to discovery settings' do + sign_in admin_user + visit admin_settings_discovery_path + + check trends_box + + click_on submit_button + + expect(page) + .to have_content(success_message) + end + + def trends_box + form_label 'form_admin_settings.trends' + end +end diff --git a/spec/system/admin/settings/registrations_spec.rb b/spec/system/admin/settings/registrations_spec.rb new file mode 100644 index 0000000000..88c750e8ee --- /dev/null +++ b/spec/system/admin/settings/registrations_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Admin::Settings::Registrations' do + it 'Saves changes to registrations settings' do + sign_in admin_user + visit admin_settings_registrations_path + + select open_mode_option, + from: registrations_mode_field + + click_on submit_button + + expect(page) + .to have_content(success_message) + end + + def open_mode_option + I18n.t('admin.settings.registrations_mode.modes.open') + end + + def registrations_mode_field + form_label 'form_admin_settings.registrations_mode' + end +end From fcb83be8b23f64d336d7d3aca2829e9b92ff1df2 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:30:13 -0400 Subject: [PATCH 12/24] Improve coverage specificity for Webhook enable/disable/secret specs (#31194) --- spec/models/webhook_spec.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb index effaf92e9c..864baf2e1a 100644 --- a/spec/models/webhook_spec.rb +++ b/spec/models/webhook_spec.rb @@ -38,28 +38,28 @@ RSpec.describe Webhook do describe '#rotate_secret!' do it 'changes the secret' do - previous_value = webhook.secret - webhook.rotate_secret! - expect(webhook.secret).to_not be_blank - expect(webhook.secret).to_not eq previous_value + expect { webhook.rotate_secret! } + .to change(webhook, :secret) + expect(webhook.secret) + .to_not be_blank end end describe '#enable!' do - before do - webhook.disable! - end + let(:webhook) { Fabricate(:webhook, enabled: false) } it 'enables the webhook' do - webhook.enable! - expect(webhook.enabled?).to be true + expect { webhook.enable! } + .to change(webhook, :enabled?).to(true) end end describe '#disable!' do + let(:webhook) { Fabricate(:webhook, enabled: true) } + it 'disables the webhook' do - webhook.disable! - expect(webhook.enabled?).to be false + expect { webhook.disable! } + .to change(webhook, :enabled?).to(false) end end end From 67faaf555723e8b202620725b7144a183543809e Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:30:57 -0400 Subject: [PATCH 13/24] Simplify account model username presence validation spec (#31013) --- Gemfile | 2 ++ Gemfile.lock | 3 +++ spec/models/account_spec.rb | 6 +----- spec/support/shoulda_matchers.rb | 8 ++++++++ 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 spec/support/shoulda_matchers.rb diff --git a/Gemfile b/Gemfile index 6538067cff..4c808fa480 100644 --- a/Gemfile +++ b/Gemfile @@ -151,6 +151,8 @@ group :test do # Test harness fo rack components gem 'rack-test', '~> 2.1' + gem 'shoulda-matchers' + # Coverage formatter for RSpec test if DISABLE_SIMPLECOV is false gem 'simplecov', '~> 0.22', require: false gem 'simplecov-lcov', '~> 0.8', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 2e9037d8a6..461a2d43a2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -790,6 +790,8 @@ GEM rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) semantic_range (3.0.0) + shoulda-matchers (6.3.1) + activesupport (>= 5.2.0) sidekiq (6.5.12) connection_pool (>= 2.2.5, < 3) rack (~> 2.0) @@ -1035,6 +1037,7 @@ DEPENDENCIES sanitize (~> 6.0) scenic (~> 1.7) selenium-webdriver + shoulda-matchers sidekiq (~> 6.5) sidekiq-bulk (~> 0.2.0) sidekiq-scheduler (~> 5.0) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 8e5648a0b0..14528ed17b 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -722,11 +722,7 @@ RSpec.describe Account do end describe 'validations' do - it 'is invalid without a username' do - account = Fabricate.build(:account, username: nil) - account.valid? - expect(account).to model_have_error_on_field(:username) - end + it { is_expected.to validate_presence_of(:username) } it 'squishes the username before validation' do account = Fabricate(:account, domain: nil, username: " \u3000bob \t \u00a0 \n ") diff --git a/spec/support/shoulda_matchers.rb b/spec/support/shoulda_matchers.rb new file mode 100644 index 0000000000..edcf9dd859 --- /dev/null +++ b/spec/support/shoulda_matchers.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Shoulda::Matchers.configure do |config| + config.integrate do |with| + with.test_framework :rspec + with.library :rails + end +end From 8922786ef48951a3d4ce719a0e40ef96d2a3792c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:32:12 -0400 Subject: [PATCH 14/24] Fix `RSpec/LetSetup` cop in api/v1/timelines/tag spec (#30796) --- spec/requests/api/v1/timelines/tag_spec.rb | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/spec/requests/api/v1/timelines/tag_spec.rb b/spec/requests/api/v1/timelines/tag_spec.rb index 5e1415bb1a..cfbfa0291c 100644 --- a/spec/requests/api/v1/timelines/tag_spec.rb +++ b/spec/requests/api/v1/timelines/tag_spec.rb @@ -8,22 +8,25 @@ RSpec.describe 'Tag' do let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } - shared_examples 'a successful request to the tag timeline' do - it 'returns the expected statuses', :aggregate_failures do - subject - - expect(response).to have_http_status(200) - expect(body_as_json.pluck(:id)).to match_array(expected_statuses.map { |status| status.id.to_s }) - end - end - describe 'GET /api/v1/timelines/tag/:hashtag' do subject do get "/api/v1/timelines/tag/#{hashtag}", headers: headers, params: params end + shared_examples 'a successful request to the tag timeline' do + it 'returns the expected statuses', :aggregate_failures do + subject + + expect(response) + .to have_http_status(200) + expect(body_as_json.pluck(:id)) + .to match_array(expected_statuses.map { |status| status.id.to_s }) + .and not_include(private_status.id) + end + end + let(:account) { Fabricate(:account) } - let!(:private_status) { PostStatusService.new.call(account, visibility: :private, text: '#life could be a dream') } # rubocop:disable RSpec/LetSetup + let!(:private_status) { PostStatusService.new.call(account, visibility: :private, text: '#life could be a dream') } let!(:life_status) { PostStatusService.new.call(account, text: 'tell me what is my #life without your #love') } let!(:war_status) { PostStatusService.new.call(user.account, text: '#war, war never changes') } let!(:love_status) { PostStatusService.new.call(account, text: 'what is #love?') } From 46828044481f2a2698483d7bfa4ca36b04980d40 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:32:22 -0400 Subject: [PATCH 15/24] Fix `Rails/ReversibleMigration` cop for `remove_index` (#30832) --- db/migrate/20161122163057_remove_unneeded_indexes.rb | 6 +++--- db/migrate/20171129172043_add_index_on_stream_entries.rb | 2 +- .../20171212195226_remove_duplicate_indexes_in_lists.rb | 4 ++-- .../20171226094803_more_faster_index_on_notifications.rb | 2 +- ...x_on_statuses_for_api_v1_accounts_account_id_statuses.rb | 2 +- db/migrate/20180617162849_remove_unused_indexes.rb | 6 +++--- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/db/migrate/20161122163057_remove_unneeded_indexes.rb b/db/migrate/20161122163057_remove_unneeded_indexes.rb index 7ed92127d7..2496184703 100644 --- a/db/migrate/20161122163057_remove_unneeded_indexes.rb +++ b/db/migrate/20161122163057_remove_unneeded_indexes.rb @@ -2,8 +2,8 @@ class RemoveUnneededIndexes < ActiveRecord::Migration[5.0] def change - remove_index :notifications, name: 'index_notifications_on_account_id' - remove_index :settings, name: 'index_settings_on_target_type_and_target_id' - remove_index :statuses_tags, name: 'index_statuses_tags_on_tag_id' + remove_index :notifications, :account_id, name: 'index_notifications_on_account_id' + remove_index :settings, [:target_type, :target_id], name: 'index_settings_on_target_type_and_target_id' + remove_index :statuses_tags, :tag_id, name: 'index_statuses_tags_on_tag_id' end end diff --git a/db/migrate/20171129172043_add_index_on_stream_entries.rb b/db/migrate/20171129172043_add_index_on_stream_entries.rb index c959135c22..e861cdc2d1 100644 --- a/db/migrate/20171129172043_add_index_on_stream_entries.rb +++ b/db/migrate/20171129172043_add_index_on_stream_entries.rb @@ -5,6 +5,6 @@ class AddIndexOnStreamEntries < ActiveRecord::Migration[5.2] def change add_index :stream_entries, [:account_id, :activity_type, :id], algorithm: :concurrently - remove_index :stream_entries, name: :index_stream_entries_on_account_id + remove_index :stream_entries, :account_id, name: :index_stream_entries_on_account_id end end diff --git a/db/migrate/20171212195226_remove_duplicate_indexes_in_lists.rb b/db/migrate/20171212195226_remove_duplicate_indexes_in_lists.rb index 362b1367df..6f51f0a063 100644 --- a/db/migrate/20171212195226_remove_duplicate_indexes_in_lists.rb +++ b/db/migrate/20171212195226_remove_duplicate_indexes_in_lists.rb @@ -2,7 +2,7 @@ class RemoveDuplicateIndexesInLists < ActiveRecord::Migration[5.2] def change - remove_index :list_accounts, name: 'index_list_accounts_on_account_id' - remove_index :list_accounts, name: 'index_list_accounts_on_list_id' + remove_index :list_accounts, :account_id, name: 'index_list_accounts_on_account_id' + remove_index :list_accounts, :list_id, name: 'index_list_accounts_on_list_id' end end diff --git a/db/migrate/20171226094803_more_faster_index_on_notifications.rb b/db/migrate/20171226094803_more_faster_index_on_notifications.rb index 429eab96a1..b2fc040e2e 100644 --- a/db/migrate/20171226094803_more_faster_index_on_notifications.rb +++ b/db/migrate/20171226094803_more_faster_index_on_notifications.rb @@ -5,6 +5,6 @@ class MoreFasterIndexOnNotifications < ActiveRecord::Migration[5.2] def change add_index :notifications, [:account_id, :id], order: { id: :desc }, algorithm: :concurrently - remove_index :notifications, name: :index_notifications_on_id_and_account_id_and_activity_type + remove_index :notifications, [:id, :account_id, :activity_type], name: :index_notifications_on_id_and_account_id_and_activity_type end end diff --git a/db/migrate/20180106000232_add_index_on_statuses_for_api_v1_accounts_account_id_statuses.rb b/db/migrate/20180106000232_add_index_on_statuses_for_api_v1_accounts_account_id_statuses.rb index 1531c4dd29..4c3a25e838 100644 --- a/db/migrate/20180106000232_add_index_on_statuses_for_api_v1_accounts_account_id_statuses.rb +++ b/db/migrate/20180106000232_add_index_on_statuses_for_api_v1_accounts_account_id_statuses.rb @@ -7,6 +7,6 @@ class AddIndexOnStatusesForApiV1AccountsAccountIdStatuses < ActiveRecord::Migrat safety_assured do add_index :statuses, [:account_id, :id, :visibility, :updated_at], order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20180106 end - remove_index :statuses, name: :index_statuses_on_account_id_id + remove_index :statuses, [:account_id, :id], name: :index_statuses_on_account_id_id end end diff --git a/db/migrate/20180617162849_remove_unused_indexes.rb b/db/migrate/20180617162849_remove_unused_indexes.rb index 14766589fd..cf8faceb3a 100644 --- a/db/migrate/20180617162849_remove_unused_indexes.rb +++ b/db/migrate/20180617162849_remove_unused_indexes.rb @@ -2,8 +2,8 @@ class RemoveUnusedIndexes < ActiveRecord::Migration[5.2] def change - remove_index :statuses, name: 'index_statuses_on_conversation_id' - remove_index :users, name: 'index_users_on_filtered_languages' - remove_index :backups, name: 'index_backups_on_user_id' + remove_index :statuses, :conversation_id, name: 'index_statuses_on_conversation_id' + remove_index :users, :filtered_languages, name: 'index_users_on_filtered_languages' + remove_index :backups, :user_id, name: 'index_backups_on_user_id' end end From 490bdb7944fd2fe323494b3bbd60130673ccfa33 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:34:09 -0400 Subject: [PATCH 16/24] Add coverage for `StatusesHelper#media_summary` method (#31726) --- spec/helpers/statuses_helper_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/helpers/statuses_helper_spec.rb b/spec/helpers/statuses_helper_spec.rb index b7531ec0b7..66eb996f99 100644 --- a/spec/helpers/statuses_helper_spec.rb +++ b/spec/helpers/statuses_helper_spec.rb @@ -23,6 +23,19 @@ describe StatusesHelper do end end + describe '#media_summary' do + it 'describes the media on a status' do + status = Fabricate :status + Fabricate :media_attachment, status: status, type: :video + Fabricate :media_attachment, status: status, type: :audio + Fabricate :media_attachment, status: status, type: :image + + result = helper.media_summary(status) + + expect(result).to eq('Attached: 1 image · 1 video · 1 audio') + end + end + describe 'fa_visibility_icon' do context 'with a status that is public' do let(:status) { Status.new(visibility: 'public') } From e1fa456c7cfa5dc41cec21a94573bc6cb1ec60cc Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:35:19 -0400 Subject: [PATCH 17/24] Add `have_cacheable_headers` matcher for responses (#31727) --- .../collections_controller_spec.rb | 13 ++--- .../activitypub/outboxes_controller_spec.rb | 14 +++--- .../activitypub/replies_controller_spec.rb | 7 +-- spec/controllers/statuses_controller_spec.rb | 9 ++-- spec/requests/accounts_spec.rb | 32 ++++++------ spec/requests/custom_stylesheets_spec.rb | 3 +- spec/requests/instance_actor_spec.rb | 3 +- spec/requests/manifest_spec.rb | 3 +- spec/requests/tags_spec.rb | 13 ++--- spec/support/examples/cache.rb | 14 ------ spec/support/matchers/cacheable_response.rb | 50 +++++++++++++++++++ 11 files changed, 96 insertions(+), 65 deletions(-) delete mode 100644 spec/support/examples/cache.rb create mode 100644 spec/support/matchers/cacheable_response.rb diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb index a5718fbd7d..0880273853 100644 --- a/spec/controllers/activitypub/collections_controller_spec.rb +++ b/spec/controllers/activitypub/collections_controller_spec.rb @@ -25,10 +25,10 @@ RSpec.describe ActivityPub::CollectionsController do context 'without signature' do let(:remote_account) { nil } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and correct items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers expect(response.media_type).to eq 'application/activity+json' expect(body_as_json[:orderedItems]) @@ -64,10 +64,11 @@ RSpec.describe ActivityPub::CollectionsController do let(:remote_account) { Fabricate(:account, domain: 'example.com') } context 'when getting a featured resource' do - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and expected items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(body_as_json[:orderedItems]) diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index 3c8e8e399f..26a52bad93 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -25,10 +25,11 @@ RSpec.describe ActivityPub::OutboxesController do context 'with page not requested' do let(:page) { nil } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and headers and items count' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(response.headers['Vary']).to be_nil expect(body[:totalItems]).to eq 4 @@ -59,10 +60,11 @@ RSpec.describe ActivityPub::OutboxesController do context 'with page requested' do let(:page) { 'true' } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and vary header and items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(response.headers['Vary']).to include 'Signature' diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb index c556e07270..c10c782c9a 100644 --- a/spec/controllers/activitypub/replies_controller_spec.rb +++ b/spec/controllers/activitypub/replies_controller_spec.rb @@ -68,10 +68,11 @@ RSpec.describe ActivityPub::RepliesController do let(:parent_visibility) { :public } let(:page_json) { body_as_json[:first] } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' end diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index fe40ee6de1..084dcfaa75 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -72,13 +72,12 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - 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) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + expect(response.headers).to include( - 'Vary' => 'Accept, Accept-Language, Cookie', 'Content-Type' => include('application/activity+json'), 'Link' => satisfy { |header| header.to_s.include?('activity+json') } ) @@ -380,13 +379,11 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - 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) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.headers).to include( - 'Vary' => 'Accept, Accept-Language, Cookie', 'Content-Type' => include('application/activity+json'), 'Link' => satisfy { |header| header.to_s.include?('activity+json') } ) diff --git a/spec/requests/accounts_spec.rb b/spec/requests/accounts_spec.rb index bf067cdc38..238524c75c 100644 --- a/spec/requests/accounts_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -130,6 +130,7 @@ describe 'Accounts show response' do it 'returns a JSON version of the account', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') .and have_attributes( media_type: eq('application/activity+json') ) @@ -137,8 +138,6 @@ describe 'Accounts show response' do 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 } @@ -179,6 +178,7 @@ describe 'Accounts show response' do it 'returns a JSON version of the account', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') .and have_attributes( media_type: eq('application/activity+json') ) @@ -186,8 +186,6 @@ describe 'Accounts show response' do 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 } @@ -215,10 +213,10 @@ describe 'Accounts show response' do 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) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') 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)) @@ -234,10 +232,11 @@ describe 'Accounts show response' do 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) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + 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)) @@ -253,10 +252,10 @@ describe 'Accounts show response' do 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) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') 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)) @@ -277,10 +276,11 @@ describe 'Accounts show response' do 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) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + 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)) diff --git a/spec/requests/custom_stylesheets_spec.rb b/spec/requests/custom_stylesheets_spec.rb index 982511297b..128d173f3a 100644 --- a/spec/requests/custom_stylesheets_spec.rb +++ b/spec/requests/custom_stylesheets_spec.rb @@ -9,11 +9,10 @@ describe 'Custom stylesheets' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers .and have_attributes( content_type: match('text/css') ) end - - it_behaves_like 'cacheable response' end end diff --git a/spec/requests/instance_actor_spec.rb b/spec/requests/instance_actor_spec.rb index 9c7ee9ff90..7e4784203f 100644 --- a/spec/requests/instance_actor_spec.rb +++ b/spec/requests/instance_actor_spec.rb @@ -17,6 +17,7 @@ RSpec.describe 'Instance actor endpoint' do it 'returns http success with correct media type and body' do expect(response) .to have_http_status(200) + .and have_cacheable_headers expect(response.content_type) .to start_with('application/activity+json') expect(body_as_json) @@ -32,8 +33,6 @@ RSpec.describe 'Instance actor endpoint' do url: about_more_url(instance_actor: true) ) end - - it_behaves_like 'cacheable response' end context 'with limited federation mode disabled' do diff --git a/spec/requests/manifest_spec.rb b/spec/requests/manifest_spec.rb index 749c4038df..55b8147d7e 100644 --- a/spec/requests/manifest_spec.rb +++ b/spec/requests/manifest_spec.rb @@ -9,6 +9,7 @@ describe 'Manifest' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers .and have_attributes( content_type: match('application/json') ) @@ -18,7 +19,5 @@ describe 'Manifest' do name: 'Mastodon' ) end - - it_behaves_like 'cacheable response' end end diff --git a/spec/requests/tags_spec.rb b/spec/requests/tags_spec.rb index c037bbef5a..7974a6b152 100644 --- a/spec/requests/tags_spec.rb +++ b/spec/requests/tags_spec.rb @@ -8,16 +8,15 @@ RSpec.describe 'Tags' do let(:tag) { Fabricate :tag } context 'with HTML format' do - # TODO: Convert the cacheable response shared example into a matcher, - # remove this example, rely on system spec (which should use matcher) + # TODO: Update the have_cacheable_headers matcher to operate on capybara sessions + # Remove this example, rely on system spec (which should use matcher) before { get tag_path(tag) } it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end context 'with JSON format' do @@ -26,11 +25,10 @@ RSpec.describe 'Tags' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.content_type) .to start_with('application/activity+json') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end context 'with RSS format' do @@ -39,11 +37,10 @@ RSpec.describe 'Tags' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.content_type) .to start_with('application/rss+xml') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end end diff --git a/spec/support/examples/cache.rb b/spec/support/examples/cache.rb deleted file mode 100644 index 60e522f426..0000000000 --- a/spec/support/examples/cache.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -shared_examples 'cacheable response' do |expects_vary: false| - it 'sets correct cache and vary headers and does not set cookies or session', :aggregate_failures do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - - expect(session).to be_empty - - expect(response.headers['Vary']).to include(expects_vary) if expects_vary - - expect(response.headers['Cache-Control']).to include('public') - end -end diff --git a/spec/support/matchers/cacheable_response.rb b/spec/support/matchers/cacheable_response.rb new file mode 100644 index 0000000000..da8570c8c5 --- /dev/null +++ b/spec/support/matchers/cacheable_response.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :have_cacheable_headers do + match do |response| + @response = response + + @errors = [].tap do |errors| + errors << check_cookies + errors << check_cookie_headers + errors << check_session + errors << check_cache_control + errors << check_vary if @expected_vary.present? + end + + @errors.compact.empty? + end + + chain :with_vary do |string| + @expected_vary = string + end + + failure_message do + <<~ERROR + Expected that the response would be cacheable but it was not: + - #{@errors.compact.join("\n - ")} + ERROR + end + + def check_vary + puts @expected_vary + pp @response.headers + "Response `Vary` header does not contain `#{@expected_vary}`" unless @response.headers['Vary'].include?(@expected_vary) + end + + def check_cookies + 'Reponse cookies are present' unless @response.cookies.empty? + end + + def check_cookie_headers + 'Response `Set-Cookies` headers are present' if @response.headers['Set-Cookies'].present? + end + + def check_session + 'The session is not empty' unless session.empty? + end + + def check_cache_control + 'The `Cache-Control` header does not contain `public`' unless @response.headers['Cache-Control'].include?('public') + end +end From 2f0d0fc127c73d74c7105441bf6dd8163b14450c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:36:59 -0400 Subject: [PATCH 18/24] Add coverage for `CLI::Accounts#fix_duplications` task (#30639) --- app/models/account.rb | 1 + lib/mastodon/cli/accounts.rb | 2 +- spec/lib/mastodon/cli/accounts_spec.rb | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/account.rb b/app/models/account.rb index 482eaa4aba..04095890ef 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -145,6 +145,7 @@ class Account < ApplicationRecord scope :with_username, ->(value) { where arel_table[:username].lower.eq(value.to_s.downcase) } scope :with_domain, ->(value) { where arel_table[:domain].lower.eq(value&.to_s&.downcase) } scope :without_memorial, -> { where(memorial: false) } + scope :duplicate_uris, -> { select(:uri, Arel.star.count).group(:uri).having(Arel.star.count.gt(1)) } after_update_commit :trigger_update_webhooks diff --git a/lib/mastodon/cli/accounts.rb b/lib/mastodon/cli/accounts.rb index d3b7ebe580..0cdf68158f 100644 --- a/lib/mastodon/cli/accounts.rb +++ b/lib/mastodon/cli/accounts.rb @@ -252,7 +252,7 @@ module Mastodon::CLI domain configuration. LONG_DESC def fix_duplicates - Account.remote.select(:uri, 'count(*)').group(:uri).having('count(*) > 1').pluck(:uri).each do |uri| + Account.remote.duplicate_uris.pluck(:uri).each do |uri| say("Duplicates found for #{uri}") begin ActivityPub::FetchRemoteAccountService.new.call(uri) unless dry_run? diff --git a/spec/lib/mastodon/cli/accounts_spec.rb b/spec/lib/mastodon/cli/accounts_spec.rb index 137f85c6ca..3988e0b027 100644 --- a/spec/lib/mastodon/cli/accounts_spec.rb +++ b/spec/lib/mastodon/cli/accounts_spec.rb @@ -613,6 +613,25 @@ describe Mastodon::CLI::Accounts do end end + describe '#fix_duplicates' do + let(:action) { :fix_duplicates } + let(:service_double) { instance_double(ActivityPub::FetchRemoteAccountService, call: nil) } + let(:uri) { 'https://host.example/same/value' } + + context 'when there are duplicate URI accounts' do + before do + Fabricate.times(2, :account, domain: 'host.example', uri: uri) + allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_double) + end + + it 'finds the duplicates and calls fetch remote account service' do + expect { subject } + .to output_results('Duplicates found') + expect(service_double).to have_received(:call).with(uri) + end + end + end + describe '#backup' do let(:action) { :backup } From 7c26e5e4a101e2784d8c627055b4caad5812015f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:37:45 -0400 Subject: [PATCH 19/24] Add `Reviewable` model concern (#31152) --- app/models/account.rb | 17 +----- app/models/concerns/reviewable.rb | 21 ++++++++ app/models/preview_card_provider.rb | 17 +----- app/models/tag.rb | 18 +------ spec/models/account_spec.rb | 2 + spec/models/preview_card_provider_spec.rb | 2 + spec/models/tag_spec.rb | 2 + .../examples/models/concerns/reviewable.rb | 54 +++++++++++++++++++ 8 files changed, 85 insertions(+), 48 deletions(-) create mode 100644 app/models/concerns/reviewable.rb create mode 100644 spec/support/examples/models/concerns/reviewable.rb diff --git a/app/models/account.rb b/app/models/account.rb index 04095890ef..c20b72658b 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -89,6 +89,7 @@ class Account < ApplicationRecord include DomainMaterializable include DomainNormalizable include Paginable + include Reviewable enum :protocol, { ostatus: 0, activitypub: 1 } enum :suspension_origin, { local: 0, remote: 1 }, prefix: true @@ -426,22 +427,6 @@ class Account < ApplicationRecord @synchronization_uri_prefix ||= "#{uri[URL_PREFIX_RE]}/" end - def requires_review? - reviewed_at.nil? - end - - def reviewed? - reviewed_at.present? - end - - def requested_review? - requested_review_at.present? - end - - def requires_review_notification? - requires_review? && !requested_review? - end - class << self def readonly_attributes super - %w(statuses_count following_count followers_count) diff --git a/app/models/concerns/reviewable.rb b/app/models/concerns/reviewable.rb new file mode 100644 index 0000000000..1f70474b35 --- /dev/null +++ b/app/models/concerns/reviewable.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Reviewable + extend ActiveSupport::Concern + + def requires_review? + reviewed_at.nil? + end + + def reviewed? + reviewed_at.present? + end + + def requested_review? + requested_review_at.present? + end + + def requires_review_notification? + requires_review? && !requested_review? + end +end diff --git a/app/models/preview_card_provider.rb b/app/models/preview_card_provider.rb index 756707e3f1..48944fe638 100644 --- a/app/models/preview_card_provider.rb +++ b/app/models/preview_card_provider.rb @@ -21,6 +21,7 @@ class PreviewCardProvider < ApplicationRecord include Paginable include DomainNormalizable include Attachmentable + include Reviewable ICON_MIME_TYPES = %w(image/x-icon image/vnd.microsoft.icon image/png).freeze LIMIT = 1.megabyte @@ -36,22 +37,6 @@ class PreviewCardProvider < ApplicationRecord scope :reviewed, -> { where.not(reviewed_at: nil) } scope :pending_review, -> { where(reviewed_at: nil) } - def requires_review? - reviewed_at.nil? - end - - def reviewed? - reviewed_at.present? - end - - def requested_review? - requested_review_at.present? - end - - def requires_review_notification? - requires_review? && !requested_review? - end - def self.matching_domain(domain) segments = domain.split('.') where(domain: segments.map.with_index { |_, i| segments[i..].join('.') }).by_domain_length.first diff --git a/app/models/tag.rb b/app/models/tag.rb index 9006e1f25d..acf514919b 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -21,6 +21,8 @@ class Tag < ApplicationRecord include Paginable + include Reviewable + # rubocop:disable Rails/HasAndBelongsToMany has_and_belongs_to_many :statuses has_and_belongs_to_many :accounts @@ -97,22 +99,6 @@ class Tag < ApplicationRecord alias trendable? trendable - def requires_review? - reviewed_at.nil? - end - - def reviewed? - reviewed_at.present? - end - - def requested_review? - requested_review_at.present? - end - - def requires_review_notification? - requires_review? && !requested_review? - end - def decaying? max_score_at && max_score_at >= Trends.tags.options[:max_score_cooldown].ago && max_score_at < 1.day.ago end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 14528ed17b..83f1585b61 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' RSpec.describe Account do + include_examples 'Reviewable' + context 'with an account record' do subject { Fabricate(:account) } diff --git a/spec/models/preview_card_provider_spec.rb b/spec/models/preview_card_provider_spec.rb index 7425b93946..8b18b3d2b7 100644 --- a/spec/models/preview_card_provider_spec.rb +++ b/spec/models/preview_card_provider_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe PreviewCardProvider do + include_examples 'Reviewable' + describe 'scopes' do let(:trendable_and_reviewed) { Fabricate(:preview_card_provider, trendable: true, reviewed_at: 5.days.ago) } let(:not_trendable_and_not_reviewed) { Fabricate(:preview_card_provider, trendable: false, reviewed_at: nil) } diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index ff0a055113..18dd26be94 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' RSpec.describe Tag do + include_examples 'Reviewable' + describe 'validations' do it 'invalid with #' do expect(described_class.new(name: '#hello_world')).to_not be_valid diff --git a/spec/support/examples/models/concerns/reviewable.rb b/spec/support/examples/models/concerns/reviewable.rb new file mode 100644 index 0000000000..562183d1cc --- /dev/null +++ b/spec/support/examples/models/concerns/reviewable.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +shared_examples 'Reviewable' do + subject { described_class.new(reviewed_at: reviewed_at, requested_review_at: requested_review_at) } + + let(:reviewed_at) { nil } + let(:requested_review_at) { nil } + + describe '#requires_review?' do + it { is_expected.to be_requires_review } + + context 'when reviewed_at is not null' do + let(:reviewed_at) { 5.days.ago } + + it { is_expected.to_not be_requires_review } + end + end + + describe '#reviewed?' do + it { is_expected.to_not be_reviewed } + + context 'when reviewed_at is not null' do + let(:reviewed_at) { 5.days.ago } + + it { is_expected.to be_reviewed } + end + end + + describe '#requested_review?' do + it { is_expected.to_not be_requested_review } + + context 'when requested_reviewed_at is not null' do + let(:requested_review_at) { 5.days.ago } + + it { is_expected.to be_requested_review } + end + end + + describe '#requires_review_notification?' do + it { is_expected.to be_requires_review_notification } + + context 'when reviewed_at is not null' do + let(:reviewed_at) { 5.days.ago } + + it { is_expected.to_not be_requires_review_notification } + end + + context 'when requested_reviewed_at is not null' do + let(:requested_review_at) { 5.days.ago } + + it { is_expected.to_not be_requires_review_notification } + end + end +end From 69dbc2303826d4a90322892f1fdf96c5ec22a948 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:38:38 -0400 Subject: [PATCH 20/24] Only enable chewy in search-tagged specs (#30583) --- spec/rails_helper.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 2a602d113a..883eaec84c 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -55,6 +55,8 @@ Sidekiq.logger = nil DatabaseCleaner.strategy = [:deletion] +Chewy.settings[:enabled] = false + Devise::Test::ControllerHelpers.module_eval do alias_method :original_sign_in, :sign_in @@ -129,6 +131,12 @@ RSpec.configure do |config| example.run end + config.around(:each, type: :search) do |example| + Chewy.settings[:enabled] = true + example.run + Chewy.settings[:enabled] = false + end + config.before :each, type: :cli do stub_reset_connection_pools end From 19849eb91d1a7f8b5700abc50cbd84181666fa2c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:39:19 -0400 Subject: [PATCH 21/24] Skip paperclip spoof detector unless opted into attachment processing specs (#31454) --- spec/rails_helper.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 883eaec84c..0b24f68f73 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -147,7 +147,10 @@ RSpec.configure do |config| config.before do |example| unless example.metadata[:attachment_processing] - allow_any_instance_of(Paperclip::Attachment).to receive(:post_process).and_return(true) # rubocop:disable RSpec/AnyInstance + # rubocop:disable RSpec/AnyInstance + allow_any_instance_of(Paperclip::Attachment).to receive(:post_process).and_return(true) + allow_any_instance_of(Paperclip::MediaTypeSpoofDetector).to receive(:spoofed?).and_return(false) + # rubocop:enable RSpec/AnyInstance end end From 8adf67f2dbde0ae249ccadd8cbd2156e14932f52 Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Tue, 3 Sep 2024 17:55:13 +0200 Subject: [PATCH 22/24] `frequentlyUsedLanguages` not updated correctly (#31386) --- app/javascript/mastodon/actions/languages.js | 12 ------------ .../compose/components/language_dropdown.jsx | 4 ---- .../containers/language_dropdown_container.js | 6 ------ app/javascript/mastodon/reducers/settings.js | 4 ++-- 4 files changed, 2 insertions(+), 24 deletions(-) delete mode 100644 app/javascript/mastodon/actions/languages.js diff --git a/app/javascript/mastodon/actions/languages.js b/app/javascript/mastodon/actions/languages.js deleted file mode 100644 index ad186ba0cc..0000000000 --- a/app/javascript/mastodon/actions/languages.js +++ /dev/null @@ -1,12 +0,0 @@ -import { saveSettings } from './settings'; - -export const LANGUAGE_USE = 'LANGUAGE_USE'; - -export const useLanguage = language => dispatch => { - dispatch({ - type: LANGUAGE_USE, - language, - }); - - dispatch(saveSettings()); -}; diff --git a/app/javascript/mastodon/features/compose/components/language_dropdown.jsx b/app/javascript/mastodon/features/compose/components/language_dropdown.jsx index 47e81cf134..b164a07cbd 100644 --- a/app/javascript/mastodon/features/compose/components/language_dropdown.jsx +++ b/app/javascript/mastodon/features/compose/components/language_dropdown.jsx @@ -240,7 +240,6 @@ class LanguageDropdown extends PureComponent { frequentlyUsedLanguages: PropTypes.arrayOf(PropTypes.string), intl: PropTypes.object.isRequired, onChange: PropTypes.func, - onClose: PropTypes.func, }; state = { @@ -257,14 +256,11 @@ class LanguageDropdown extends PureComponent { }; handleClose = () => { - const { value, onClose } = this.props; - if (this.state.open && this.activeElement) { this.activeElement.focus({ preventScroll: true }); } this.setState({ open: false }); - onClose(value); }; handleChange = value => { diff --git a/app/javascript/mastodon/features/compose/containers/language_dropdown_container.js b/app/javascript/mastodon/features/compose/containers/language_dropdown_container.js index ba4b5f05a5..64c90afa92 100644 --- a/app/javascript/mastodon/features/compose/containers/language_dropdown_container.js +++ b/app/javascript/mastodon/features/compose/containers/language_dropdown_container.js @@ -4,7 +4,6 @@ import { connect } from 'react-redux'; import { changeComposeLanguage } from 'mastodon/actions/compose'; -import { useLanguage } from 'mastodon/actions/languages'; import LanguageDropdown from '../components/language_dropdown'; @@ -28,11 +27,6 @@ const mapDispatchToProps = dispatch => ({ dispatch(changeComposeLanguage(value)); }, - onClose (value) { - // eslint-disable-next-line react-hooks/rules-of-hooks -- this is not a react hook - dispatch(useLanguage(value)); - }, - }); export default connect(mapStateToProps, mapDispatchToProps)(LanguageDropdown); diff --git a/app/javascript/mastodon/reducers/settings.js b/app/javascript/mastodon/reducers/settings.js index 6f55241dc1..e5ff2ff910 100644 --- a/app/javascript/mastodon/reducers/settings.js +++ b/app/javascript/mastodon/reducers/settings.js @@ -1,8 +1,8 @@ import { Map as ImmutableMap, fromJS } from 'immutable'; import { COLUMN_ADD, COLUMN_REMOVE, COLUMN_MOVE, COLUMN_PARAMS_CHANGE } from '../actions/columns'; +import { COMPOSE_LANGUAGE_CHANGE } from '../actions/compose'; import { EMOJI_USE } from '../actions/emojis'; -import { LANGUAGE_USE } from '../actions/languages'; import { LIST_DELETE_SUCCESS, LIST_FETCH_FAIL } from '../actions/lists'; import { NOTIFICATIONS_FILTER_SET } from '../actions/notifications'; import { SETTING_CHANGE, SETTING_SAVE } from '../actions/settings'; @@ -175,7 +175,7 @@ export default function settings(state = initialState, action) { return changeColumnParams(state, action.uuid, action.path, action.value); case EMOJI_USE: return updateFrequentEmojis(state, action.emoji); - case LANGUAGE_USE: + case COMPOSE_LANGUAGE_CHANGE: return updateFrequentLanguages(state, action.language); case SETTING_SAVE: return state.set('saved', true); From 8c928faff3b9de1087b5cf983fd0e575bd2181dc Mon Sep 17 00:00:00 2001 From: zunda Date: Tue, 3 Sep 2024 16:03:00 +0000 Subject: [PATCH 23/24] Refresh Heroku related thingy for heroku-24 stack (#31135) --- .profile | 1 - Aptfile | 10 +++++----- Procfile | 2 +- app.json | 9 ++++++++- 4 files changed, 14 insertions(+), 8 deletions(-) delete mode 100644 .profile diff --git a/.profile b/.profile deleted file mode 100644 index f4826ea303..0000000000 --- a/.profile +++ /dev/null @@ -1 +0,0 @@ -LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/app/.apt/lib/x86_64-linux-gnu:/app/.apt/usr/lib/x86_64-linux-gnu/mesa:/app/.apt/usr/lib/x86_64-linux-gnu/pulseaudio:/app/.apt/usr/lib/x86_64-linux-gnu/openblas-pthread diff --git a/Aptfile b/Aptfile index 5e033f1365..06c91d4c7b 100644 --- a/Aptfile +++ b/Aptfile @@ -1,5 +1,5 @@ -ffmpeg -libopenblas0-pthread -libpq-dev -libxdamage1 -libxfixes3 +libidn12 +# for idn-ruby on heroku-24 stack + +# use https://github.com/heroku/heroku-buildpack-activestorage-preview +# in place for ffmpeg and its dependent packages to reduce slag size diff --git a/Procfile b/Procfile index d15c835b86..f033fd36c6 100644 --- a/Procfile +++ b/Procfile @@ -11,4 +11,4 @@ worker: bundle exec sidekiq # # and let the main app use the separate app: # -# heroku config:set STREAMING_API_BASE_URL=wss://.herokuapp.com -a +# heroku config:set STREAMING_API_BASE_URL=wss://.herokuapp.com -a diff --git a/app.json b/app.json index 4f05a64f51..5e5a3dc1e7 100644 --- a/app.json +++ b/app.json @@ -90,9 +90,15 @@ } }, "buildpacks": [ + { + "url": "https://github.com/heroku/heroku-buildpack-activestorage-preview" + }, { "url": "https://github.com/heroku/heroku-buildpack-apt" }, + { + "url": "heroku/nodejs" + }, { "url": "heroku/ruby" } @@ -100,5 +106,6 @@ "scripts": { "postdeploy": "bundle exec rails db:migrate && bundle exec rails db:seed" }, - "addons": ["heroku-postgresql", "heroku-redis"] + "addons": ["heroku-postgresql", "heroku-redis"], + "stack": "heroku-24" } From c9ea91f8683cd5c0cfac14071a17e3956ac6d3b0 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 12:10:48 -0400 Subject: [PATCH 24/24] Add coverage for `api/v1/annual_reports` area (#31730) --- .../generated_annual_report_fabricator.rb | 8 +++ spec/requests/api/v1/annual_reports_spec.rb | 57 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 spec/fabricators/generated_annual_report_fabricator.rb create mode 100644 spec/requests/api/v1/annual_reports_spec.rb diff --git a/spec/fabricators/generated_annual_report_fabricator.rb b/spec/fabricators/generated_annual_report_fabricator.rb new file mode 100644 index 0000000000..462d0cf4bc --- /dev/null +++ b/spec/fabricators/generated_annual_report_fabricator.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Fabricator(:generated_annual_report) do + account { Fabricate.build(:account) } + data { { test: :data } } + schema_version { AnnualReport::SCHEMA } + year { sequence(:year) { |i| 2000 + i } } +end diff --git a/spec/requests/api/v1/annual_reports_spec.rb b/spec/requests/api/v1/annual_reports_spec.rb new file mode 100644 index 0000000000..60cd8ed526 --- /dev/null +++ b/spec/requests/api/v1/annual_reports_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'API V1 Annual Reports' do + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + describe 'GET /api/v1/annual_reports' do + context 'when not authorized' do + it 'returns http unauthorized' do + get api_v1_annual_reports_path + + expect(response) + .to have_http_status(401) + end + end + + context 'with wrong scope' do + before do + get api_v1_annual_reports_path, headers: headers + end + + it_behaves_like 'forbidden for wrong scope', 'write write:accounts' + end + + context 'with correct scope' do + let(:scopes) { 'read:accounts' } + + it 'returns http success' do + get api_v1_annual_reports_path, headers: headers + + expect(response) + .to have_http_status(200) + + expect(body_as_json) + .to be_present + end + end + end + + describe 'POST /api/v1/annual_reports/:id/read' do + context 'with correct scope' do + let(:scopes) { 'write:accounts' } + + it 'returns success and marks the report as read' do + annual_report = Fabricate :generated_annual_report, account: user.account + + expect { post read_api_v1_annual_report_path(id: annual_report.year), headers: headers } + .to change { annual_report.reload.viewed? }.to(true) + expect(response) + .to have_http_status(200) + end + end + end +end