From a2a66300d99ed53d63a7f065ffbed53d2ce074e3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 11 Apr 2023 11:25:29 +0200 Subject: Clean up the post deployment migration generator (#24233) --- .../post_deployment_migration_generator_spec.rb | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 spec/generators/post_deployment_migration_generator_spec.rb (limited to 'spec') diff --git a/spec/generators/post_deployment_migration_generator_spec.rb b/spec/generators/post_deployment_migration_generator_spec.rb new file mode 100644 index 000000000..d552880e3 --- /dev/null +++ b/spec/generators/post_deployment_migration_generator_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'rails/generators/testing/behaviour' +require 'rails/generators/testing/assertions' + +require 'generators/post_deployment_migration/post_deployment_migration_generator' + +describe PostDeploymentMigrationGenerator, type: :generator do + include Rails::Generators::Testing::Behaviour + include Rails::Generators::Testing::Assertions + include FileUtils + + tests described_class + destination File.expand_path('../../tmp', __dir__) + before { prepare_destination } + after { rm_rf(destination_root) } + + describe 'the migration' do + it 'generates expected file' do + run_generator %w(Changes) + + assert_migration('db/post_migrate/changes.rb', /disable_ddl/) + assert_migration('db/post_migrate/changes.rb', /change/) + end + end +end -- cgit From 36eeb70d5315be045a638d77f8ff0a71dce61076 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 11 Apr 2023 11:35:39 +0200 Subject: Spec coverage on Settings/ controllers specs (#24221) --- .../settings/aliases_controller_spec.rb | 46 ++++++++++++++++++++ .../settings/featured_tags_controller_spec.rb | 37 ++++++++++++---- .../migration/redirects_controller_spec.rb | 45 +++++++++++++++++++- .../settings/pictures_controller_spec.rb | 30 +++++++++++++ .../preferences/appearance_controller_spec.rb | 9 ++++ ...actor_authentication_methods_controller_spec.rb | 49 ++++++++++++++++++---- spec/fabricators/featured_tag_fabricator.rb | 7 ++++ 7 files changed, 204 insertions(+), 19 deletions(-) create mode 100644 spec/fabricators/featured_tag_fabricator.rb (limited to 'spec') diff --git a/spec/controllers/settings/aliases_controller_spec.rb b/spec/controllers/settings/aliases_controller_spec.rb index 805f65988..ef8724faf 100644 --- a/spec/controllers/settings/aliases_controller_spec.rb +++ b/spec/controllers/settings/aliases_controller_spec.rb @@ -18,4 +18,50 @@ describe Settings::AliasesController do expect(response).to have_http_status(200) end end + + describe 'POST #create' do + context 'with valid alias' do + before { stub_resolver } + + it 'creates an alias for the user' do + expect do + post :create, params: { account_alias: { acct: 'new@example.com' } } + end.to change(AccountAlias, :count).by(1) + + expect(response).to redirect_to(settings_aliases_path) + end + end + + context 'with invalid alias' do + it 'does not create an alias for the user' do + expect do + post :create, params: { account_alias: { acct: 'format-wrong' } } + end.to_not change(AccountAlias, :count) + + expect(response).to have_http_status(200) + end + end + end + + describe 'DELETE #destroy' do + let(:account_alias) do + AccountAlias.new(account: user.account, acct: 'new@example.com').tap do |account_alias| + account_alias.save(validate: false) + end + end + + it 'removes an alias' do + delete :destroy, params: { id: account_alias.id } + + expect(response).to redirect_to(settings_aliases_path) + expect { account_alias.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + private + + def stub_resolver + resolver = instance_double(ResolveAccountService, call: Fabricate(:account)) + allow(ResolveAccountService).to receive(:new).and_return(resolver) + end end diff --git a/spec/controllers/settings/featured_tags_controller_spec.rb b/spec/controllers/settings/featured_tags_controller_spec.rb index 5c61351af..fc25e7aa8 100644 --- a/spec/controllers/settings/featured_tags_controller_spec.rb +++ b/spec/controllers/settings/featured_tags_controller_spec.rb @@ -11,19 +11,19 @@ describe Settings::FeaturedTagsController do end end - describe 'POST #create' do - context 'when user is not sign in' do - subject { post :create } + context 'when user is not signed in' do + subject { post :create } - it_behaves_like 'authenticate user' - end + it_behaves_like 'authenticate user' + end - context 'when user is sign in' do - subject { post :create, params: { featured_tag: params } } + context 'when user is signed in' do + let(:user) { Fabricate(:user, password: '12345678') } - let(:user) { Fabricate(:user, password: '12345678') } + before { sign_in user, scope: :user } - before { sign_in user, scope: :user } + describe 'POST #create' do + subject { post :create, params: { featured_tag: params } } context 'when parameter is valid' do let(:params) { { name: 'test' } } @@ -41,5 +41,24 @@ describe Settings::FeaturedTagsController do end end end + + describe 'GET to #index' do + it 'responds with success' do + get :index + + expect(response).to have_http_status(200) + end + end + + describe 'DELETE to #destroy' do + let(:featured_tag) { Fabricate(:featured_tag, account: user.account) } + + it 'removes the featured tag' do + delete :destroy, params: { id: featured_tag.id } + + expect(response).to redirect_to(settings_featured_tags_path) + expect { featured_tag.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end end end diff --git a/spec/controllers/settings/migration/redirects_controller_spec.rb b/spec/controllers/settings/migration/redirects_controller_spec.rb index 50d9e1927..54897bb7f 100644 --- a/spec/controllers/settings/migration/redirects_controller_spec.rb +++ b/spec/controllers/settings/migration/redirects_controller_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe Settings::Migration::RedirectsController do render_views - let!(:user) { Fabricate(:user) } + let!(:user) { Fabricate(:user, password: 'testtest') } before do sign_in user, scope: :user @@ -17,4 +17,47 @@ describe Settings::Migration::RedirectsController do expect(response).to have_http_status(200) end end + + describe 'POST #create' do + context 'with valid params' do + before { stub_resolver } + + it 'redirects to the settings migration path' do + post :create, params: { form_redirect: { acct: 'new@host.com', current_password: 'testtest' } } + + expect(response).to redirect_to(settings_migration_path) + end + end + + context 'with non valid params' do + it 'returns success and renders the new page' do + post :create, params: { form_redirect: { acct: '' } } + + expect(response).to have_http_status(200) + expect(response).to render_template(:new) + end + end + end + + describe 'DELETE #destroy' do + let(:account) { Fabricate(:account) } + + before do + user.account.update(moved_to_account_id: account.id) + end + + it 'resets the account and sends an update' do + delete :destroy + + expect(response).to redirect_to(settings_migration_path) + expect(user.account.reload.moved_to_account).to be_nil + end + end + + private + + def stub_resolver + resolver = instance_double(ResolveAccountService, call: Fabricate(:account)) + allow(ResolveAccountService).to receive(:new).and_return(resolver) + end end diff --git a/spec/controllers/settings/pictures_controller_spec.rb b/spec/controllers/settings/pictures_controller_spec.rb index 2368dc55d..705878f03 100644 --- a/spec/controllers/settings/pictures_controller_spec.rb +++ b/spec/controllers/settings/pictures_controller_spec.rb @@ -18,5 +18,35 @@ describe Settings::PicturesController do expect(response).to have_http_status(400) end end + + context 'with valid picture id' do + context 'when account updates correctly' do + let(:service) { instance_double(UpdateAccountService, call: true) } + + before do + allow(UpdateAccountService).to receive(:new).and_return(service) + end + + it 'updates the account' do + delete :destroy, params: { id: 'avatar' } + expect(response).to redirect_to(settings_profile_path) + expect(response).to have_http_status(303) + expect(service).to have_received(:call).with(user.account, { 'avatar' => nil, 'avatar_remote_url' => '' }) + end + end + + context 'when account cannot update' do + let(:service) { instance_double(UpdateAccountService, call: false) } + + before do + allow(UpdateAccountService).to receive(:new).and_return(service) + end + + it 'redirects to profile' do + delete :destroy, params: { id: 'avatar' } + expect(response).to redirect_to(settings_profile_path) + end + end + end end end diff --git a/spec/controllers/settings/preferences/appearance_controller_spec.rb b/spec/controllers/settings/preferences/appearance_controller_spec.rb index 7c7f716b7..df0237a6b 100644 --- a/spec/controllers/settings/preferences/appearance_controller_spec.rb +++ b/spec/controllers/settings/preferences/appearance_controller_spec.rb @@ -14,7 +14,16 @@ describe Settings::Preferences::AppearanceController do 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 'redirects correctly' do + put :update, params: { user: { setting_theme: 'contrast' } } + + expect(response).to redirect_to(settings_preferences_appearance_path) + end + end end diff --git a/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb b/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb index 66ffe89f3..153eca1a5 100644 --- a/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb @@ -5,14 +5,24 @@ require 'rails_helper' describe Settings::TwoFactorAuthenticationMethodsController do render_views - let(:user) { Fabricate(:user) } + context 'when not signed in' do + describe 'GET to #index' do + it 'redirects' do + get :index - describe 'GET #index' do - context 'when signed in' do - before do - sign_in user, scope: :user + expect(response).to redirect_to '/auth/sign_in' end + end + end + + context 'when signed in' do + let(:user) { Fabricate(:user) } + before do + sign_in user, scope: :user + end + + describe 'GET #index' do describe 'when user has enabled otp' do before do user.update(otp_required_for_login: true) @@ -38,11 +48,32 @@ describe Settings::TwoFactorAuthenticationMethodsController do end end - context 'when not signed in' do - it 'redirects' do - get :index + describe 'POST to #disable' do + before do + user.update(otp_required_for_login: true) + end - expect(response).to redirect_to '/auth/sign_in' + context 'when user has not passed challenge' do + it 'renders challenge page' do + post :disable + + expect(response).to have_http_status(200) + expect(response).to render_template('auth/challenges/new') + end + end + + context 'when user has passed challenge' do + before do + mailer = instance_double(ApplicationMailer::MessageDelivery, deliver_later!: true) + allow(UserMailer).to receive(:two_factor_disabled).with(user).and_return(mailer) + end + + it 'redirects to settings page' do + post :disable, session: { challenge_passed_at: 10.minutes.ago } + + expect(UserMailer).to have_received(:two_factor_disabled).with(user) + expect(response).to redirect_to(settings_otp_authentication_path) + end end end end diff --git a/spec/fabricators/featured_tag_fabricator.rb b/spec/fabricators/featured_tag_fabricator.rb new file mode 100644 index 000000000..747d8e36a --- /dev/null +++ b/spec/fabricators/featured_tag_fabricator.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:featured_tag) do + account + tag + name 'Tag' +end -- cgit From f53d009778e2ae7a0a7b246147dff8e6bbec3755 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 12 Apr 2023 12:47:05 +0200 Subject: Refactor `Status._insert_record` slightly and tighten the test around reblogs of discarded statuses (#24516) --- app/models/status.rb | 48 +++++++++++++++++++++--------------- spec/services/reblog_service_spec.rb | 5 +++- 2 files changed, 32 insertions(+), 21 deletions(-) (limited to 'spec') diff --git a/app/models/status.rb b/app/models/status.rb index 2757497db..302049e20 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -391,33 +391,41 @@ class Status < ApplicationRecord super || build_status_stat end - # Hack to use a "INSERT INTO ... SELECT ..." query instead of "INSERT INTO ... VALUES ..." query + # This is a hack to ensure that no reblogs of discarded statuses are created, + # as this cannot be enforced through database constraints the same way we do + # for reblogs of deleted statuses. + # + # To achieve this, we redefine the internal method responsible for issuing + # the "INSERT" statement and replace the "INSERT INTO ... VALUES ..." query + # with an "INSERT INTO ... SELECT ..." query with a "WHERE deleted_at IS NULL" + # clause on the reblogged status to ensure consistency at the database level. + # + # Otherwise, the code is kept as close as possible to ActiveRecord::Persistence + # code, and actually calls it if we are not handling a reblog. def self._insert_record(values) - if values.is_a?(Hash) && values['reblog_of_id'].present? - primary_key = self.primary_key - primary_key_value = nil + return super unless values.is_a?(Hash) && values['reblog_of_id'].present? - if primary_key - primary_key_value = values[primary_key] + primary_key = self.primary_key + primary_key_value = nil - if !primary_key_value && prefetch_primary_key? - primary_key_value = next_sequence_value - values[primary_key] = primary_key_value - end + if primary_key + primary_key_value = values[primary_key] + + if !primary_key_value && prefetch_primary_key? + primary_key_value = next_sequence_value + values[primary_key] = primary_key_value end + end - # The following line is where we differ from stock ActiveRecord implementation - im = _compile_reblog_insert(values) + # The following line is where we differ from stock ActiveRecord implementation + im = _compile_reblog_insert(values) - # Since we are using SELECT instead of VALUES, a non-error `nil` return is possible. - # For our purposes, it's equivalent to a foreign key constraint violation - result = connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) - raise ActiveRecord::InvalidForeignKey, "(reblog_of_id)=(#{values['reblog_of_id']}) is not present in table \"statuses\"" if result.nil? + # Since we are using SELECT instead of VALUES, a non-error `nil` return is possible. + # For our purposes, it's equivalent to a foreign key constraint violation + result = connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) + raise ActiveRecord::InvalidForeignKey, "(reblog_of_id)=(#{values['reblog_of_id']}) is not present in table \"statuses\"" if result.nil? - result - else - super - end + result end def self._compile_reblog_insert(values) diff --git a/spec/services/reblog_service_spec.rb b/spec/services/reblog_service_spec.rb index c00472229..fdf5ec923 100644 --- a/spec/services/reblog_service_spec.rb +++ b/spec/services/reblog_service_spec.rb @@ -38,7 +38,10 @@ RSpec.describe ReblogService, type: :service do let(:status) { Fabricate(:status, account: alice, visibility: :public) } before do - status.discard + # Update the in-database attribute without reflecting the change in + # the object. This cannot simulate all race conditions, but it is + # pretty close. + Status.where(id: status.id).update_all(deleted_at: Time.now.utc) # rubocop:disable Rails/SkipsModelValidations end it 'raises an exception' do -- cgit From 10f0de42129d17fd28fc6ff92b77d49156b0185b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 14 Apr 2023 08:42:10 -0400 Subject: Refactor race condition reblog service spec (#24526) --- spec/services/reblog_service_spec.rb | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'spec') diff --git a/spec/services/reblog_service_spec.rb b/spec/services/reblog_service_spec.rb index fdf5ec923..2ad6d30f6 100644 --- a/spec/services/reblog_service_spec.rb +++ b/spec/services/reblog_service_spec.rb @@ -35,13 +35,25 @@ RSpec.describe ReblogService, type: :service do end context 'when the reblogged status is discarded in the meantime' do - let(:status) { Fabricate(:status, account: alice, visibility: :public) } + let(:status) { Fabricate(:status, account: alice, visibility: :public, text: 'discard-status-text') } + # Add a callback to discard the status being reblogged after the + # validations pass but before the database commit is executed. before do - # Update the in-database attribute without reflecting the change in - # the object. This cannot simulate all race conditions, but it is - # pretty close. - Status.where(id: status.id).update_all(deleted_at: Time.now.utc) # rubocop:disable Rails/SkipsModelValidations + Status.class_eval do + before_save :discard_status + def discard_status + Status + .where(id: reblog_of_id) + .where(text: 'discard-status-text') + .update_all(deleted_at: Time.now.utc) # rubocop:disable Rails/SkipsModelValidations + end + end + end + + # Remove race condition simulating `discard_status` callback. + after do + Status._save_callbacks.delete(:discard_status) end it 'raises an exception' do -- cgit From 4601e0dcbb1c10dba16708662145dfa2595fee33 Mon Sep 17 00:00:00 2001 From: Heitor de Melo Cardozo Date: Mon, 17 Apr 2023 06:06:06 -0300 Subject: Add user handle to notification mail recipient address (#24240) Co-authored-by: luccamps Co-authored-by: Leonardo Negreiros de Oliveira Co-authored-by: Marcio Flavio Co-authored-by: Gabriel Quaresma --- app/mailers/notification_mailer.rb | 10 +++++----- spec/mailers/notification_mailer_spec.rb | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'spec') diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index ab73826ab..c428fd30d 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -14,7 +14,7 @@ class NotificationMailer < ApplicationMailer locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: @me.user.email, subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct) + mail to: email_address_with_name(@me.user.email, @me.user.account.username), subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct) end end @@ -25,7 +25,7 @@ class NotificationMailer < ApplicationMailer return unless @me.user.functional? locale_for_account(@me) do - mail to: @me.user.email, subject: I18n.t('notification_mailer.follow.subject', name: @account.acct) + mail to: email_address_with_name(@me.user.email, @me.user.account.username), subject: I18n.t('notification_mailer.follow.subject', name: @account.acct) end end @@ -38,7 +38,7 @@ class NotificationMailer < ApplicationMailer locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: @me.user.email, subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct) + mail to: email_address_with_name(@me.user.email, @me.user.account.username), subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct) end end @@ -51,7 +51,7 @@ class NotificationMailer < ApplicationMailer locale_for_account(@me) do thread_by_conversation(@status.conversation) - mail to: @me.user.email, subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct) + mail to: email_address_with_name(@me.user.email, @me.user.account.username), subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct) end end @@ -62,7 +62,7 @@ class NotificationMailer < ApplicationMailer return unless @me.user.functional? locale_for_account(@me) do - mail to: @me.user.email, subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct) + mail to: email_address_with_name(@me.user.email, @me.user.account.username), subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct) end end diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index a6db08d85..341fe6f23 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -29,7 +29,7 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders the headers' do expect(mail.subject).to eq('You were mentioned by bob') - expect(mail.to).to eq([receiver.email]) + expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -46,7 +46,7 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders the headers' do expect(mail.subject).to eq('bob is now following you') - expect(mail.to).to eq([receiver.email]) + expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -62,7 +62,7 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders the headers' do expect(mail.subject).to eq('bob favourited your post') - expect(mail.to).to eq([receiver.email]) + expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -79,7 +79,7 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders the headers' do expect(mail.subject).to eq('bob boosted your post') - expect(mail.to).to eq([receiver.email]) + expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do @@ -96,7 +96,7 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders the headers' do expect(mail.subject).to eq('Pending follower: bob') - expect(mail.to).to eq([receiver.email]) + expect(mail[:to].value).to eq("#{receiver.account.username} <#{receiver.email}>") end it 'renders the body' do -- cgit From bc75e62ca6e16d3dad43fd35ccca335de547cfb3 Mon Sep 17 00:00:00 2001 From: Heitor de Melo Cardozo Date: Mon, 17 Apr 2023 09:16:36 -0300 Subject: Change moderation search an account using the username with @ (#24242) --- app/models/account_filter.rb | 2 +- spec/models/account_filter_spec.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb index 1666ea883..55d34e85c 100644 --- a/app/models/account_filter.rb +++ b/app/models/account_filter.rb @@ -55,7 +55,7 @@ class AccountFilter when 'by_domain' Account.where(domain: value.to_s.strip) when 'username' - Account.matches_username(value.to_s.strip) + Account.matches_username(value.to_s.strip.delete_prefix('@')) when 'display_name' Account.matches_display_name(value.to_s.strip) when 'email' diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb index 3032260fe..cb00e7609 100644 --- a/spec/models/account_filter_spec.rb +++ b/spec/models/account_filter_spec.rb @@ -44,4 +44,23 @@ describe AccountFilter do expect(filter.results).to match_array [remote_account_one] end end + + describe 'with username' do + let!(:local_account) { Fabricate(:account, domain: nil, username: 'validUserName') } + + it 'works with @ at the beginning of the username' do + filter = described_class.new(username: '@validUserName') + expect(filter.results).to match_array [local_account] + end + + it 'does not work with more than one @ at the beginning of the username' do + filter = described_class.new(username: '@@validUserName') + expect(filter.results).to_not match_array [local_account] + end + + it 'does not work with @ outside the beginning of the username' do + filter = described_class.new(username: 'validUserName@') + expect(filter.results).to_not match_array [local_account] + end + end end -- cgit From 4db8230194258a9a1c3d17d7261608515f3f2067 Mon Sep 17 00:00:00 2001 From: Robert R George Date: Tue, 18 Apr 2023 02:33:30 -0700 Subject: Add trend management to admin API (#24257) --- .../links/preview_card_providers_controller.rb | 72 ++++++++++++++++++++++ .../api/v1/admin/trends/links_controller.rb | 31 +++++++++- .../api/v1/admin/trends/statuses_controller.rb | 31 +++++++++- .../api/v1/admin/trends/tags_controller.rb | 23 ++++++- app/models/preview_card_provider.rb | 1 + .../rest/admin/trends/link_serializer.rb | 9 +++ .../links/preview_card_provider_serializer.rb | 10 +++ .../rest/admin/trends/status_serializer.rb | 9 +++ config/routes.rb | 30 ++++++++- .../preview_card_providers_controller_spec.rb | 68 ++++++++++++++++++++ .../api/v1/admin/trends/links_controller_spec.rb | 49 ++++++++++++++- .../v1/admin/trends/statuses_controller_spec.rb | 49 ++++++++++++++- .../api/v1/admin/trends/tags_controller_spec.rb | 49 ++++++++++++++- 13 files changed, 419 insertions(+), 12 deletions(-) create mode 100644 app/controllers/api/v1/admin/trends/links/preview_card_providers_controller.rb create mode 100644 app/serializers/rest/admin/trends/link_serializer.rb create mode 100644 app/serializers/rest/admin/trends/links/preview_card_provider_serializer.rb create mode 100644 app/serializers/rest/admin/trends/status_serializer.rb create mode 100644 spec/controllers/api/v1/admin/trends/links/preview_card_providers_controller_spec.rb (limited to 'spec') diff --git a/app/controllers/api/v1/admin/trends/links/preview_card_providers_controller.rb b/app/controllers/api/v1/admin/trends/links/preview_card_providers_controller.rb new file mode 100644 index 000000000..5d9fcc82c --- /dev/null +++ b/app/controllers/api/v1/admin/trends/links/preview_card_providers_controller.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +class Api::V1::Admin::Trends::Links::PreviewCardProvidersController < Api::BaseController + include Authorization + + LIMIT = 100 + + before_action -> { authorize_if_got_token! :'admin:read' }, only: :index + before_action -> { authorize_if_got_token! :'admin:write' }, except: :index + before_action :set_providers, only: :index + + after_action :verify_authorized + after_action :insert_pagination_headers, only: :index + + PAGINATION_PARAMS = %i(limit).freeze + + def index + authorize :preview_card_provider, :index? + + render json: @providers, each_serializer: REST::Admin::Trends::Links::PreviewCardProviderSerializer + end + + def approve + authorize :preview_card_provider, :review? + + provider = PreviewCardProvider.find(params[:id]) + provider.update(trendable: true, reviewed_at: Time.now.utc) + render json: provider, serializer: REST::Admin::Trends::Links::PreviewCardProviderSerializer + end + + def reject + authorize :preview_card_provider, :review? + + provider = PreviewCardProvider.find(params[:id]) + provider.update(trendable: false, reviewed_at: Time.now.utc) + render json: provider, serializer: REST::Admin::Trends::Links::PreviewCardProviderSerializer + end + + private + + def set_providers + @providers = PreviewCardProvider.all.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + api_v1_admin_trends_links_preview_card_providers_url(pagination_params(max_id: pagination_max_id)) if records_continue? + end + + def prev_path + api_v1_admin_trends_links_preview_card_providers_url(pagination_params(min_id: pagination_since_id)) unless @providers.empty? + end + + def pagination_max_id + @providers.last.id + end + + def pagination_since_id + @providers.first.id + end + + def records_continue? + @providers.size == limit_param(LIMIT) + end + + def pagination_params(core_params) + params.slice(*PAGINATION_PARAMS).permit(*PAGINATION_PARAMS).merge(core_params) + end +end diff --git a/app/controllers/api/v1/admin/trends/links_controller.rb b/app/controllers/api/v1/admin/trends/links_controller.rb index cc6388980..7f4ca4828 100644 --- a/app/controllers/api/v1/admin/trends/links_controller.rb +++ b/app/controllers/api/v1/admin/trends/links_controller.rb @@ -1,7 +1,36 @@ # frozen_string_literal: true class Api::V1::Admin::Trends::LinksController < Api::V1::Trends::LinksController - before_action -> { authorize_if_got_token! :'admin:read' } + include Authorization + + before_action -> { authorize_if_got_token! :'admin:read' }, only: :index + before_action -> { authorize_if_got_token! :'admin:write' }, except: :index + + after_action :verify_authorized, except: :index + + def index + if current_user&.can?(:manage_taxonomies) + render json: @links, each_serializer: REST::Admin::Trends::LinkSerializer + else + super + end + end + + def approve + authorize :preview_card, :review? + + link = PreviewCard.find(params[:id]) + link.update(trendable: true) + render json: link, serializer: REST::Admin::Trends::LinkSerializer + end + + def reject + authorize :preview_card, :review? + + link = PreviewCard.find(params[:id]) + link.update(trendable: false) + render json: link, serializer: REST::Admin::Trends::LinkSerializer + end private diff --git a/app/controllers/api/v1/admin/trends/statuses_controller.rb b/app/controllers/api/v1/admin/trends/statuses_controller.rb index c39f77363..34b6580df 100644 --- a/app/controllers/api/v1/admin/trends/statuses_controller.rb +++ b/app/controllers/api/v1/admin/trends/statuses_controller.rb @@ -1,7 +1,36 @@ # frozen_string_literal: true class Api::V1::Admin::Trends::StatusesController < Api::V1::Trends::StatusesController - before_action -> { authorize_if_got_token! :'admin:read' } + include Authorization + + before_action -> { authorize_if_got_token! :'admin:read' }, only: :index + before_action -> { authorize_if_got_token! :'admin:write' }, except: :index + + after_action :verify_authorized, except: :index + + def index + if current_user&.can?(:manage_taxonomies) + render json: @statuses, each_serializer: REST::Admin::Trends::StatusSerializer + else + super + end + end + + def approve + authorize [:admin, :status], :review? + + status = Status.find(params[:id]) + status.update(trendable: true) + render json: status, serializer: REST::Admin::Trends::StatusSerializer + end + + def reject + authorize [:admin, :status], :review? + + status = Status.find(params[:id]) + status.update(trendable: false) + render json: status, serializer: REST::Admin::Trends::StatusSerializer + end private diff --git a/app/controllers/api/v1/admin/trends/tags_controller.rb b/app/controllers/api/v1/admin/trends/tags_controller.rb index e77df3021..2eeea9522 100644 --- a/app/controllers/api/v1/admin/trends/tags_controller.rb +++ b/app/controllers/api/v1/admin/trends/tags_controller.rb @@ -1,7 +1,12 @@ # frozen_string_literal: true class Api::V1::Admin::Trends::TagsController < Api::V1::Trends::TagsController - before_action -> { authorize_if_got_token! :'admin:read' } + include Authorization + + before_action -> { authorize_if_got_token! :'admin:read' }, only: :index + before_action -> { authorize_if_got_token! :'admin:write' }, except: :index + + after_action :verify_authorized, except: :index def index if current_user&.can?(:manage_taxonomies) @@ -11,6 +16,22 @@ class Api::V1::Admin::Trends::TagsController < Api::V1::Trends::TagsController end end + def approve + authorize :tag, :review? + + tag = Tag.find(params[:id]) + tag.update(trendable: true, reviewed_at: Time.now.utc) + render json: tag, serializer: REST::Admin::TagSerializer + end + + def reject + authorize :tag, :review? + + tag = Tag.find(params[:id]) + tag.update(trendable: false, reviewed_at: Time.now.utc) + render json: tag, serializer: REST::Admin::TagSerializer + end + private def enabled? diff --git a/app/models/preview_card_provider.rb b/app/models/preview_card_provider.rb index 1dd95fc91..9f5f6d3cb 100644 --- a/app/models/preview_card_provider.rb +++ b/app/models/preview_card_provider.rb @@ -18,6 +18,7 @@ # class PreviewCardProvider < ApplicationRecord + include Paginable include DomainNormalizable include Attachmentable diff --git a/app/serializers/rest/admin/trends/link_serializer.rb b/app/serializers/rest/admin/trends/link_serializer.rb new file mode 100644 index 000000000..c93e6c178 --- /dev/null +++ b/app/serializers/rest/admin/trends/link_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class REST::Admin::Trends::LinkSerializer < REST::Trends::LinkSerializer + attributes :id, :requires_review + + def requires_review + object.requires_review? + end +end diff --git a/app/serializers/rest/admin/trends/links/preview_card_provider_serializer.rb b/app/serializers/rest/admin/trends/links/preview_card_provider_serializer.rb new file mode 100644 index 000000000..fba0259fb --- /dev/null +++ b/app/serializers/rest/admin/trends/links/preview_card_provider_serializer.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class REST::Admin::Trends::Links::PreviewCardProviderSerializer < ActiveModel::Serializer + attributes :id, :domain, :trendable, :reviewed_at, + :requested_review_at, :requires_review + + def requires_review + object.requires_review? + end +end diff --git a/app/serializers/rest/admin/trends/status_serializer.rb b/app/serializers/rest/admin/trends/status_serializer.rb new file mode 100644 index 000000000..e46be30ab --- /dev/null +++ b/app/serializers/rest/admin/trends/status_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class REST::Admin::Trends::StatusSerializer < REST::StatusSerializer + attributes :requires_review + + def requires_review + object.requires_review? + end +end diff --git a/config/routes.rb b/config/routes.rb index 22ef10866..3be088cee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -660,9 +660,33 @@ Rails.application.routes.draw do resources :ip_blocks, only: [:index, :show, :update, :create, :destroy] namespace :trends do - resources :tags, only: [:index] - resources :links, only: [:index] - resources :statuses, only: [:index] + resources :tags, only: [:index] do + member do + post :approve + post :reject + end + end + resources :links, only: [:index] do + member do + post :approve + post :reject + end + end + resources :statuses, only: [:index] do + member do + post :approve + post :reject + end + end + + namespace :links do + resources :preview_card_providers, only: [:index], path: :publishers do + member do + post :approve + post :reject + end + end + end end post :measures, to: 'measures#create' diff --git a/spec/controllers/api/v1/admin/trends/links/preview_card_providers_controller_spec.rb b/spec/controllers/api/v1/admin/trends/links/preview_card_providers_controller_spec.rb new file mode 100644 index 000000000..883a55b7b --- /dev/null +++ b/spec/controllers/api/v1/admin/trends/links/preview_card_providers_controller_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Admin::Trends::Links::PreviewCardProvidersController do + render_views + + let(:role) { UserRole.find_by(name: 'Admin') } + let(:user) { Fabricate(:user, role: role) } + let(:scopes) { 'admin:read admin:write' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:account) { Fabricate(:account) } + let(:preview_card_provider) { Fabricate(:preview_card_provider) } + + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + shared_examples 'forbidden for wrong scope' do |wrong_scope| + let(:scopes) { wrong_scope } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + shared_examples 'forbidden for wrong role' do |wrong_role| + let(:role) { UserRole.find_by(name: wrong_role) } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + describe 'GET #index' do + it 'returns http success' do + get :index, params: { account_id: account.id, limit: 2 } + + expect(response).to have_http_status(200) + end + end + + describe 'POST #approve' do + before do + post :approve, params: { id: preview_card_provider.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + + describe 'POST #reject' do + before do + post :reject, params: { id: preview_card_provider.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end +end diff --git a/spec/controllers/api/v1/admin/trends/links_controller_spec.rb b/spec/controllers/api/v1/admin/trends/links_controller_spec.rb index a64292f06..9c144d3fa 100644 --- a/spec/controllers/api/v1/admin/trends/links_controller_spec.rb +++ b/spec/controllers/api/v1/admin/trends/links_controller_spec.rb @@ -5,14 +5,33 @@ require 'rails_helper' describe Api::V1::Admin::Trends::LinksController do render_views - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'admin:read') } + let(:role) { UserRole.find_by(name: 'Admin') } + let(:user) { Fabricate(:user, role: role) } + let(:scopes) { 'admin:read admin:write' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:account) { Fabricate(:account) } + let(:preview_card) { Fabricate(:preview_card) } before do allow(controller).to receive(:doorkeeper_token) { token } end + shared_examples 'forbidden for wrong scope' do |wrong_scope| + let(:scopes) { wrong_scope } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + shared_examples 'forbidden for wrong role' do |wrong_role| + let(:role) { UserRole.find_by(name: wrong_role) } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + describe 'GET #index' do it 'returns http success' do get :index, params: { account_id: account.id, limit: 2 } @@ -20,4 +39,30 @@ describe Api::V1::Admin::Trends::LinksController do expect(response).to have_http_status(200) end end + + describe 'POST #approve' do + before do + post :approve, params: { id: preview_card.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + + describe 'POST #reject' do + before do + post :reject, params: { id: preview_card.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end end diff --git a/spec/controllers/api/v1/admin/trends/statuses_controller_spec.rb b/spec/controllers/api/v1/admin/trends/statuses_controller_spec.rb index 821cc499f..d25186b37 100644 --- a/spec/controllers/api/v1/admin/trends/statuses_controller_spec.rb +++ b/spec/controllers/api/v1/admin/trends/statuses_controller_spec.rb @@ -5,14 +5,33 @@ require 'rails_helper' describe Api::V1::Admin::Trends::StatusesController do render_views - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'admin:read') } + let(:role) { UserRole.find_by(name: 'Admin') } + let(:user) { Fabricate(:user, role: role) } + let(:scopes) { 'admin:read admin:write' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:account) { Fabricate(:account) } + let(:status) { Fabricate(:status) } before do allow(controller).to receive(:doorkeeper_token) { token } end + shared_examples 'forbidden for wrong scope' do |wrong_scope| + let(:scopes) { wrong_scope } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + shared_examples 'forbidden for wrong role' do |wrong_role| + let(:role) { UserRole.find_by(name: wrong_role) } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + describe 'GET #index' do it 'returns http success' do get :index, params: { account_id: account.id, limit: 2 } @@ -20,4 +39,30 @@ describe Api::V1::Admin::Trends::StatusesController do expect(response).to have_http_status(200) end end + + describe 'POST #approve' do + before do + post :approve, params: { id: status.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + + describe 'POST #reject' do + before do + post :reject, params: { id: status.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end end diff --git a/spec/controllers/api/v1/admin/trends/tags_controller_spec.rb b/spec/controllers/api/v1/admin/trends/tags_controller_spec.rb index 480306ce7..5ee443d57 100644 --- a/spec/controllers/api/v1/admin/trends/tags_controller_spec.rb +++ b/spec/controllers/api/v1/admin/trends/tags_controller_spec.rb @@ -5,14 +5,33 @@ require 'rails_helper' describe Api::V1::Admin::Trends::TagsController do render_views - let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'admin:read') } + let(:role) { UserRole.find_by(name: 'Admin') } + let(:user) { Fabricate(:user, role: role) } + let(:scopes) { 'admin:read admin:write' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:account) { Fabricate(:account) } + let(:tag) { Fabricate(:tag) } before do allow(controller).to receive(:doorkeeper_token) { token } end + shared_examples 'forbidden for wrong scope' do |wrong_scope| + let(:scopes) { wrong_scope } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + + shared_examples 'forbidden for wrong role' do |wrong_role| + let(:role) { UserRole.find_by(name: wrong_role) } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end + describe 'GET #index' do it 'returns http success' do get :index, params: { account_id: account.id, limit: 2 } @@ -20,4 +39,30 @@ describe Api::V1::Admin::Trends::TagsController do expect(response).to have_http_status(200) end end + + describe 'POST #approve' do + before do + post :approve, params: { id: tag.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + + describe 'POST #reject' do + before do + post :reject, params: { id: tag.id } + end + + it_behaves_like 'forbidden for wrong scope', 'write:statuses' + it_behaves_like 'forbidden for wrong role', '' + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end end -- cgit