From cb27d8999798d96c17f84a705639fc59f9d12d14 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 2 Nov 2022 16:34:47 +0100 Subject: Change migration to migrate admins to Owner role rather than Admin role (#19671) --- db/post_migrate/20220617202502_migrate_roles.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'db') diff --git a/db/post_migrate/20220617202502_migrate_roles.rb b/db/post_migrate/20220617202502_migrate_roles.rb index b7a7b2201..950699d9c 100644 --- a/db/post_migrate/20220617202502_migrate_roles.rb +++ b/db/post_migrate/20220617202502_migrate_roles.rb @@ -9,18 +9,19 @@ class MigrateRoles < ActiveRecord::Migration[5.2] def up load Rails.root.join('db', 'seeds', '03_roles.rb') - admin_role = UserRole.find_by(name: 'Admin') + owner_role = UserRole.find_by(name: 'Owner') moderator_role = UserRole.find_by(name: 'Moderator') - User.where(admin: true).in_batches.update_all(role_id: admin_role.id) + User.where(admin: true).in_batches.update_all(role_id: owner_role.id) User.where(moderator: true).in_batches.update_all(role_id: moderator_role.id) end def down admin_role = UserRole.find_by(name: 'Admin') + owner_role = UserRole.find_by(name: 'Owner') moderator_role = UserRole.find_by(name: 'Moderator') - User.where(role_id: admin_role.id).in_batches.update_all(admin: true) if admin_role + User.where(role_id: [admin_role.id, owner_role.id]).in_batches.update_all(admin: true) if admin_role User.where(role_id: moderator_role.id).in_batches.update_all(moderator: true) if moderator_role end end -- cgit From 1dca08b76f25d15365127ded37202d783a50e298 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 3 Nov 2022 16:06:42 +0100 Subject: Fix admin action logs page (#19649) * Add tests * Fix crash when trying to display orphaned action logs * Add migration for older admin action logs --- app/helpers/admin/action_logs_helper.rb | 20 ++- .../20221101190723_backfill_admin_action_logs.rb | 150 +++++++++++++++++++++ db/schema.rb | 2 +- lib/tasks/tests.rake | 51 +++++-- .../admin/action_logs_controller_spec.rb | 13 ++ 5 files changed, 221 insertions(+), 15 deletions(-) create mode 100644 db/post_migrate/20221101190723_backfill_admin_action_logs.rb (limited to 'db') diff --git a/app/helpers/admin/action_logs_helper.rb b/app/helpers/admin/action_logs_helper.rb index fd1977ac5..215ecea0d 100644 --- a/app/helpers/admin/action_logs_helper.rb +++ b/app/helpers/admin/action_logs_helper.rb @@ -4,15 +4,19 @@ module Admin::ActionLogsHelper def log_target(log) case log.target_type when 'Account' - link_to log.human_identifier, admin_account_path(log.target_id) + link_to (log.human_identifier.presence || I18n.t('admin.action_logs.deleted_account')), admin_account_path(log.target_id) when 'User' - link_to log.human_identifier, admin_account_path(log.route_param) + if log.route_param.present? + link_to log.human_identifier, admin_account_path(log.route_param) + else + I18n.t('admin.action_logs.deleted_account') + end when 'UserRole' link_to log.human_identifier, admin_roles_path(log.target_id) when 'Report' - link_to "##{log.human_identifier}", admin_report_path(log.target_id) + link_to "##{log.human_identifier.presence || log.target_id}", admin_report_path(log.target_id) when 'DomainBlock', 'DomainAllow', 'EmailDomainBlock', 'UnavailableDomain' - link_to log.human_identifier, "https://#{log.human_identifier}" + link_to log.human_identifier, "https://#{log.human_identifier.presence}" when 'Status' link_to log.human_identifier, log.permalink when 'AccountWarning' @@ -22,9 +26,13 @@ module Admin::ActionLogsHelper when 'IpBlock', 'Instance', 'CustomEmoji' log.human_identifier when 'CanonicalEmailBlock' - content_tag(:samp, log.human_identifier[0...7], title: log.human_identifier) + content_tag(:samp, (log.human_identifier.presence || '')[0...7], title: log.human_identifier) when 'Appeal' - link_to log.human_identifier, disputes_strike_path(log.route_param) + if log.route_param.present? + link_to log.human_identifier, disputes_strike_path(log.route_param.presence) + else + I18n.t('admin.action_logs.deleted_account') + end end end end diff --git a/db/post_migrate/20221101190723_backfill_admin_action_logs.rb b/db/post_migrate/20221101190723_backfill_admin_action_logs.rb new file mode 100644 index 000000000..9a64d1715 --- /dev/null +++ b/db/post_migrate/20221101190723_backfill_admin_action_logs.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +class BackfillAdminActionLogs < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + class Account < ApplicationRecord + # Dummy class, to make migration possible across version changes + has_one :user, inverse_of: :account + + def local? + domain.nil? + end + + def acct + local? ? username : "#{username}@#{domain}" + end + end + + class User < ApplicationRecord + # Dummy class, to make migration possible across version changes + belongs_to :account + end + + class Status < ApplicationRecord + include RoutingHelper + + # Dummy class, to make migration possible across version changes + belongs_to :account + + def local? + attributes['local'] || attributes['uri'].nil? + end + + def uri + local? ? activity_account_status_url(account, self) : attributes['uri'] + end + end + + class DomainBlock < ApplicationRecord; end + class DomainAllow < ApplicationRecord; end + class EmailDomainBlock < ApplicationRecord; end + class UnavailableDomain < ApplicationRecord; end + + class AccountWarning < ApplicationRecord + # Dummy class, to make migration possible across version changes + belongs_to :account + end + + class Announcement < ApplicationRecord; end + class IpBlock < ApplicationRecord; end + class CustomEmoji < ApplicationRecord; end + class CanonicalEmailBlock < ApplicationRecord; end + + class Appeal < ApplicationRecord + # Dummy class, to make migration possible across version changes + belongs_to :account + end + + class AdminActionLog < ApplicationRecord + # Dummy class, to make migration possible across version changes + + # Cannot use usual polymorphic support because of namespacing issues + belongs_to :status, foreign_key: :target_id + belongs_to :account, foreign_key: :target_id + belongs_to :user, foreign_key: :user_id + belongs_to :domain_block, foreign_key: :target_id + belongs_to :domain_allow, foreign_key: :target_id + belongs_to :email_domain_block, foreign_key: :target_id + belongs_to :unavailable_domain, foreign_key: :target_id + belongs_to :account_warning, foreign_key: :target_id + belongs_to :announcement, foreign_key: :target_id + belongs_to :ip_block, foreign_key: :target_id + belongs_to :custom_emoji, foreign_key: :target_id + belongs_to :canonical_email_block, foreign_key: :target_id + belongs_to :appeal, foreign_key: :target_id + end + + def up + safety_assured do + AdminActionLog.includes(:account).where(target_type: 'Account', human_identifier: nil).find_each do |log| + next if log.account.nil? + log.update(human_identifier: log.account.acct) + end + + AdminActionLog.includes(user: :account).where(target_type: 'User', human_identifier: nil).find_each do |log| + next if log.user.nil? + log.update(human_identifier: log.user.account.acct, route_param: log.user.account_id) + end + + Admin::ActionLog.where(target_type: 'Report', human_identifier: nil).in_batches.update_all('human_identifier = target_id::text') + + AdminActionLog.includes(:domain_block).where(target_type: 'DomainBlock').find_each do |log| + next if log.domain_block.nil? + log.update(human_identifier: log.domain_block.domain) + end + + AdminActionLog.includes(:domain_allow).where(target_type: 'DomainAllow').find_each do |log| + next if log.domain_allow.nil? + log.update(human_identifier: log.domain_allow.domain) + end + + AdminActionLog.includes(:email_domain_block).where(target_type: 'EmailDomainBlock').find_each do |log| + next if log.email_domain_block.nil? + log.update(human_identifier: log.email_domain_block.domain) + end + + AdminActionLog.includes(:unavailable_domain).where(target_type: 'UnavailableDomain').find_each do |log| + next if log.unavailable_domain.nil? + log.update(human_identifier: log.unavailable_domain.domain) + end + + AdminActionLog.includes(status: :account).where(target_type: 'Status', human_identifier: nil).find_each do |log| + next if log.status.nil? + log.update(human_identifier: log.status.account.acct, permalink: log.status.uri) + end + + AdminActionLog.includes(account_warning: :account).where(target_type: 'AccountWarning', human_identifier: nil).find_each do |log| + next if log.account_warning.nil? + log.update(human_identifier: log.account_warning.account.acct) + end + + AdminActionLog.includes(:announcement).where(target_type: 'Announcement', human_identifier: nil).find_each do |log| + next if log.announcement.nil? + log.update(human_identifier: log.announcement.text) + end + + AdminActionLog.includes(:ip_block).where(target_type: 'IpBlock', human_identifier: nil).find_each do |log| + next if log.ip_block.nil? + log.update(human_identifier: "#{log.ip_block.ip}/#{log.ip_block.ip.prefix}") + end + + AdminActionLog.includes(:custom_emoji).where(target_type: 'CustomEmoji', human_identifier: nil).find_each do |log| + next if log.custom_emoji.nil? + log.update(human_identifier: log.custom_emoji.shortcode) + end + + AdminActionLog.includes(:canonical_email_block).where(target_type: 'CanonicalEmailBlock', human_identifier: nil).find_each do |log| + next if log.canonical_email_block.nil? + log.update(human_identifier: log.canonical_email_block.canonical_email_hash) + end + + AdminActionLog.includes(appeal: :account).where(target_type: 'Appeal', human_identifier: nil).find_each do |log| + next if log.appeal.nil? + log.update(human_identifier: log.appeal.account.acct, route_param: log.appeal.account_warning_id) + end + end + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index d7e40b133..12ec37c11 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: 2022_10_25_171544) do +ActiveRecord::Schema.define(version: 2022_11_01_190723) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/tasks/tests.rake b/lib/tasks/tests.rake index 65bff6a8e..96d2f7112 100644 --- a/lib/tasks/tests.rake +++ b/lib/tasks/tests.rake @@ -53,6 +53,41 @@ namespace :tests do VALUES (1, 2, 'test', '{ "home", "public" }', true, true, now(), now()), (2, 2, 'take', '{ "home" }', false, false, now(), now()); + + -- Orphaned admin action logs + + INSERT INTO "admin_action_logs" + (account_id, action, target_type, target_id, created_at, updated_at) + VALUES + (1, 'destroy', 'Account', 1312, now(), now()), + (1, 'destroy', 'User', 1312, now(), now()), + (1, 'destroy', 'Report', 1312, now(), now()), + (1, 'destroy', 'DomainBlock', 1312, now(), now()), + (1, 'destroy', 'EmailDomainBlock', 1312, now(), now()), + (1, 'destroy', 'Status', 1312, now(), now()), + (1, 'destroy', 'CustomEmoji', 1312, now(), now()); + + -- Admin action logs with linked objects + + INSERT INTO "domain_blocks" + (id, domain, created_at, updated_at) + VALUES + (1, 'example.org', now(), now()); + + INSERT INTO "email_domain_blocks" + (id, domain, created_at, updated_at) + VALUES + (1, 'example.org', now(), now()); + + INSERT INTO "admin_action_logs" + (account_id, action, target_type, target_id, created_at, updated_at) + VALUES + (1, 'destroy', 'Account', 1, now(), now()), + (1, 'destroy', 'User', 1, now(), now()), + (1, 'destroy', 'DomainBlock', 1312, now(), now()), + (1, 'destroy', 'EmailDomainBlock', 1312, now(), now()), + (1, 'destroy', 'Status', 1, now(), now()), + (1, 'destroy', 'CustomEmoji', 3, now(), now()); SQL end @@ -207,18 +242,18 @@ namespace :tests do -- custom emoji INSERT INTO "custom_emojis" - (shortcode, created_at, updated_at) + (id, shortcode, created_at, updated_at) VALUES - ('test', now(), now()), - ('Test', now(), now()), - ('blobcat', now(), now()); + (1, 'test', now(), now()), + (2, 'Test', now(), now()), + (3, 'blobcat', now(), now()); INSERT INTO "custom_emojis" - (shortcode, domain, uri, created_at, updated_at) + (id, shortcode, domain, uri, created_at, updated_at) VALUES - ('blobcat', 'remote.org', 'https://remote.org/emoji/blobcat', now(), now()), - ('blobcat', 'Remote.org', 'https://remote.org/emoji/blobcat', now(), now()), - ('Blobcat', 'remote.org', 'https://remote.org/emoji/Blobcat', now(), now()); + (4, 'blobcat', 'remote.org', 'https://remote.org/emoji/blobcat', now(), now()), + (5, 'blobcat', 'Remote.org', 'https://remote.org/emoji/blobcat', now(), now()), + (6, 'Blobcat', 'remote.org', 'https://remote.org/emoji/Blobcat', now(), now()); -- favourites diff --git a/spec/controllers/admin/action_logs_controller_spec.rb b/spec/controllers/admin/action_logs_controller_spec.rb index c1957258f..7cd8cdf46 100644 --- a/spec/controllers/admin/action_logs_controller_spec.rb +++ b/spec/controllers/admin/action_logs_controller_spec.rb @@ -3,6 +3,19 @@ require 'rails_helper' describe Admin::ActionLogsController, type: :controller do + render_views + + # Action logs typically cause issues when their targets are not in the database + let!(:account) { Fabricate(:account) } + + let!(:orphaned_logs) do + %w( + Account User UserRole Report DomainBlock DomainAllow + EmailDomainBlock UnavailableDomain Status AccountWarning + Announcement IpBlock Instance CustomEmoji CanonicalEmailBlock Appeal + ).map { |type| Admin::ActionLog.new(account: account, action: 'destroy', target_type: type, target_id: 1312).save! } + end + describe 'GET #index' do it 'returns 200' do sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin')) -- cgit From b1a219552e935d0c988e1f20b9fdceeb042d2c2d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 4 Nov 2022 16:08:29 +0100 Subject: Fix featured tags not saving preferred casing (#19732) --- app/models/featured_tag.rb | 25 +++++++++------------- config/locales/simple_form.en.yml | 2 +- .../20221104133904_add_name_to_featured_tags.rb | 5 +++++ db/schema.rb | 3 ++- 4 files changed, 18 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20221104133904_add_name_to_featured_tags.rb (limited to 'db') diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index d4ed74302..78185b2a9 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -10,13 +10,16 @@ # last_status_at :datetime # created_at :datetime not null # updated_at :datetime not null +# name :string # class FeaturedTag < ApplicationRecord belongs_to :account, inverse_of: :featured_tags belongs_to :tag, inverse_of: :featured_tags, optional: true # Set after validation - validate :validate_tag_name, on: :create + validates :name, presence: true, format: { with: /\A(#{Tag::HASHTAG_NAME_RE})\z/i }, on: :create + + validate :validate_tag_uniqueness, on: :create validate :validate_featured_tags_limit, on: :create before_validation :strip_name @@ -26,18 +29,14 @@ class FeaturedTag < ApplicationRecord scope :by_name, ->(name) { joins(:tag).where(tag: { name: HashtagNormalizer.new.normalize(name) }) } - delegate :display_name, to: :tag - - attr_writer :name - LIMIT = 10 def sign? true end - def name - tag_id.present? ? tag.name : @name + def display_name + attributes['name'] || tag.display_name end def increment(timestamp) @@ -51,13 +50,11 @@ class FeaturedTag < ApplicationRecord private def strip_name - return unless defined?(@name) - - @name = @name&.strip&.gsub(/\A#/, '') + self.name = name&.strip&.gsub(/\A#/, '') end def set_tag - self.tag = Tag.find_or_create_by_names(@name)&.first + self.tag = Tag.find_or_create_by_names(name)&.first end def reset_data @@ -69,9 +66,7 @@ class FeaturedTag < ApplicationRecord errors.add(:base, I18n.t('featured_tags.errors.limit')) if account.featured_tags.count >= LIMIT end - def validate_tag_name - errors.add(:name, :blank) if @name.blank? - errors.add(:name, :invalid) unless @name.match?(/\A(#{Tag::HASHTAG_NAME_RE})\z/i) - errors.add(:name, :taken) if FeaturedTag.by_name(@name).where(account_id: account_id).exists? + def validate_tag_uniqueness + errors.add(:name, :taken) if FeaturedTag.by_name(name).where(account_id: account_id).exists? end end diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 64281d7b7..6edf7b4e9 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -67,7 +67,7 @@ en: domain: This can be the domain name that shows up in the e-mail address or the MX record it uses. They will be checked upon sign-up. with_dns_records: An attempt to resolve the given domain's DNS records will be made and the results will also be blocked featured_tag: - name: 'You might want to use one of these:' + name: 'Here are some of the hashtags you used the most recently:' filters: action: Chose which action to perform when a post matches the filter actions: diff --git a/db/migrate/20221104133904_add_name_to_featured_tags.rb b/db/migrate/20221104133904_add_name_to_featured_tags.rb new file mode 100644 index 000000000..7c8c8ebfb --- /dev/null +++ b/db/migrate/20221104133904_add_name_to_featured_tags.rb @@ -0,0 +1,5 @@ +class AddNameToFeaturedTags < ActiveRecord::Migration[6.1] + def change + add_column :featured_tags, :name, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 12ec37c11..09d07fcca 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: 2022_11_01_190723) do +ActiveRecord::Schema.define(version: 2022_11_04_133904) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -442,6 +442,7 @@ ActiveRecord::Schema.define(version: 2022_11_01_190723) do t.datetime "last_status_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "name" t.index ["account_id", "tag_id"], name: "index_featured_tags_on_account_id_and_tag_id", unique: true t.index ["tag_id"], name: "index_featured_tags_on_tag_id" end -- cgit