From e445a8af64908b2bdb721bec74c113e8258a129b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 6 Sep 2019 13:55:51 +0200 Subject: Add timeline read markers API (#11762) Fix #4093 --- app/models/marker.rb | 23 +++++++++++++++++++++++ app/models/user.rb | 1 + 2 files changed, 24 insertions(+) create mode 100644 app/models/marker.rb (limited to 'app/models') diff --git a/app/models/marker.rb b/app/models/marker.rb new file mode 100644 index 000000000..a5bd2176a --- /dev/null +++ b/app/models/marker.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: markers +# +# id :bigint(8) not null, primary key +# user_id :bigint(8) +# timeline :string default(""), not null +# last_read_id :bigint(8) default(0), not null +# lock_version :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# + +class Marker < ApplicationRecord + TIMELINES = %w(home notifications).freeze + + belongs_to :user + + validates :timeline, :last_read_id, presence: true + validates :timeline, inclusion: { in: TIMELINES } +end diff --git a/app/models/user.rb b/app/models/user.rb index a4a20d975..95f1d8fc5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,6 +74,7 @@ class User < ApplicationRecord has_many :applications, class_name: 'Doorkeeper::Application', as: :owner has_many :backups, inverse_of: :user has_many :invites, inverse_of: :user + has_many :markers, inverse_of: :user, dependent: :destroy has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? } -- cgit From a75009a65e1335047dff5488b4e67bdc03677590 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 8 Sep 2019 19:17:57 +0200 Subject: Change half-life of trend decay (#11774) --- app/models/trending_tags.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb index e1b92b175..8cdade42d 100644 --- a/app/models/trending_tags.rb +++ b/app/models/trending_tags.rb @@ -7,8 +7,8 @@ class TrendingTags THRESHOLD = 5 LIMIT = 10 REVIEW_THRESHOLD = 3 - MAX_SCORE_COOLDOWN = 3.days.freeze - MAX_SCORE_HALFLIFE = 6.hours.freeze + MAX_SCORE_COOLDOWN = 2.days.freeze + MAX_SCORE_HALFLIFE = 2.hours.freeze class << self include Redisable @@ -83,6 +83,7 @@ class TrendingTags # Trim older items redis.zremrangebyrank(KEY, 0, -(LIMIT + 1)) + redis.zremrangebyscore(KEY, '(0.3', '-inf') end def get(limit, filtered: true) -- cgit From 261e52268c05d2da4459a23e2898555dd5db5771 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 9 Sep 2019 12:50:09 +0200 Subject: Add batch approve/reject for pending hashtags in admin UI (#11791) --- app/controllers/admin/tags_controller.rb | 41 +++++++++++++++++++++++++++--- app/javascript/styles/mastodon/tables.scss | 10 ++++++++ app/models/form/tag_batch.rb | 33 ++++++++++++++++++++++++ app/views/admin/tags/_tag.html.haml | 30 ++++++++++++---------- app/views/admin/tags/index.html.haml | 37 ++++++++++++++++++++++++++- config/locales/en.yml | 1 + config/routes.rb | 9 ++++++- 7 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 app/models/form/tag_batch.rb (limited to 'app/models') diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 8bd4e5f8b..376ebe44d 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -3,12 +3,33 @@ module Admin class TagsController < BaseController before_action :set_tags, only: :index - before_action :set_tag, except: :index - before_action :set_usage_by_domain, except: :index - before_action :set_counters, except: :index + before_action :set_tag, except: [:index, :batch, :approve_all, :reject_all] + before_action :set_usage_by_domain, except: [:index, :batch, :approve_all, :reject_all] + before_action :set_counters, except: [:index, :batch, :approve_all, :reject_all] def index authorize :tag, :index? + + @form = Form::TagBatch.new + end + + def batch + @form = Form::TagBatch.new(form_tag_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + flash[:alert] = I18n.t('admin.accounts.no_account_selected') + ensure + redirect_to admin_tags_path(filter_params) + end + + def approve_all + Form::TagBatch.new(current_account: current_account, tag_ids: Tag.pending_review.pluck(:id), action: 'approve').save + redirect_to admin_tags_path(filter_params) + end + + def reject_all + Form::TagBatch.new(current_account: current_account, tag_ids: Tag.pending_review.pluck(:id), action: 'reject').save + redirect_to admin_tags_path(filter_params) end def show @@ -61,7 +82,7 @@ module Admin end def filter_params - params.slice(:context, :review).permit(:context, :review) + params.slice(:context, :review, :page).permit(:context, :review, :page) end def tag_params @@ -75,5 +96,17 @@ module Admin date.to_time(:utc).beginning_of_day.to_i end end + + def form_tag_batch_params + params.require(:form_tag_batch).permit(:action, tag_ids: []) + end + + def action_from_button + if params[:approve] + 'approve' + elsif params[:reject] + 'reject' + end + end end end diff --git a/app/javascript/styles/mastodon/tables.scss b/app/javascript/styles/mastodon/tables.scss index fe6beba5d..2aef099e6 100644 --- a/app/javascript/styles/mastodon/tables.scss +++ b/app/javascript/styles/mastodon/tables.scss @@ -211,6 +211,16 @@ a.table-action-link { padding: 0; } } + + .directory__tag { + margin: 0; + width: 100%; + + a { + background: transparent; + border-radius: 0; + } + } } .status__content { diff --git a/app/models/form/tag_batch.rb b/app/models/form/tag_batch.rb new file mode 100644 index 000000000..fd517a1a6 --- /dev/null +++ b/app/models/form/tag_batch.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class Form::TagBatch + include ActiveModel::Model + include Authorization + + attr_accessor :tag_ids, :action, :current_account + + def save + case action + when 'approve' + approve! + when 'reject' + reject! + end + end + + private + + def tags + Tag.where(id: tag_ids) + end + + def approve! + tags.each { |tag| authorize(tag, :update?) } + tags.update_all(trendable: true, reviewed_at: Time.now.utc) + end + + def reject! + tags.each { |tag| authorize(tag, :update?) } + tags.update_all(trendable: false, reviewed_at: Time.now.utc) + end +end diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml index 91af8e492..670f3bc05 100644 --- a/app/views/admin/tags/_tag.html.haml +++ b/app/views/admin/tags/_tag.html.haml @@ -1,16 +1,20 @@ -.directory__tag - = link_to admin_tag_path(tag.id) do - %h4 - = fa_icon 'hashtag' - = tag.name +.batch-table__row + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :tag_ids, { multiple: true, include_hidden: false }, tag.id - %small - = t('admin.tags.in_directory', count: tag.accounts_count) - • - = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) + .directory__tag + = link_to admin_tag_path(tag.id) do + %h4 + = fa_icon 'hashtag' + = tag.name - - if tag.trending? - = fa_icon 'fire fw' - = t('admin.tags.trending_right_now') + %small + = t('admin.tags.in_directory', count: tag.accounts_count) + • + = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) - .trends__item__current= number_to_human tag.history.first[:uses], strip_insignificant_zeros: true + - if tag.trending? + = fa_icon 'fire fw' + = t('admin.tags.trending_right_now') + + .trends__item__current= number_to_human tag.history.first[:uses], strip_insignificant_zeros: true diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml index d994955ef..324d13d3e 100644 --- a/app/views/admin/tags/index.html.haml +++ b/app/views/admin/tags/index.html.haml @@ -1,6 +1,9 @@ - content_for :page_title do = t('admin.tags.title') +- content_for :header_tags do + = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous' + .filters .filter-subset %strong= t('admin.tags.context') @@ -18,5 +21,37 @@ %hr.spacer/ -= render @tags += form_for(@form, url: batch_admin_tags_path) do |f| + = hidden_field_tag :page, params[:page] || 1 + = hidden_field_tag :context, params[:context] + = hidden_field_tag :review, params[:review] + + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + - if params[:review] == 'pending_review' + = f.button safe_join([fa_icon('check'), t('admin.accounts.approve')]), name: :approve, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('times'), t('admin.accounts.reject')]), name: :reject, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + - else + %span.neutral-hint= t('generic.no_batch_actions_available') + + .batch-table__body + - if @tags.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'tag', collection: @tags, locals: { f: f } + = paginate @tags + +- if params[:review] == 'pending_review' + %hr.spacer/ + + %div{ style: 'overflow: hidden' } + %div{ style: 'float: right' } + = link_to t('admin.accounts.reject_all'), reject_all_admin_tags_path, method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' + + %div + = link_to t('admin.accounts.approve_all'), approve_all_admin_tags_path, method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' diff --git a/config/locales/en.yml b/config/locales/en.yml index 687f5f2a0..42d8e0eb8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -727,6 +727,7 @@ en: all: All changes_saved_msg: Changes successfully saved! copy: Copy + no_batch_actions_available: No batch actions available on this page order_by: Order by save_changes: Save changes validation_errors: diff --git a/config/routes.rb b/config/routes.rb index 1ebf9e066..534e68814 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -251,7 +251,14 @@ Rails.application.routes.draw do end resources :account_moderation_notes, only: [:create, :destroy] - resources :tags, only: [:index, :show, :update] + + resources :tags, only: [:index, :show, :update] do + collection do + post :approve_all + post :reject_all + post :batch + end + end end get '/admin', to: redirect('/admin/dashboard', status: 302) -- cgit From 1110ea1a9162d5488e1ed5dbccd0803618e713f8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 9 Sep 2019 22:44:17 +0200 Subject: Add batch actions and categories to admin UI for custom emojis (#11793) --- app/controllers/admin/custom_emojis_controller.rb | 102 +++++++------------- app/javascript/styles/mastodon/tables.scss | 41 ++++++++ app/models/custom_emoji.rb | 6 ++ app/models/custom_emoji_category.rb | 2 + app/models/custom_emoji_filter.rb | 8 +- app/models/form/custom_emoji_batch.rb | 106 +++++++++++++++++++++ .../admin/custom_emojis/_custom_emoji.html.haml | 55 ++++++----- app/views/admin/custom_emojis/index.html.haml | 66 ++++++++++--- config/locales/en.yml | 3 + config/routes.rb | 8 +- .../admin/custom_emojis_controller_spec.rb | 60 ------------ 11 files changed, 281 insertions(+), 176 deletions(-) create mode 100644 app/models/form/custom_emoji_batch.rb (limited to 'app/models') diff --git a/app/controllers/admin/custom_emojis_controller.rb b/app/controllers/admin/custom_emojis_controller.rb index f77699166..2af90f051 100644 --- a/app/controllers/admin/custom_emojis_controller.rb +++ b/app/controllers/admin/custom_emojis_controller.rb @@ -2,19 +2,20 @@ module Admin class CustomEmojisController < BaseController - before_action :set_custom_emoji, except: [:index, :new, :create] - before_action :set_filter_params - include ObfuscateFilename + obfuscate_filename [:custom_emoji, :image] def index authorize :custom_emoji, :index? + @custom_emojis = filtered_custom_emojis.eager_load(:local_counterpart).page(params[:page]) + @form = Form::CustomEmojiBatch.new end def new authorize :custom_emoji, :create? + @custom_emoji = CustomEmoji.new end @@ -31,69 +32,17 @@ module Admin end end - def update - authorize @custom_emoji, :update? - - if @custom_emoji.update(resource_params) - log_action :update, @custom_emoji - flash[:notice] = I18n.t('admin.custom_emojis.updated_msg') - else - flash[:alert] = I18n.t('admin.custom_emojis.update_failed_msg') - end - redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params) - end - - def destroy - authorize @custom_emoji, :destroy? - @custom_emoji.destroy! - log_action :destroy, @custom_emoji - flash[:notice] = I18n.t('admin.custom_emojis.destroyed_msg') - redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params) - end - - def copy - authorize @custom_emoji, :copy? - - emoji = CustomEmoji.find_or_initialize_by(domain: nil, - shortcode: @custom_emoji.shortcode) - emoji.image = @custom_emoji.image - - if emoji.save - log_action :create, emoji - flash[:notice] = I18n.t('admin.custom_emojis.copied_msg') - else - flash[:alert] = I18n.t('admin.custom_emojis.copy_failed_msg') - end - - redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params) - end - - def enable - authorize @custom_emoji, :enable? - @custom_emoji.update!(disabled: false) - log_action :enable, @custom_emoji - flash[:notice] = I18n.t('admin.custom_emojis.enabled_msg') - redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params) - end - - def disable - authorize @custom_emoji, :disable? - @custom_emoji.update!(disabled: true) - log_action :disable, @custom_emoji - flash[:notice] = I18n.t('admin.custom_emojis.disabled_msg') - redirect_to admin_custom_emojis_path(page: params[:page], **@filter_params) + def batch + @form = Form::CustomEmojiBatch.new(form_custom_emoji_batch_params.merge(current_account: current_account, action: action_from_button)) + @form.save + rescue ActionController::ParameterMissing + flash[:alert] = I18n.t('admin.accounts.no_account_selected') + ensure + redirect_to admin_custom_emojis_path(filter_params) end private - def set_custom_emoji - @custom_emoji = CustomEmoji.find(params[:id]) - end - - def set_filter_params - @filter_params = filter_params.to_hash.symbolize_keys - end - def resource_params params.require(:custom_emoji).permit(:shortcode, :image, :visible_in_picker) end @@ -103,12 +52,29 @@ module Admin end def filter_params - params.permit( - :local, - :remote, - :by_domain, - :shortcode - ) + params.slice(:local, :remote, :by_domain, :shortcode, :page).permit(:local, :remote, :by_domain, :shortcode, :page) + end + + def action_from_button + if params[:update] + 'update' + elsif params[:list] + 'list' + elsif params[:unlist] + 'unlist' + elsif params[:enable] + 'enable' + elsif params[:disable] + 'disable' + elsif params[:copy] + 'copy' + elsif params[:delete] + 'delete' + end + end + + def form_custom_emoji_batch_params + params.require(:form_custom_emoji_batch).permit(:action, :category_id, :category_name, custom_emoji_ids: []) end end end diff --git a/app/javascript/styles/mastodon/tables.scss b/app/javascript/styles/mastodon/tables.scss index 2aef099e6..d6403986f 100644 --- a/app/javascript/styles/mastodon/tables.scss +++ b/app/javascript/styles/mastodon/tables.scss @@ -180,6 +180,18 @@ a.table-action-link { } } + &__form { + padding: 16px; + border: 1px solid darken($ui-base-color, 8%); + border-top: 0; + background: $ui-base-color; + + .fields-row { + padding-top: 0; + margin-bottom: 0; + } + } + &__row { border: 1px solid darken($ui-base-color, 8%); border-top: 0; @@ -210,6 +222,35 @@ a.table-action-link { &--unpadded { padding: 0; } + + &--with-image { + display: flex; + align-items: center; + } + + &__image { + flex: 0 0 auto; + display: flex; + justify-content: center; + align-items: center; + margin-right: 10px; + + .emojione { + width: 32px; + height: 32px; + } + } + + &__text { + flex: 1 1 auto; + } + + &__extra { + flex: 0 0 auto; + text-align: right; + color: $darker-text-color; + font-weight: 500; + } } .directory__tag { diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index b21ad9042..0a4201a14 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -59,6 +59,12 @@ class CustomEmoji < ApplicationRecord :emoji end + def copy! + copy = self.class.find_or_initialize_by(domain: nil, shortcode: shortcode) + copy.image = image + copy.save! + end + class << self def from_text(text, domain) return [] if text.blank? diff --git a/app/models/custom_emoji_category.rb b/app/models/custom_emoji_category.rb index 7d8c0ee2d..3c87f2b2e 100644 --- a/app/models/custom_emoji_category.rb +++ b/app/models/custom_emoji_category.rb @@ -12,4 +12,6 @@ class CustomEmojiCategory < ApplicationRecord has_many :emojis, class_name: 'CustomEmoji', foreign_key: 'category_id', inverse_of: :category + + validates :name, presence: true, uniqueness: true end diff --git a/app/models/custom_emoji_filter.rb b/app/models/custom_emoji_filter.rb index 7649055d2..15b8da1d1 100644 --- a/app/models/custom_emoji_filter.rb +++ b/app/models/custom_emoji_filter.rb @@ -11,6 +11,8 @@ class CustomEmojiFilter scope = CustomEmoji.alphabetic params.each do |key, value| + next if key.to_s == 'page' + scope.merge!(scope_for(key, value)) if value.present? end @@ -22,13 +24,13 @@ class CustomEmojiFilter def scope_for(key, value) case key.to_s when 'local' - CustomEmoji.local + CustomEmoji.local.left_joins(:category).reorder(Arel.sql('custom_emoji_categories.name ASC NULLS FIRST, custom_emojis.shortcode ASC')) when 'remote' CustomEmoji.remote when 'by_domain' - CustomEmoji.where(domain: value.downcase) + CustomEmoji.where(domain: value.strip.downcase) when 'shortcode' - CustomEmoji.search(value) + CustomEmoji.search(value.strip) else raise "Unknown filter: #{key}" end diff --git a/app/models/form/custom_emoji_batch.rb b/app/models/form/custom_emoji_batch.rb new file mode 100644 index 000000000..076e8c9e3 --- /dev/null +++ b/app/models/form/custom_emoji_batch.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +class Form::CustomEmojiBatch + include ActiveModel::Model + include Authorization + include AccountableConcern + + attr_accessor :custom_emoji_ids, :action, :current_account, + :category_id, :category_name, :visible_in_picker + + def save + case action + when 'update' + update! + when 'list' + list! + when 'unlist' + unlist! + when 'enable' + enable! + when 'disable' + disable! + when 'copy' + copy! + when 'delete' + delete! + end + end + + private + + def custom_emojis + CustomEmoji.where(id: custom_emoji_ids) + end + + def update! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + + category = begin + if category_id.present? + CustomEmojiCategory.find(category_id) + elsif category_name.present? + CustomEmojiCategory.create!(name: category_name) + end + end + + custom_emojis.each do |custom_emoji| + custom_emoji.update(category_id: category&.id) + log_action :update, custom_emoji + end + end + + def list! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + + custom_emojis.each do |custom_emoji| + custom_emoji.update(visible_in_picker: true) + log_action :update, custom_emoji + end + end + + def unlist! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :update?) } + + custom_emojis.each do |custom_emoji| + custom_emoji.update(visible_in_picker: false) + log_action :update, custom_emoji + end + end + + def enable! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :enable?) } + + custom_emojis.each do |custom_emoji| + custom_emoji.update(disabled: false) + log_action :enable, custom_emoji + end + end + + def disable! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :disable?) } + + custom_emojis.each do |custom_emoji| + custom_emoji.update(disabled: true) + log_action :disable, custom_emoji + end + end + + def copy! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :copy?) } + + custom_emojis.each do |custom_emoji| + copied_custom_emoji = custom_emoji.copy! + log_action :create, copied_custom_emoji + end + end + + def delete! + custom_emojis.each { |custom_emoji| authorize(custom_emoji, :destroy?) } + + custom_emojis.each do |custom_emoji| + custom_emoji.destroy + log_action :destroy, custom_emoji + end + end +end diff --git a/app/views/admin/custom_emojis/_custom_emoji.html.haml b/app/views/admin/custom_emojis/_custom_emoji.html.haml index fbaa9a174..9e06a3b42 100644 --- a/app/views/admin/custom_emojis/_custom_emoji.html.haml +++ b/app/views/admin/custom_emojis/_custom_emoji.html.haml @@ -1,28 +1,31 @@ -%tr - %td - = custom_emoji_tag(custom_emoji) - %td - %samp= ":#{custom_emoji.shortcode}:" - %td - - if custom_emoji.local? - = t('admin.accounts.location.local') - - else - = link_to custom_emoji.domain, admin_custom_emojis_path(by_domain: custom_emoji.domain) - %td - - if custom_emoji.local? - - if custom_emoji.visible_in_picker - = table_link_to 'eye', t('admin.custom_emojis.listed'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: false }, page: params[:page], **@filter_params), method: :patch +.batch-table__row + %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox + = f.check_box :custom_emoji_ids, { multiple: true, include_hidden: false }, custom_emoji.id + .batch-table__row__content.batch-table__row__content--with-image + .batch-table__row__content__image + = custom_emoji_tag(custom_emoji) + + .batch-table__row__content__text + %samp= ":#{custom_emoji.shortcode}:" + + - if custom_emoji.local? + %span.account-role.bot= custom_emoji.category&.name || t('admin.custom_emojis.uncategorized') + + .batch-table__row__content__extra + - if custom_emoji.local? + = t('admin.accounts.location.local') - else - = table_link_to 'eye-slash', t('admin.custom_emojis.unlisted'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: true }, page: params[:page], **@filter_params), method: :patch - - else - - if custom_emoji.local_counterpart.present? - = link_to safe_join([custom_emoji_tag(custom_emoji.local_counterpart), t('admin.custom_emojis.overwrite')]), copy_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, class: 'table-action-link' + = custom_emoji.domain + + %br/ + + - if custom_emoji.disabled? + = t('admin.custom_emojis.disabled') - else - = table_link_to 'copy', t('admin.custom_emojis.copy'), copy_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post - %td - - if custom_emoji.disabled? - = table_link_to 'power-off', t('admin.custom_emojis.enable'), enable_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } - - else - = table_link_to 'power-off', t('admin.custom_emojis.disable'), disable_admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :post, data: { confirm: t('admin.accounts.are_you_sure') } - %td - = table_link_to 'times', t('admin.custom_emojis.delete'), admin_custom_emoji_path(custom_emoji, page: params[:page], **@filter_params), method: :delete, data: { confirm: t('admin.accounts.are_you_sure') } + = t('admin.custom_emojis.enabled') + - if custom_emoji.local? + • + - if custom_emoji.visible_in_picker? + = t('admin.custom_emojis.listed') + - else + = t('admin.custom_emojis.unlisted') diff --git a/app/views/admin/custom_emojis/index.html.haml b/app/views/admin/custom_emojis/index.html.haml index 3a119276c..7320ce1bb 100644 --- a/app/views/admin/custom_emojis/index.html.haml +++ b/app/views/admin/custom_emojis/index.html.haml @@ -1,6 +1,9 @@ - content_for :page_title do = t('admin.custom_emojis.title') +- content_for :header_tags do + = javascript_pack_tag 'admin', integrity: true, async: true, crossorigin: 'anonymous' + .filters .filter-subset %strong= t('admin.accounts.location.title') @@ -20,8 +23,7 @@ = form_tag admin_custom_emojis_url, method: 'GET', class: 'simple_form' do .fields-group - Admin::FilterHelper::CUSTOM_EMOJI_FILTERS.each do |key| - - if params[key].present? - = hidden_field_tag key, params[key] + = hidden_field_tag key, params[key] if params[key].present? - %i(shortcode by_domain).each do |key| .input.string.optional @@ -31,18 +33,54 @@ %button= t('admin.accounts.search') = link_to t('admin.accounts.reset'), admin_custom_emojis_path, class: 'button negative' -.table-wrapper - %table.table - %thead - %tr - %th= t('admin.custom_emojis.emoji') - %th= t('admin.custom_emojis.shortcode') - %th= t('admin.accounts.domain') - %th - %th - %th - %tbody - = render @custom_emojis += form_for(@form, url: batch_admin_custom_emojis_path) do |f| + = hidden_field_tag :page, params[:page] || 1 + + - Admin::FilterHelper::CUSTOM_EMOJI_FILTERS.each do |key| + = hidden_field_tag key, params[key] if params[key].present? + + .batch-table + .batch-table__toolbar + %label.batch-table__toolbar__select.batch-checkbox-all + = check_box_tag :batch_checkbox_all, nil, false + .batch-table__toolbar__actions + - if params[:local] == '1' + = f.button safe_join([fa_icon('save'), t('generic.save_changes')]), name: :update, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('eye'), t('admin.custom_emojis.list')]), name: :list, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('eye-slash'), t('admin.custom_emojis.unlist')]), name: :unlist, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('power-off'), t('admin.custom_emojis.enable')]), name: :enable, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('power-off'), t('admin.custom_emojis.disable')]), name: :disable, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + = f.button safe_join([fa_icon('times'), t('admin.custom_emojis.delete')]), name: :delete, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + - unless params[:local] == '1' + = f.button safe_join([fa_icon('copy'), t('admin.custom_emojis.copy')]), name: :copy, class: 'table-action-link', type: :submit, data: { confirm: t('admin.reports.are_you_sure') } + + - if params[:local] == '1' + .batch-table__form.simple_form + .fields-row + .fields-group.fields-row__column.fields-row__column-6 + .input.select.optional + .label_input + = f.select :category_id, options_from_collection_for_select(CustomEmojiCategory.all, 'id', 'name'), prompt: t('admin.custom_emojis.assign_category'), class: 'select optional', 'aria-label': t('admin.custom_emojis.assign_category') + + .fields-group.fields-row__column.fields-row__column-6 + .input.string.optional + .label_input + = f.text_field :category_name, class: 'string optional', placeholder: t('admin.custom_emojis.create_new_category'), 'aria-label': t('admin.custom_emojis.create_new_category') + + .batch-table__body + - if @custom_emojis.empty? + = nothing_here 'nothing-here--under-tabs' + - else + = render partial: 'custom_emoji', collection: @custom_emojis, locals: { f: f } = paginate @custom_emojis + +%hr.spacer/ + = link_to t('admin.custom_emojis.upload'), new_admin_custom_emoji_path, class: 'button' diff --git a/config/locales/en.yml b/config/locales/en.yml index 42d8e0eb8..52cb4a269 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -225,10 +225,12 @@ en: deleted_status: "(deleted status)" title: Audit log custom_emojis: + assign_category: Assign category by_domain: Domain copied_msg: Successfully created local copy of the emoji copy: Copy copy_failed_msg: Could not make a local copy of that emoji + create_new_category: Create new category created_msg: Emoji successfully created! delete: Delete destroyed_msg: Emojo successfully destroyed! @@ -245,6 +247,7 @@ en: shortcode: Shortcode shortcode_hint: At least 2 characters, only alphanumeric characters and underscores title: Custom emojis + uncategorized: Uncategorized unlisted: Unlisted update_failed_msg: Could not update that emoji updated_msg: Emoji successfully updated! diff --git a/config/routes.rb b/config/routes.rb index 534e68814..d22a9e56a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -242,11 +242,9 @@ Rails.application.routes.draw do resource :two_factor_authentication, only: [:destroy] end - resources :custom_emojis, only: [:index, :new, :create, :update, :destroy] do - member do - post :copy - post :enable - post :disable + resources :custom_emojis, only: [:index, :new, :create] do + collection do + post :batch end end diff --git a/spec/controllers/admin/custom_emojis_controller_spec.rb b/spec/controllers/admin/custom_emojis_controller_spec.rb index b7e2894e9..a8d96948c 100644 --- a/spec/controllers/admin/custom_emojis_controller_spec.rb +++ b/spec/controllers/admin/custom_emojis_controller_spec.rb @@ -52,64 +52,4 @@ describe Admin::CustomEmojisController do end end end - - describe 'PUT #update' do - let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test') } - let(:image) { fixture_file_upload(Rails.root.join('spec', 'fixtures', 'files', 'emojo.png'), 'image/png') } - - before do - put :update, params: { id: custom_emoji.id, custom_emoji: params } - end - - context 'when parameter is valid' do - let(:params) { { shortcode: 'updated', image: image } } - - it 'succeeds in updating custom emoji' do - expect(flash[:notice]).to eq I18n.t('admin.custom_emojis.updated_msg') - expect(custom_emoji.reload).to have_attributes(shortcode: 'updated') - end - end - - context 'when parameter is invalid' do - let(:params) { { shortcode: 'u', image: image } } - - it 'fails to update custom emoji' do - expect(flash[:alert]).to eq I18n.t('admin.custom_emojis.update_failed_msg') - expect(custom_emoji.reload).to have_attributes(shortcode: 'test') - end - end - end - - describe 'POST #copy' do - subject { post :copy, params: { id: custom_emoji.id } } - - let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test') } - - it 'copies custom emoji' do - expect { subject }.to change { CustomEmoji.where(shortcode: 'test').count }.by(1) - expect(flash[:notice]).to eq I18n.t('admin.custom_emojis.copied_msg') - end - end - - describe 'POST #enable' do - let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test', disabled: true) } - - before { post :enable, params: { id: custom_emoji.id } } - - it 'enables custom emoji' do - expect(response).to redirect_to admin_custom_emojis_path - expect(custom_emoji.reload).to have_attributes(disabled: false) - end - end - - describe 'POST #disable' do - let(:custom_emoji) { Fabricate(:custom_emoji, shortcode: 'test', disabled: false) } - - before { post :disable, params: { id: custom_emoji.id } } - - it 'enables custom emoji' do - expect(response).to redirect_to admin_custom_emojis_path - expect(custom_emoji.reload).to have_attributes(disabled: true) - end - end end -- cgit From 86748148256b504c0411119628435b1445959309 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 10 Sep 2019 13:48:48 +0200 Subject: Change tootctl to use inline parallelization instead of Sidekiq (#11776) - Remove --background option - Add --concurrency(=5) option - Add progress bars --- Gemfile | 1 + Gemfile.lock | 1 + app/models/media_attachment.rb | 1 + app/models/preview_card.rb | 2 + app/workers/maintenance/destroy_media_worker.rb | 14 -- .../maintenance/redownload_account_media_worker.rb | 16 -- app/workers/maintenance/uncache_media_worker.rb | 18 -- app/workers/maintenance/uncache_preview_worker.rb | 18 -- lib/mastodon/accounts_cli.rb | 192 +++++++++------------ lib/mastodon/cache_cli.rb | 16 +- lib/mastodon/cli_helper.rb | 49 ++++++ lib/mastodon/domains_cli.rb | 27 ++- lib/mastodon/feeds_cli.rb | 42 +---- lib/mastodon/media_cli.rb | 57 ++---- lib/mastodon/preview_cards_cli.rb | 83 +++------ 15 files changed, 200 insertions(+), 337 deletions(-) delete mode 100644 app/workers/maintenance/destroy_media_worker.rb delete mode 100644 app/workers/maintenance/redownload_account_media_worker.rb delete mode 100644 app/workers/maintenance/uncache_media_worker.rb delete mode 100644 app/workers/maintenance/uncache_preview_worker.rb (limited to 'app/models') diff --git a/Gemfile b/Gemfile index 73edb2a6a..af0e8e2fc 100644 --- a/Gemfile +++ b/Gemfile @@ -77,6 +77,7 @@ gem 'rails-settings-cached', '~> 0.6' gem 'redis', '~> 4.1', require: ['redis', 'redis/connection/hiredis'] gem 'mario-redis-lock', '~> 1.2', require: 'redis_lock' gem 'rqrcode', '~> 0.10' +gem 'ruby-progressbar', '~> 1.10' gem 'sanitize', '~> 5.1' gem 'sidekiq', '~> 5.2' gem 'sidekiq-scheduler', '~> 3.0' diff --git a/Gemfile.lock b/Gemfile.lock index 125cca4ba..6e931c611 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -769,6 +769,7 @@ DEPENDENCIES rspec-sidekiq (~> 3.0) rubocop (~> 0.74) rubocop-rails (~> 2.3) + ruby-progressbar (~> 1.10) sanitize (~> 5.1) sidekiq (~> 5.2) sidekiq-bulk (~> 0.2.0) diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 44f8e6be6..b58025015 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -129,6 +129,7 @@ class MediaAttachment < ApplicationRecord scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) } scope :local, -> { where(remote_url: '') } scope :remote, -> { where.not(remote_url: '') } + scope :cached, -> { remote.where.not(file_file_name: nil) } default_scope { order(id: :asc) } diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index a792b352b..9d6c1938a 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -43,6 +43,8 @@ class PreviewCard < ApplicationRecord validates_attachment_size :image, less_than: LIMIT remotable_attachment :image, LIMIT + scope :cached, -> { where.not(image_file_name: [nil, '']) } + before_save :extract_dimensions, if: :link? def save_with_optional_image! diff --git a/app/workers/maintenance/destroy_media_worker.rb b/app/workers/maintenance/destroy_media_worker.rb deleted file mode 100644 index cde33d6d7..000000000 --- a/app/workers/maintenance/destroy_media_worker.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -class Maintenance::DestroyMediaWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(media_attachment_id) - media = media_attachment_id.is_a?(MediaAttachment) ? media_attachment_id : MediaAttachment.find(media_attachment_id) - media.destroy - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/app/workers/maintenance/redownload_account_media_worker.rb b/app/workers/maintenance/redownload_account_media_worker.rb deleted file mode 100644 index 6afbe6e19..000000000 --- a/app/workers/maintenance/redownload_account_media_worker.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -class Maintenance::RedownloadAccountMediaWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull', retry: false - - def perform(account_id) - account = account_id.is_a?(Account) ? account_id : Account.find(account_id) - account.reset_avatar! - account.reset_header! - account.save - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/app/workers/maintenance/uncache_media_worker.rb b/app/workers/maintenance/uncache_media_worker.rb deleted file mode 100644 index 4bc62ef75..000000000 --- a/app/workers/maintenance/uncache_media_worker.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class Maintenance::UncacheMediaWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(media_attachment_id) - media = media_attachment_id.is_a?(MediaAttachment) ? media_attachment_id : MediaAttachment.find(media_attachment_id) - - return if media.file.blank? - - media.file.destroy - media.save - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/app/workers/maintenance/uncache_preview_worker.rb b/app/workers/maintenance/uncache_preview_worker.rb deleted file mode 100644 index 810ffd8cc..000000000 --- a/app/workers/maintenance/uncache_preview_worker.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class Maintenance::UncachePreviewWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(preview_card_id) - preview_card = PreviewCard.find(preview_card_id) - - return if preview_card.image.blank? - - preview_card.image.destroy - preview_card.save - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index d1854acc0..b16bf2e38 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -7,6 +7,8 @@ require_relative 'cli_helper' module Mastodon class AccountsCLI < Thor + include CLIHelper + def self.exit_on_failure? true end @@ -26,18 +28,20 @@ module Mastodon if options[:all] processed = 0 delay = 0 + scope = Account.local.without_suspended + progress = create_progress_bar(scope.count) - Account.local.without_suspended.find_in_batches do |accounts| + scope.find_in_batches do |accounts| accounts.each do |account| rotate_keys_for_account(account, delay) + progress.increment processed += 1 - say('.', :green, false) end delay += 5.minutes end - say + progress.finish say("OK, rotated keys for #{processed} accounts", :green) elsif username.present? rotate_keys_for_account(Account.find_local(username)) @@ -206,6 +210,8 @@ module Mastodon say('OK', :green) end + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] option :dry_run, type: :boolean desc 'cull', 'Remove remote accounts that no longer exist' long_desc <<-LONG_DESC @@ -215,63 +221,45 @@ module Mastodon Accounts that have had confirmed activity within the last week are excluded from the checks. - - Domains that are unreachable are not checked. - - With the --dry-run option, no deletes will actually be carried - out. LONG_DESC def cull skip_threshold = 7.days.ago - culled = 0 - dry_run_culled = [] - skip_domains = Set.new dry_run = options[:dry_run] ? ' (DRY RUN)' : '' + skip_domains = Concurrent::Set.new - Account.remote.where(protocol: :activitypub).partitioned.find_each do |account| - next if account.updated_at >= skip_threshold || (account.last_webfingered_at.present? && account.last_webfingered_at >= skip_threshold) + processed, culled = parallelize_with_progress(Account.remote.where(protocol: :activitypub).partitioned) do |account| + next if account.updated_at >= skip_threshold || (account.last_webfingered_at.present? && account.last_webfingered_at >= skip_threshold) || skip_domains.include?(account.domain) code = 0 - unless skip_domains.include?(account.domain) - begin - code = Request.new(:head, account.uri).perform(&:code) - rescue HTTP::ConnectionError - skip_domains << account.domain - rescue StandardError - next - end + + begin + code = Request.new(:head, account.uri).perform(&:code) + rescue HTTP::ConnectionError + skip_domains << account.domain end if [404, 410].include?(code) - if options[:dry_run] - dry_run_culled << account.acct - else - SuspendAccountService.new.call(account, destroy: true) - end - culled += 1 - say('+', :green, false) + SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run] + 1 else - account.touch # Touch account even during dry run to avoid getting the account into the window again - say('.', nil, false) + # Touch account even during dry run to avoid getting the account into the window again + account.touch end end - say - say("Removed #{culled} accounts. #{skip_domains.size} servers skipped#{dry_run}", skip_domains.empty? ? :green : :yellow) + say("Visited #{processed} accounts, removed #{culled}#{dry_run}", :green) unless skip_domains.empty? - say('The following servers were not available during the check:', :yellow) + say('The following domains were not available during the check:', :yellow) skip_domains.each { |domain| say(' ' + domain) } end - - unless dry_run_culled.empty? - say('The following accounts would have been deleted:', :green) - dry_run_culled.each { |account| say(' ' + account) } - end end option :all, type: :boolean option :domain + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] + option :dry_run, type: :boolean desc 'refresh [USERNAME]', 'Fetch remote user data and files' long_desc <<-LONG_DESC Fetch remote user data and files for one or multiple accounts. @@ -280,21 +268,23 @@ module Mastodon Through the --domain option, this can be narrowed down to a specific domain only. Otherwise, a single remote account must be specified with USERNAME. - - All processing is done in the background through Sidekiq. LONG_DESC def refresh(username = nil) + dry_run = options[:dry_run] ? ' (DRY RUN)' : '' + if options[:domain] || options[:all] - queued = 0 scope = Account.remote scope = scope.where(domain: options[:domain]) if options[:domain] - scope.select(:id).reorder(nil).find_in_batches do |accounts| - Maintenance::RedownloadAccountMediaWorker.push_bulk(accounts.map(&:id)) - queued += accounts.size + processed, = parallelize_with_progress(scope) do |account| + next if options[:dry_run] + + account.reset_avatar! + account.reset_header! + account.save end - say("Scheduled refreshment of #{queued} accounts", :green, true) + say("Refreshed #{processed} accounts#{dry_run}", :green, true) elsif username.present? username, domain = username.split('@') account = Account.find_remote(username, domain) @@ -304,76 +294,53 @@ module Mastodon exit(1) end - Maintenance::RedownloadAccountMediaWorker.perform_async(account.id) - say('OK', :green) + unless options[:dry_run] + account.reset_avatar! + account.reset_header! + account.save + end + + say("OK#{dry_run}", :green) else say('No account(s) given', :red) exit(1) end end - desc 'follow ACCT', 'Make all local accounts follow account specified by ACCT' - long_desc <<-LONG_DESC - Make all local accounts follow another local account specified by ACCT. - ACCT should be the username only. - LONG_DESC - def follow(acct) - if acct.include? '@' - say('Target account name should not contain a target instance, since it has to be a local account.', :red) - exit(1) - end - - target_account = ResolveAccountService.new.call(acct) - processed = 0 - failed = 0 + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] + desc 'follow USERNAME', 'Make all local accounts follow account specified by USERNAME' + def follow(username) + target_account = Account.find_local(username) if target_account.nil? - say("Target account (#{acct}) could not be resolved", :red) + say('No such account', :red) exit(1) end - Account.local.without_suspended.find_each do |account| - begin - FollowService.new.call(account, target_account) - processed += 1 - say('.', :green, false) - rescue StandardError - failed += 1 - say('.', :red, false) - end + processed, = parallelize_with_progress(Account.local.without_suspended) do |account| + FollowService.new.call(account, target_account) end - say("OK, followed target from #{processed} accounts, skipped #{failed}", :green) + say("OK, followed target from #{processed} accounts", :green) end + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] desc 'unfollow ACCT', 'Make all local accounts unfollow account specified by ACCT' - long_desc <<-LONG_DESC - Make all local accounts unfollow an account specified by ACCT. ACCT can be - a simple username, in case of a local user. It can also be in the format - username@domain, in case of a remote user. - LONG_DESC def unfollow(acct) target_account = Account.find_remote(*acct.split('@')) - processed = 0 - failed = 0 if target_account.nil? - say("Target account (#{acct}) was not found", :red) + say('No such account', :red) exit(1) end - target_account.followers.local.find_each do |account| - begin - UnfollowService.new.call(account, target_account) - processed += 1 - say('.', :green, false) - rescue StandardError - failed += 1 - say('.', :red, false) - end + parallelize_with_progress(target_account.followers.local) do |account| + UnfollowService.new.call(account, target_account) end - say("OK, unfollowed target from #{processed} accounts, skipped #{failed}", :green) + say("OK, unfollowed target from #{processed} accounts", :green) end option :follows, type: :boolean, default: false @@ -396,51 +363,50 @@ module Mastodon account = Account.find_local(username) if account.nil? - say('No user with such username', :red) + say('No such account', :red) exit(1) end - if options[:follows] - processed = 0 - failed = 0 + total = 0 + total += Account.where(id: ::Follow.where(account: account).select(:target_account_id)).count if options[:follows] + total += Account.where(id: ::Follow.where(target_account: account).select(:account_id)).count if options[:followers] + progress = create_progress_bar(total) + processed = 0 - say("Unfollowing #{account.username}'s followees, this might take a while...") + if options[:follows] + scope = Account.where(id: ::Follow.where(account: account).select(:target_account_id)) - Account.where(id: ::Follow.where(account: account).select(:target_account_id)).find_each do |target_account| + scope.find_each do |target_account| begin UnfollowService.new.call(account, target_account) + rescue => e + progress.log pastel.red("Error processing #{target_account.id}: #{e}") + ensure + progress.increment processed += 1 - say('.', :green, false) - rescue StandardError - failed += 1 - say('.', :red, false) end end BootstrapTimelineWorker.perform_async(account.id) - - say("OK, unfollowed #{processed} followees, skipped #{failed}", :green) end if options[:followers] - processed = 0 - failed = 0 - - say("Removing #{account.username}'s followers, this might take a while...") + scope = Account.where(id: ::Follow.where(target_account: account).select(:account_id)) - Account.where(id: ::Follow.where(target_account: account).select(:account_id)).find_each do |target_account| + scope.find_each do |target_account| begin UnfollowService.new.call(target_account, account) + rescue => e + progress.log pastel.red("Error processing #{target_account.id}: #{e}") + ensure + progress.increment processed += 1 - say('.', :green, false) - rescue StandardError - failed += 1 - say('.', :red, false) end end - - say("OK, removed #{processed} followers, skipped #{failed}", :green) end + + progress.finish + say("Processed #{processed} relationships", :green, true) end option :number, type: :numeric, aliases: [:n] diff --git a/lib/mastodon/cache_cli.rb b/lib/mastodon/cache_cli.rb index 5b0eea91b..803404c34 100644 --- a/lib/mastodon/cache_cli.rb +++ b/lib/mastodon/cache_cli.rb @@ -6,6 +6,8 @@ require_relative 'cli_helper' module Mastodon class CacheCLI < Thor + include CLIHelper + def self.exit_on_failure? true end @@ -16,6 +18,8 @@ module Mastodon say('OK', :green) end + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] desc 'recount TYPE', 'Update hard-cached counters' long_desc <<~LONG_DESC Update hard-cached counters of TYPE by counting referenced @@ -25,32 +29,24 @@ module Mastodon size of the database. LONG_DESC def recount(type) - processed = 0 - case type when 'accounts' - Account.local.includes(:account_stat).find_each do |account| + processed, = parallelize_with_progress(Account.local.includes(:account_stat)) do |account| account_stat = account.account_stat account_stat.following_count = account.active_relationships.count account_stat.followers_count = account.passive_relationships.count account_stat.statuses_count = account.statuses.where.not(visibility: :direct).count account_stat.save if account_stat.changed? - - processed += 1 - say('.', :green, false) end when 'statuses' - Status.includes(:status_stat).find_each do |status| + processed, = parallelize_with_progress(Status.includes(:status_stat)) do |status| status_stat = status.status_stat status_stat.replies_count = status.replies.where.not(visibility: :direct).count status_stat.reblogs_count = status.reblogs.count status_stat.favourites_count = status.favourites.count status_stat.save if status_stat.changed? - - processed += 1 - say('.', :green, false) end else say("Unknown type: #{type}", :red) diff --git a/lib/mastodon/cli_helper.rb b/lib/mastodon/cli_helper.rb index 2f807d08c..da7348349 100644 --- a/lib/mastodon/cli_helper.rb +++ b/lib/mastodon/cli_helper.rb @@ -7,3 +7,52 @@ ActiveRecord::Base.logger = dev_null ActiveJob::Base.logger = dev_null HttpLog.configuration.logger = dev_null Paperclip.options[:log] = false + +module Mastodon + module CLIHelper + def create_progress_bar(total = nil) + ProgressBar.create(total: total, format: '%c/%u |%b%i| %e') + end + + def parallelize_with_progress(scope) + ActiveRecord::Base.configurations[Rails.env]['pool'] = options[:concurrency] + + progress = create_progress_bar(scope.count) + pool = Concurrent::FixedThreadPool.new(options[:concurrency]) + total = Concurrent::AtomicFixnum.new(0) + aggregate = Concurrent::AtomicFixnum.new(0) + + scope.reorder(nil).find_in_batches do |items| + futures = [] + + items.each do |item| + futures << Concurrent::Future.execute(executor: pool) do + ActiveRecord::Base.connection_pool.with_connection do + begin + progress.log("Processing #{item.id}") if options[:verbose] + + result = yield(item) + aggregate.increment(result) if result.is_a?(Integer) + rescue => e + progress.log pastel.red("Error processing #{item.id}: #{e}") + ensure + progress.increment + end + end + end + end + + total.increment(items.size) + futures.map(&:value) + end + + progress.finish + + [total.value, aggregate.value] + end + + def pastel + @pastel ||= Pastel.new + end + end +end diff --git a/lib/mastodon/domains_cli.rb b/lib/mastodon/domains_cli.rb index 17cafd1bc..c612c2d72 100644 --- a/lib/mastodon/domains_cli.rb +++ b/lib/mastodon/domains_cli.rb @@ -7,10 +7,14 @@ require_relative 'cli_helper' module Mastodon class DomainsCLI < Thor + include CLIHelper + def self.exit_on_failure? true end + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] option :dry_run, type: :boolean option :whitelist_mode, type: :boolean desc 'purge [DOMAIN]', 'Remove accounts from a DOMAIN without a trace' @@ -24,7 +28,6 @@ module Mastodon are removed from the database. LONG_DESC def purge(domain = nil) - removed = 0 dry_run = options[:dry_run] ? ' (DRY RUN)' : '' scope = begin @@ -38,25 +41,22 @@ module Mastodon end end - scope.find_each do |account| + processed, = parallelize_with_progress(scope) do |account| SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run] - removed += 1 - say('.', :green, false) end DomainBlock.where(domain: domain).destroy_all unless options[:dry_run] - say - say("Removed #{removed} accounts#{dry_run}", :green) + say("Removed #{processed} accounts#{dry_run}", :green) custom_emojis = CustomEmoji.where(domain: domain) custom_emojis_count = custom_emojis.count custom_emojis.destroy_all unless options[:dry_run] + say("Removed #{custom_emojis_count} custom emojis", :green) end option :concurrency, type: :numeric, default: 50, aliases: [:c] - option :silent, type: :boolean, default: false, aliases: [:s] option :format, type: :string, default: 'summary', aliases: [:f] option :exclude_suspended, type: :boolean, default: false, aliases: [:x] desc 'crawl [START]', 'Crawl all known peers, optionally beginning at START' @@ -69,8 +69,6 @@ module Mastodon The --concurrency (-c) option controls the number of threads performing HTTP requests at the same time. More threads means the crawl may complete faster. - The --silent (-s) option controls progress output. - The --format (-f) option controls how the data is displayed at the end. By default (`summary`), a summary of the statistics is returned. The other options are `domains`, which returns a newline-delimited list of all discovered peers, @@ -87,6 +85,7 @@ module Mastodon start_at = Time.now.to_f seed = start ? [start] : Account.remote.domains blocked_domains = Regexp.new('\\.?' + DomainBlock.where(severity: 1).pluck(:domain).join('|') + '$') + progress = create_progress_bar pool = Concurrent::ThreadPoolExecutor.new(min_threads: 0, max_threads: options[:concurrency], idletime: 10, auto_terminate: true, max_queue: 0) @@ -95,7 +94,6 @@ module Mastodon next if options[:exclude_suspended] && domain.match(blocked_domains) stats[domain] = nil - processed.increment begin Request.new(:get, "https://#{domain}/api/v1/instance").perform do |res| @@ -115,11 +113,11 @@ module Mastodon next unless res.code == 200 stats[domain]['activity'] = Oj.load(res.to_s) end - - say('.', :green, false) unless options[:silent] rescue StandardError failed.increment - say('.', :red, false) unless options[:silent] + ensure + processed.increment + progress.increment unless progress.finished? end end @@ -133,10 +131,9 @@ module Mastodon pool.shutdown pool.wait_for_termination(20) ensure + progress.finish pool.shutdown - say unless options[:silent] - case options[:format] when 'summary' stats_to_summary(stats, processed, failed, start_at) diff --git a/lib/mastodon/feeds_cli.rb b/lib/mastodon/feeds_cli.rb index fe11c3df4..ea7c90dff 100644 --- a/lib/mastodon/feeds_cli.rb +++ b/lib/mastodon/feeds_cli.rb @@ -6,55 +6,33 @@ require_relative 'cli_helper' module Mastodon class FeedsCLI < Thor + include CLIHelper + def self.exit_on_failure? true end option :all, type: :boolean, default: false - option :background, type: :boolean, default: false + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] option :dry_run, type: :boolean, default: false - option :verbose, type: :boolean, default: false desc 'build [USERNAME]', 'Build home and list feeds for one or all users' long_desc <<-LONG_DESC Build home and list feeds that are stored in Redis from the database. With the --all option, all active users will be processed. Otherwise, a single user specified by USERNAME. - - With the --background option, regeneration will be queued into Sidekiq, - and the command will exit as soon as possible. - - With the --dry-run option, no work will be done. - - With the --verbose option, when accounts are processed sequentially in the - foreground, the IDs of the accounts will be printed. LONG_DESC def build(username = nil) dry_run = options[:dry_run] ? '(DRY RUN)' : '' if options[:all] || username.nil? - processed = 0 - queued = 0 - User.active.select(:id, :account_id).reorder(nil).find_in_batches do |users| - if options[:background] - RegenerationWorker.push_bulk(users.map(&:account_id)) unless options[:dry_run] - queued += users.size - else - users.each do |user| - RegenerationWorker.new.perform(user.account_id) unless options[:dry_run] - options[:verbose] ? say(user.account_id) : say('.', :green, false) - processed += 1 - end - end + processed, = parallelize_with_progress(Account.joins(:user).merge(User.active)) do |account| + PrecomputeFeedService.new.call(account) unless options[:dry_run] end - if options[:background] - say("Scheduled feed regeneration for #{queued} accounts #{dry_run}", :green, true) - else - say - say("Regenerated feeds for #{processed} accounts #{dry_run}", :green, true) - end + say("Regenerated feeds for #{processed} accounts #{dry_run}", :green, true) elsif username.present? account = Account.find_local(username) @@ -63,11 +41,7 @@ module Mastodon exit(1) end - if options[:background] - RegenerationWorker.perform_async(account.id) unless options[:dry_run] - else - RegenerationWorker.new.perform(account.id) unless options[:dry_run] - end + PrecomputeFeedService.new.call(account) unless options[:dry_run] say("OK #{dry_run}", :green, true) else diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index 6152d5a09..0659b6b65 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -7,14 +7,15 @@ require_relative 'cli_helper' module Mastodon class MediaCLI < Thor include ActionView::Helpers::NumberHelper + include CLIHelper def self.exit_on_failure? true end - option :days, type: :numeric, default: 7 - option :background, type: :boolean, default: false - option :verbose, type: :boolean, default: false + option :days, type: :numeric, default: 7, aliases: [:d] + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, default: false, aliases: [:v] option :dry_run, type: :boolean, default: false desc 'remove', 'Remove remote media files' long_desc <<-DESC @@ -22,49 +23,25 @@ module Mastodon The --days option specifies how old media attachments have to be before they are removed. It defaults to 7 days. - - With the --background option, instead of deleting the files sequentially, - they will be queued into Sidekiq and the command will exit as soon as - possible. In Sidekiq they will be processed with higher concurrency, but - it may impact other operations of the Mastodon server, and it may overload - the underlying file storage. - - With the --dry-run option, no work will be done. - - With the --verbose option, when media attachments are processed sequentially in the - foreground, the IDs of the media attachments will be printed. DESC def remove - time_ago = options[:days].days.ago - queued = 0 - processed = 0 - size = 0 - dry_run = options[:dry_run] ? '(DRY RUN)' : '' + time_ago = options[:days].days.ago + dry_run = options[:dry_run] ? '(DRY RUN)' : '' - if options[:background] - MediaAttachment.where.not(remote_url: '').where.not(file_file_name: nil).where('created_at < ?', time_ago).select(:id, :file_file_size).reorder(nil).find_in_batches do |media_attachments| - queued += media_attachments.size - size += media_attachments.reduce(0) { |sum, m| sum + (m.file_file_size || 0) } - Maintenance::UncacheMediaWorker.push_bulk(media_attachments.map(&:id)) unless options[:dry_run] - end - else - MediaAttachment.where.not(remote_url: '').where.not(file_file_name: nil).where('created_at < ?', time_ago).reorder(nil).find_in_batches do |media_attachments| - media_attachments.each do |m| - size += m.file_file_size || 0 - Maintenance::UncacheMediaWorker.new.perform(m) unless options[:dry_run] - options[:verbose] ? say(m.id) : say('.', :green, false) - processed += 1 - end - end - end + processed, aggregate = parallelize_with_progress(MediaAttachment.cached.where.not(remote_url: '').where('created_at < ?', time_ago)) do |media_attachment| + next if media_attachment.file.blank? + + size = media_attachment.file_file_size - say + unless options[:dry_run] + media_attachment.file.destroy + media_attachment.save + end - if options[:background] - say("Scheduled the deletion of #{queued} media attachments (approx. #{number_to_human_size(size)}) #{dry_run}", :green, true) - else - say("Removed #{processed} media attachments (approx. #{number_to_human_size(size)}) #{dry_run}", :green, true) + size end + + say("Removed #{processed} media attachments (approx. #{number_to_human_size(aggregate)}) #{dry_run}", :green, true) end end end diff --git a/lib/mastodon/preview_cards_cli.rb b/lib/mastodon/preview_cards_cli.rb index 465fe7d0b..cf4407250 100644 --- a/lib/mastodon/preview_cards_cli.rb +++ b/lib/mastodon/preview_cards_cli.rb @@ -8,87 +8,52 @@ require_relative 'cli_helper' module Mastodon class PreviewCardsCLI < Thor include ActionView::Helpers::NumberHelper + include CLIHelper def self.exit_on_failure? true end option :days, type: :numeric, default: 180 - option :background, type: :boolean, default: false - option :verbose, type: :boolean, default: false + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, aliases: [:v] option :dry_run, type: :boolean, default: false option :link, type: :boolean, default: false desc 'remove', 'Remove preview cards' long_desc <<-DESC - Removes locally thumbnails for previews. + Removes local thumbnails for preview cards. The --days option specifies how old preview cards have to be before - they are removed. It defaults to 180 days. + they are removed. It defaults to 180 days. Since preview cards will + not be re-fetched unless the link is re-posted after 2 weeks from + last time, it is not recommended to delete preview cards within the + last 14 days. - With the --background option, instead of deleting the files sequentially, - they will be queued into Sidekiq and the command will exit as soon as - possible. In Sidekiq they will be processed with higher concurrency, but - it may impact other operations of the Mastodon server, and it may overload - the underlying file storage. - - With the --dry-run option, no work will be done. - - With the --verbose option, when preview cards are processed sequentially in the - foreground, the IDs of the preview cards will be printed. - - With the --link option, delete only link-type preview cards. + With the --link option, only link-type preview cards will be deleted, + leaving video and photo cards untouched. DESC def remove - prompt = TTY::Prompt.new - time_ago = options[:days].days.ago - queued = 0 - processed = 0 - size = 0 - dry_run = options[:dry_run] ? '(DRY RUN)' : '' - link = options[:link] ? 'link-type ' : '' - scope = PreviewCard.where.not(image_file_name: nil) - scope = scope.where.not(image_file_name: '') - scope = scope.where(type: :link) if options[:link] - scope = scope.where('updated_at < ?', time_ago) + time_ago = options[:days].days.ago + dry_run = options[:dry_run] ? ' (DRY RUN)' : '' + link = options[:link] ? 'link-type ' : '' + scope = PreviewCard.cached + scope = scope.where(type: :link) if options[:link] + scope = scope.where('updated_at < ?', time_ago) - if time_ago > 2.weeks.ago - prompt.say "\n" - prompt.say('The preview cards less than the past two weeks will not be re-acquired even when needed.') - prompt.say "\n" + processed, aggregate = parallelize_with_progress(scope) do |preview_card| + next if preview_card.image.blank? - unless prompt.yes?('Are you sure you want to delete the preview cards?', default: false) - prompt.say "\n" - prompt.warn 'Nothing execute. Bye!' - prompt.say "\n" - exit(1) - end - end + size = preview_card.image_file_size - if options[:background] - scope.select(:id, :image_file_size).reorder(nil).find_in_batches do |preview_cards| - queued += preview_cards.size - size += preview_cards.reduce(0) { |sum, p| sum + (p.image_file_size || 0) } - Maintenance::UncachePreviewWorker.push_bulk(preview_cards.map(&:id)) unless options[:dry_run] + unless options[:dry_run] + preview_card.image.destroy + preview_card.save end - else - scope.select(:id, :image_file_size).reorder(nil).find_in_batches do |preview_cards| - preview_cards.each do |p| - size += p.image_file_size || 0 - Maintenance::UncachePreviewWorker.new.perform(p.id) unless options[:dry_run] - options[:verbose] ? say(p.id) : say('.', :green, false) - processed += 1 - end - end + size end - say - - if options[:background] - say("Scheduled the deletion of #{queued} #{link}preview cards (approx. #{number_to_human_size(size)}) #{dry_run}", :green, true) - else - say("Removed #{processed} #{link}preview cards (approx. #{number_to_human_size(size)}) #{dry_run}", :green, true) - end + say("Removed #{processed} #{link}preview cards (approx. #{number_to_human_size(aggregate)})#{dry_run}", :green, true) end end end -- cgit From 031ca25014e0ba88d3dcc3086947b41449a672e2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 10 Sep 2019 15:29:12 +0200 Subject: Add retry for failed media downloads and `tootctl media refresh` (#11775) --- app/lib/activitypub/activity/create.rb | 21 +++++++------ app/models/concerns/remotable.rb | 12 ++++---- app/models/media_attachment.rb | 2 +- app/workers/redownload_media_worker.rb | 19 ++++++++++++ lib/mastodon/media_cli.rb | 54 ++++++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 16 deletions(-) create mode 100644 app/workers/redownload_media_worker.rb (limited to 'app/models') diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 000b77df5..dea7fd43c 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -189,22 +189,25 @@ class ActivityPub::Activity::Create < ActivityPub::Activity media_attachments = [] as_array(@object['attachment']).each do |attachment| - next if attachment['url'].blank? + next if attachment['url'].blank? || media_attachments.size >= 4 - href = Addressable::URI.parse(attachment['url']).normalize.to_s - media_attachment = MediaAttachment.create(account: @account, remote_url: href, description: attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil) - media_attachments << media_attachment + begin + href = Addressable::URI.parse(attachment['url']).normalize.to_s + media_attachment = MediaAttachment.create(account: @account, remote_url: href, description: attachment['name'].presence, focus: attachment['focalPoint'], blurhash: supported_blurhash?(attachment['blurhash']) ? attachment['blurhash'] : nil) + media_attachments << media_attachment - next if unsupported_media_type?(attachment['mediaType']) || skip_download? + next if unsupported_media_type?(attachment['mediaType']) || skip_download? - media_attachment.file_remote_url = href - media_attachment.save + media_attachment.file_remote_url = href + media_attachment.save + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError + RedownloadMediaWorker.perform_in(rand(30..600).seconds, media_attachment.id) + end end media_attachments rescue Addressable::URI::InvalidURIError => e - Rails.logger.debug e - + Rails.logger.debug "Invalid URL in attachment: #{e}" media_attachments end diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb index 9372a963b..082302619 100644 --- a/app/models/concerns/remotable.rb +++ b/app/models/concerns/remotable.rb @@ -4,7 +4,7 @@ module Remotable extend ActiveSupport::Concern class_methods do - def remotable_attachment(attachment_name, limit) + def remotable_attachment(attachment_name, limit, suppress_errors: true) attribute_name = "#{attachment_name}_remote_url".to_sym method_name = "#{attribute_name}=".to_sym alt_method_name = "reset_#{attachment_name}!".to_sym @@ -22,7 +22,7 @@ module Remotable begin Request.new(:get, url).perform do |response| - next if response.code != 200 + raise Mastodon::UnexpectedResponseError, response unless (200...300).cover?(response.code) content_type = parse_content_type(response.headers.get('content-type').last) extname = detect_extname_from_content_type(content_type) @@ -41,11 +41,11 @@ module Remotable self[attribute_name] = url if has_attribute?(attribute_name) end - rescue HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError => e + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError => e + Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" + raise e unless suppress_errors + rescue Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Paperclip::Error, Mastodon::DimensionsValidationError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" - nil - rescue Paperclip::Error, Mastodon::DimensionsValidationError => e - Rails.logger.debug "Error processing remote #{attachment_name}: #{e}" nil end end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index b58025015..a2b73f150 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -118,7 +118,7 @@ class MediaAttachment < ApplicationRecord validates_attachment_content_type :file, content_type: IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES validates_attachment_size :file, less_than: IMAGE_LIMIT, unless: :larger_media_format? validates_attachment_size :file, less_than: VIDEO_LIMIT, if: :larger_media_format? - remotable_attachment :file, VIDEO_LIMIT + remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false include Attachmentable diff --git a/app/workers/redownload_media_worker.rb b/app/workers/redownload_media_worker.rb new file mode 100644 index 000000000..98e995918 --- /dev/null +++ b/app/workers/redownload_media_worker.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class RedownloadMediaWorker + include Sidekiq::Worker + include ExponentialBackoff + + sidekiq_options queue: 'pull', retry: 3 + + def perform(id) + media_attachment = MediaAttachment.find(id) + + return if media_attachment.remote_url.blank? + + media_attachment.reset_file! + media_attachment.save + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index 0659b6b65..ec2f36c30 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -43,5 +43,59 @@ module Mastodon say("Removed #{processed} media attachments (approx. #{number_to_human_size(aggregate)}) #{dry_run}", :green, true) end + + option :account, type: :string + option :domain, type: :string + option :status, type: :numeric + option :concurrency, type: :numeric, default: 5, aliases: [:c] + option :verbose, type: :boolean, default: false, aliases: [:v] + option :dry_run, type: :boolean, default: false + desc 'refresh', 'Fetch remote media files' + long_desc <<-DESC + Re-downloads media attachments from other servers. You must specify the + source of media attachments with one of the following options: + + Use the --status option to download attachments from a specific status, + using the status local numeric ID. + + Use the --account option to download attachments from a specific account, + using username@domain handle of the account. + + Use the --domain option to download attachments from a specific domain. + DESC + def refresh + dry_run = options[:dry_run] ? ' (DRY RUN)' : '' + + if options[:status] + scope = MediaAttachment.where(status_id: options[:status]) + elsif options[:account] + username, domain = username.split('@') + account = Account.find_remote(username, domain) + + if account.nil? + say('No such account', :red) + exit(1) + end + + scope = MediaAttachment.where(account_id: account.id) + elsif options[:domain] + scope = MediaAttachment.joins(:account).merge(Account.by_domain_and_subdomains(options[:domain])) + else + exit(1) + end + + processed, aggregate = parallelize_with_progress(scope) do |media_attachment| + next if media_attachment.remote_url.blank? + + unless options[:dry_run] + media_attachment.reset_file! + media_attachment.save + end + + media_attachment.file_file_size + end + + say("Downloaded #{processed} media attachments (approx. #{number_to_human_size(aggregate)})#{dry_run}", :green, true) + end end end -- cgit From 4fe127664b0ae22a528b4a4467ab2de92e3da3ef Mon Sep 17 00:00:00 2001 From: Tao Bror Bojlén Date: Wed, 11 Sep 2019 07:44:58 +0100 Subject: add admin setting for default search engine indexing (fix #11750) (#11804) --- app/lib/settings/scoped_settings.rb | 1 + app/models/form/admin_settings.rb | 2 ++ app/views/admin/settings/edit.html.haml | 3 +++ config/locales/en.yml | 3 +++ spec/controllers/application_controller_spec.rb | 1 + 5 files changed, 10 insertions(+) (limited to 'app/models') diff --git a/app/lib/settings/scoped_settings.rb b/app/lib/settings/scoped_settings.rb index 3653ab114..9ca39510a 100644 --- a/app/lib/settings/scoped_settings.rb +++ b/app/lib/settings/scoped_settings.rb @@ -4,6 +4,7 @@ module Settings class ScopedSettings DEFAULTING_TO_UNSCOPED = %w( theme + noindex ).freeze def initialize(object) diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index 6bc3ca9f5..24196e182 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -32,6 +32,7 @@ class Form::AdminSettings trends show_domain_blocks show_domain_blocks_rationale + noindex ).freeze BOOLEAN_KEYS = %i( @@ -45,6 +46,7 @@ class Form::AdminSettings profile_directory spam_check_enabled trends + noindex ).freeze UPLOAD_KEYS = %i( diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 28880c087..752386b3c 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -71,6 +71,9 @@ .fields-group = f.input :trends, as: :boolean, wrapper: :with_label, label: t('admin.settings.trends.title'), hint: t('admin.settings.trends.desc_html') + .fields-group + = f.input :noindex, as: :boolean, wrapper: :with_label, label: t('admin.settings.default_noindex.title'), hint: t('admin.settings.default_noindex.desc_html') + .fields-group = f.input :spam_check_enabled, as: :boolean, wrapper: :with_label, label: t('admin.settings.spam_check_enabled.title'), hint: t('admin.settings.spam_check_enabled.desc_html') diff --git a/config/locales/en.yml b/config/locales/en.yml index 52cb4a269..0a5ca31c1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -427,6 +427,9 @@ en: custom_css: desc_html: Modify the look with CSS loaded on every page title: Custom CSS + default_noindex: + desc_html: Affects all users who have not changed this setting themselves + title: Opt users out of search engine indexing by default domain_blocks: all: To everyone disabled: To no one diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 1811500df..da4a794cd 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -110,6 +110,7 @@ describe ApplicationController, type: :controller do sign_in current_user allow(Setting).to receive(:[]).with('theme').and_return 'contrast' + allow(Setting).to receive(:[]).with('noindex').and_return false expect(controller.view_context.current_theme).to eq 'contrast' end -- cgit From c5d37f18cb3f4d6212fb8f3e1c4e1e027f677ec5 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 11 Sep 2019 16:32:44 +0200 Subject: Change deletes to preserve soft-deleted statuses in unresolved reports (#11805) Change all account actions except "none" to resolve all unresolved reports Refactor `SuspendAccountService` to be more readable --- app/controllers/admin/accounts_controller.rb | 2 +- app/controllers/admin/report_notes_controller.rb | 9 ++-- .../api/v1/admin/accounts_controller.rb | 2 +- app/lib/activitypub/activity/delete.rb | 3 +- app/models/account.rb | 1 + app/models/admin/account_action.rb | 24 +++++++-- app/models/form/account_batch.rb | 2 +- app/models/form/status_batch.rb | 2 +- app/models/report.rb | 1 + app/models/status.rb | 4 ++ app/models/user.rb | 4 ++ app/services/block_domain_service.rb | 2 +- app/services/remove_status_service.rb | 7 +-- app/services/suspend_account_service.rb | 62 ++++++++++++++++------ app/services/unallow_domain_service.rb | 2 +- app/workers/admin/suspension_worker.rb | 2 +- lib/mastodon/accounts_cli.rb | 4 +- lib/mastodon/domains_cli.rb | 2 +- .../admin/reported_statuses_controller_spec.rb | 2 +- spec/controllers/admin/statuses_controller_spec.rb | 2 +- spec/models/form/status_batch_spec.rb | 4 +- 21 files changed, 98 insertions(+), 45 deletions(-) (limited to 'app/models') diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 2fa1dfe5f..68b6352f8 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -41,7 +41,7 @@ module Admin def reject authorize @account.user, :reject? - SuspendAccountService.new.call(@account, including_user: true, destroy: true, skip_distribution: true) + SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false) redirect_to admin_pending_accounts_path end diff --git a/app/controllers/admin/report_notes_controller.rb b/app/controllers/admin/report_notes_controller.rb index bcb3f2026..b816c5b5d 100644 --- a/app/controllers/admin/report_notes_controller.rb +++ b/app/controllers/admin/report_notes_controller.rb @@ -5,10 +5,10 @@ module Admin before_action :set_report_note, only: [:destroy] def create - authorize ReportNote, :create? + authorize :report_note, :create? @report_note = current_account.report_notes.new(resource_params) - @report = @report_note.report + @report = @report_note.report if @report_note.save if params[:create_and_resolve] @@ -26,9 +26,8 @@ module Admin redirect_to admin_report_path(@report), notice: I18n.t('admin.report_notes.created_msg') else - @report_notes = @report.notes.latest - @report_history = @report.history - @form = Form::StatusBatch.new + @report_notes = (@report.notes.latest + @report.history + @report.target_account.targeted_account_warnings.latest.custom).sort_by(&:created_at) + @form = Form::StatusBatch.new render template: 'admin/reports/show' end diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb index c306180ca..c35ea5ab2 100644 --- a/app/controllers/api/v1/admin/accounts_controller.rb +++ b/app/controllers/api/v1/admin/accounts_controller.rb @@ -58,7 +58,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController def reject authorize @account.user, :reject? - SuspendAccountService.new.call(@account, including_user: true, destroy: true, skip_distribution: true) + SuspendAccountService.new.call(@account, reserve_email: false, reserve_username: false) render json: @account, serializer: REST::Admin::AccountSerializer end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 345060462..dc9ff580c 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -13,8 +13,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def delete_person lock_or_return("delete_in_progress:#{@account.id}") do - SuspendAccountService.new.call(@account) - @account.destroy! + SuspendAccountService.new.call(@account, reserve_username: false) end end diff --git a/app/models/account.rb b/app/models/account.rb index 8c9388b95..55fe53fae 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -115,6 +115,7 @@ class Account < ApplicationRecord :approved?, :pending?, :disabled?, + :unconfirmed_or_pending?, :role, :admin?, :moderator?, diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index c7da8b52c..b30a82369 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -83,19 +83,23 @@ class Admin::AccountAction # A log entry is only interesting if the warning contains # custom text from someone. Otherwise it's just noise. + log_action(:create, warning) if warning.text.present? end def process_reports! - return if report_id.blank? + # If we're doing "mark as resolved" on a single report, + # then we want to keep other reports open in case they + # contain new actionable information. + # + # Otherwise, we will mark all unresolved reports about + # the account as resolved. - authorize(report, :update?) + reports.each { |report| authorize(report, :update?) } - if type == 'none' + reports.each do |report| log_action(:resolve, report) report.resolve!(current_account) - else - Report.where(target_account: target_account).unresolved.update_all(action_taken: true, action_taken_by_account_id: current_account.id) end end @@ -141,6 +145,16 @@ class Admin::AccountAction @report.status_ids if @report && include_statuses end + def reports + @reports ||= begin + if type == 'none' && with_report? + [report] + else + Report.where(target_account: target_account).unresolved + end + end + end + def warning_preset @warning_preset ||= AccountWarningPreset.find(warning_preset_id) if warning_preset_id.present? end diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index f1b7a4566..0b285fde9 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -69,6 +69,6 @@ class Form::AccountBatch records = accounts.includes(:user) records.each { |account| authorize(account.user, :reject?) } - .each { |account| SuspendAccountService.new.call(account, including_user: true, destroy: true, skip_distribution: true) } + .each { |account| SuspendAccountService.new.call(account, reserve_email: false, reserve_username: false) } end end diff --git a/app/models/form/status_batch.rb b/app/models/form/status_batch.rb index e09cc2594..c4943a7ea 100644 --- a/app/models/form/status_batch.rb +++ b/app/models/form/status_batch.rb @@ -35,7 +35,7 @@ class Form::StatusBatch def delete_statuses Status.where(id: status_ids).reorder(nil).find_each do |status| status.discard - RemovalWorker.perform_async(status.id, redraft: false) + RemovalWorker.perform_async(status.id, immediate: true) Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true) log_action :destroy, status end diff --git a/app/models/report.rb b/app/models/report.rb index 1e707ff1c..fb2e040ee 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -59,6 +59,7 @@ class Report < ApplicationRecord end def resolve!(acting_account) + RemovalWorker.push_bulk(Status.with_discarded.discarded.where(id: status_ids).pluck(:id)) { |status_id| [status_id, { immediate: true }] } update!(action_taken: true, action_taken_by_account_id: acting_account.id) end diff --git a/app/models/status.rb b/app/models/status.rb index 9cfaddcec..471bb03b4 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -214,6 +214,10 @@ class Status < ApplicationRecord !sensitive? && with_media? end + def reported? + @reported ||= Report.where(target_account: account).unresolved.where('? = ANY(status_ids)', id).exists? + end + def emojis return @emojis if defined?(@emojis) diff --git a/app/models/user.rb b/app/models/user.rb index 95f1d8fc5..78b82a68f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -171,6 +171,10 @@ class User < ApplicationRecord confirmed? && approved? && !disabled? && !account.suspended? end + def unconfirmed_or_pending? + !(confirmed? && approved?) + end + def inactive_message !approved? ? :pending : super end diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index 0ec6be503..ae461abf2 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -53,7 +53,7 @@ class BlockDomainService < BaseService def suspend_accounts! blocked_domain_accounts.without_suspended.reorder(nil).find_each do |account| - SuspendAccountService.new.call(account, suspended_at: @domain_block.created_at) + SuspendAccountService.new.call(account, reserve_username: true, suspended_at: @domain_block.created_at) end end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 685c1d4bf..f9352ed3d 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -8,7 +8,8 @@ class RemoveStatusService < BaseService # @param [Status] status # @param [Hash] options # @option [Boolean] :redraft - # @options [Boolean] :original_removed + # @option [Boolean] :immediate + # @option [Boolean] :original_removed def call(status, **options) @payload = Oj.dump(event: :delete, payload: status.id.to_s) @status = status @@ -31,7 +32,7 @@ class RemoveStatusService < BaseService remove_from_spam_check remove_media - @status.destroy! + @status.destroy! if @options[:immediate] || !@status.reported? else raise Mastodon::RaceConditionError end @@ -150,7 +151,7 @@ class RemoveStatusService < BaseService end def remove_media - return if @options[:redraft] + return if @options[:redraft] || (!@options[:immediate] && @status.reported?) @status.media_attachments.destroy_all end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 85da7e921..ecc893931 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -15,7 +15,6 @@ class SuspendAccountService < BaseService favourites follow_requests list_accounts - media_attachments mute_relationships muted_by_relationships notifications @@ -32,14 +31,26 @@ class SuspendAccountService < BaseService targeted_reports ).freeze - # Suspend an account and remove as much of its data as possible + # Suspend or remove an account and remove as much of its data + # as possible. If it's a local account and it has not been confirmed + # or never been approved, then side effects are skipped and both + # the user and account records are removed fully. Otherwise, + # it is controlled by options. # @param [Account] # @param [Hash] options - # @option [Boolean] :including_user Remove the user record as well - # @option [Boolean] :destroy Remove the account record instead of suspending + # @option [Boolean] :reserve_email Keep user record. Only applicable for local accounts + # @option [Boolean] :reserve_username Keep account record + # @option [Boolean] :skip_side_effects Side effects are ActivityPub and streaming API payloads + # @option [Time] :suspended_at Only applicable when :reserve_username is true def call(account, **options) @account = account - @options = options + @options = { reserve_username: true, reserve_email: true }.merge(options) + + if @account.local? && @account.user_unconfirmed_or_pending? + @options[:reserve_email] = false + @options[:reserve_username] = false + @options[:skip_side_effects] = true + end reject_follows! purge_user! @@ -60,27 +71,39 @@ class SuspendAccountService < BaseService def purge_user! return if !@account.local? || @account.user.nil? - if @options[:including_user] - @options[:destroy] = true if !@account.user_confirmed? || @account.user_pending? - @account.user.destroy - else + if @options[:reserve_email] @account.user.disable! @account.user.invites.where(uses: 0).destroy_all + else + @account.user.destroy end end def purge_content! - distribute_delete_actor! if @account.local? && !@options[:skip_distribution] + distribute_delete_actor! if @account.local? && !@options[:skip_side_effects] @account.statuses.reorder(nil).find_in_batches do |statuses| - BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:destroy]) + statuses.reject! { |status| reported_status_ids.include?(status.id) } if @options[:reserve_username] + BatchedRemoveStatusService.new.call(statuses, skip_side_effects: @options[:skip_side_effects]) + end + + @account.media_attachments.reorder(nil).find_each do |media_attachment| + next if @options[:reserve_username] && reported_status_ids.include?(media_attachment.status_id) + + media_attachment.destroy + end + + @account.polls.reorder(nil).find_each do |poll| + next if @options[:reserve_username] && reported_status_ids.include?(poll.status_id) + + poll.destroy end associations_for_destruction.each do |association_name| destroy_all(@account.public_send(association_name)) end - @account.destroy if @options[:destroy] + @account.destroy unless @options[:reserve_username] end def purge_profile! @@ -88,11 +111,13 @@ class SuspendAccountService < BaseService # there is no point wasting time updating # its values first - return if @options[:destroy] + return unless @options[:reserve_username] @account.silenced_at = nil @account.suspended_at = @options[:suspended_at] || Time.now.utc @account.locked = false + @account.memorial = false + @account.discoverable = false @account.display_name = '' @account.note = '' @account.fields = [] @@ -100,6 +125,7 @@ class SuspendAccountService < BaseService @account.followers_count = 0 @account.following_count = 0 @account.moved_to_account = nil + @account.trust_level = :untrusted @account.avatar.destroy @account.header.destroy @account.save! @@ -135,11 +161,15 @@ class SuspendAccountService < BaseService Account.inboxes - delivery_inboxes end + def reported_status_ids + @reported_status_ids ||= Report.where(target_account: @account).unresolved.pluck(:status_ids).flatten.uniq + end + def associations_for_destruction - if @options[:destroy] - ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY - else + if @options[:reserve_username] ASSOCIATIONS_ON_SUSPEND + else + ASSOCIATIONS_ON_SUSPEND + ASSOCIATIONS_ON_DESTROY end end end diff --git a/app/services/unallow_domain_service.rb b/app/services/unallow_domain_service.rb index d4387c1a1..bd1ad328d 100644 --- a/app/services/unallow_domain_service.rb +++ b/app/services/unallow_domain_service.rb @@ -3,7 +3,7 @@ class UnallowDomainService < BaseService def call(domain_allow) Account.where(domain: domain_allow.domain).find_each do |account| - SuspendAccountService.new.call(account, destroy: true) + SuspendAccountService.new.call(account, reserve_username: false) end domain_allow.destroy diff --git a/app/workers/admin/suspension_worker.rb b/app/workers/admin/suspension_worker.rb index ae8b24d8c..83c815efd 100644 --- a/app/workers/admin/suspension_worker.rb +++ b/app/workers/admin/suspension_worker.rb @@ -6,6 +6,6 @@ class Admin::SuspensionWorker sidekiq_options queue: 'pull' def perform(account_id, remove_user = false) - SuspendAccountService.new.call(Account.find(account_id), including_user: remove_user) + SuspendAccountService.new.call(Account.find(account_id), reserve_username: true, reserve_email: !remove_user) end end diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index b16bf2e38..a09a6ab04 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -185,7 +185,7 @@ module Mastodon end say("Deleting user with #{account.statuses_count} statuses, this might take a while...") - SuspendAccountService.new.call(account, including_user: true) + SuspendAccountService.new.call(account, reserve_email: false) say('OK', :green) end @@ -239,7 +239,7 @@ module Mastodon end if [404, 410].include?(code) - SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run] + SuspendAccountService.new.call(account, reserve_username: false) unless options[:dry_run] 1 else # Touch account even during dry run to avoid getting the account into the window again diff --git a/lib/mastodon/domains_cli.rb b/lib/mastodon/domains_cli.rb index c612c2d72..8e52de1c3 100644 --- a/lib/mastodon/domains_cli.rb +++ b/lib/mastodon/domains_cli.rb @@ -42,7 +42,7 @@ module Mastodon end processed, = parallelize_with_progress(scope) do |account| - SuspendAccountService.new.call(account, destroy: true) unless options[:dry_run] + SuspendAccountService.new.call(account, reserve_username: false, skip_side_effects: true) unless options[:dry_run] end DomainBlock.where(domain: domain).destroy_all unless options[:dry_run] diff --git a/spec/controllers/admin/reported_statuses_controller_spec.rb b/spec/controllers/admin/reported_statuses_controller_spec.rb index bd146b795..2a1598123 100644 --- a/spec/controllers/admin/reported_statuses_controller_spec.rb +++ b/spec/controllers/admin/reported_statuses_controller_spec.rb @@ -47,7 +47,7 @@ describe Admin::ReportedStatusesController do it 'removes a status' do allow(RemovalWorker).to receive(:perform_async) subject.call - expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false) + expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, immediate: true) end end diff --git a/spec/controllers/admin/statuses_controller_spec.rb b/spec/controllers/admin/statuses_controller_spec.rb index 6b06343ef..d9690d83f 100644 --- a/spec/controllers/admin/statuses_controller_spec.rb +++ b/spec/controllers/admin/statuses_controller_spec.rb @@ -65,7 +65,7 @@ describe Admin::StatusesController do it 'removes a status' do allow(RemovalWorker).to receive(:perform_async) subject.call - expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, redraft: false) + expect(RemovalWorker).to have_received(:perform_async).with(status_ids.first, immediate: true) end end diff --git a/spec/models/form/status_batch_spec.rb b/spec/models/form/status_batch_spec.rb index f9c58c90f..68d84a737 100644 --- a/spec/models/form/status_batch_spec.rb +++ b/spec/models/form/status_batch_spec.rb @@ -41,12 +41,12 @@ describe Form::StatusBatch do it 'call RemovalWorker' do form.save - expect(RemovalWorker).to have_received(:perform_async).with(status.id, redraft: false) + expect(RemovalWorker).to have_received(:perform_async).with(status.id, immediate: true) end it 'do not call RemovalWorker' do form.save - expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id, redraft: false) + expect(RemovalWorker).not_to have_received(:perform_async).with(another_status.id, immediate: true) end end end -- cgit From b6381bdc7dcfe5713b99e2aae0491115350c7724 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 13 Sep 2019 16:00:34 +0200 Subject: Change max length of media descriptions from 420 to 1500 characters (#11819) Fix #11658 --- app/javascript/mastodon/features/ui/components/focal_point_modal.js | 2 +- app/models/media_attachment.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/javascript/mastodon/features/ui/components/focal_point_modal.js b/app/javascript/mastodon/features/ui/components/focal_point_modal.js index 735e445e8..d13138a76 100644 --- a/app/javascript/mastodon/features/ui/components/focal_point_modal.js +++ b/app/javascript/mastodon/features/ui/components/focal_point_modal.js @@ -223,7 +223,7 @@ class FocalPointModal extends ImmutablePureComponent {
- +