From 00d988955f63551e86dd33ca1a26f73f7d0c7b45 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 23 Mar 2019 02:23:48 +0100 Subject: If registrations have been re-opened when user confirms account, approve (#10349) --- app/models/user.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index 9d0d49676..d703f9588 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -124,7 +124,8 @@ class User < ApplicationRecord end def confirm - new_user = !confirmed? + new_user = !confirmed? + self.approved = true if open_registrations? super @@ -136,7 +137,8 @@ class User < ApplicationRecord end def confirm! - new_user = !confirmed? + new_user = !confirmed? + self.approved = true if open_registrations? skip_confirmation! save! @@ -264,7 +266,11 @@ class User < ApplicationRecord private def set_approved - self.approved = Setting.registrations_mode == 'open' || invited? + self.approved = open_registrations? || invited? + end + + def open_registrations? + Setting.registrations_mode == 'open' end def sanitize_languages -- cgit From 555c4e11baf58401c1bdd915e4ecef679e6ae514 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 23 Mar 2019 14:07:04 +0100 Subject: Add validations to admin settings (#10348) * Add validations to admin settings - Validate correct HTML markup - Validate presence of contact username & e-mail - Validate that all usernames are valid - Validate that enums have expected values * Fix code style issue * Fix tests --- app/controllers/admin/settings_controller.rb | 73 ++---------- app/models/form/admin_settings.rb | 131 ++++++++++++++------- app/validators/existing_username_validator.rb | 20 ++++ app/validators/html_validator.rb | 14 +++ app/views/admin/settings/edit.html.haml | 1 + config/locales/en.yml | 5 + config/navigation.rb | 2 +- spec/controllers/admin/settings_controller_spec.rb | 4 + 8 files changed, 140 insertions(+), 110 deletions(-) create mode 100644 app/validators/existing_username_validator.rb create mode 100644 app/validators/html_validator.rb (limited to 'app/models') diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index a763597f2..dc1c79b7f 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -2,84 +2,29 @@ module Admin class SettingsController < BaseController - ADMIN_SETTINGS = %w( - site_contact_username - site_contact_email - site_title - site_short_description - site_description - site_extended_description - site_terms - registrations_mode - closed_registrations_message - open_deletion - timeline_preview - show_staff_badge - bootstrap_timeline_accounts - theme - thumbnail - hero - mascot - min_invite_role - activity_api_enabled - peers_api_enabled - show_known_fediverse_at_about_page - preview_sensitive_media - custom_css - profile_directory - ).freeze - - BOOLEAN_SETTINGS = %w( - open_deletion - timeline_preview - show_staff_badge - activity_api_enabled - peers_api_enabled - show_known_fediverse_at_about_page - preview_sensitive_media - profile_directory - ).freeze - - UPLOAD_SETTINGS = %w( - thumbnail - hero - mascot - ).freeze - def edit authorize :settings, :show? + @admin_settings = Form::AdminSettings.new end def update authorize :settings, :update? - settings_params.each do |key, value| - if UPLOAD_SETTINGS.include?(key) - upload = SiteUpload.where(var: key).first_or_initialize(var: key) - upload.update(file: value) - else - setting = Setting.where(var: key).first_or_initialize(var: key) - setting.update(value: value_for_update(key, value)) - end - end + @admin_settings = Form::AdminSettings.new(settings_params) - flash[:notice] = I18n.t('generic.changes_saved_msg') - redirect_to edit_admin_settings_path + if @admin_settings.save + flash[:notice] = I18n.t('generic.changes_saved_msg') + redirect_to edit_admin_settings_path + else + render :edit + end end private def settings_params - params.require(:form_admin_settings).permit(ADMIN_SETTINGS) - end - - def value_for_update(key, value) - if BOOLEAN_SETTINGS.include?(key) - value == '1' - else - value - end + params.require(:form_admin_settings).permit(*Form::AdminSettings::KEYS) end end end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index a21394a52..2d3aa726d 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -3,49 +3,90 @@ class Form::AdminSettings include ActiveModel::Model - delegate( - :site_contact_username, - :site_contact_username=, - :site_contact_email, - :site_contact_email=, - :site_title, - :site_title=, - :site_short_description, - :site_short_description=, - :site_description, - :site_description=, - :site_extended_description, - :site_extended_description=, - :site_terms, - :site_terms=, - :registrations_mode, - :registrations_mode=, - :closed_registrations_message, - :closed_registrations_message=, - :open_deletion, - :open_deletion=, - :timeline_preview, - :timeline_preview=, - :show_staff_badge, - :show_staff_badge=, - :bootstrap_timeline_accounts, - :bootstrap_timeline_accounts=, - :theme, - :theme=, - :min_invite_role, - :min_invite_role=, - :activity_api_enabled, - :activity_api_enabled=, - :peers_api_enabled, - :peers_api_enabled=, - :show_known_fediverse_at_about_page, - :show_known_fediverse_at_about_page=, - :preview_sensitive_media, - :preview_sensitive_media=, - :custom_css, - :custom_css=, - :profile_directory, - :profile_directory=, - to: Setting - ) + KEYS = %i( + site_contact_username + site_contact_email + site_title + site_short_description + site_description + site_extended_description + site_terms + registrations_mode + closed_registrations_message + open_deletion + timeline_preview + show_staff_badge + bootstrap_timeline_accounts + theme + min_invite_role + activity_api_enabled + peers_api_enabled + show_known_fediverse_at_about_page + preview_sensitive_media + custom_css + profile_directory + ).freeze + + BOOLEAN_KEYS = %i( + open_deletion + timeline_preview + show_staff_badge + activity_api_enabled + peers_api_enabled + show_known_fediverse_at_about_page + preview_sensitive_media + profile_directory + ).freeze + + UPLOAD_KEYS = %i( + thumbnail + hero + mascot + ).freeze + + attr_accessor(*KEYS) + + validates :site_short_description, :site_description, :site_extended_description, :site_terms, :closed_registrations_message, html: true + validates :registrations_mode, inclusion: { in: %w(open approved none) } + validates :min_invite_role, inclusion: { in: %w(disabled user moderator admin) } + validates :site_contact_email, :site_contact_username, presence: true + validates :site_contact_username, existing_username: true + validates :bootstrap_timeline_accounts, existing_username: { multiple: true } + + def initialize(_attributes = {}) + super + initialize_attributes + end + + def save + return false unless valid? + + KEYS.each do |key| + value = instance_variable_get("@#{key}") + + if UPLOAD_KEYS.include?(key) + upload = SiteUpload.where(var: key).first_or_initialize(var: key) + upload.update(file: value) + else + setting = Setting.where(var: key).first_or_initialize(var: key) + setting.update(value: typecast_value(key, value)) + end + end + end + + private + + def initialize_attributes + KEYS.each do |key| + instance_variable_set("@#{key}", Setting.public_send(key)) if instance_variable_get("@#{key}").nil? + end + end + + def typecast_value(key, value) + if BOOLEAN_KEYS.include?(key) + value == '1' + else + value + end + end end diff --git a/app/validators/existing_username_validator.rb b/app/validators/existing_username_validator.rb new file mode 100644 index 000000000..4388a0c98 --- /dev/null +++ b/app/validators/existing_username_validator.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class ExistingUsernameValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if value.blank? + + if options[:multiple] + missing_usernames = value.split(',').map { |username| username unless Account.find_local(username) }.compact + record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: missing_usernames.join(', '))) if missing_usernames.any? + else + record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) unless Account.find_local(value) + end + end + + private + + def valid_html?(str) + Nokogiri::HTML.fragment(str).to_s == str + end +end diff --git a/app/validators/html_validator.rb b/app/validators/html_validator.rb new file mode 100644 index 000000000..882c35d41 --- /dev/null +++ b/app/validators/html_validator.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class HtmlValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if value.blank? + record.errors.add(attribute, I18n.t('html_validator.invalid_markup')) unless valid_html?(value) + end + + private + + def valid_html?(str) + Nokogiri::HTML.fragment(str).to_s == str + end +end diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index d9b4bf01b..1c2c00f10 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -2,6 +2,7 @@ = t('admin.settings.title') = simple_form_for @admin_settings, url: admin_settings_path, html: { method: :patch } do |f| + = render 'shared/error_messages', object: @admin_settings .fields-group = f.input :site_title, wrapper: :with_label, label: t('admin.settings.site_title') diff --git a/config/locales/en.yml b/config/locales/en.yml index ba42e7ce1..d5ed20623 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -586,6 +586,9 @@ en: content: We're sorry, but something went wrong on our end. title: This page is not correct noscript_html: To use the Mastodon web application, please enable JavaScript. Alternatively, try one of the native apps for Mastodon for your platform. + existing_username_validator: + not_found: could not find a local user with that username + not_found_multiple: could not find %{usernames} exports: archive_takeout: date: Date @@ -633,6 +636,8 @@ en: validation_errors: one: Something isn't quite right yet! Please review the error below other: Something isn't quite right yet! Please review %{count} errors below + html_validator: + invalid_markup: contains invalid HTML markup identity_proofs: active: Active authorize: Yes, authorize diff --git a/config/navigation.rb b/config/navigation.rb index 07aec4b9d..f136141b3 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -37,7 +37,7 @@ SimpleNavigation::Configuration.run do |navigation| primary.item :admin, safe_join([fa_icon('cogs fw'), t('admin.title')]), admin_dashboard_url, if: proc { current_user.staff? } do |admin| admin.item :dashboard, safe_join([fa_icon('tachometer fw'), t('admin.dashboard.title')]), admin_dashboard_url - admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), edit_admin_settings_url, if: -> { current_user.admin? } + admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), edit_admin_settings_url, if: -> { current_user.admin? }, highlights_on: %r{/admin/settings} admin.item :custom_emojis, safe_join([fa_icon('smile-o fw'), t('admin.custom_emojis.title')]), admin_custom_emojis_url, highlights_on: %r{/admin/custom_emojis} admin.item :relays, safe_join([fa_icon('exchange fw'), t('admin.relays.title')]), admin_relays_url, if: -> { current_user.admin? }, highlights_on: %r{/admin/relays} admin.item :subscriptions, safe_join([fa_icon('paper-plane-o fw'), t('admin.subscriptions.title')]), admin_subscriptions_url, if: -> { current_user.admin? } diff --git a/spec/controllers/admin/settings_controller_spec.rb b/spec/controllers/admin/settings_controller_spec.rb index 34f6bbdae..6cf0ee20a 100644 --- a/spec/controllers/admin/settings_controller_spec.rb +++ b/spec/controllers/admin/settings_controller_spec.rb @@ -19,6 +19,10 @@ RSpec.describe Admin::SettingsController, type: :controller do end describe 'PUT #update' do + before do + allow_any_instance_of(Form::AdminSettings).to receive(:valid?).and_return(true) + end + describe 'for a record that doesnt exist' do around do |example| before = Setting.site_extended_description -- cgit From 45b849bad98ffad9ce9ce39f1aa984ca4b7329f9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 24 Mar 2019 12:36:26 +0100 Subject: Fix MergeWorker being queued for remote users (#10355) --- app/models/follow_request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index c5451a050..96ac7eaa5 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -26,7 +26,7 @@ class FollowRequest < ApplicationRecord def authorize! account.follow!(target_account, reblogs: show_reblogs, uri: uri) - MergeWorker.perform_async(target_account.id, account.id) + MergeWorker.perform_async(target_account.id, account.id) if account.local? destroy! end -- cgit From 94e5e834f2ddbc791ab27e2ef17eb2f23140a6ba Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 26 Mar 2019 00:36:35 +0100 Subject: Improve performance of list of blocked domains by caching counts (#10374) --- app/models/instance.rb | 6 +++++- app/views/admin/instances/index.html.haml | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/instance.rb b/app/models/instance.rb index 7448d465c..7bf000d40 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -7,7 +7,7 @@ class Instance def initialize(resource) @domain = resource.domain - @accounts_count = resource.accounts_count + @accounts_count = resource.is_a?(DomainBlock) ? nil : resource.accounts_count @domain_block = resource.is_a?(DomainBlock) ? resource : DomainBlock.find_by(domain: domain) end @@ -15,6 +15,10 @@ class Instance Rails.cache.fetch("#{cache_key}/sample_accounts", expires_in: 12.hours) { Account.where(domain: domain).searchable.joins(:account_stat).popular.limit(3) } end + def cached_accounts_count + @accounts_count || Rails.cache.fetch("#{cache_key}/count", expires_in: 12.hours) { Account.where(domain: domain).count } + end + def to_param domain end diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index 235927140..9574c3147 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -33,7 +33,7 @@ %h4 = instance.domain %small - = t('admin.instances.known_accounts', count: instance.accounts_count) + = t('admin.instances.known_accounts', count: instance.cached_accounts_count) - if instance.domain_block - if !instance.domain_block.noop? -- cgit From e11796432514afb49f3d891f805973a37f00fcf1 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 26 Mar 2019 01:24:19 +0100 Subject: Change icons of features on admin dashboard to remove bias (#10366) Red crosses implied that it was bad/unexpected that certain features were not enabled. In reality, they are options, so showing a green or grey power-off icon is more appropriate. Add status of timeline preview as well Fix sample accounts changing too frequently due to wrong query Sample accounts are intended to be sorted by popularity --- app/controllers/admin/dashboard_controller.rb | 1 + app/controllers/directories_controller.rb | 2 +- app/helpers/admin/dashboard_helper.rb | 10 +++ app/javascript/styles/mastodon/admin.scss | 5 ++ app/models/account.rb | 2 +- app/views/admin/dashboard/index.html.haml | 94 +++++++-------------------- config/locales/en.yml | 1 + 7 files changed, 43 insertions(+), 72 deletions(-) create mode 100644 app/helpers/admin/dashboard_helper.rb (limited to 'app/models') diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 22bbcec19..f23ed1508 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -29,6 +29,7 @@ module Admin @hidden_service = ENV['ALLOW_ACCESS_TO_HIDDEN_SERVICE'] == 'true' @trending_hashtags = TrendingTags.get(7) @profile_directory = Setting.profile_directory + @timeline_preview = Setting.timeline_preview end private diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb index ff7ff4a42..594907674 100644 --- a/app/controllers/directories_controller.rb +++ b/app/controllers/directories_controller.rb @@ -32,7 +32,7 @@ class DirectoriesController < ApplicationController end def set_accounts - @accounts = Account.discoverable.page(params[:page]).per(40).tap do |query| + @accounts = Account.discoverable.by_recent_status.page(params[:page]).per(40).tap do |query| query.merge!(Account.tagged_with(@tag.id)) if @tag end end diff --git a/app/helpers/admin/dashboard_helper.rb b/app/helpers/admin/dashboard_helper.rb new file mode 100644 index 000000000..4ee2cdef4 --- /dev/null +++ b/app/helpers/admin/dashboard_helper.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Admin::DashboardHelper + def feature_hint(feature, enabled) + indicator = safe_join([enabled ? t('simple_form.yes') : t('simple_form.no'), fa_icon('power-off fw')], ' ') + class_names = enabled ? 'pull-right positive-hint' : 'pull-right neutral-hint' + + safe_join([feature, content_tag(:span, indicator, class: class_names)]) + end +end diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index f6bfe44cf..fd5c08f04 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -220,6 +220,11 @@ $content-width: 840px; color: $error-value-color; font-weight: 500; } + + .neutral-hint { + color: $dark-text-color; + font-weight: 500; + } } @media screen and (max-width: $no-columns-breakpoint) { diff --git a/app/models/account.rb b/app/models/account.rb index c2a0709f9..51e01246e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -94,7 +94,7 @@ class Account < ApplicationRecord scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } - scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat).where(AccountStat.arel_table[:followers_count].gteq(MIN_FOLLOWERS_DISCOVERY)).by_recent_status } + scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat).where(AccountStat.arel_table[:followers_count].gteq(MIN_FOLLOWERS_DISCOVERY)) } scope :tagged_with, ->(tag) { joins(:accounts_tags).where(accounts_tags: { tag_id: tag }) } scope :by_recent_status, -> { order(Arel.sql('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc')) } scope :popular, -> { order('account_stats.followers_count desc') } diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index fa3d70e9e..d448e3862 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -40,35 +40,17 @@ %h4= t 'admin.dashboard.features' %ul %li - = link_to t('admin.dashboard.feature_registrations'), edit_admin_settings_path - - if @registrations_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = link_to t('admin.dashboard.feature_invites'), edit_admin_settings_path - - if @invites_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = link_to t('admin.dashboard.feature_deletions'), edit_admin_settings_path - - if @deletions_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = link_to t('admin.dashboard.feature_profile_directory'), edit_admin_settings_path - - if @profile_directory - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = link_to t('admin.dashboard.feature_relay'), admin_relays_path - - if @relay_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' + = feature_hint(link_to(t('admin.dashboard.feature_registrations'), edit_admin_settings_path), @registrations_enabled) + %li + = feature_hint(link_to(t('admin.dashboard.feature_invites'), edit_admin_settings_path), @invites_enabled) + %li + = feature_hint(link_to(t('admin.dashboard.feature_deletions'), edit_admin_settings_path), @deletions_enabled) + %li + = feature_hint(link_to(t('admin.dashboard.feature_profile_directory'), edit_admin_settings_path), @profile_directory) + %li + = feature_hint(link_to(t('admin.dashboard.feature_timeline_preview'), edit_admin_settings_path), @timeline_preview) + %li + = feature_hint(link_to(t('admin.dashboard.feature_relay'), admin_relays_path), @relay_enabled) .dashboard__widgets__versions %div @@ -103,47 +85,19 @@ %h4= t 'admin.dashboard.config' %ul %li - = t('admin.dashboard.search') - - if @search_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = t('admin.dashboard.single_user_mode') - - if @single_user_mode - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - LDAP - - if @ldap_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - CAS - - if @cas_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - SAML - - if @saml_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - PAM - - if @pam_enabled - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' - %li - = t 'admin.dashboard.hidden_service' - - if @hidden_service - %span.pull-right.positive-hint= fa_icon 'check fw' - - else - %span.pull-right.negative-hint= fa_icon 'times fw' + = feature_hint(t('admin.dashboard.search'), @search_enabled) + %li + = feature_hint(t('admin.dashboard.single_user_mode'), @single_user_mode) + %li + = feature_hint('LDAP', @ldap_enabled) + %li + = feature_hint('CAS', @cas_enabled) + %li + = feature_hint('SAML', @saml_enabled) + %li + = feature_hint('PAM', @pam_enabled) + %li + = feature_hint(t('admin.dashboard.hidden_service'), @hidden_service) .dashboard__widgets__trends %div diff --git a/config/locales/en.yml b/config/locales/en.yml index b0bf2539c..ad1332fd2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -245,6 +245,7 @@ en: feature_profile_directory: Profile directory feature_registrations: Registrations feature_relay: Federation relay + feature_timeline_preview: Timeline preview features: Features hidden_service: Federation with hidden services open_reports: open reports -- cgit