From 2eb17360df289b4c2e0065a79196a2cb1e1702ea Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 22 Apr 2021 14:26:11 +0200 Subject: Fix delete of local reply to local parent not being forwarded (#16096) --- app/lib/status_reach_finder.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/lib') diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 3aab3bde0..0e755d433 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -62,7 +62,11 @@ class StatusReachFinder end def followers_inboxes - @status.account.followers.inboxes + if @status.reply? && @status.thread.account.local? && @status.distributable? + @status.account.followers.or(@status.thread.account.followers).inboxes + else + @status.account.followers.inboxes + end end def relay_inboxes -- cgit From f4b7c6b61914070e590507bcb33e4345d3f9b0b9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Apr 2021 13:35:39 +0200 Subject: Fix nil error when removing status caused by race condition (#16099) --- app/lib/status_reach_finder.rb | 2 +- app/models/status.rb | 4 ++++ app/workers/activitypub/distribution_worker.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'app/lib') diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 0e755d433..735d66a4f 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -62,7 +62,7 @@ class StatusReachFinder end def followers_inboxes - if @status.reply? && @status.thread.account.local? && @status.distributable? + if @status.in_reply_to_local_account? && @status.distributable? @status.account.followers.or(@status.thread.account.followers).inboxes else @status.account.followers.inboxes diff --git a/app/models/status.rb b/app/models/status.rb index 74e81f612..847921ac2 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -161,6 +161,10 @@ class Status < ApplicationRecord attributes['local'] || uri.nil? end + def in_reply_to_local_account? + reply? && thread&.account&.local? + end + def reblog? !reblog_of_id.nil? end diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 9b4814644..09898ca49 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -35,7 +35,7 @@ class ActivityPub::DistributionWorker # Deliver the status to all followers. # If the status is a reply to another local status, also forward it to that # status' authors' followers. - @inboxes ||= if @status.reply? && @status.thread.account.local? && @status.distributable? + @inboxes ||= if @status.in_reply_to_local_account? && @status.distributable? @account.followers.or(@status.thread.account.followers).inboxes else @account.followers.inboxes -- cgit From daccc07dc170627b17564402296f6c8631d0cd97 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Apr 2021 17:01:43 +0200 Subject: Change auto-following admin-selected accounts, show in recommendations (#16078) --- app/controllers/api/v1/suggestions_controller.rb | 10 ++-- app/lib/potential_friendship_tracker.rb | 10 ---- app/models/account_suggestions.rb | 25 +++++--- app/models/account_suggestions/global_source.rb | 37 ++++++++++++ .../past_interactions_source.rb | 36 ++++++++++++ app/models/account_suggestions/setting_source.rb | 68 ++++++++++++++++++++++ app/models/account_suggestions/source.rb | 34 +++++++++++ app/models/account_suggestions/suggestion.rb | 7 +++ app/models/follow_recommendation.rb | 15 ----- app/models/form/admin_settings.rb | 2 - app/services/bootstrap_timeline_service.rb | 37 +----------- app/validators/existing_username_validator.rb | 24 ++++++-- app/views/admin/settings/edit.html.haml | 5 +- config/locales/en.yml | 7 +-- config/settings.yml | 1 - spec/services/bootstrap_timeline_service_spec.rb | 38 ------------ 16 files changed, 228 insertions(+), 128 deletions(-) create mode 100644 app/models/account_suggestions/global_source.rb create mode 100644 app/models/account_suggestions/past_interactions_source.rb create mode 100644 app/models/account_suggestions/setting_source.rb create mode 100644 app/models/account_suggestions/source.rb create mode 100644 app/models/account_suggestions/suggestion.rb (limited to 'app/lib') diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index b2788cc76..9737ae5cb 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -5,20 +5,20 @@ class Api::V1::SuggestionsController < Api::BaseController before_action -> { doorkeeper_authorize! :read } before_action :require_user! - before_action :set_accounts def index - render json: @accounts, each_serializer: REST::AccountSerializer + suggestions = suggestions_source.get(current_account, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) + render json: suggestions.map(&:account), each_serializer: REST::AccountSerializer end def destroy - PotentialFriendshipTracker.remove(current_account.id, params[:id]) + suggestions_source.remove(current_account, params[:id]) render_empty end private - def set_accounts - @accounts = PotentialFriendshipTracker.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + def suggestions_source + AccountSuggestions::PastInteractionsSource.new end end diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb index e72d454b6..f5bc20346 100644 --- a/app/lib/potential_friendship_tracker.rb +++ b/app/lib/potential_friendship_tracker.rb @@ -27,15 +27,5 @@ class PotentialFriendshipTracker def remove(account_id, target_account_id) redis.zrem("interactions:#{account_id}", target_account_id) end - - def get(account, limit) - account_ids = redis.zrevrange("interactions:#{account.id}", 0, limit) - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.searchable.where(id: account_ids).index_by(&:id) - - account_ids.map { |id| accounts[id.to_i] }.compact - end end end diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb index 7fe9d618e..d1774e62f 100644 --- a/app/models/account_suggestions.rb +++ b/app/models/account_suggestions.rb @@ -1,17 +1,28 @@ # frozen_string_literal: true class AccountSuggestions - class Suggestion < ActiveModelSerializers::Model - attributes :account, :source - end + SOURCES = [ + AccountSuggestions::SettingSource, + AccountSuggestions::PastInteractionsSource, + AccountSuggestions::GlobalSource, + ].freeze def self.get(account, limit) - suggestions = PotentialFriendshipTracker.get(account, limit).map { |target_account| Suggestion.new(account: target_account, source: :past_interaction) } - suggestions.concat(FollowRecommendation.get(account, limit - suggestions.size, suggestions.map { |suggestion| suggestion.account.id }).map { |target_account| Suggestion.new(account: target_account, source: :global) }) if suggestions.size < limit - suggestions + SOURCES.each_with_object([]) do |source_class, suggestions| + source_suggestions = source_class.new.get( + account, + skip_account_ids: suggestions.map(&:account_id), + limit: limit - suggestions.size + ) + + suggestions.concat(source_suggestions) + end end def self.remove(account, target_account_id) - PotentialFriendshipTracker.remove(account.id, target_account_id) + SOURCES.each do |source_class| + source = source_class.new + source.remove(account, target_account_id) + end end end diff --git a/app/models/account_suggestions/global_source.rb b/app/models/account_suggestions/global_source.rb new file mode 100644 index 000000000..ac764de50 --- /dev/null +++ b/app/models/account_suggestions/global_source.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class AccountSuggestions::GlobalSource < AccountSuggestions::Source + def key + :global + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_locale(account.user_locale) - [account.id] - skip_account_ids + + as_ordered_suggestions( + scope(account).where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + end + + def account_ids_for_locale(locale) + Redis.current.zrevrange("follow_recommendations:#{locale}", 0, -1).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/past_interactions_source.rb b/app/models/account_suggestions/past_interactions_source.rb new file mode 100644 index 000000000..d169394f1 --- /dev/null +++ b/app/models/account_suggestions/past_interactions_source.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class AccountSuggestions::PastInteractionsSource < AccountSuggestions::Source + include Redisable + + def key + :past_interactions + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_account(account.id, limit + skip_account_ids.size) - skip_account_ids + + as_ordered_suggestions( + scope.where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(account, target_account_id) + redis.zrem("interactions:#{account.id}", target_account_id) + end + + private + + def scope + Account.searchable + end + + def account_ids_for_account(account_id, limit) + redis.zrevrange("interactions:#{account_id}", 0, limit).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb new file mode 100644 index 000000000..be9eff233 --- /dev/null +++ b/app/models/account_suggestions/setting_source.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class AccountSuggestions::SettingSource < AccountSuggestions::Source + def key + :staff + end + + def get(account, skip_account_ids: [], limit: 40) + return [] unless setting_enabled? + + as_ordered_suggestions( + scope(account).where(setting_to_where_condition).where.not(id: skip_account_ids), + usernames_and_domains + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where(locked: false) + .where.not(id: account.id) + end + + def usernames_and_domains + @usernames_and_domains ||= setting_to_usernames_and_domains + end + + def setting_enabled? + setting.present? + end + + def setting_to_where_condition + usernames_and_domains.map do |(username, domain)| + Arel::Nodes::Grouping.new( + Account.arel_table[:username].lower.eq(username.downcase).and( + Account.arel_table[:domain].lower.eq(domain&.downcase) + ) + ) + end.reduce(:or) + end + + def setting_to_usernames_and_domains + setting.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@', 2) + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [username, domain] + end.compact + end + + def setting + Setting.bootstrap_timeline_accounts + end + + def to_ordered_list_key(account) + [account.username, account.domain] + end +end diff --git a/app/models/account_suggestions/source.rb b/app/models/account_suggestions/source.rb new file mode 100644 index 000000000..bd1068d20 --- /dev/null +++ b/app/models/account_suggestions/source.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class AccountSuggestions::Source + def key + raise NotImplementedError + end + + def get(_account, **kwargs) + raise NotImplementedError + end + + def remove(_account, target_account_id) + raise NotImplementedError + end + + protected + + def as_ordered_suggestions(scope, ordered_list) + return [] if ordered_list.empty? + + map = scope.index_by(&method(:to_ordered_list_key)) + + ordered_list.map { |ordered_list_key| map[ordered_list_key] }.compact.map do |account| + AccountSuggestions::Suggestion.new( + account: account, + source: key + ) + end + end + + def to_ordered_list_key(_account) + raise NotImplementedError + end +end diff --git a/app/models/account_suggestions/suggestion.rb b/app/models/account_suggestions/suggestion.rb new file mode 100644 index 000000000..2c6f4d27f --- /dev/null +++ b/app/models/account_suggestions/suggestion.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AccountSuggestions::Suggestion < ActiveModelSerializers::Model + attributes :account, :source + + delegate :id, to: :account, prefix: true +end diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb index c4355224d..6670b6560 100644 --- a/app/models/follow_recommendation.rb +++ b/app/models/follow_recommendation.rb @@ -21,19 +21,4 @@ class FollowRecommendation < ApplicationRecord def readonly? true end - - def self.get(account, limit, exclude_account_ids = []) - account_ids = Redis.current.zrevrange("follow_recommendations:#{account.user_locale}", 0, -1).map(&:to_i) - exclude_account_ids - [account.id] - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.followable_by(account) - .not_excluded_by_account(account) - .not_domain_blocked_by_account(account) - .where(id: account_ids) - .limit(limit) - .index_by(&:id) - - account_ids.map { |id| accounts[id] }.compact - end end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index b5c3dcdbe..6fc7c56fd 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -16,7 +16,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts bootstrap_timeline_accounts theme min_invite_role @@ -41,7 +40,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts activity_api_enabled peers_api_enabled show_known_fediverse_at_about_page diff --git a/app/services/bootstrap_timeline_service.rb b/app/services/bootstrap_timeline_service.rb index 8412aa7e7..e1a1b98c3 100644 --- a/app/services/bootstrap_timeline_service.rb +++ b/app/services/bootstrap_timeline_service.rb @@ -5,48 +5,13 @@ class BootstrapTimelineService < BaseService @source_account = source_account autofollow_inviter! - autofollow_bootstrap_timeline_accounts! if Setting.enable_bootstrap_timeline_accounts end private def autofollow_inviter! return unless @source_account&.user&.invite&.autofollow? - FollowService.new.call(@source_account, @source_account.user.invite.user.account) - end - - def autofollow_bootstrap_timeline_accounts! - bootstrap_timeline_accounts.each do |target_account| - begin - FollowService.new.call(@source_account, target_account) - rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError - nil - end - end - end - - def bootstrap_timeline_accounts - return @bootstrap_timeline_accounts if defined?(@bootstrap_timeline_accounts) - - @bootstrap_timeline_accounts = bootstrap_timeline_accounts_usernames.empty? ? admin_accounts : local_unlocked_accounts(bootstrap_timeline_accounts_usernames) - end - - def bootstrap_timeline_accounts_usernames - @bootstrap_timeline_accounts_usernames ||= (Setting.bootstrap_timeline_accounts || '').split(',').map { |str| str.strip.gsub(/\A@/, '') }.reject(&:blank?) - end - def admin_accounts - User.admins - .includes(:account) - .where(accounts: { locked: false }) - .map(&:account) - end - - def local_unlocked_accounts(usernames) - Account.local - .without_suspended - .where(username: usernames) - .where(locked: false) - .where(moved_to_account_id: nil) + FollowService.new.call(@source_account, @source_account.user.invite.user.account) end end diff --git a/app/validators/existing_username_validator.rb b/app/validators/existing_username_validator.rb index 723302ec9..afbe0c635 100644 --- a/app/validators/existing_username_validator.rb +++ b/app/validators/existing_username_validator.rb @@ -4,11 +4,25 @@ class ExistingUsernameValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) return if value.blank? - if options[:multiple] - missing_usernames = value.split(',').map { |username| username.strip.gsub(/\A@/, '') }.filter_map { |username| username unless Account.find_local(username) } - record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: missing_usernames.join(', '))) if missing_usernames.any? - else - record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) unless Account.find_local(value.strip.gsub(/\A@/, '')) + usernames_and_domains = begin + value.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@') + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [str, username, domain] + end.compact + end + + usernames_with_no_accounts = usernames_and_domains.filter_map do |(str, username, domain)| + str unless Account.find_remote(username, domain) + end + + if usernames_with_no_accounts.any? && options[:multiple] + record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: usernames_with_no_accounts.join(', '))) + elsif usernames_with_no_accounts.any? || usernames_and_domains.size > 1 + record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) end end end diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 7783dbfeb..33bfc43d3 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -50,10 +50,7 @@ %hr.spacer/ .fields-group - = f.input :enable_bootstrap_timeline_accounts, as: :boolean, wrapper: :with_label, label: t('admin.settings.enable_bootstrap_timeline_accounts.title'), hint: t('admin.settings.enable_bootstrap_timeline_accounts.desc_html') - - .fields-group - = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html'), disabled: !Setting.enable_bootstrap_timeline_accounts + = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html') %hr.spacer/ diff --git a/config/locales/en.yml b/config/locales/en.yml index 765f71250..1b41ee063 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -564,8 +564,8 @@ en: desc_html: Counts of locally published posts, active users, and new registrations in weekly buckets title: Publish aggregate statistics about user activity in the API bootstrap_timeline_accounts: - desc_html: Separate multiple usernames by comma. Only local and unlocked accounts will work. Default when empty is all local admins. - title: Default follows for new users + desc_html: Separate multiple usernames by comma. These accounts will be guaranteed to be shown in follow recommendations + title: Recommend these accounts to new users contact_information: email: Business e-mail username: Contact username @@ -582,9 +582,6 @@ en: users: To logged-in local users domain_blocks_rationale: title: Show rationale - enable_bootstrap_timeline_accounts: - desc_html: Make new users automatically follow configured accounts so their home feed doesn't start out empty - title: Enable default follows for new users hero: desc_html: Displayed on the frontpage. At least 600x100px recommended. When not set, falls back to server thumbnail title: Hero image diff --git a/config/settings.yml b/config/settings.yml index b79ea620c..06cee2532 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -62,7 +62,6 @@ defaults: &defaults - mod - moderator disallowed_hashtags: # space separated string or list of hashtags without the hash - enable_bootstrap_timeline_accounts: true bootstrap_timeline_accounts: '' activity_api_enabled: true peers_api_enabled: true diff --git a/spec/services/bootstrap_timeline_service_spec.rb b/spec/services/bootstrap_timeline_service_spec.rb index a28d2407c..880ca4f0d 100644 --- a/spec/services/bootstrap_timeline_service_spec.rb +++ b/spec/services/bootstrap_timeline_service_spec.rb @@ -1,42 +1,4 @@ require 'rails_helper' RSpec.describe BootstrapTimelineService, type: :service do - subject { described_class.new } - - describe '#call' do - let(:source_account) { Fabricate(:account) } - - context 'when setting is empty' do - let!(:admin) { Fabricate(:user, admin: true) } - - before do - Setting.bootstrap_timeline_accounts = nil - subject.call(source_account) - end - - it 'follows admin accounts from account' do - expect(source_account.following?(admin.account)).to be true - end - end - - context 'when setting is set' do - let!(:alice) { Fabricate(:account, username: 'alice') } - let!(:bob) { Fabricate(:account, username: 'bob') } - let!(:eve) { Fabricate(:account, username: 'eve', suspended: true) } - - before do - Setting.bootstrap_timeline_accounts = 'alice, @bob, eve, unknown' - subject.call(source_account) - end - - it 'follows found accounts from account' do - expect(source_account.following?(alice)).to be true - expect(source_account.following?(bob)).to be true - end - - it 'does not follow suspended account' do - expect(source_account.following?(eve)).to be false - end - end - end end -- cgit From 036556d3509fac5fa487a0d5ff3cf95767e8d84f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 5 May 2021 19:44:01 +0200 Subject: Fix media processing getting stuck on too much stdin/stderr (#16136) * Fix media processing getting stuck on too much stdin/stderr See thoughtbot/terrapin#5 * Remove dependency on paperclip-av-transcoder gem * Remove dependency on streamio-ffmpeg gem * Disable stdin on ffmpeg process --- Gemfile | 2 - Gemfile.lock | 11 ---- app/lib/video_metadata_extractor.rb | 54 +++++++++++++++++ app/models/media_attachment.rb | 4 +- config/application.rb | 4 +- lib/paperclip/attachment_extensions.rb | 4 ++ lib/paperclip/gif_transcoder.rb | 3 +- lib/paperclip/image_extractor.rb | 14 ++--- lib/paperclip/transcoder.rb | 102 +++++++++++++++++++++++++++++++++ lib/paperclip/transcoder_extensions.rb | 14 ----- lib/paperclip/video_transcoder.rb | 26 --------- lib/terrapin/multi_pipe_extensions.rb | 63 ++++++++++++++++++++ 12 files changed, 234 insertions(+), 67 deletions(-) create mode 100644 app/lib/video_metadata_extractor.rb create mode 100644 lib/paperclip/transcoder.rb delete mode 100644 lib/paperclip/transcoder_extensions.rb delete mode 100644 lib/paperclip/video_transcoder.rb create mode 100644 lib/terrapin/multi_pipe_extensions.rb (limited to 'app/lib') diff --git a/Gemfile b/Gemfile index fb80e24d5..6ca0a81de 100644 --- a/Gemfile +++ b/Gemfile @@ -21,8 +21,6 @@ gem 'aws-sdk-s3', '~> 1.94', require: false gem 'fog-core', '<= 2.1.0' gem 'fog-openstack', '~> 0.3', require: false gem 'paperclip', '~> 6.0' -gem 'paperclip-av-transcoder', '~> 0.6' -gem 'streamio-ffmpeg', '~> 3.0' gem 'blurhash', '~> 0.1' gem 'active_model_serializers', '~> 0.10' diff --git a/Gemfile.lock b/Gemfile.lock index 3c36e07bc..b1ae4fd22 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -77,8 +77,6 @@ GEM ast (2.4.2) attr_encrypted (3.1.0) encryptor (~> 3.0.0) - av (0.9.0) - cocaine (~> 0.5.3) awrence (1.1.1) aws-eventstream (1.1.1) aws-partitions (1.449.0) @@ -156,8 +154,6 @@ GEM cld3 (3.4.2) ffi (>= 1.1.0, < 1.16.0) climate_control (0.2.0) - cocaine (0.5.8) - climate_control (>= 0.0.3, < 1.0) coderay (1.1.3) color_diff (0.1) concurrent-ruby (1.1.8) @@ -402,9 +398,6 @@ GEM mime-types mimemagic (~> 0.3.0) terrapin (~> 0.6.0) - paperclip-av-transcoder (0.6.4) - av (~> 0.9.0) - paperclip (>= 2.5.2) parallel (1.20.1) parallel_tests (3.7.0) parallel @@ -605,8 +598,6 @@ GEM stackprof (0.2.16) statsd-ruby (1.5.0) stoplight (2.2.1) - streamio-ffmpeg (3.0.2) - multi_json (~> 1.8) strong_migrations (0.7.6) activerecord (>= 5) temple (0.8.2) @@ -750,7 +741,6 @@ DEPENDENCIES omniauth-saml (~> 1.10) ox (~> 2.14) paperclip (~> 6.0) - paperclip-av-transcoder (~> 0.6) parallel (~> 1.20) parallel_tests (~> 3.7) parslet @@ -795,7 +785,6 @@ DEPENDENCIES sprockets-rails (~> 3.2) stackprof stoplight (~> 2.2.1) - streamio-ffmpeg (~> 3.0) strong_migrations (~> 0.7) thor (~> 1.1) tty-prompt (~> 0.23) diff --git a/app/lib/video_metadata_extractor.rb b/app/lib/video_metadata_extractor.rb new file mode 100644 index 000000000..03e40f923 --- /dev/null +++ b/app/lib/video_metadata_extractor.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class VideoMetadataExtractor + attr_reader :duration, :bitrate, :video_codec, :audio_codec, + :colorspace, :width, :height, :frame_rate + + def initialize(path) + @path = path + @metadata = Oj.load(ffmpeg_command_output, mode: :strict, symbol_keys: true) + + parse_metadata + rescue Terrapin::ExitStatusError, Oj::ParseError + @invalid = true + rescue Terrapin::CommandNotFoundError + raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffprobe` command. Please install ffmpeg.' + end + + def valid? + !@invalid + end + + private + + def ffmpeg_command_output + command = Terrapin::CommandLine.new('ffprobe', '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel') + command.run(path: @path, format: 'json', loglevel: 'fatal') + end + + def parse_metadata + if @metadata.key?(:format) + @duration = @metadata[:format][:duration].to_f + @bitrate = @metadata[:format][:bit_rate].to_i + end + + if @metadata.key?(:streams) + video_streams = @metadata[:streams].select { |stream| stream[:codec_type] == 'video' } + audio_streams = @metadata[:streams].select { |stream| stream[:codec_type] == 'audio' } + + if (video_stream = video_streams.first) + @video_codec = video_stream[:codec_name] + @colorspace = video_stream[:pix_fmt] + @width = video_stream[:width] + @height = video_stream[:height] + @frame_rate = video_stream[:avg_frame_rate] == '0/0' ? nil : Rational(video_stream[:avg_frame_rate]) + end + + if (audio_stream = audio_streams.first) + @audio_codec = audio_stream[:codec_name] + end + end + + @invalid = true if @metadata.key?(:error) + end +end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 5cf4d8127..3515f6895 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -287,7 +287,7 @@ class MediaAttachment < ApplicationRecord if instance.file_content_type == 'image/gif' [:gif_transcoder, :blurhash_transcoder] elsif VIDEO_MIME_TYPES.include?(instance.file_content_type) - [:video_transcoder, :blurhash_transcoder, :type_corrector] + [:transcoder, :blurhash_transcoder, :type_corrector] elsif AUDIO_MIME_TYPES.include?(instance.file_content_type) [:image_extractor, :transcoder, :type_corrector] else @@ -388,7 +388,7 @@ class MediaAttachment < ApplicationRecord # paths but ultimately the same file, so it makes sense to memoize the # result while disregarding the path def ffmpeg_data(path = nil) - @ffmpeg_data ||= FFMPEG::Movie.new(path) + @ffmpeg_data ||= VideoMetadataExtractor.new(path) end def enqueue_processing diff --git a/config/application.rb b/config/application.rb index 9aa1594ce..37a996224 100644 --- a/config/application.rb +++ b/config/application.rb @@ -13,12 +13,12 @@ require_relative '../lib/redis/namespace_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' -require_relative '../lib/paperclip/transcoder_extensions' require_relative '../lib/paperclip/lazy_thumbnail' require_relative '../lib/paperclip/gif_transcoder' -require_relative '../lib/paperclip/video_transcoder' +require_relative '../lib/paperclip/transcoder' require_relative '../lib/paperclip/type_corrector' require_relative '../lib/paperclip/response_with_limit_adapter' +require_relative '../lib/terrapin/multi_pipe_extensions' require_relative '../lib/mastodon/snowflake' require_relative '../lib/mastodon/version' require_relative '../lib/devise/two_factor_ldap_authenticatable' diff --git a/lib/paperclip/attachment_extensions.rb b/lib/paperclip/attachment_extensions.rb index e25a34213..271f8b603 100644 --- a/lib/paperclip/attachment_extensions.rb +++ b/lib/paperclip/attachment_extensions.rb @@ -2,6 +2,10 @@ module Paperclip module AttachmentExtensions + def meta + instance_read(:meta) + end + # We overwrite this method to support delayed processing in # Sidekiq. Since we process the original file to reduce disk # usage, and we still want to generate thumbnails straight diff --git a/lib/paperclip/gif_transcoder.rb b/lib/paperclip/gif_transcoder.rb index 9f3c8e8be..74aa1a0b2 100644 --- a/lib/paperclip/gif_transcoder.rb +++ b/lib/paperclip/gif_transcoder.rb @@ -100,7 +100,8 @@ end module Paperclip # This transcoder is only to be used for the MediaAttachment model - # to convert animated gifs to webm + # to convert animated GIFs to videos + class GifTranscoder < Paperclip::Processor def make return File.open(@file.path) unless needs_convert? diff --git a/lib/paperclip/image_extractor.rb b/lib/paperclip/image_extractor.rb index aab675a06..17fe4326f 100644 --- a/lib/paperclip/image_extractor.rb +++ b/lib/paperclip/image_extractor.rb @@ -31,21 +31,17 @@ module Paperclip private def extract_image_from_file! - ::Av.logger = Paperclip.logger - - cli = ::Av.cli dst = Tempfile.new([File.basename(@file.path, '.*'), '.png']) dst.binmode - cli.add_source(@file.path) - cli.add_destination(dst.path) - cli.add_output_param loglevel: 'fatal' - begin - cli.run - rescue Cocaine::ExitStatusError, ::Av::CommandError + command = Terrapin::CommandLine.new('ffmpeg', '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger) + command.run(source: @file.path, destination: dst.path, loglevel: 'fatal') + rescue Terrapin::ExitStatusError dst.close(true) return nil + rescue Terrapin::CommandNotFoundError + raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffmpeg` command. Please install ffmpeg.' end dst diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb new file mode 100644 index 000000000..e99704086 --- /dev/null +++ b/lib/paperclip/transcoder.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +module Paperclip + # This transcoder is only to be used for the MediaAttachment model + # to check when uploaded videos are actually gifv's + class Transcoder < Paperclip::Processor + def initialize(file, options = {}, attachment = nil) + super + + @current_format = File.extname(@file.path) + @basename = File.basename(@file.path, @current_format) + @format = options[:format] + @time = options[:time] || 3 + @passthrough_options = options[:passthrough_options] + @convert_options = options[:convert_options].dup + end + + def make + metadata = VideoMetadataExtractor.new(@file.path) + + unless metadata.valid? + log("Unsupported file #{@file.path}") + return File.open(@file.path) + end + + update_attachment_type(metadata) + update_options_from_metadata(metadata) + + destination = Tempfile.new([@basename, @format ? ".#{@format}" : '']) + destination.binmode + + @output_options = @convert_options[:output]&.dup || {} + @input_options = @convert_options[:input]&.dup || {} + + case @format.to_s + when /jpg$/, /jpeg$/, /png$/, /gif$/ + @input_options['ss'] = @time + + @output_options['f'] = 'image2' + @output_options['vframes'] = 1 + when 'mp4' + @output_options['acodec'] = 'aac' + @output_options['strict'] = 'experimental' + end + + command_arguments, interpolations = prepare_command(destination) + + begin + command = Terrapin::CommandLine.new('ffmpeg', command_arguments.join(' '), logger: Paperclip.logger) + command.run(interpolations) + rescue Terrapin::ExitStatusError => e + raise Paperclip::Error, "Error while transcoding #{@basename}: #{e}" + rescue Terrapin::CommandNotFoundError + raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffmpeg` command. Please install ffmpeg.' + end + + destination + end + + private + + def prepare_command(destination) + command_arguments = ['-nostdin'] + interpolations = {} + interpolation_keys = 0 + + @input_options.each_pair do |key, value| + interpolation_key = interpolation_keys + command_arguments << "-#{key} :#{interpolation_key}" + interpolations[interpolation_key] = value + interpolation_keys += 1 + end + + command_arguments << '-i :source' + interpolations[:source] = @file.path + + @output_options.each_pair do |key, value| + interpolation_key = interpolation_keys + command_arguments << "-#{key} :#{interpolation_key}" + interpolations[interpolation_key] = value + interpolation_keys += 1 + end + + command_arguments << '-y :destination' + interpolations[:destination] = destination.path + + [command_arguments, interpolations] + end + + def update_options_from_metadata(metadata) + return unless @passthrough_options && @passthrough_options[:video_codecs].include?(metadata.video_codec) && @passthrough_options[:audio_codecs].include?(metadata.audio_codec) && @passthrough_options[:colorspaces].include?(metadata.colorspace) + + @format = @passthrough_options[:options][:format] || @format + @time = @passthrough_options[:options][:time] || @time + @convert_options = @passthrough_options[:options][:convert_options].dup + end + + def update_attachment_type(metadata) + @attachment.instance.type = MediaAttachment.types[:gifv] unless metadata.audio_codec + end + end +end diff --git a/lib/paperclip/transcoder_extensions.rb b/lib/paperclip/transcoder_extensions.rb deleted file mode 100644 index c0b2447f3..000000000 --- a/lib/paperclip/transcoder_extensions.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module Paperclip - module TranscoderExtensions - # Prevent the transcoder from modifying our meta hash - def initialize(file, options = {}, attachment = nil) - meta_value = attachment&.instance_read(:meta) - super - attachment&.instance_write(:meta, meta_value) - end - end -end - -Paperclip::Transcoder.prepend(Paperclip::TranscoderExtensions) diff --git a/lib/paperclip/video_transcoder.rb b/lib/paperclip/video_transcoder.rb deleted file mode 100644 index 4d9544231..000000000 --- a/lib/paperclip/video_transcoder.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module Paperclip - # This transcoder is only to be used for the MediaAttachment model - # to check when uploaded videos are actually gifv's - class VideoTranscoder < Paperclip::Processor - def make - movie = FFMPEG::Movie.new(@file.path) - - attachment.instance.type = MediaAttachment.types[:gifv] unless movie.audio_codec - - Paperclip::Transcoder.make(file, actual_options(movie), attachment) - end - - private - - def actual_options(movie) - opts = options[:passthrough_options] - if opts && opts[:video_codecs].include?(movie.video_codec) && opts[:audio_codecs].include?(movie.audio_codec) && opts[:colorspaces].include?(movie.colorspace) - opts[:options] - else - options - end - end - end -end diff --git a/lib/terrapin/multi_pipe_extensions.rb b/lib/terrapin/multi_pipe_extensions.rb new file mode 100644 index 000000000..51d7de37c --- /dev/null +++ b/lib/terrapin/multi_pipe_extensions.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: false +# Fix adapted from https://github.com/thoughtbot/terrapin/pull/5 + +module Terrapin + module MultiPipeExtensions + def read + read_streams(@stdout_in, @stderr_in) + end + + def close_read + begin + @stdout_in.close + rescue IOError + # Do nothing + end + + begin + @stderr_in.close + rescue IOError + # Do nothing + end + end + + def read_streams(output, error) + @stdout_output = '' + @stderr_output = '' + + read_fds = [output, error] + + until read_fds.empty? + to_read, = IO.select(read_fds) + + if to_read.include?(output) + @stdout_output << read_stream(output) + read_fds.delete(output) if output.closed? + end + + if to_read.include?(error) + @stderr_output << read_stream(error) + read_fds.delete(error) if error.closed? + end + end + end + + def read_stream(io) + result = '' + + begin + while (partial_result = io.read_nonblock(8192)) + result << partial_result + end + rescue EOFError, Errno::EPIPE + io.close + rescue Errno::EINTR, Errno::EWOULDBLOCK, Errno::EAGAIN + # Do nothing + end + + result + end + end +end + +Terrapin::CommandLine::MultiPipe.prepend(Terrapin::MultiPipeExtensions) -- cgit From 7cb34b32f8bc925b56c79dbcf053671f93f2eb42 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Thu, 6 May 2021 06:39:02 +0900 Subject: Add management of delivery availability in Federation settings (#15771) * Add management of delivery availavility in Federation settings * fix translate * Remove useless object creation * Fix DeepSource issue * Add shortcut for all * Fix DeepSource(skipcq) * Change 'remove' to 'clear' * Fix style * Change class method name (exhausted_deliveries_key_by) --- app/controllers/admin/instances_controller.rb | 44 +++++++++++++++++++++- app/helpers/admin/action_logs_helper.rb | 4 +- app/lib/delivery_failure_tracker.rb | 26 +++++++++++++ app/models/admin/action_log_filter.rb | 2 + app/models/instance.rb | 3 ++ app/models/instance_filter.rb | 8 +++- app/policies/delivery_policy.rb | 15 ++++++++ .../instances/_exhausted_deliveries_days.haml | 2 + app/views/admin/instances/_instance.html.haml | 8 ++++ app/views/admin/instances/index.html.haml | 18 +++++++++ app/views/admin/instances/show.html.haml | 25 ++++++++++++ config/locales/en.yml | 21 +++++++++++ config/routes.rb | 9 ++++- 13 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 app/policies/delivery_policy.rb create mode 100644 app/views/admin/instances/_exhausted_deliveries_days.haml (limited to 'app/lib') diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index b5918d231..748c5de5a 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -3,7 +3,8 @@ module Admin class InstancesController < BaseController before_action :set_instances, only: :index - before_action :set_instance, only: :show + before_action :set_instance, except: :index + before_action :set_exhausted_deliveries_days, only: :show def index authorize :instance, :index? @@ -13,14 +14,55 @@ module Admin authorize :instance, :show? end + def clear_delivery_errors + authorize :delivery, :clear_delivery_errors? + + @instance.delivery_failure_tracker.clear_failures! + redirect_to admin_instance_path(@instance.domain) + end + + def restart_delivery + authorize :delivery, :restart_delivery? + + last_unavailable_domain = unavailable_domain + + if last_unavailable_domain.present? + @instance.delivery_failure_tracker.track_success! + log_action :destroy, last_unavailable_domain + end + + redirect_to admin_instance_path(@instance.domain) + end + + def stop_delivery + authorize :delivery, :stop_delivery? + + UnavailableDomain.create(domain: @instance.domain) + log_action :create, unavailable_domain + redirect_to admin_instance_path(@instance.domain) + end + private def set_instance @instance = Instance.find(params[:id]) end + def set_exhausted_deliveries_days + @exhausted_deliveries_days = @instance.delivery_failure_tracker.exhausted_deliveries_days + end + def set_instances @instances = filtered_instances.page(params[:page]) + warning_domains_map = DeliveryFailureTracker.warning_domains_map + + @instances.each do |instance| + instance.failure_days = warning_domains_map[instance.domain] + end + end + + def unavailable_domain + UnavailableDomain.find_by(domain: @instance.domain) end def filtered_instances diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index 0f3ca36e2..e9a298a24 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -21,7 +21,7 @@ module Admin::ActionLogsHelper record.shortcode when 'Report' link_to "##{record.id}", admin_report_path(record) - when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock' + when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain' link_to record.domain, "https://#{record.domain}" when 'Status' link_to record.account.acct, ActivityPub::TagManager.instance.url_for(record) @@ -38,7 +38,7 @@ module Admin::ActionLogsHelper case type when 'CustomEmoji' attributes['shortcode'] - when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock' + when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain' link_to attributes['domain'], "https://#{attributes['domain']}" when 'Status' tmp_status = Status.new(attributes.except('reblogs_count', 'favourites_count')) diff --git a/app/lib/delivery_failure_tracker.rb b/app/lib/delivery_failure_tracker.rb index 2cd6ef7ad..8907ade4c 100644 --- a/app/lib/delivery_failure_tracker.rb +++ b/app/lib/delivery_failure_tracker.rb @@ -17,6 +17,10 @@ class DeliveryFailureTracker UnavailableDomain.find_by(domain: @host)&.destroy end + def clear_failures! + Redis.current.del(exhausted_deliveries_key) + end + def days Redis.current.scard(exhausted_deliveries_key) || 0 end @@ -25,6 +29,10 @@ class DeliveryFailureTracker !UnavailableDomain.where(domain: @host).exists? end + def exhausted_deliveries_days + Redis.current.smembers(exhausted_deliveries_key).sort.map { |date| Date.new(date.slice(0, 4).to_i, date.slice(4, 2).to_i, date.slice(6, 2).to_i) } + end + alias reset! track_success! class << self @@ -44,6 +52,24 @@ class DeliveryFailureTracker def reset!(url) new(url).reset! end + + def warning_domains + domains = Redis.current.keys(exhausted_deliveries_key_by('*')).map do |key| + key.delete_prefix(exhausted_deliveries_key_by('')) + end + + domains - UnavailableDomain.all.pluck(:domain) + end + + def warning_domains_map + warning_domains.index_with { |domain| Redis.current.scard(exhausted_deliveries_key_by(domain)) } + end + + private + + def exhausted_deliveries_key_by(host) + "exhausted_deliveries:#{host}" + end end private diff --git a/app/models/admin/action_log_filter.rb b/app/models/admin/action_log_filter.rb index 3a1b67e06..a1c156a8b 100644 --- a/app/models/admin/action_log_filter.rb +++ b/app/models/admin/action_log_filter.rb @@ -17,12 +17,14 @@ class Admin::ActionLogFilter create_domain_allow: { target_type: 'DomainAllow', action: 'create' }.freeze, create_domain_block: { target_type: 'DomainBlock', action: 'create' }.freeze, create_email_domain_block: { target_type: 'EmailDomainBlock', action: 'create' }.freeze, + create_unavailable_domain: { target_type: 'UnavailableDomain', action: 'create' }.freeze, demote_user: { target_type: 'User', action: 'demote' }.freeze, destroy_announcement: { target_type: 'Announcement', action: 'destroy' }.freeze, destroy_custom_emoji: { target_type: 'CustomEmoji', action: 'destroy' }.freeze, destroy_domain_allow: { target_type: 'DomainAllow', action: 'destroy' }.freeze, destroy_domain_block: { target_type: 'DomainBlock', action: 'destroy' }.freeze, destroy_email_domain_block: { target_type: 'EmailDomainBlock', action: 'destroy' }.freeze, + destroy_unavailable_domain: { target_type: 'UnavailableDomain', action: 'destroy' }.freeze, destroy_status: { target_type: 'Status', action: 'destroy' }.freeze, disable_2fa_user: { target_type: 'User', action: 'disable' }.freeze, disable_custom_emoji: { target_type: 'CustomEmoji', action: 'disable' }.freeze, diff --git a/app/models/instance.rb b/app/models/instance.rb index 29be03662..8949be054 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -10,10 +10,13 @@ class Instance < ApplicationRecord self.primary_key = :domain + attr_accessor :failure_days + has_many :accounts, foreign_key: :domain, primary_key: :domain belongs_to :domain_block, foreign_key: :domain, primary_key: :domain belongs_to :domain_allow, foreign_key: :domain, primary_key: :domain + belongs_to :unavailable_domain, foreign_key: :domain, primary_key: :domain # skipcq: RB-RL1031 scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } diff --git a/app/models/instance_filter.rb b/app/models/instance_filter.rb index 0598d8fea..9e533c4aa 100644 --- a/app/models/instance_filter.rb +++ b/app/models/instance_filter.rb @@ -4,6 +4,8 @@ class InstanceFilter KEYS = %i( limited by_domain + warning + unavailable ).freeze attr_reader :params @@ -13,7 +15,7 @@ class InstanceFilter end def results - scope = Instance.includes(:domain_block, :domain_allow).order(accounts_count: :desc) + scope = Instance.includes(:domain_block, :domain_allow, :unavailable_domain).order(accounts_count: :desc) params.each do |key, value| scope.merge!(scope_for(key, value.to_s.strip)) if value.present? @@ -32,6 +34,10 @@ class InstanceFilter Instance.joins(:domain_allow).reorder(Arel.sql('domain_allows.id desc')) when 'by_domain' Instance.matches_domain(value) + when 'warning' + Instance.where(domain: DeliveryFailureTracker.warning_domains) + when 'unavailable' + Instance.joins(:unavailable_domain) else raise "Unknown filter: #{key}" end diff --git a/app/policies/delivery_policy.rb b/app/policies/delivery_policy.rb new file mode 100644 index 000000000..24d06c168 --- /dev/null +++ b/app/policies/delivery_policy.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class DeliveryPolicy < ApplicationPolicy + def clear_delivery_errors? + admin? + end + + def restart_delivery? + admin? + end + + def stop_delivery? + admin? + end +end diff --git a/app/views/admin/instances/_exhausted_deliveries_days.haml b/app/views/admin/instances/_exhausted_deliveries_days.haml new file mode 100644 index 000000000..e581f542d --- /dev/null +++ b/app/views/admin/instances/_exhausted_deliveries_days.haml @@ -0,0 +1,2 @@ +%li.negative-hint + = l(exhausted_deliveries_days) diff --git a/app/views/admin/instances/_instance.html.haml b/app/views/admin/instances/_instance.html.haml index 188d0d984..990cf9ec8 100644 --- a/app/views/admin/instances/_instance.html.haml +++ b/app/views/admin/instances/_instance.html.haml @@ -22,4 +22,12 @@ = t('admin.accounts.whitelisted') - else = t('admin.accounts.no_limits_imposed') + - if instance.failure_days + = ' / ' + %span.negative-hint + = t('admin.instances.delivery.warning_message', count: instance.failure_days) + - if instance.unavailable_domain + = ' / ' + %span.negative-hint + = t('admin.instances.delivery.unavailable_message') .trends__item__current{ title: t('admin.instances.known_accounts', count: instance.accounts_count) }= number_to_human instance.accounts_count, strip_insignificant_zeros: true diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index 7c7958786..797948d94 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -16,6 +16,24 @@ - unless whitelist_mode? %li= filter_link_to t('admin.instances.moderation.limited'), limited: '1' + .filter-subset + %strong= t('admin.instances.delivery.title') + %ul + %li= filter_link_to t('admin.instances.delivery.all'), warning: nil, unavailable: nil + %li= filter_link_to t('admin.instances.delivery.warning'), warning: '1', unavailable: nil + %li= filter_link_to t('admin.instances.delivery.unavailable'), warning: nil, unavailable: '1' + + .back-link + = link_to admin_instances_path() do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_all') + = link_to admin_instances_path(limited: 1) do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_limited') + = link_to admin_instances_path(warning: 1) do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_warning') + - unless whitelist_mode? = form_tag admin_instances_url, method: 'GET', class: 'simple_form' do .fields-group diff --git a/app/views/admin/instances/show.html.haml b/app/views/admin/instances/show.html.haml index 0b9382771..462529338 100644 --- a/app/views/admin/instances/show.html.haml +++ b/app/views/admin/instances/show.html.haml @@ -1,6 +1,18 @@ - content_for :page_title do = @instance.domain +.filters + .back-link + = link_to admin_instances_path() do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_all') + = link_to admin_instances_path(limited: 1) do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_limited') + = link_to admin_instances_path(warning: 1) do + %i.fa.fa-chevron-left.fa-fw + = t('admin.instances.back_to_warning') + .dashboard__counters %div = link_to admin_accounts_path(remote: '1', by_domain: @instance.domain) do @@ -48,6 +60,13 @@ = simple_format(h(@instance.public_comment)) .speech-bubble__owner= t 'admin.instances.public_comment' +- unless @exhausted_deliveries_days.empty? + %h4= t 'admin.instances.delivery_error_days' + %ul + = render partial: 'exhausted_deliveries_days', collection: @exhausted_deliveries_days + %p.hint + = t 'admin.instances.delivery_error_hint', count: DeliveryFailureTracker::FAILURE_DAYS_THRESHOLD + %hr.spacer/ %div.action-buttons @@ -59,3 +78,9 @@ = link_to t('admin.domain_blocks.undo'), admin_domain_block_path(@instance.domain_block), class: 'button' - else = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @instance.domain), class: 'button' + - if @instance.delivery_failure_tracker.available? + - unless @exhausted_deliveries_days.empty? + = link_to t('admin.instances.delivery.clear'), clear_delivery_errors_admin_instance_path(@instance), data: { confirm: t('admin.accounts.are_you_sure'), method: :post }, class: 'button' + = link_to t('admin.instances.delivery.stop'), stop_delivery_admin_instance_path(@instance), data: { confirm: t('admin.accounts.are_you_sure'), method: :post }, class: 'button' + - else + = link_to t('admin.instances.delivery.restart'), restart_delivery_admin_instance_path(@instance), data: { confirm: t('admin.accounts.are_you_sure'), method: :post }, class: 'button' diff --git a/config/locales/en.yml b/config/locales/en.yml index 1b41ee063..bfa489817 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -230,6 +230,7 @@ en: create_domain_block: Create Domain Block create_email_domain_block: Create E-mail Domain Block create_ip_block: Create IP rule + create_unavailable_domain: Create Unavailable Domain demote_user: Demote User destroy_announcement: Delete Announcement destroy_custom_emoji: Delete Custom Emoji @@ -238,6 +239,7 @@ en: destroy_email_domain_block: Delete e-mail domain block destroy_ip_block: Delete IP rule destroy_status: Delete Post + destroy_unavailable_domain: Delete Unavailable Domain disable_2fa_user: Disable 2FA disable_custom_emoji: Disable Custom Emoji disable_user: Disable User @@ -271,6 +273,7 @@ en: create_domain_block_html: "%{name} blocked domain %{target}" create_email_domain_block_html: "%{name} blocked e-mail domain %{target}" create_ip_block_html: "%{name} created rule for IP %{target}" + create_unavailable_domain_html: "%{name} stopped delivery to domain %{target}" demote_user_html: "%{name} demoted user %{target}" destroy_announcement_html: "%{name} deleted announcement %{target}" destroy_custom_emoji_html: "%{name} destroyed emoji %{target}" @@ -279,6 +282,7 @@ en: destroy_email_domain_block_html: "%{name} unblocked e-mail domain %{target}" destroy_ip_block_html: "%{name} deleted rule for IP %{target}" destroy_status_html: "%{name} removed post by %{target}" + destroy_unavailable_domain_html: "%{name} resumed delivery to domain %{target}" disable_2fa_user_html: "%{name} disabled two factor requirement for user %{target}" disable_custom_emoji_html: "%{name} disabled emoji %{target}" disable_user_html: "%{name} disabled login for user %{target}" @@ -451,8 +455,25 @@ en: title: Follow recommendations unsuppress: Restore follow recommendation instances: + back_to_all: All + back_to_limited: Limited + back_to_warning: Warning by_domain: Domain + delivery: + all: All + clear: Clear delivery errors + restart: Restart delivery + stop: Stop delivery + title: Delivery + unavailable: Unavailable + unavailable_message: Delivery unavailable + warning: Warning + warning_message: + one: Delivery failure %{count} day + other: Delivery failure %{count} days delivery_available: Delivery is available + delivery_error_days: Delivery error days + delivery_error_hint: If delivery is not possible for %{count} days, it will be automatically marked as undeliverable. empty: No domains found. known_accounts: one: "%{count} known account" diff --git a/config/routes.rb b/config/routes.rb index 4661a7c11..8ca7fccdd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -217,7 +217,14 @@ Rails.application.routes.draw do end end - resources :instances, only: [:index, :show], constraints: { id: /[^\/]+/ } + resources :instances, only: [:index, :show], constraints: { id: /[^\/]+/ } do + member do + post :clear_delivery_errors + post :restart_delivery + post :stop_delivery + end + end + resources :rules resources :reports, only: [:index, :show] do -- cgit From 74081433d0078784b7c2139f6caaa812740632b2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 14:33:43 +0200 Subject: Change trending hashtags to be affected be reblogs (#16164) If a status with a hashtag becomes very popular, it stands to reason that the hashtag should have a chance at trending Fix no stats being recorded for hashtags that are not allowed to trend, and stop ignoring bots Remove references to hashtags in profile directory from the code and the admin UI --- app/controllers/directories_controller.rb | 10 ------ app/lib/activitypub/activity/announce.rb | 4 +++ app/lib/activitypub/activity/create.rb | 2 +- app/models/account.rb | 11 ++----- app/models/account_tag_stat.rb | 24 -------------- app/models/tag.rb | 38 +++++----------------- app/models/tag_filter.rb | 2 -- app/models/trending_tags.rb | 12 ++++--- app/services/process_hashtags_service.rb | 3 +- app/services/reblog_service.rb | 11 +++++++ app/views/admin/tags/_tag.html.haml | 2 -- app/views/admin/tags/index.html.haml | 9 ++--- app/views/admin/tags/show.html.haml | 13 ++------ config/locales/en.yml | 7 ++-- config/locales/simple_form.en.yml | 2 +- config/routes.rb | 2 -- .../20210502233513_drop_account_tag_stats.rb | 13 ++++++++ db/schema.rb | 10 ------ spec/models/account_tag_stat_spec.rb | 38 ---------------------- spec/models/trending_tags_spec.rb | 6 ++-- 20 files changed, 59 insertions(+), 160 deletions(-) delete mode 100644 app/models/account_tag_stat.rb create mode 100644 db/post_migrate/20210502233513_drop_account_tag_stats.rb delete mode 100644 spec/models/account_tag_stat_spec.rb (limited to 'app/lib') diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb index f198ad5ba..f28c5b2af 100644 --- a/app/controllers/directories_controller.rb +++ b/app/controllers/directories_controller.rb @@ -6,7 +6,6 @@ class DirectoriesController < ApplicationController before_action :authenticate_user!, if: :whitelist_mode? before_action :require_enabled! before_action :set_instance_presenter - before_action :set_tag, only: :show before_action :set_accounts skip_before_action :require_functional!, unless: :whitelist_mode? @@ -15,23 +14,14 @@ class DirectoriesController < ApplicationController render :index end - def show - render :index - end - private def require_enabled! return not_found unless Setting.profile_directory end - def set_tag - @tag = Tag.discoverable.find_normalized!(params[:id]) - end - def set_accounts @accounts = Account.local.discoverable.by_recent_status.page(params[:page]).per(20).tap do |query| - query.merge!(Account.tagged_with(@tag.id)) if @tag query.merge!(Account.not_excluded_by_account(current_account)) if current_account end end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index a1081522e..9f778ffb9 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -22,6 +22,10 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity visibility: visibility_from_audience ) + original_status.tags.each do |tag| + tag.use!(@account) + end + distribute(@status) end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index c7a655d9d..e46361c14 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -164,7 +164,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def attach_tags(status) @tags.each do |tag| status.tags << tag - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(@account, status: status, at_time: status.created_at) if status.public_visibility? end @mentions.each do |mention| diff --git a/app/models/account.rb b/app/models/account.rb index a573365de..994459338 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -111,7 +111,6 @@ class Account < ApplicationRecord scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } - scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) } scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) } scope :popular, -> { order('account_stats.followers_count desc') } @@ -279,19 +278,13 @@ class Account < ApplicationRecord if hashtags_map.key?(tag.name) hashtags_map.delete(tag.name) else - transaction do - tags.delete(tag) - tag.decrement_count!(:accounts_count) - end + tags.delete(tag) end end # Add hashtags that were so far missing hashtags_map.each_value do |tag| - transaction do - tags << tag - tag.increment_count!(:accounts_count) - end + tags << tag end end diff --git a/app/models/account_tag_stat.rb b/app/models/account_tag_stat.rb deleted file mode 100644 index 3c36c155a..000000000 --- a/app/models/account_tag_stat.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true -# == Schema Information -# -# Table name: account_tag_stats -# -# id :bigint(8) not null, primary key -# tag_id :bigint(8) not null -# accounts_count :bigint(8) default(0), not null -# hidden :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# - -class AccountTagStat < ApplicationRecord - belongs_to :tag, inverse_of: :account_tag_stat - - def increment_count!(key) - update(key => public_send(key) + 1) - end - - def decrement_count!(key) - update(key => [public_send(key) - 1, 0].max) - end -end diff --git a/app/models/tag.rb b/app/models/tag.rb index efffc7eee..735c30608 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -20,10 +20,8 @@ class Tag < ApplicationRecord has_and_belongs_to_many :statuses has_and_belongs_to_many :accounts - has_and_belongs_to_many :sample_accounts, -> { local.discoverable.popular.limit(3) }, class_name: 'Account' has_many :featured_tags, dependent: :destroy, inverse_of: :tag - has_one :account_tag_stat, dependent: :destroy HASHTAG_SEPARATORS = "_\u00B7\u200c" HASHTAG_NAME_RE = "([[:word:]_][[:word:]#{HASHTAG_SEPARATORS}]*[[:alpha:]#{HASHTAG_SEPARATORS}][[:word:]#{HASHTAG_SEPARATORS}]*[[:word:]_])|([[:word:]_]*[[:alpha:]][[:word:]_]*)" @@ -38,29 +36,11 @@ class Tag < ApplicationRecord scope :usable, -> { where(usable: [true, nil]) } scope :listable, -> { where(listable: [true, nil]) } scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } - scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } - # Search with case-sensitive to use B-tree index. - scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } - - delegate :accounts_count, - :accounts_count=, - :increment_count!, - :decrement_count!, - to: :account_tag_stat - - after_save :save_account_tag_stat + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index update_index('tags#tag', :self) - def account_tag_stat - super || build_account_tag_stat - end - - def cached_sample_accounts - Rails.cache.fetch("#{cache_key}/sample_accounts", expires_in: 12.hours) { sample_accounts } - end - def to_param name end @@ -95,6 +75,10 @@ class Tag < ApplicationRecord requested_review_at.present? end + def use!(account, status: nil, at_time: Time.now.utc) + TrendingTags.record_use!(self, account, status: status, at_time: at_time) + end + def trending? TrendingTags.trending?(self) end @@ -127,9 +111,10 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - striped_term = term.strip - query = Tag.listable.matches_name(striped_term) - query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] + stripped_term = term.strip + + query = Tag.listable.matches_name(stripped_term) + query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) .limit(limit) @@ -161,11 +146,6 @@ class Tag < ApplicationRecord private - def save_account_tag_stat - return unless account_tag_stat&.changed? - account_tag_stat.save - end - def validate_name_change errors.add(:name, I18n.t('tags.does_not_match_previous_name')) unless name_was.mb_chars.casecmp(name.mb_chars).zero? end diff --git a/app/models/tag_filter.rb b/app/models/tag_filter.rb index a9ff5b703..85bfcbea5 100644 --- a/app/models/tag_filter.rb +++ b/app/models/tag_filter.rb @@ -33,8 +33,6 @@ class TagFilter def scope_for(key, value) case key.to_s - when 'directory' - Tag.discoverable when 'reviewed' Tag.reviewed.order(reviewed_at: :desc) when 'unreviewed' diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb index 9c2aa0ee8..31890b082 100644 --- a/app/models/trending_tags.rb +++ b/app/models/trending_tags.rb @@ -13,19 +13,23 @@ class TrendingTags class << self include Redisable - def record_use!(tag, account, at_time = Time.now.utc) - return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?) + def record_use!(tag, account, status: nil, at_time: Time.now.utc) + return unless tag.usable? && !account.silenced? + # Even if a tag is not allowed to trend, we still need to + # record the stats since they can be displayed in other places increment_historical_use!(tag.id, at_time) increment_unique_use!(tag.id, account.id, at_time) increment_use!(tag.id, at_time) - tag.update(last_status_at: Time.now.utc) if tag.last_status_at.nil? || tag.last_status_at < 12.hours.ago + # Only update when the tag was last used once every 12 hours + # and only if a status is given (lets use ignore reblogs) + tag.update(last_status_at: at_time) if status.present? && (tag.last_status_at.nil? || (tag.last_status_at < at_time && tag.last_status_at < 12.hours.ago)) end def update!(at_time = Time.now.utc) tag_ids = redis.smembers("#{KEY}:used:#{at_time.beginning_of_day.to_i}") + redis.zrange(KEY, 0, -1) - tags = Tag.where(id: tag_ids.uniq) + tags = Tag.trendable.where(id: tag_ids.uniq) # First pass to calculate scores and update the set diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index e8e139b05..c42b79db8 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -8,8 +8,7 @@ class ProcessHashtagsService < BaseService Tag.find_or_create_by_names(tags) do |tag| status.tags << tag records << tag - - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(status.account, status: status, at_time: status.created_at) if status.public_visibility? end return unless status.distributable? diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 5032397b3..744bdf567 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -35,6 +35,7 @@ class ReblogService < BaseService create_notification(reblog) bump_potential_friendship(account, reblog) + record_use(account, reblog) reblog end @@ -59,6 +60,16 @@ class ReblogService < BaseService PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog) end + def record_use(account, reblog) + return unless reblog.public_visibility? + + original_status = reblog.reblog + + original_status.tags.each do |tag| + tag.use!(account) + end + end + def build_json(reblog) Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account)) end diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml index 287d28e53..adf4ca7b2 100644 --- a/app/views/admin/tags/_tag.html.haml +++ b/app/views/admin/tags/_tag.html.haml @@ -10,8 +10,6 @@ = tag.name %small - = t('admin.tags.in_directory', count: tag.accounts_count) - • = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) - if tag.trending? diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml index d7719d45d..d78f3c6d1 100644 --- a/app/views/admin/tags/index.html.haml +++ b/app/views/admin/tags/index.html.haml @@ -5,12 +5,6 @@ = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' .filters - .filter-subset - %strong= t('admin.tags.context') - %ul - %li= filter_link_to t('generic.all'), directory: nil - %li= filter_link_to t('admin.tags.directory'), directory: '1' - .filter-subset %strong= t('admin.tags.review') %ul @@ -23,8 +17,9 @@ %strong= t('generic.order_by') %ul %li= filter_link_to t('admin.tags.most_recent'), popular: nil, active: nil - %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil %li= filter_link_to t('admin.tags.last_active'), active: '1', popular: nil + %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil + = form_tag admin_tags_url, method: 'GET', class: 'simple_form' do .fields-group diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml index c9a147587..c4caffda1 100644 --- a/app/views/admin/tags/show.html.haml +++ b/app/views/admin/tags/show.html.haml @@ -10,15 +10,6 @@ %div .dashboard__counters__num= number_with_delimiter @accounts_week .dashboard__counters__label= t 'admin.tags.accounts_week' - %div - - if @tag.accounts_count > 0 - = link_to explore_hashtag_path(@tag) do - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' - - else - %div - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' %hr.spacer/ @@ -30,8 +21,8 @@ .fields-group = f.input :usable, as: :boolean, wrapper: :with_label - = f.input :trendable, as: :boolean, wrapper: :with_label, disabled: !Setting.trends - = f.input :listable, as: :boolean, wrapper: :with_label, disabled: !Setting.profile_directory + = f.input :trendable, as: :boolean, wrapper: :with_label + = f.input :listable, as: :boolean, wrapper: :with_label .actions = f.button :button, t('generic.save_changes'), type: :submit diff --git a/config/locales/en.yml b/config/locales/en.yml index bfa489817..d8ad5bd84 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -698,12 +698,9 @@ en: accounts_today: Unique uses today accounts_week: Unique uses this week breakdown: Breakdown of today's usage by source - context: Context - directory: In directory - in_directory: "%{count} in directory" - last_active: Last active + last_active: Recently used most_popular: Most popular - most_recent: Most recent + most_recent: Recently created name: Hashtag review: Review status reviewed: Reviewed diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 8ff880ebc..c4388ffc5 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -208,7 +208,7 @@ en: rule: text: Rule tag: - listable: Allow this hashtag to appear in searches and on the profile directory + listable: Allow this hashtag to appear in searches and suggestions name: Hashtag trendable: Allow this hashtag to appear under trends usable: Allow posts to use this hashtag diff --git a/config/routes.rb b/config/routes.rb index 8ca7fccdd..2373d8a51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -97,8 +97,6 @@ Rails.application.routes.draw do post '/interact/:id', to: 'remote_interaction#create' get '/explore', to: 'directories#index', as: :explore - get '/explore/:id', to: 'directories#show', as: :explore_hashtag - get '/settings', to: redirect('/settings/profile') namespace :settings do diff --git a/db/post_migrate/20210502233513_drop_account_tag_stats.rb b/db/post_migrate/20210502233513_drop_account_tag_stats.rb new file mode 100644 index 000000000..80adadcab --- /dev/null +++ b/db/post_migrate/20210502233513_drop_account_tag_stats.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DropAccountTagStats < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + drop_table :account_tag_stats + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index 88e906079..19b1afe00 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -115,15 +115,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true end - create_table "account_tag_stats", force: :cascade do |t| - t.bigint "tag_id", null: false - t.bigint "accounts_count", default: 0, null: false - t.boolean "hidden", default: false, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["tag_id"], name: "index_account_tag_stats_on_tag_id", unique: true - end - create_table "account_warning_presets", force: :cascade do |t| t.text "text", default: "", null: false t.datetime "created_at", null: false @@ -985,7 +976,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade - add_foreign_key "account_tag_stats", "tags", on_delete: :cascade add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_warnings", "accounts", on_delete: :nullify add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify diff --git a/spec/models/account_tag_stat_spec.rb b/spec/models/account_tag_stat_spec.rb deleted file mode 100644 index 6d3057f35..000000000 --- a/spec/models/account_tag_stat_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe AccountTagStat, type: :model do - key = 'accounts_count' - let(:account_tag_stat) { Fabricate(:tag).account_tag_stat } - - describe '#increment_count!' do - it 'calls #update' do - args = { key => account_tag_stat.public_send(key) + 1 } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.increment_count!(key) - end - - it 'increments value by 1' do - expect do - account_tag_stat.increment_count!(key) - end.to change { account_tag_stat.accounts_count }.by(1) - end - end - - describe '#decrement_count!' do - it 'calls #update' do - args = { key => [account_tag_stat.public_send(key) - 1, 0].max } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.decrement_count!(key) - end - - it 'decrements value by 1' do - account_tag_stat.update(key => 1) - - expect do - account_tag_stat.decrement_count!(key) - end.to change { account_tag_stat.accounts_count }.by(-1) - end - end -end diff --git a/spec/models/trending_tags_spec.rb b/spec/models/trending_tags_spec.rb index b6122c994..dfbc7d6f8 100644 --- a/spec/models/trending_tags_spec.rb +++ b/spec/models/trending_tags_spec.rb @@ -7,9 +7,9 @@ RSpec.describe TrendingTags do describe '.update!' do let!(:at_time) { Time.now.utc } - let!(:tag1) { Fabricate(:tag, name: 'Catstodon') } - let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon') } - let!(:tag3) { Fabricate(:tag, name: 'OCs') } + let!(:tag1) { Fabricate(:tag, name: 'Catstodon', trendable: true) } + let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon', trendable: true) } + let!(:tag3) { Fabricate(:tag, name: 'OCs', trendable: true) } before do allow(Redis.current).to receive(:pfcount) do |key| -- cgit