From a3b5675aa83f7fec4036888e0d94b626ad973f75 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 14 Dec 2020 05:09:14 +0100 Subject: Change number format on about page from full to shortened (#15327) --- app/views/about/more.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/views') diff --git a/app/views/about/more.html.haml b/app/views/about/more.html.haml index c2168e1f5..109b5fa86 100644 --- a/app/views/about/more.html.haml +++ b/app/views/about/more.html.haml @@ -17,11 +17,11 @@ .row__information-board .information-board__section %span= t 'about.user_count_before' - %strong= number_with_delimiter @instance_presenter.user_count + %strong= number_to_human @instance_presenter.user_count, strip_insignificant_zeros: true %span= t 'about.user_count_after', count: @instance_presenter.user_count .information-board__section %span= t 'about.status_count_before' - %strong= number_with_delimiter @instance_presenter.status_count + %strong= number_to_human @instance_presenter.status_count, strip_insignificant_zeros: true %span= t 'about.status_count_after', count: @instance_presenter.status_count .row__mascot .landing-page__mascot -- cgit From 216b85b053d091306e3311a21f5b050f70a75130 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 14 Dec 2020 09:06:34 +0100 Subject: Fix performance on instances list in admin UI (#15282) - Reduce duplicate queries - Remove n+1 queries - Add accounts count to detailed view - Add separate action log entry for updating existing domain blocks --- Gemfile | 1 + Gemfile.lock | 4 + app/controllers/admin/domain_blocks_controller.rb | 5 +- app/controllers/admin/instances_controller.rb | 44 +-------- .../api/v1/instances/peers_controller.rb | 2 +- app/models/account.rb | 6 +- app/models/concerns/domain_materializable.rb | 13 +++ app/models/domain_allow.rb | 1 + app/models/domain_block.rb | 1 + app/models/instance.rb | 63 ++++++++++--- app/models/instance_filter.rb | 31 ++++--- app/models/unavailable_domain.rb | 2 + app/policies/domain_block_policy.rb | 4 + app/presenters/instance_presenter.rb | 2 +- app/views/admin/instances/_instance.html.haml | 25 +++++ app/views/admin/instances/index.html.haml | 36 ++------ app/views/admin/instances/show.html.haml | 50 +++++----- .../scheduler/instance_refresh_scheduler.rb | 11 +++ config/brakeman.ignore | 101 ++++++++++++++++++--- config/locales/en.yml | 3 + config/sidekiq.yml | 3 + db/migrate/20201206004238_create_instances.rb | 9 ++ db/schema.rb | 27 +++++- db/views/instances_v01.sql | 17 ++++ lib/mastodon/domains_cli.rb | 4 +- .../controllers/admin/instances_controller_spec.rb | 6 +- spec/models/account_spec.rb | 21 ----- 27 files changed, 326 insertions(+), 166 deletions(-) create mode 100644 app/models/concerns/domain_materializable.rb create mode 100644 app/views/admin/instances/_instance.html.haml create mode 100644 app/workers/scheduler/instance_refresh_scheduler.rb create mode 100644 db/migrate/20201206004238_create_instances.rb create mode 100644 db/views/instances_v01.sql (limited to 'app/views') diff --git a/Gemfile b/Gemfile index 63becb7cc..3aee5d7cc 100644 --- a/Gemfile +++ b/Gemfile @@ -82,6 +82,7 @@ gem 'mario-redis-lock', '~> 1.2', require: 'redis_lock' gem 'rqrcode', '~> 1.1' gem 'ruby-progressbar', '~> 1.10' gem 'sanitize', '~> 5.2' +gem 'scenic', '~> 1.5' gem 'sidekiq', '~> 6.1' gem 'sidekiq-scheduler', '~> 3.0' gem 'sidekiq-unique-jobs', '~> 6.0' diff --git a/Gemfile.lock b/Gemfile.lock index f7192d084..c4c8d9904 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -561,6 +561,9 @@ GEM crass (~> 1.0.2) nokogiri (>= 1.8.0) nokogumbo (~> 2.0) + scenic (1.5.4) + activerecord (>= 4.0.0) + railties (>= 4.0.0) securecompare (1.0.0) semantic_range (2.3.0) sidekiq (6.1.2) @@ -782,6 +785,7 @@ DEPENDENCIES rubocop-rails (~> 2.8) ruby-progressbar (~> 1.10) sanitize (~> 5.2) + scenic (~> 1.5) sidekiq (~> 6.1) sidekiq-bulk (~> 0.2.0) sidekiq-scheduler (~> 3.0) diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index 74a36b79c..6a5b41a74 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -29,6 +29,7 @@ module Admin @domain_block = existing_domain_block @domain_block.update(resource_params) end + if @domain_block.save DomainBlockWorker.perform_async(@domain_block.id) log_action :create, @domain_block @@ -40,7 +41,7 @@ module Admin end def update - authorize :domain_block, :create? + authorize :domain_block, :update? @domain_block.update(update_params) @@ -48,7 +49,7 @@ module Admin if @domain_block.save DomainBlockWorker.perform_async(@domain_block.id, severity_changed) - log_action :create, @domain_block + log_action :update, @domain_block redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') else render :edit diff --git a/app/controllers/admin/instances_controller.rb b/app/controllers/admin/instances_controller.rb index 1790becbf..b5918d231 100644 --- a/app/controllers/admin/instances_controller.rb +++ b/app/controllers/admin/instances_controller.rb @@ -2,65 +2,31 @@ module Admin class InstancesController < BaseController - before_action :set_domain_block, only: :show - before_action :set_domain_allow, only: :show + before_action :set_instances, only: :index before_action :set_instance, only: :show def index authorize :instance, :index? - - @instances = ordered_instances end def show authorize :instance, :show? - - @following_count = Follow.where(account: Account.where(domain: params[:id])).count - @followers_count = Follow.where(target_account: Account.where(domain: params[:id])).count - @reports_count = Report.where(target_account: Account.where(domain: params[:id])).count - @blocks_count = Block.where(target_account: Account.where(domain: params[:id])).count - @available = DeliveryFailureTracker.available?(params[:id]) - @media_storage = MediaAttachment.where(account: Account.where(domain: params[:id])).sum(:file_file_size) - @private_comment = @domain_block&.private_comment - @public_comment = @domain_block&.public_comment end private - def set_domain_block - @domain_block = DomainBlock.rule_for(params[:id]) - end - - def set_domain_allow - @domain_allow = DomainAllow.rule_for(params[:id]) - end - def set_instance - resource = Account.by_domain_accounts.find_by(domain: params[:id]) - resource ||= @domain_block - resource ||= @domain_allow + @instance = Instance.find(params[:id]) + end - if resource - @instance = Instance.new(resource) - else - not_found - end + def set_instances + @instances = filtered_instances.page(params[:page]) end def filtered_instances InstanceFilter.new(whitelist_mode? ? { allowed: true } : filter_params).results end - def paginated_instances - filtered_instances.page(params[:page]) - end - - helper_method :paginated_instances - - def ordered_instances - paginated_instances.map { |resource| Instance.new(resource) } - end - def filter_params params.slice(*InstanceFilter::KEYS).permit(*InstanceFilter::KEYS) end diff --git a/app/controllers/api/v1/instances/peers_controller.rb b/app/controllers/api/v1/instances/peers_controller.rb index 9fa440935..2877fec52 100644 --- a/app/controllers/api/v1/instances/peers_controller.rb +++ b/app/controllers/api/v1/instances/peers_controller.rb @@ -8,7 +8,7 @@ class Api::V1::Instances::PeersController < Api::BaseController def index expires_in 1.day, public: true - render_with_cache(expires_in: 1.day) { Account.remote.domains } + render_with_cache(expires_in: 1.day) { Instance.where.not(domain: DomainBlock.select(:domain)).pluck(:domain) } end private diff --git a/app/models/account.rb b/app/models/account.rb index ed11a514d..e21b353e9 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -67,6 +67,7 @@ class Account < ApplicationRecord include Paginable include AccountCounters include DomainNormalizable + include DomainMaterializable include AccountMerging TRUST_LEVELS = { @@ -103,7 +104,6 @@ class Account < ApplicationRecord scope :bots, -> { where(actor_type: %w(Application Service)) } scope :groups, -> { where(actor_type: 'Group') } scope :alphabetic, -> { order(domain: :asc, username: :asc) } - scope :by_domain_accounts, -> { group(:domain).select(:domain, 'COUNT(*) AS accounts_count').order('accounts_count desc') } scope :matches_username, ->(value) { where(arel_table[:username].matches("#{value}%")) } scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } @@ -438,10 +438,6 @@ class Account < ApplicationRecord super - %w(statuses_count following_count followers_count) end - def domains - reorder(nil).pluck(Arel.sql('distinct accounts.domain')) - end - def inboxes urls = reorder(nil).where(protocol: :activitypub).group(:preferred_inbox_url).pluck(Arel.sql("coalesce(nullif(accounts.shared_inbox_url, ''), accounts.inbox_url) AS preferred_inbox_url")) DeliveryFailureTracker.without_unavailable(urls) diff --git a/app/models/concerns/domain_materializable.rb b/app/models/concerns/domain_materializable.rb new file mode 100644 index 000000000..88337f8c0 --- /dev/null +++ b/app/models/concerns/domain_materializable.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module DomainMaterializable + extend ActiveSupport::Concern + + included do + after_create_commit :refresh_instances_view + end + + def refresh_instances_view + Instance.refresh unless domain.nil? || Instance.where(domain: domain).exists? + end +end diff --git a/app/models/domain_allow.rb b/app/models/domain_allow.rb index 5fe0e3a29..4b0a89c18 100644 --- a/app/models/domain_allow.rb +++ b/app/models/domain_allow.rb @@ -12,6 +12,7 @@ class DomainAllow < ApplicationRecord include DomainNormalizable + include DomainMaterializable validates :domain, presence: true, uniqueness: true, domain: true diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 2b18e01fa..829d7583b 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -16,6 +16,7 @@ class DomainBlock < ApplicationRecord include DomainNormalizable + include DomainMaterializable enum severity: [:silence, :suspend, :noop] diff --git a/app/models/instance.rb b/app/models/instance.rb index 3c740f8a2..29be03662 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -1,26 +1,63 @@ # frozen_string_literal: true +# == Schema Information +# +# Table name: instances +# +# domain :string primary key +# accounts_count :bigint(8) +# -class Instance - include ActiveModel::Model +class Instance < ApplicationRecord + self.primary_key = :domain - attr_accessor :domain, :accounts_count, :domain_block + has_many :accounts, foreign_key: :domain, primary_key: :domain - def initialize(resource) - @domain = resource.domain - @accounts_count = resource.respond_to?(:accounts_count) ? resource.accounts_count : nil - @domain_block = resource.is_a?(DomainBlock) ? resource : DomainBlock.rule_for(domain) - @domain_allow = resource.is_a?(DomainAllow) ? resource : DomainAllow.rule_for(domain) + belongs_to :domain_block, foreign_key: :domain, primary_key: :domain + belongs_to :domain_allow, foreign_key: :domain, primary_key: :domain + + scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } + + def self.refresh + Scenic.database.refresh_materialized_view(table_name, concurrently: true, cascade: false) end - def countable? - @accounts_count.present? + def readonly? + true end - def to_param - domain + def delivery_failure_tracker + @delivery_failure_tracker ||= DeliveryFailureTracker.new(domain) + end + + def following_count + @following_count ||= Follow.where(account: accounts).count + end + + def followers_count + @followers_count ||= Follow.where(target_account: accounts).count + end + + def reports_count + @reports_count ||= Report.where(target_account: accounts).count end - def cache_key + def blocks_count + @blocks_count ||= Block.where(target_account: accounts).count + end + + def public_comment + domain_block&.public_comment + end + + def private_comment + domain_block&.private_comment + end + + def media_storage + @media_storage ||= MediaAttachment.where(account: accounts).sum(:file_file_size) + end + + def to_param domain end end diff --git a/app/models/instance_filter.rb b/app/models/instance_filter.rb index 9c467bc27..0598d8fea 100644 --- a/app/models/instance_filter.rb +++ b/app/models/instance_filter.rb @@ -13,18 +13,27 @@ class InstanceFilter end def results - if params[:limited].present? - scope = DomainBlock - scope = scope.matches_domain(params[:by_domain]) if params[:by_domain].present? - scope.order(id: :desc) - elsif params[:allowed].present? - scope = DomainAllow - scope = scope.matches_domain(params[:by_domain]) if params[:by_domain].present? - scope.order(id: :desc) + scope = Instance.includes(:domain_block, :domain_allow).order(accounts_count: :desc) + + params.each do |key, value| + scope.merge!(scope_for(key, value.to_s.strip)) if value.present? + end + + scope + end + + private + + def scope_for(key, value) + case key.to_s + when 'limited' + Instance.joins(:domain_block).reorder(Arel.sql('domain_blocks.id desc')) + when 'allowed' + Instance.joins(:domain_allow).reorder(Arel.sql('domain_allows.id desc')) + when 'by_domain' + Instance.matches_domain(value) else - scope = Account.remote - scope = scope.matches_domain(params[:by_domain]) if params[:by_domain].present? - scope.by_domain_accounts + raise "Unknown filter: #{key}" end end end diff --git a/app/models/unavailable_domain.rb b/app/models/unavailable_domain.rb index e2918b586..5e8870bde 100644 --- a/app/models/unavailable_domain.rb +++ b/app/models/unavailable_domain.rb @@ -12,6 +12,8 @@ class UnavailableDomain < ApplicationRecord include DomainNormalizable + validates :domain, presence: true, uniqueness: true + after_commit :reset_cache! private diff --git a/app/policies/domain_block_policy.rb b/app/policies/domain_block_policy.rb index 47c0a81af..543259cce 100644 --- a/app/policies/domain_block_policy.rb +++ b/app/policies/domain_block_policy.rb @@ -13,6 +13,10 @@ class DomainBlockPolicy < ApplicationPolicy admin? end + def update? + admin? + end + def destroy? admin? end diff --git a/app/presenters/instance_presenter.rb b/app/presenters/instance_presenter.rb index c150bf742..1bfdd40ac 100644 --- a/app/presenters/instance_presenter.rb +++ b/app/presenters/instance_presenter.rb @@ -29,7 +29,7 @@ class InstancePresenter end def domain_count - Rails.cache.fetch('distinct_domain_count') { Account.distinct.count(:domain) } + Rails.cache.fetch('distinct_domain_count') { Instance.count } end def sample_accounts diff --git a/app/views/admin/instances/_instance.html.haml b/app/views/admin/instances/_instance.html.haml new file mode 100644 index 000000000..188d0d984 --- /dev/null +++ b/app/views/admin/instances/_instance.html.haml @@ -0,0 +1,25 @@ +.directory__tag + = link_to admin_instance_path(instance) do + %h4 + = instance.domain + %small + - if instance.domain_block + - first_item = true + - if !instance.domain_block.noop? + = t("admin.domain_blocks.severity.#{instance.domain_block.severity}") + - first_item = false + - unless instance.domain_block.suspend? + - if instance.domain_block.reject_media? + - unless first_item + • + = t('admin.domain_blocks.rejecting_media') + - first_item = false + - if instance.domain_block.reject_reports? + - unless first_item + • + = t('admin.domain_blocks.rejecting_reports') + - elsif whitelist_mode? + = t('admin.accounts.whitelisted') + - else + = t('admin.accounts.no_limits_imposed') + .trends__item__current{ title: t('admin.instances.known_accounts', count: instance.accounts_count) }= number_to_human instance.accounts_count, strip_insignificant_zeros: true diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index 696ba3c7f..5f20e7ec0 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -32,32 +32,10 @@ %hr.spacer/ -- @instances.each do |instance| - .directory__tag - = link_to admin_instance_path(instance) do - %h4 - = instance.domain - %small - - if instance.domain_block - - first_item = true - - if !instance.domain_block.noop? - = t("admin.domain_blocks.severity.#{instance.domain_block.severity}") - - first_item = false - - unless instance.domain_block.suspend? - - if instance.domain_block.reject_media? - - unless first_item - • - = t('admin.domain_blocks.rejecting_media') - - first_item = false - - if instance.domain_block.reject_reports? - - unless first_item - • - = t('admin.domain_blocks.rejecting_reports') - - elsif whitelist_mode? - = t('admin.accounts.whitelisted') - - else - = t('admin.accounts.no_limits_imposed') - - if instance.countable? - .trends__item__current{ title: t('admin.instances.known_accounts', count: instance.accounts_count) }= number_to_human instance.accounts_count, strip_insignificant_zeros: true - -= paginate paginated_instances +- if @instances.empty? + %div.muted-hint.center-text + = t 'admin.instances.empty' +- else + = render @instances + += paginate @instances diff --git a/app/views/admin/instances/show.html.haml b/app/views/admin/instances/show.html.haml index 92e14c0df..0b9382771 100644 --- a/app/views/admin/instances/show.html.haml +++ b/app/views/admin/instances/show.html.haml @@ -2,58 +2,60 @@ = @instance.domain .dashboard__counters + %div + = link_to admin_accounts_path(remote: '1', by_domain: @instance.domain) do + .dashboard__counters__num= number_with_delimiter @instance.accounts_count + .dashboard__counters__label= t 'admin.accounts.title' + %div + = link_to admin_reports_path(by_target_domain: @instance.domain) do + .dashboard__counters__num= number_with_delimiter @instance.reports_count + .dashboard__counters__label= t 'admin.instances.total_reported' %div %div - .dashboard__counters__num= number_with_delimiter @following_count - .dashboard__counters__label= t 'admin.instances.total_followed_by_them' + .dashboard__counters__num= number_to_human_size @instance.media_storage + .dashboard__counters__label= t 'admin.instances.total_storage' %div %div - .dashboard__counters__num= number_with_delimiter @followers_count - .dashboard__counters__label= t 'admin.instances.total_followed_by_us' + .dashboard__counters__num= number_with_delimiter @instance.following_count + .dashboard__counters__label= t 'admin.instances.total_followed_by_them' %div %div - .dashboard__counters__num= number_to_human_size @media_storage - .dashboard__counters__label= t 'admin.instances.total_storage' + .dashboard__counters__num= number_with_delimiter @instance.followers_count + .dashboard__counters__label= t 'admin.instances.total_followed_by_us' %div %div - .dashboard__counters__num= number_with_delimiter @blocks_count + .dashboard__counters__num= number_with_delimiter @instance.blocks_count .dashboard__counters__label= t 'admin.instances.total_blocked_by_us' - %div - = link_to admin_reports_path(by_target_domain: @instance.domain) do - .dashboard__counters__num= number_with_delimiter @reports_count - .dashboard__counters__label= t 'admin.instances.total_reported' + %div %div .dashboard__counters__num - - if @available + - if @instance.delivery_failure_tracker.available? = fa_icon 'check' - else = fa_icon 'times' .dashboard__counters__label= t 'admin.instances.delivery_available' -- if @private_comment.present? +- if @instance.private_comment.present? .speech-bubble .speech-bubble__bubble - = simple_format(h(@private_comment)) + = simple_format(h(@instance.private_comment)) .speech-bubble__owner= t 'admin.instances.private_comment' -- if @public_comment.present? +- if @instance.public_comment.present? .speech-bubble .speech-bubble__bubble - = simple_format(h(@public_comment)) + = simple_format(h(@instance.public_comment)) .speech-bubble__owner= t 'admin.instances.public_comment' %hr.spacer/ %div.action-buttons %div - = link_to t('admin.accounts.title'), admin_accounts_path(remote: '1', by_domain: @instance.domain), class: 'button' - - %div - - if @domain_allow - = link_to t('admin.domain_allows.undo'), admin_domain_allow_path(@domain_allow), class: 'button button--destructive', data: { confirm: t('admin.accounts.are_you_sure'), method: :delete } - - elsif @domain_block - = link_to t('admin.domain_blocks.edit'), edit_admin_domain_block_path(@domain_block), class: 'button' - = link_to t('admin.domain_blocks.undo'), admin_domain_block_path(@domain_block), class: 'button' + - if @instance.domain_allow + = link_to t('admin.domain_allows.undo'), admin_domain_allow_path(@instance.domain_allow), class: 'button button--destructive', data: { confirm: t('admin.accounts.are_you_sure'), method: :delete } + - elsif @instance.domain_block + = link_to t('admin.domain_blocks.edit'), edit_admin_domain_block_path(@instance.domain_block), class: 'button' + = link_to t('admin.domain_blocks.undo'), admin_domain_block_path(@instance.domain_block), class: 'button' - else = link_to t('admin.domain_blocks.add_new'), new_admin_domain_block_path(_domain: @instance.domain), class: 'button' diff --git a/app/workers/scheduler/instance_refresh_scheduler.rb b/app/workers/scheduler/instance_refresh_scheduler.rb new file mode 100644 index 000000000..917404bec --- /dev/null +++ b/app/workers/scheduler/instance_refresh_scheduler.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class Scheduler::InstanceRefreshScheduler + include Sidekiq::Worker + + sidekiq_options lock: :until_executed, retry: 0 + + def perform + Instance.refresh + end +end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index baa993c78..dcbfd02b4 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -102,6 +102,37 @@ "confidence": "Weak", "note": "" }, + { + "warning_type": "Dynamic Render Path", + "warning_code": 15, + "fingerprint": "4704e8093e3e0561bf705f892e8fc6780419f8255f4440b1c0afd09339bd6446", + "check_name": "Render", + "message": "Render path contains parameter value", + "file": "app/views/admin/instances/index.html.haml", + "line": 39, + "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", + "code": "render(action => filtered_instances.page(params[:page]), {})", + "render_path": [ + { + "type": "controller", + "class": "Admin::InstancesController", + "method": "index", + "line": 10, + "file": "app/controllers/admin/instances_controller.rb", + "rendered": { + "name": "admin/instances/index", + "file": "app/views/admin/instances/index.html.haml" + } + } + ], + "location": { + "type": "template", + "template": "admin/instances/index" + }, + "user_input": "params[:page]", + "confidence": "Weak", + "note": "" + }, { "warning_type": "Redirect", "warning_code": 18, @@ -122,6 +153,26 @@ "confidence": "High", "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "6e4051854bb62e2ddbc671f82d6c2328892e1134b8b28105ecba9b0122540714", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 491, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" WITH first_degree AS (\\n SELECT target_account_id\\n FROM follows\\n WHERE account_id = ?\\n UNION ALL\\n SELECT ?\\n )\\n SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)\\n WHERE accounts.id IN (SELECT * FROM first_degree)\\n AND #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, account.id, limit, offset])", + "render_path": null, + "location": { + "type": "method", + "class": "Account", + "method": "advanced_search_for" + }, + "user_input": "textsearch", + "confidence": "Medium", + "note": "" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -163,23 +214,23 @@ "note": "" }, { - "warning_type": "Mass Assignment", - "warning_code": 105, - "fingerprint": "8f63dec68951d9bcf7eddb15af9392b2e1333003089c41fb76688dfd3579f394", - "check_name": "PermitAttributes", - "message": "Potentially dangerous key allowed for mass assignment", - "file": "app/controllers/api/v1/crypto/deliveries_controller.rb", - "line": 23, - "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", - "code": "params.require(:device).permit(:account_id, :device_id, :type, :body, :hmac)", + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "9251d682c4e2840e1b2fea91e7d758efe2097ecb7f6255c065e3750d25eb178c", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 460, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" SELECT\\n accounts.*,\\n ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, limit, offset])", "render_path": null, "location": { "type": "method", - "class": "Api::V1::Crypto::DeliveriesController", - "method": "resource_params" + "class": "Account", + "method": "search_for" }, - "user_input": ":account_id", - "confidence": "High", + "user_input": "textsearch", + "confidence": "Medium", "note": "" }, { @@ -273,6 +324,26 @@ "confidence": "High", "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "e21d8fee7a5805761679877ca35ed1029c64c45ef3b4012a30262623e1ba8bb9", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/account.rb", + "line": 507, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "find_by_sql([\" SELECT\\n accounts.*,\\n (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n FROM accounts\\n LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)\\n WHERE #{query} @@ #{textsearch}\\n AND accounts.suspended_at IS NULL\\n AND accounts.moved_to_account_id IS NULL\\n GROUP BY accounts.id\\n ORDER BY rank DESC\\n LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, limit, offset])", + "render_path": null, + "location": { + "type": "method", + "class": "Account", + "method": "advanced_search_for" + }, + "user_input": "textsearch", + "confidence": "Medium", + "note": "" + }, { "warning_type": "Mass Assignment", "warning_code": 105, @@ -294,6 +365,6 @@ "note": "" } ], - "updated": "2020-06-01 18:18:02 +0200", - "brakeman_version": "4.8.0" + "updated": "2020-12-07 01:17:13 +0100", + "brakeman_version": "4.10.0" } diff --git a/config/locales/en.yml b/config/locales/en.yml index 59f561aa3..f89f50e4d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -255,6 +255,7 @@ en: unsuspend_account: Unsuspend Account update_announcement: Update Announcement update_custom_emoji: Update Custom Emoji + update_domain_block: Update Domain Block update_status: Update Status actions: assigned_to_self_report: "%{name} assigned report %{target} to themselves" @@ -295,6 +296,7 @@ en: unsuspend_account: "%{name} unsuspended %{target}'s account" update_announcement: "%{name} updated announcement %{target}" update_custom_emoji: "%{name} updated emoji %{target}" + update_domain_block: "%{name} updated domain block for %{target}" update_status: "%{name} updated status by %{target}" deleted_status: "(deleted status)" empty: No logs found. @@ -437,6 +439,7 @@ en: instances: by_domain: Domain delivery_available: Delivery is available + empty: No domains found. known_accounts: one: "%{count} known account" other: "%{count} known accounts" diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 5de25de23..a71c1098e 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -36,3 +36,6 @@ pghero_scheduler: cron: '0 0 * * *' class: Scheduler::PgheroScheduler + instance_refresh_scheduler: + cron: '0 * * * *' + class: Scheduler::InstanceRefreshScheduler diff --git a/db/migrate/20201206004238_create_instances.rb b/db/migrate/20201206004238_create_instances.rb new file mode 100644 index 000000000..a4b866894 --- /dev/null +++ b/db/migrate/20201206004238_create_instances.rb @@ -0,0 +1,9 @@ +class CreateInstances < ActiveRecord::Migration[5.2] + def change + create_view :instances, materialized: true + + # To be able to refresh the view concurrently, + # at least one unique index is required + safety_assured { add_index :instances, :domain, unique: true } + end +end diff --git a/db/schema.rb b/db/schema.rb index 873c37f67..2f9d369be 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_10_17_234926) do +ActiveRecord::Schema.define(version: 2020_12_06_004238) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1045,4 +1045,29 @@ ActiveRecord::Schema.define(version: 2020_10_17_234926) do add_foreign_key "web_push_subscriptions", "users", on_delete: :cascade add_foreign_key "web_settings", "users", name: "fk_11910667b2", on_delete: :cascade add_foreign_key "webauthn_credentials", "users" + + create_view "instances", materialized: true, sql_definition: <<-SQL + WITH domain_counts(domain, accounts_count) AS ( + SELECT accounts.domain, + count(*) AS accounts_count + FROM accounts + WHERE (accounts.domain IS NOT NULL) + GROUP BY accounts.domain + ) + SELECT domain_counts.domain, + domain_counts.accounts_count + FROM domain_counts + UNION + SELECT domain_blocks.domain, + COALESCE(domain_counts.accounts_count, (0)::bigint) AS accounts_count + FROM (domain_blocks + LEFT JOIN domain_counts ON (((domain_counts.domain)::text = (domain_blocks.domain)::text))) + UNION + SELECT domain_allows.domain, + COALESCE(domain_counts.accounts_count, (0)::bigint) AS accounts_count + FROM (domain_allows + LEFT JOIN domain_counts ON (((domain_counts.domain)::text = (domain_allows.domain)::text))); + SQL + add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true + end diff --git a/db/views/instances_v01.sql b/db/views/instances_v01.sql new file mode 100644 index 000000000..94acd61a1 --- /dev/null +++ b/db/views/instances_v01.sql @@ -0,0 +1,17 @@ +WITH domain_counts(domain, accounts_count) +AS ( + SELECT domain, COUNT(*) as accounts_count + FROM accounts + WHERE domain IS NOT NULL + GROUP BY domain +) +SELECT domain, accounts_count +FROM domain_counts +UNION +SELECT domain_blocks.domain, COALESCE(domain_counts.accounts_count, 0) +FROM domain_blocks +LEFT OUTER JOIN domain_counts ON domain_counts.domain = domain_blocks.domain +UNION +SELECT domain_allows.domain, COALESCE(domain_counts.accounts_count, 0) +FROM domain_allows +LEFT OUTER JOIN domain_counts ON domain_counts.domain = domain_allows.domain diff --git a/lib/mastodon/domains_cli.rb b/lib/mastodon/domains_cli.rb index 5433ddd9d..3c2dfd4ec 100644 --- a/lib/mastodon/domains_cli.rb +++ b/lib/mastodon/domains_cli.rb @@ -53,6 +53,8 @@ module Mastodon custom_emojis_count = custom_emojis.count custom_emojis.destroy_all unless options[:dry_run] + Instance.refresh unless options[:dry_run] + say("Removed #{custom_emojis_count} custom emojis", :green) end @@ -83,7 +85,7 @@ module Mastodon processed = Concurrent::AtomicFixnum.new(0) failed = Concurrent::AtomicFixnum.new(0) start_at = Time.now.to_f - seed = start ? [start] : Account.remote.domains + seed = start ? [start] : Instance.pluck(:domain) blocked_domains = Regexp.new('\\.?' + DomainBlock.where(severity: 1).pluck(:domain).join('|') + '$') progress = create_progress_bar diff --git a/spec/controllers/admin/instances_controller_spec.rb b/spec/controllers/admin/instances_controller_spec.rb index 412b81443..8c0b309f2 100644 --- a/spec/controllers/admin/instances_controller_spec.rb +++ b/spec/controllers/admin/instances_controller_spec.rb @@ -9,10 +9,10 @@ RSpec.describe Admin::InstancesController, type: :controller do describe 'GET #index' do around do |example| - default_per_page = Account.default_per_page - Account.paginates_per 1 + default_per_page = Instance.default_per_page + Instance.paginates_per 1 example.run - Account.paginates_per default_per_page + Instance.paginates_per default_per_page end it 'renders instances' do diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 75f628076..1d000ed4d 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -440,13 +440,6 @@ RSpec.describe Account, type: :model do end end - describe '.domains' do - it 'returns domains' do - Fabricate(:account, domain: 'domain') - expect(Account.remote.domains).to match_array(['domain']) - end - end - describe '#statuses_count' do subject { Fabricate(:account) } @@ -737,20 +730,6 @@ RSpec.describe Account, type: :model do end end - describe 'by_domain_accounts' do - it 'returns accounts grouped by domain sorted by accounts' do - 2.times { Fabricate(:account, domain: 'example.com') } - Fabricate(:account, domain: 'example2.com') - - results = Account.where('id > 0').by_domain_accounts - expect(results.length).to eq 2 - expect(results.first.domain).to eq 'example.com' - expect(results.first.accounts_count).to eq 2 - expect(results.last.domain).to eq 'example2.com' - expect(results.last.accounts_count).to eq 1 - end - end - describe 'local' do it 'returns an array of accounts who do not have a domain' do account_1 = Fabricate(:account, domain: nil) -- cgit From 47e507fa61be6dc39dd9821e1d07c33e993cc246 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 14 Dec 2020 10:03:09 +0100 Subject: Add ability to require invite request text (#15326) Fixes #15273 Co-authored-by: Claire --- app/javascript/packs/admin.js | 26 ++++++++++++++++++++++++++ app/javascript/styles/mastodon/forms.scss | 15 ++++++++++----- app/models/form/admin_settings.rb | 2 ++ app/models/user.rb | 3 ++- app/views/about/_registration.html.haml | 2 +- app/views/admin/settings/edit.html.haml | 6 ++++++ app/views/auth/registrations/new.html.haml | 2 +- config/locales/en.yml | 3 +++ config/settings.yml | 1 + 9 files changed, 52 insertions(+), 8 deletions(-) (limited to 'app/views') diff --git a/app/javascript/packs/admin.js b/app/javascript/packs/admin.js index 1c44fb45b..8fd1b8a8e 100644 --- a/app/javascript/packs/admin.js +++ b/app/javascript/packs/admin.js @@ -67,10 +67,36 @@ const onEnableBootstrapTimelineAccountsChange = (target) => { delegate(document, '#form_admin_settings_enable_bootstrap_timeline_accounts', 'change', ({ target }) => onEnableBootstrapTimelineAccountsChange(target)); +const onChangeRegistrationMode = (target) => { + const enabled = target.value === 'approved'; + + [].forEach.call(document.querySelectorAll('#form_admin_settings_require_invite_text'), (input) => { + input.disabled = !enabled; + if (enabled) { + let element = input; + do { + element.classList.remove('disabled'); + element = element.parentElement; + } while (element && !element.classList.contains('fields-group')); + } else { + let element = input; + do { + element.classList.add('disabled'); + element = element.parentElement; + } while (element && !element.classList.contains('fields-group')); + } + }); +}; + +delegate(document, '#form_admin_settings_registrations_mode', 'change', ({ target }) => onChangeRegistrationMode(target)); + ready(() => { const domainBlockSeverityInput = document.getElementById('domain_block_severity'); if (domainBlockSeverityInput) onDomainBlockSeverityChange(domainBlockSeverityInput); const enableBootstrapTimelineAccounts = document.getElementById('form_admin_settings_enable_bootstrap_timeline_accounts'); if (enableBootstrapTimelineAccounts) onEnableBootstrapTimelineAccountsChange(enableBootstrapTimelineAccounts); + + const registrationMode = document.getElementById('form_admin_settings_registrations_mode'); + if (registrationMode) onChangeRegistrationMode(registrationMode); }); diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 92d89e6f2..e0604303b 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -377,11 +377,6 @@ code { box-shadow: none; } - &:focus:invalid:not(:placeholder-shown), - &:required:invalid:not(:placeholder-shown) { - border-color: lighten($error-red, 12%); - } - &:required:valid { border-color: $valid-value-color; } @@ -397,6 +392,16 @@ code { } } + input[type=text], + input[type=number], + input[type=email], + input[type=password] { + &:focus:invalid:not(:placeholder-shown), + &:required:invalid:not(:placeholder-shown) { + border-color: lighten($error-red, 12%); + } + } + .input.field_with_errors { label { color: lighten($error-red, 12%); diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index 390836f28..e9f78da21 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -35,6 +35,7 @@ class Form::AdminSettings show_domain_blocks show_domain_blocks_rationale noindex + require_invite_text ).freeze BOOLEAN_KEYS = %i( @@ -51,6 +52,7 @@ class Form::AdminSettings trends trendable_by_default noindex + require_invite_text ).freeze UPLOAD_KEYS = %i( diff --git a/app/models/user.rb b/app/models/user.rb index 981dc6d47..6088f1994 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,7 +82,8 @@ class User < ApplicationRecord has_many :webauthn_credentials, 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? } + accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text } + validates :invite_request, presence: true, on: :create, if: -> { Setting.require_invite_text } validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? validates_with BlacklistedEmailValidator, on: :create diff --git a/app/views/about/_registration.html.haml b/app/views/about/_registration.html.haml index 6160ca4d4..e4d614d71 100644 --- a/app/views/about/_registration.html.haml +++ b/app/views/about/_registration.html.haml @@ -16,7 +16,7 @@ - if approved_registrations? .fields-group = f.simple_fields_for :invite_request do |invite_request_fields| - = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: false + = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: Setting.require_invite_text .fields-group = f.input :agreement, as: :boolean, wrapper: :with_label, label: t('auth.checkbox_agreement_html', rules_path: about_more_path, terms_path: terms_path), required: true, disabled: closed_registrations? diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 9e28766b1..a162490b5 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -43,6 +43,12 @@ %hr.spacer/ + .fields-group + = f.input :require_invite_text, as: :boolean, wrapper: :with_label, label: t('admin.settings.registrations.require_invite_text.title'), hint: t('admin.settings.registrations.require_invite_text.desc_html'), disabled: !approved_registrations? + .fields-group + + %hr.spacer/ + .fields-group = f.input :enable_bootstrap_timeline_accounts, as: :boolean, wrapper: :with_label, label: t('admin.settings.enable_bootstrap_timeline_accounts.title') .fields-group diff --git a/app/views/auth/registrations/new.html.haml b/app/views/auth/registrations/new.html.haml index de541847f..6981195ed 100644 --- a/app/views/auth/registrations/new.html.haml +++ b/app/views/auth/registrations/new.html.haml @@ -31,7 +31,7 @@ - if approved_registrations? && !@invite.present? .fields-group = f.simple_fields_for :invite_request, resource.invite_request || resource.build_invite_request do |invite_request_fields| - = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: false + = invite_request_fields.input :text, as: :text, wrapper: :with_block_label, required: Setting.require_invite_text = f.input :invite_code, as: :hidden diff --git a/config/locales/en.yml b/config/locales/en.yml index f89f50e4d..114f86fd1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -586,6 +586,9 @@ en: min_invite_role: disabled: No one title: Allow invitations by + require_invite_text: + desc_html: When registrations require manual approval, make the “Why do you want to join?” invite request text mandatory rather than optional + title: Require new users to fill an invite request text registrations_mode: modes: approved: Approval required for sign up diff --git a/config/settings.yml b/config/settings.yml index 217745f28..9cf68a096 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -70,6 +70,7 @@ defaults: &defaults spam_check_enabled: true show_domain_blocks: 'disabled' show_domain_blocks_rationale: 'disabled' + require_invite_text: false development: <<: *defaults -- cgit From 1390cc194bd07b85fe36e31a0cab22981315974d Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 15 Dec 2020 04:30:15 +0100 Subject: Add indication to admin UI of whether a report has been forwarded (#13237) * Add indication to admin UI of whether a report has been forwarded * Rework how forwarded status is displayed Co-authored-by: Claire --- app/models/report.rb | 1 + app/services/report_service.rb | 3 ++- app/views/admin/reports/index.html.haml | 4 ++++ app/views/admin/reports/show.html.haml | 10 ++++++++++ config/locales/en.yml | 2 ++ db/migrate/20200309150742_add_forwarded_to_reports.rb | 5 +++++ db/schema.rb | 1 + 7 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20200309150742_add_forwarded_to_reports.rb (limited to 'app/views') diff --git a/app/models/report.rb b/app/models/report.rb index f31bcfd2e..cd08120e4 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -14,6 +14,7 @@ # target_account_id :bigint(8) not null # assigned_account_id :bigint(8) # uri :string +# forwarded :boolean # class Report < ApplicationRecord diff --git a/app/services/report_service.rb b/app/services/report_service.rb index 1e955c1e7..9d9c7d6c9 100644 --- a/app/services/report_service.rb +++ b/app/services/report_service.rb @@ -24,7 +24,8 @@ class ReportService < BaseService target_account: @target_account, status_ids: @status_ids, comment: @comment, - uri: @options[:uri] + uri: @options[:uri], + forwarded: ActiveModel::Type::Boolean.new.cast(@options[:forward]) ) end diff --git a/app/views/admin/reports/index.html.haml b/app/views/admin/reports/index.html.haml index bb441380e..721c55f71 100644 --- a/app/views/admin/reports/index.html.haml +++ b/app/views/admin/reports/index.html.haml @@ -59,6 +59,10 @@ = fa_icon('camera') = report.media_attachments.count + - if report.forwarded? + · + = t('admin.reports.forwarded_to', domain: target_account.domain) + .report-card__summary__item__assigned - if report.assigned_account.present? = admin_account_link_to report.assigned_account diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 2681419ca..b060c553f 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -46,6 +46,16 @@ %td{ colspan: 2 } - if @report.action_taken? = table_link_to 'envelope-open', t('admin.reports.reopen'), admin_report_path(@report, outcome: 'reopen'), method: :put + - unless @report.target_account.local? + %tr + %th= t('admin.reports.forwarded') + %td{ colspan: 3 } + - if @report.forwarded.nil? + \- + - elsif @report.forwarded? + = t('simple_form.yes') + - else + = t('simple_form.no') - if !@report.action_taken_by_account.nil? %tr %th= t('admin.reports.action_taken_by') diff --git a/config/locales/en.yml b/config/locales/en.yml index 114f86fd1..3ff01ac86 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -517,6 +517,8 @@ en: comment: none: None created_at: Reported + forwarded: Forwarded + forwarded_to: Forwarded to %{domain} mark_as_resolved: Mark as resolved mark_as_unresolved: Mark as unresolved notes: diff --git a/db/migrate/20200309150742_add_forwarded_to_reports.rb b/db/migrate/20200309150742_add_forwarded_to_reports.rb new file mode 100644 index 000000000..df278240b --- /dev/null +++ b/db/migrate/20200309150742_add_forwarded_to_reports.rb @@ -0,0 +1,5 @@ +class AddForwardedToReports < ActiveRecord::Migration[5.2] + def change + add_column :reports, :forwarded, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 2f9d369be..55822a4b3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -717,6 +717,7 @@ ActiveRecord::Schema.define(version: 2020_12_06_004238) do t.bigint "target_account_id", null: false t.bigint "assigned_account_id" t.string "uri" + t.boolean "forwarded" t.index ["account_id"], name: "index_reports_on_account_id" t.index ["target_account_id"], name: "index_reports_on_target_account_id" end -- cgit From 75d2762fdf600de6785f612caf182b32064a9bf6 Mon Sep 17 00:00:00 2001 From: Mashiro Date: Tue, 15 Dec 2020 13:28:14 +0800 Subject: Add "invite request content" display in user account admin page (#15265) * feat: display `invite_request_text` in admin's user account page * fix: move invite_request to the bottom of accounts page * fix: remove time display, remove formate, change code terminology * fix: remove escape --- app/views/admin/accounts/show.html.haml | 10 ++++++++++ config/locales/en.yml | 1 + 2 files changed, 11 insertions(+) (limited to 'app/views') diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index d5978eddd..ae527cc23 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -242,3 +242,13 @@ .actions = f.button :button, t('admin.account_moderation_notes.create'), type: :submit + + %hr.spacer/ + + - if @account.user&.invite_request&.text&.present? + %div.speech-bubble + %div.speech-bubble__bubble + = @account.user&.invite_request&.text + %div.speech-bubble__owner + = admin_account_link_to @account + = t('admin.accounts.invite_request_text') diff --git a/config/locales/en.yml b/config/locales/en.yml index 3ff01ac86..1f5798fcc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -131,6 +131,7 @@ en: follows: Follows header: Header inbox_url: Inbox URL + invite_request_text: Reasons for joining invited_by: Invited by ip: IP joined: Joined -- cgit From 1f564051b6b447e3663852c482982b3ff5a2f238 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Dec 2020 12:56:43 +0100 Subject: Change RTL detection to rely on unicode-bidi paragraph by paragraph (#14573) --- app/helpers/statuses_helper.rb | 20 -------------- .../mastodon/components/autosuggest_input.js | 8 +----- .../mastodon/components/autosuggest_textarea.js | 8 +----- .../mastodon/components/status_content.js | 18 ++++-------- .../features/compose/components/reply_indicator.js | 6 +--- app/javascript/mastodon/rtl.js | 32 ---------------------- app/javascript/styles/mailer.scss | 12 +++++++- app/javascript/styles/mastodon/components.scss | 2 ++ app/views/notification_mailer/_status.html.haml | 4 +-- app/views/statuses/_detailed_status.html.haml | 2 +- app/views/statuses/_simple_status.html.haml | 2 +- spec/helpers/statuses_helper_spec.rb | 18 ------------ 12 files changed, 26 insertions(+), 106 deletions(-) delete mode 100644 app/javascript/mastodon/rtl.js (limited to 'app/views') diff --git a/app/helpers/statuses_helper.rb b/app/helpers/statuses_helper.rb index daed9048f..1f654f34f 100644 --- a/app/helpers/statuses_helper.rb +++ b/app/helpers/statuses_helper.rb @@ -92,22 +92,6 @@ module StatusesHelper end end - def rtl_status?(status) - status.local? ? rtl?(status.text) : rtl?(strip_tags(status.text)) - end - - def rtl?(text) - text = simplified_text(text) - rtl_words = text.scan(/[\p{Hebrew}\p{Arabic}\p{Syriac}\p{Thaana}\p{Nko}]+/m) - - if rtl_words.present? - total_size = text.size.to_f - rtl_size(rtl_words) / total_size > 0.3 - else - false - end - end - def fa_visibility_icon(status) case status.visibility when 'public' @@ -143,10 +127,6 @@ module StatusesHelper end end - def rtl_size(words) - words.reduce(0) { |acc, elem| acc + elem.size }.to_f - end - def embedded_view? params[:controller] == EMBEDDED_CONTROLLER && params[:action] == EMBEDDED_ACTION end diff --git a/app/javascript/mastodon/components/autosuggest_input.js b/app/javascript/mastodon/components/autosuggest_input.js index 6d2035add..5187f95c8 100644 --- a/app/javascript/mastodon/components/autosuggest_input.js +++ b/app/javascript/mastodon/components/autosuggest_input.js @@ -4,7 +4,6 @@ import AutosuggestEmoji from './autosuggest_emoji'; import AutosuggestHashtag from './autosuggest_hashtag'; import ImmutablePropTypes from 'react-immutable-proptypes'; import PropTypes from 'prop-types'; -import { isRtl } from '../rtl'; import ImmutablePureComponent from 'react-immutable-pure-component'; import classNames from 'classnames'; import { List as ImmutableList } from 'immutable'; @@ -189,11 +188,6 @@ export default class AutosuggestInput extends ImmutablePureComponent { render () { const { value, suggestions, disabled, placeholder, onKeyUp, autoFocus, className, id, maxLength } = this.props; const { suggestionsHidden } = this.state; - const style = { direction: 'ltr' }; - - if (isRtl(value)) { - style.direction = 'rtl'; - } return (
@@ -212,7 +206,7 @@ export default class AutosuggestInput extends ImmutablePureComponent { onKeyUp={onKeyUp} onFocus={this.onFocus} onBlur={this.onBlur} - style={style} + dir='auto' aria-autocomplete='list' id={id} className={className} diff --git a/app/javascript/mastodon/components/autosuggest_textarea.js b/app/javascript/mastodon/components/autosuggest_textarea.js index 58ec4f6eb..08b9cd80b 100644 --- a/app/javascript/mastodon/components/autosuggest_textarea.js +++ b/app/javascript/mastodon/components/autosuggest_textarea.js @@ -4,7 +4,6 @@ import AutosuggestEmoji from './autosuggest_emoji'; import AutosuggestHashtag from './autosuggest_hashtag'; import ImmutablePropTypes from 'react-immutable-proptypes'; import PropTypes from 'prop-types'; -import { isRtl } from '../rtl'; import ImmutablePureComponent from 'react-immutable-pure-component'; import Textarea from 'react-textarea-autosize'; import classNames from 'classnames'; @@ -195,11 +194,6 @@ export default class AutosuggestTextarea extends ImmutablePureComponent { render () { const { value, suggestions, disabled, placeholder, onKeyUp, autoFocus, children } = this.props; const { suggestionsHidden } = this.state; - const style = { direction: 'ltr' }; - - if (isRtl(value)) { - style.direction = 'rtl'; - } return [
@@ -220,7 +214,7 @@ export default class AutosuggestTextarea extends ImmutablePureComponent { onFocus={this.onFocus} onBlur={this.onBlur} onPaste={this.onPaste} - style={style} + dir='auto' aria-autocomplete='list' /> diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js index 3200f2d82..185a2a663 100644 --- a/app/javascript/mastodon/components/status_content.js +++ b/app/javascript/mastodon/components/status_content.js @@ -1,7 +1,6 @@ import React from 'react'; import ImmutablePropTypes from 'react-immutable-proptypes'; import PropTypes from 'prop-types'; -import { isRtl } from '../rtl'; import { FormattedMessage } from 'react-intl'; import Permalink from './permalink'; import classnames from 'classnames'; @@ -186,17 +185,12 @@ export default class StatusContent extends React.PureComponent { const content = { __html: status.get('contentHtml') }; const spoilerContent = { __html: status.get('spoilerHtml') }; - const directionStyle = { direction: 'ltr' }; const classNames = classnames('status__content', { 'status__content--with-action': this.props.onClick && this.context.router, 'status__content--with-spoiler': status.get('spoiler_text').length > 0, 'status__content--collapsed': renderReadMore, }); - if (isRtl(status.get('search_index'))) { - directionStyle.direction = 'rtl'; - } - const showThreadButton = (