From 5d710b1139b34f2ed6bd556f448fa7248354e758 Mon Sep 17 00:00:00 2001 From: Eugen Date: Sun, 16 Apr 2017 12:51:30 +0200 Subject: [PATCH] Make file attachment on MediaAttachment optional (#1865) Create MediaAttachment but without actual file download when domain is blocked with reject_media set to true Clean up old media files when creating a new domain block with reject_media set to true Return remote_url in media attachments API if local file is not present Undo domain block action in admin UI Ability to enable reject_media from admin UI --- .../admin/domain_blocks_controller.rb | 14 ++++++-- app/controllers/application_controller.rb | 4 ++- app/models/domain_block.rb | 2 ++ app/services/block_domain_service.rb | 32 ++++++++++++++++--- app/services/process_feed_service.rb | 10 ++++-- app/services/suspend_account_service.rb | 3 +- app/services/unblock_domain_service.rb | 15 +++++++++ app/views/admin/domain_blocks/index.html.haml | 9 +++++- app/views/admin/domain_blocks/new.html.haml | 3 ++ app/views/admin/domain_blocks/show.html.haml | 9 ++++++ app/views/api/v1/statuses/_media.rabl | 4 +-- config/locales/en.yml | 16 ++++++++++ config/routes.rb | 2 +- .../stream_entries/show.html.haml_spec.rb | 1 - 14 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 app/services/unblock_domain_service.rb create mode 100644 app/views/admin/domain_blocks/show.html.haml diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index a8b56c0859..5d146d9469 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -15,16 +15,26 @@ module Admin if @domain_block.save DomainBlockWorker.perform_async(@domain_block.id) - redirect_to admin_domain_blocks_path, notice: 'Domain block is now being processed' + redirect_to admin_domain_blocks_path, notice: I18n.t('admin.domain_block.created_msg') else render action: :new end end + def show + @domain_block = DomainBlock.find(params[:id]) + end + + def destroy + @domain_block = DomainBlock.find(params[:id]) + UnblockDomainService.new.call(@domain_block, resource_params[:retroactive]) + redirect_to admin_domain_blocks_path, notice: I18n.t('admin.domain_block.destroyed_msg') + end + private def resource_params - params.require(:domain_block).permit(:domain, :severity) + params.require(:domain_block).permit(:domain, :severity, :reject_media, :retroactive) end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0c320177dd..e8d7de2188 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,9 @@ class ApplicationController < ActionController::Base force_ssl if: "Rails.env.production? && ENV['LOCAL_HTTPS'] == 'true'" include Localized - helper_method :current_account, :single_user_mode? + + helper_method :current_account + helper_method :single_user_mode? rescue_from ActionController::RoutingError, with: :not_found rescue_from ActiveRecord::RecordNotFound, with: :not_found diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 3548ccd692..89c81f766a 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -3,6 +3,8 @@ class DomainBlock < ApplicationRecord enum severity: [:silence, :suspend] + attr_accessor :retroactive + validates :domain, presence: true, uniqueness: true def self.blocked?(domain) diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index 6c131bd341..97d2ebcd76 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -3,12 +3,34 @@ class BlockDomainService < BaseService def call(domain_block) if domain_block.silence? - Account.where(domain: domain_block.domain).update_all(silenced: true) + silence_accounts!(domain_block.domain) + clear_media!(domain_block.domain) if domain_block.reject_media? else - Account.where(domain: domain_block.domain).find_each do |account| - account.subscription(api_subscription_url(account.id)).unsubscribe if account.subscribed? - SuspendAccountService.new.call(account) - end + suspend_accounts!(domain_block.domain) + end + end + + private + + def silence_accounts!(domain) + Account.where(domain: domain).update_all(silenced: true) + end + + def clear_media!(domain) + Account.where(domain: domain).find_each do |account| + account.avatar.destroy + account.header.destroy + end + + MediaAttachment.where(account: Account.where(domain: domain)).find_each do |attachment| + attachment.file.destroy + end + end + + def suspend_accounts!(domain) + Account.where(domain: domain).where(suspended: false).find_each do |account| + account.subscription(api_subscription_url(account.id)).unsubscribe if account.subscribed? + SuspendAccountService.new.call(account) end end end diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb index 321f53f22e..fa0633b275 100644 --- a/app/services/process_feed_service.rb +++ b/app/services/process_feed_service.rb @@ -179,12 +179,12 @@ class ProcessFeedService < BaseService end def hashtags_from_xml(parent, xml) - tags = xml.xpath('./xmlns:category', xmlns: TagManager::XMLNS).map { |category| category['term'] }.select { |t| !t.blank? } + tags = xml.xpath('./xmlns:category', xmlns: TagManager::XMLNS).map { |category| category['term'] }.select(&:present?) ProcessHashtagsService.new.call(parent, tags) end def media_from_xml(parent, xml) - return if DomainBlock.find_by(domain: parent.account.domain)&.reject_media? + do_not_download = DomainBlock.find_by(domain: parent.account.domain)&.reject_media? xml.xpath('./xmlns:link[@rel="enclosure"]', xmlns: TagManager::XMLNS).each do |link| next unless link['href'] @@ -192,7 +192,11 @@ class ProcessFeedService < BaseService media = MediaAttachment.where(status: parent, remote_url: link['href']).first_or_initialize(account: parent.account, status: parent, remote_url: link['href']) parsed_url = URI.parse(link['href']) - next if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.empty? + next if !%w[http https].include?(parsed_url.scheme) || parsed_url.host.empty? + + media.save + + next if do_not_download begin media.file_remote_url = link['href'] diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 42ff4dcb78..66517470e6 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -13,6 +13,7 @@ class SuspendAccountService < BaseService def purge_content @account.statuses.reorder(nil).find_each do |status| + # This federates out deletes to previous followers RemoveStatusService.new.call(status) end @@ -29,9 +30,7 @@ class SuspendAccountService < BaseService @account.display_name = '' @account.note = '' @account.avatar.destroy - @account.avatar.clear @account.header.destroy - @account.header.clear @account.save! end diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb new file mode 100644 index 0000000000..9794e439d1 --- /dev/null +++ b/app/services/unblock_domain_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class UnblockDomainService < BaseService + def call(domain_block, retroactive) + if retroactive + if domain_block.silence? + Account.where(domain: domain_block.domain).update_all(silenced: false) + else + Account.where(domain: domain_block.domain).update_all(suspended: false) + end + end + + domain_block.destroy + end +end diff --git a/app/views/admin/domain_blocks/index.html.haml b/app/views/admin/domain_blocks/index.html.haml index 6f4ba9b579..da9a07bbc0 100644 --- a/app/views/admin/domain_blocks/index.html.haml +++ b/app/views/admin/domain_blocks/index.html.haml @@ -6,12 +6,19 @@ %tr %th= t('admin.domain_block.domain') %th= t('admin.domain_block.severity') + %th= t('admin.domain_block.reject_media') + %th %tbody - @blocks.each do |block| %tr %td %samp= block.domain - %td= block.severity + %td= t("admin.domain_block.severities.#{block.severity}") + %td + - if block.reject_media? || block.suspend? + %i.fa.fa-check + %td + = table_link_to 'undo', t('admin.domain_block.undo'), admin_domain_block_path(block) = paginate @blocks = link_to t('admin.domain_block.add_new'), new_admin_domain_block_path, class: 'button' diff --git a/app/views/admin/domain_blocks/new.html.haml b/app/views/admin/domain_blocks/new.html.haml index 53aab21ff4..603faeb554 100644 --- a/app/views/admin/domain_blocks/new.html.haml +++ b/app/views/admin/domain_blocks/new.html.haml @@ -10,5 +10,8 @@ = f.input :severity, collection: DomainBlock.severities.keys, wrapper: :with_label, include_blank: false, label_method: lambda { |type| I18n.t("admin.domain_block.new.severity.#{type}") } %p.hint= t('admin.domain_block.new.severity.desc_html') + + = f.input :reject_media, as: :boolean, wrapper: :with_label, label: I18n.t('admin.domain_block.reject_media'), hint: I18n.t('admin.domain_block.reject_media_hint') + .actions = f.button :button, t('admin.domain_block.new.create'), type: :submit diff --git a/app/views/admin/domain_blocks/show.html.haml b/app/views/admin/domain_blocks/show.html.haml new file mode 100644 index 0000000000..bf9011c528 --- /dev/null +++ b/app/views/admin/domain_blocks/show.html.haml @@ -0,0 +1,9 @@ +- content_for :page_title do + = t('admin.domain_block.show.title', domain: @domain_block.domain) + += simple_form_for @domain_block, url: admin_domain_block_path(@domain_block), method: :delete do |f| + + = f.input :retroactive, as: :boolean, wrapper: :with_label, label: I18n.t("admin.domain_block.show.retroactive.#{@domain_block.severity}"), hint: I18n.t('admin.domain_block.show.affected_accounts', count: Account.where(domain: @domain_block.domain).count) + + .actions + = f.button :button, t('admin.domain_block.show.undo'), type: :submit diff --git a/app/views/api/v1/statuses/_media.rabl b/app/views/api/v1/statuses/_media.rabl index 2f56c6d07d..80d80ea05a 100644 --- a/app/views/api/v1/statuses/_media.rabl +++ b/app/views/api/v1/statuses/_media.rabl @@ -1,5 +1,5 @@ attributes :id, :remote_url, :type -node(:url) { |media| full_asset_url(media.file.url(:original)) } -node(:preview_url) { |media| full_asset_url(media.file.url(:small)) } +node(:url) { |media| media.file.blank? ? media.remote_url : full_asset_url(media.file.url(:original)) } +node(:preview_url) { |media| media.file.blank? ? media.remote_url : full_asset_url(media.file.url(:small)) } node(:text_url) { |media| media.local? ? medium_url(media) : nil } diff --git a/config/locales/en.yml b/config/locales/en.yml index 474de39853..325df50450 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -81,6 +81,8 @@ en: web: Web domain_block: add_new: Add new + created_msg: Domain block is now being processed + destroyed_msg: Domain block has been undone domain: Domain new: create: Create block @@ -90,8 +92,22 @@ en: silence: Silence suspend: Suspend title: New domain block + reject_media: Reject media files + reject_media_hint: Removes locally stored media files and refuses to download any in the future. Irrelevant for suspensions + severities: + silence: Silence + suspend: Suspend severity: Severity + show: + affected_accounts: + one: One account in the database affected + other: "%{count} accounts in the database affected" + retroactive: + silence: Unsilence all existing accounts from this domain + suspend: Unsuspend all existing accounts from this domain + title: Undo domain block for %{domain} title: Domain Blocks + undo: Undo pubsubhubbub: callback_url: Callback URL confirmed: Confirmed diff --git a/config/routes.rb b/config/routes.rb index 31909a4f4d..fd186c3206 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -78,7 +78,7 @@ Rails.application.routes.draw do namespace :admin do resources :pubsubhubbub, only: [:index] - resources :domain_blocks, only: [:index, :new, :create] + resources :domain_blocks, only: [:index, :new, :create, :show, :destroy] resources :settings, only: [:index, :update] resources :reports, only: [:index, :show, :update] do diff --git a/spec/views/stream_entries/show.html.haml_spec.rb b/spec/views/stream_entries/show.html.haml_spec.rb index 5526d6780e..acc0c39f5b 100644 --- a/spec/views/stream_entries/show.html.haml_spec.rb +++ b/spec/views/stream_entries/show.html.haml_spec.rb @@ -61,5 +61,4 @@ describe 'stream_entries/show.html.haml' do expect(mf2.entry.in_reply_to.format.author.format.name.to_s).to eq alice.display_name expect(mf2.entry.in_reply_to.format.author.format.url.to_s).not_to be_empty end - end