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 --- 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 + 7 files changed, 88 insertions(+), 29 deletions(-) create mode 100644 app/models/concerns/domain_materializable.rb (limited to 'app/models') 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 -- 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/models') 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/models') 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