From 9cc283f0b48104f5761b1e4f96040c18428d25e9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 21 Apr 2021 18:31:24 +0200 Subject: Change the nouns "toot" and "status" to "post" (#16080) --- spec/mailers/notification_mailer_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec') diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 38916b54f..3ae106218 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -59,12 +59,12 @@ RSpec.describe NotificationMailer, type: :mailer do include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob' it "renders the headers" do - expect(mail.subject).to eq("bob favourited your status") + expect(mail.subject).to eq("bob favourited your post") expect(mail.to).to eq([receiver.email]) end it "renders the body" do - expect(mail.body.encoded).to match("Your status was favourited by bob") + expect(mail.body.encoded).to match("Your post was favourited by bob") expect(mail.body.encoded).to include 'The body of the own status' end end @@ -76,12 +76,12 @@ RSpec.describe NotificationMailer, type: :mailer do include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob' it "renders the headers" do - expect(mail.subject).to eq("bob boosted your status") + expect(mail.subject).to eq("bob boosted your post") expect(mail.to).to eq([receiver.email]) end it "renders the body" do - expect(mail.body.encoded).to match("Your status was boosted by bob") + expect(mail.body.encoded).to match("Your post was boosted by bob") expect(mail.body.encoded).to include 'The body of the own status' end end -- cgit From a6564d56d6af0467eb8cfa96444b4503f437e0a6 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 23 Apr 2021 22:51:21 +0200 Subject: Fix edge case where accepted follow cannot be processed because of follow limit (#16098) --- app/models/follow_request.rb | 2 +- spec/models/follow_request_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 59fefcdf6..0b6f7629a 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -29,7 +29,7 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri) + account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri, bypass_limit: true) MergeWorker.perform_async(target_account.id, account.id) if account.local? destroy! end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index cc484a5b9..36ce8ee60 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do let(:target_account) { Fabricate(:account) } it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do - expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri) + expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri, bypass_limit: true) expect(MergeWorker).to receive(:perform_async).with(target_account.id, account.id) expect(follow_request).to receive(:destroy!) follow_request.authorize! -- cgit From daccc07dc170627b17564402296f6c8631d0cd97 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Apr 2021 17:01:43 +0200 Subject: Change auto-following admin-selected accounts, show in recommendations (#16078) --- app/controllers/api/v1/suggestions_controller.rb | 10 ++-- app/lib/potential_friendship_tracker.rb | 10 ---- app/models/account_suggestions.rb | 25 +++++--- app/models/account_suggestions/global_source.rb | 37 ++++++++++++ .../past_interactions_source.rb | 36 ++++++++++++ app/models/account_suggestions/setting_source.rb | 68 ++++++++++++++++++++++ app/models/account_suggestions/source.rb | 34 +++++++++++ app/models/account_suggestions/suggestion.rb | 7 +++ app/models/follow_recommendation.rb | 15 ----- app/models/form/admin_settings.rb | 2 - app/services/bootstrap_timeline_service.rb | 37 +----------- app/validators/existing_username_validator.rb | 24 ++++++-- app/views/admin/settings/edit.html.haml | 5 +- config/locales/en.yml | 7 +-- config/settings.yml | 1 - spec/services/bootstrap_timeline_service_spec.rb | 38 ------------ 16 files changed, 228 insertions(+), 128 deletions(-) create mode 100644 app/models/account_suggestions/global_source.rb create mode 100644 app/models/account_suggestions/past_interactions_source.rb create mode 100644 app/models/account_suggestions/setting_source.rb create mode 100644 app/models/account_suggestions/source.rb create mode 100644 app/models/account_suggestions/suggestion.rb (limited to 'spec') diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index b2788cc76..9737ae5cb 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -5,20 +5,20 @@ class Api::V1::SuggestionsController < Api::BaseController before_action -> { doorkeeper_authorize! :read } before_action :require_user! - before_action :set_accounts def index - render json: @accounts, each_serializer: REST::AccountSerializer + suggestions = suggestions_source.get(current_account, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) + render json: suggestions.map(&:account), each_serializer: REST::AccountSerializer end def destroy - PotentialFriendshipTracker.remove(current_account.id, params[:id]) + suggestions_source.remove(current_account, params[:id]) render_empty end private - def set_accounts - @accounts = PotentialFriendshipTracker.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + def suggestions_source + AccountSuggestions::PastInteractionsSource.new end end diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb index e72d454b6..f5bc20346 100644 --- a/app/lib/potential_friendship_tracker.rb +++ b/app/lib/potential_friendship_tracker.rb @@ -27,15 +27,5 @@ class PotentialFriendshipTracker def remove(account_id, target_account_id) redis.zrem("interactions:#{account_id}", target_account_id) end - - def get(account, limit) - account_ids = redis.zrevrange("interactions:#{account.id}", 0, limit) - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.searchable.where(id: account_ids).index_by(&:id) - - account_ids.map { |id| accounts[id.to_i] }.compact - end end end diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb index 7fe9d618e..d1774e62f 100644 --- a/app/models/account_suggestions.rb +++ b/app/models/account_suggestions.rb @@ -1,17 +1,28 @@ # frozen_string_literal: true class AccountSuggestions - class Suggestion < ActiveModelSerializers::Model - attributes :account, :source - end + SOURCES = [ + AccountSuggestions::SettingSource, + AccountSuggestions::PastInteractionsSource, + AccountSuggestions::GlobalSource, + ].freeze def self.get(account, limit) - suggestions = PotentialFriendshipTracker.get(account, limit).map { |target_account| Suggestion.new(account: target_account, source: :past_interaction) } - suggestions.concat(FollowRecommendation.get(account, limit - suggestions.size, suggestions.map { |suggestion| suggestion.account.id }).map { |target_account| Suggestion.new(account: target_account, source: :global) }) if suggestions.size < limit - suggestions + SOURCES.each_with_object([]) do |source_class, suggestions| + source_suggestions = source_class.new.get( + account, + skip_account_ids: suggestions.map(&:account_id), + limit: limit - suggestions.size + ) + + suggestions.concat(source_suggestions) + end end def self.remove(account, target_account_id) - PotentialFriendshipTracker.remove(account.id, target_account_id) + SOURCES.each do |source_class| + source = source_class.new + source.remove(account, target_account_id) + end end end diff --git a/app/models/account_suggestions/global_source.rb b/app/models/account_suggestions/global_source.rb new file mode 100644 index 000000000..ac764de50 --- /dev/null +++ b/app/models/account_suggestions/global_source.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class AccountSuggestions::GlobalSource < AccountSuggestions::Source + def key + :global + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_locale(account.user_locale) - [account.id] - skip_account_ids + + as_ordered_suggestions( + scope(account).where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + end + + def account_ids_for_locale(locale) + Redis.current.zrevrange("follow_recommendations:#{locale}", 0, -1).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/past_interactions_source.rb b/app/models/account_suggestions/past_interactions_source.rb new file mode 100644 index 000000000..d169394f1 --- /dev/null +++ b/app/models/account_suggestions/past_interactions_source.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class AccountSuggestions::PastInteractionsSource < AccountSuggestions::Source + include Redisable + + def key + :past_interactions + end + + def get(account, skip_account_ids: [], limit: 40) + account_ids = account_ids_for_account(account.id, limit + skip_account_ids.size) - skip_account_ids + + as_ordered_suggestions( + scope.where(id: account_ids), + account_ids + ).take(limit) + end + + def remove(account, target_account_id) + redis.zrem("interactions:#{account.id}", target_account_id) + end + + private + + def scope + Account.searchable + end + + def account_ids_for_account(account_id, limit) + redis.zrevrange("interactions:#{account_id}", 0, limit).map(&:to_i) + end + + def to_ordered_list_key(account) + account.id + end +end diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb new file mode 100644 index 000000000..be9eff233 --- /dev/null +++ b/app/models/account_suggestions/setting_source.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class AccountSuggestions::SettingSource < AccountSuggestions::Source + def key + :staff + end + + def get(account, skip_account_ids: [], limit: 40) + return [] unless setting_enabled? + + as_ordered_suggestions( + scope(account).where(setting_to_where_condition).where.not(id: skip_account_ids), + usernames_and_domains + ).take(limit) + end + + def remove(_account, _target_account_id) + nil + end + + private + + def scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where(locked: false) + .where.not(id: account.id) + end + + def usernames_and_domains + @usernames_and_domains ||= setting_to_usernames_and_domains + end + + def setting_enabled? + setting.present? + end + + def setting_to_where_condition + usernames_and_domains.map do |(username, domain)| + Arel::Nodes::Grouping.new( + Account.arel_table[:username].lower.eq(username.downcase).and( + Account.arel_table[:domain].lower.eq(domain&.downcase) + ) + ) + end.reduce(:or) + end + + def setting_to_usernames_and_domains + setting.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@', 2) + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [username, domain] + end.compact + end + + def setting + Setting.bootstrap_timeline_accounts + end + + def to_ordered_list_key(account) + [account.username, account.domain] + end +end diff --git a/app/models/account_suggestions/source.rb b/app/models/account_suggestions/source.rb new file mode 100644 index 000000000..bd1068d20 --- /dev/null +++ b/app/models/account_suggestions/source.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class AccountSuggestions::Source + def key + raise NotImplementedError + end + + def get(_account, **kwargs) + raise NotImplementedError + end + + def remove(_account, target_account_id) + raise NotImplementedError + end + + protected + + def as_ordered_suggestions(scope, ordered_list) + return [] if ordered_list.empty? + + map = scope.index_by(&method(:to_ordered_list_key)) + + ordered_list.map { |ordered_list_key| map[ordered_list_key] }.compact.map do |account| + AccountSuggestions::Suggestion.new( + account: account, + source: key + ) + end + end + + def to_ordered_list_key(_account) + raise NotImplementedError + end +end diff --git a/app/models/account_suggestions/suggestion.rb b/app/models/account_suggestions/suggestion.rb new file mode 100644 index 000000000..2c6f4d27f --- /dev/null +++ b/app/models/account_suggestions/suggestion.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AccountSuggestions::Suggestion < ActiveModelSerializers::Model + attributes :account, :source + + delegate :id, to: :account, prefix: true +end diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb index c4355224d..6670b6560 100644 --- a/app/models/follow_recommendation.rb +++ b/app/models/follow_recommendation.rb @@ -21,19 +21,4 @@ class FollowRecommendation < ApplicationRecord def readonly? true end - - def self.get(account, limit, exclude_account_ids = []) - account_ids = Redis.current.zrevrange("follow_recommendations:#{account.user_locale}", 0, -1).map(&:to_i) - exclude_account_ids - [account.id] - - return [] if account_ids.empty? || limit < 1 - - accounts = Account.followable_by(account) - .not_excluded_by_account(account) - .not_domain_blocked_by_account(account) - .where(id: account_ids) - .limit(limit) - .index_by(&:id) - - account_ids.map { |id| accounts[id] }.compact - end end diff --git a/app/models/form/admin_settings.rb b/app/models/form/admin_settings.rb index b5c3dcdbe..6fc7c56fd 100644 --- a/app/models/form/admin_settings.rb +++ b/app/models/form/admin_settings.rb @@ -16,7 +16,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts bootstrap_timeline_accounts theme min_invite_role @@ -41,7 +40,6 @@ class Form::AdminSettings open_deletion timeline_preview show_staff_badge - enable_bootstrap_timeline_accounts activity_api_enabled peers_api_enabled show_known_fediverse_at_about_page diff --git a/app/services/bootstrap_timeline_service.rb b/app/services/bootstrap_timeline_service.rb index 8412aa7e7..e1a1b98c3 100644 --- a/app/services/bootstrap_timeline_service.rb +++ b/app/services/bootstrap_timeline_service.rb @@ -5,48 +5,13 @@ class BootstrapTimelineService < BaseService @source_account = source_account autofollow_inviter! - autofollow_bootstrap_timeline_accounts! if Setting.enable_bootstrap_timeline_accounts end private def autofollow_inviter! return unless @source_account&.user&.invite&.autofollow? - FollowService.new.call(@source_account, @source_account.user.invite.user.account) - end - - def autofollow_bootstrap_timeline_accounts! - bootstrap_timeline_accounts.each do |target_account| - begin - FollowService.new.call(@source_account, target_account) - rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError - nil - end - end - end - - def bootstrap_timeline_accounts - return @bootstrap_timeline_accounts if defined?(@bootstrap_timeline_accounts) - - @bootstrap_timeline_accounts = bootstrap_timeline_accounts_usernames.empty? ? admin_accounts : local_unlocked_accounts(bootstrap_timeline_accounts_usernames) - end - - def bootstrap_timeline_accounts_usernames - @bootstrap_timeline_accounts_usernames ||= (Setting.bootstrap_timeline_accounts || '').split(',').map { |str| str.strip.gsub(/\A@/, '') }.reject(&:blank?) - end - def admin_accounts - User.admins - .includes(:account) - .where(accounts: { locked: false }) - .map(&:account) - end - - def local_unlocked_accounts(usernames) - Account.local - .without_suspended - .where(username: usernames) - .where(locked: false) - .where(moved_to_account_id: nil) + FollowService.new.call(@source_account, @source_account.user.invite.user.account) end end diff --git a/app/validators/existing_username_validator.rb b/app/validators/existing_username_validator.rb index 723302ec9..afbe0c635 100644 --- a/app/validators/existing_username_validator.rb +++ b/app/validators/existing_username_validator.rb @@ -4,11 +4,25 @@ class ExistingUsernameValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) return if value.blank? - if options[:multiple] - missing_usernames = value.split(',').map { |username| username.strip.gsub(/\A@/, '') }.filter_map { |username| username unless Account.find_local(username) } - 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.strip.gsub(/\A@/, '')) + usernames_and_domains = begin + value.split(',').map do |str| + username, domain = str.strip.gsub(/\A@/, '').split('@') + domain = nil if TagManager.instance.local_domain?(domain) + + next if username.blank? + + [str, username, domain] + end.compact + end + + usernames_with_no_accounts = usernames_and_domains.filter_map do |(str, username, domain)| + str unless Account.find_remote(username, domain) + end + + if usernames_with_no_accounts.any? && options[:multiple] + record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: usernames_with_no_accounts.join(', '))) + elsif usernames_with_no_accounts.any? || usernames_and_domains.size > 1 + record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) end end end diff --git a/app/views/admin/settings/edit.html.haml b/app/views/admin/settings/edit.html.haml index 7783dbfeb..33bfc43d3 100644 --- a/app/views/admin/settings/edit.html.haml +++ b/app/views/admin/settings/edit.html.haml @@ -50,10 +50,7 @@ %hr.spacer/ .fields-group - = f.input :enable_bootstrap_timeline_accounts, as: :boolean, wrapper: :with_label, label: t('admin.settings.enable_bootstrap_timeline_accounts.title'), hint: t('admin.settings.enable_bootstrap_timeline_accounts.desc_html') - - .fields-group - = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html'), disabled: !Setting.enable_bootstrap_timeline_accounts + = f.input :bootstrap_timeline_accounts, wrapper: :with_block_label, label: t('admin.settings.bootstrap_timeline_accounts.title'), hint: t('admin.settings.bootstrap_timeline_accounts.desc_html') %hr.spacer/ diff --git a/config/locales/en.yml b/config/locales/en.yml index 765f71250..1b41ee063 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -564,8 +564,8 @@ en: desc_html: Counts of locally published posts, active users, and new registrations in weekly buckets title: Publish aggregate statistics about user activity in the API bootstrap_timeline_accounts: - desc_html: Separate multiple usernames by comma. Only local and unlocked accounts will work. Default when empty is all local admins. - title: Default follows for new users + desc_html: Separate multiple usernames by comma. These accounts will be guaranteed to be shown in follow recommendations + title: Recommend these accounts to new users contact_information: email: Business e-mail username: Contact username @@ -582,9 +582,6 @@ en: users: To logged-in local users domain_blocks_rationale: title: Show rationale - enable_bootstrap_timeline_accounts: - desc_html: Make new users automatically follow configured accounts so their home feed doesn't start out empty - title: Enable default follows for new users hero: desc_html: Displayed on the frontpage. At least 600x100px recommended. When not set, falls back to server thumbnail title: Hero image diff --git a/config/settings.yml b/config/settings.yml index b79ea620c..06cee2532 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -62,7 +62,6 @@ defaults: &defaults - mod - moderator disallowed_hashtags: # space separated string or list of hashtags without the hash - enable_bootstrap_timeline_accounts: true bootstrap_timeline_accounts: '' activity_api_enabled: true peers_api_enabled: true diff --git a/spec/services/bootstrap_timeline_service_spec.rb b/spec/services/bootstrap_timeline_service_spec.rb index a28d2407c..880ca4f0d 100644 --- a/spec/services/bootstrap_timeline_service_spec.rb +++ b/spec/services/bootstrap_timeline_service_spec.rb @@ -1,42 +1,4 @@ require 'rails_helper' RSpec.describe BootstrapTimelineService, type: :service do - subject { described_class.new } - - describe '#call' do - let(:source_account) { Fabricate(:account) } - - context 'when setting is empty' do - let!(:admin) { Fabricate(:user, admin: true) } - - before do - Setting.bootstrap_timeline_accounts = nil - subject.call(source_account) - end - - it 'follows admin accounts from account' do - expect(source_account.following?(admin.account)).to be true - end - end - - context 'when setting is set' do - let!(:alice) { Fabricate(:account, username: 'alice') } - let!(:bob) { Fabricate(:account, username: 'bob') } - let!(:eve) { Fabricate(:account, username: 'eve', suspended: true) } - - before do - Setting.bootstrap_timeline_accounts = 'alice, @bob, eve, unknown' - subject.call(source_account) - end - - it 'follows found accounts from account' do - expect(source_account.following?(alice)).to be true - expect(source_account.following?(bob)).to be true - end - - it 'does not follow suspended account' do - expect(source_account.following?(eve)).to be false - end - end - end end -- cgit From 7f0c49c58ab75bbd6b4dc44d47a8c4ab57db9d51 Mon Sep 17 00:00:00 2001 From: abcang Date: Sun, 25 Apr 2021 13:33:28 +0900 Subject: Improve tag search query (#16104) --- app/models/tag.rb | 14 ++++++-------- ...10421121431_add_case_insensitive_btree_index_to_tags.rb | 13 +++++++++++++ db/schema.rb | 4 ++-- spec/models/tag_spec.rb | 14 ++++++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb (limited to 'spec') diff --git a/app/models/tag.rb b/app/models/tag.rb index bb93a52e2..efffc7eee 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -40,7 +40,8 @@ class Tag < ApplicationRecord scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } - scope :matches_name, ->(value) { where(arel_table[:name].matches("#{value}%")) } + # Search with case-sensitive to use B-tree index. + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } delegate :accounts_count, :accounts_count=, @@ -126,10 +127,9 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - normalized_term = normalize(term.strip) - pattern = sanitize_sql_like(normalized_term) + '%' - query = Tag.listable.where(arel_table[:name].lower.matches(pattern)) - query = query.where(arel_table[:name].lower.eq(normalized_term).or(arel_table[:reviewed_at].not_eq(nil))) if options[:exclude_unreviewed] + striped_term = term.strip + query = Tag.listable.matches_name(striped_term) + query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) .limit(limit) @@ -145,7 +145,7 @@ class Tag < ApplicationRecord end def matching_name(name_or_names) - names = Array(name_or_names).map { |name| normalize(name).mb_chars.downcase.to_s } + names = Array(name_or_names).map { |name| arel_table.lower(normalize(name)) } if names.size == 1 where(arel_table[:name].lower.eq(names.first)) @@ -154,8 +154,6 @@ class Tag < ApplicationRecord end end - private - def normalize(str) str.gsub(/\A#/, '') end diff --git a/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb b/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb new file mode 100644 index 000000000..ed359e8cd --- /dev/null +++ b/db/migrate/20210421121431_add_case_insensitive_btree_index_to_tags.rb @@ -0,0 +1,13 @@ +class AddCaseInsensitiveBtreeIndexToTags < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + safety_assured { execute 'CREATE UNIQUE INDEX CONCURRENTLY index_tags_on_name_lower_btree ON tags (lower(name) text_pattern_ops)' } + remove_index :tags, name: 'index_tags_on_name_lower' + end + + def down + safety_assured { execute 'CREATE UNIQUE INDEX CONCURRENTLY index_tags_on_name_lower ON tags (lower(name))' } + remove_index :tags, name: 'index_tags_on_name_lower_btree' + end +end diff --git a/db/schema.rb b/db/schema.rb index dcbbf4aea..8dc0661fc 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: 2021_04_16_200740) do +ActiveRecord::Schema.define(version: 2021_04_21_121431) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -862,7 +862,7 @@ ActiveRecord::Schema.define(version: 2021_04_16_200740) do t.datetime "last_status_at" t.float "max_score" t.datetime "max_score_at" - t.index "lower((name)::text)", name: "index_tags_on_name_lower", unique: true + t.index "lower((name)::text) text_pattern_ops", name: "index_tags_on_name_lower_btree", unique: true end create_table "tombstones", force: :cascade do |t| diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index df876593c..3949dbce5 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -96,6 +96,20 @@ RSpec.describe Tag, type: :model do end end + describe '.matches_name' do + it 'returns tags for multibyte case-insensitive names' do + upcase_string = 'abcABCabcABCやゆよ' + downcase_string = 'abcabcabcabcやゆよ'; + + tag = Fabricate(:tag, name: downcase_string) + expect(Tag.matches_name(upcase_string)).to eq [tag] + end + + it 'uses the LIKE operator' do + expect(Tag.matches_name('100%abc').to_sql).to eq %q[SELECT "tags".* FROM "tags" WHERE LOWER("tags"."name") LIKE LOWER('100\\%abc%')] + end + end + describe '.matching_name' do it 'returns tags for multibyte case-insensitive names' do upcase_string = 'abcABCabcABCやゆよ' -- cgit From 8c44b723bb7505eb5923019b454ce9abd7ea59c0 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 May 2021 15:45:19 +0200 Subject: Change confirmations controller to redirect to / for approved users (#16151) Clicking the confirmation link multiple times currently leads to entering account settings, which can be confusing. This commit changes that so that it redirects to the root path, so it behaves the same way as clicking only once in most cases. --- app/controllers/auth/confirmations_controller.rb | 4 +- .../auth/confirmations_controller_spec.rb | 46 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 898525269..1475bbcef 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -17,7 +17,9 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController private def require_unconfirmed! - redirect_to edit_user_registration_path if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank? + if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank? + redirect_to(current_user.approved? ? root_path : edit_user_registration_path) + end end def set_body_classes diff --git a/spec/controllers/auth/confirmations_controller_spec.rb b/spec/controllers/auth/confirmations_controller_spec.rb index 0b6b74ff9..8469119d2 100644 --- a/spec/controllers/auth/confirmations_controller_spec.rb +++ b/spec/controllers/auth/confirmations_controller_spec.rb @@ -32,6 +32,52 @@ describe Auth::ConfirmationsController, type: :controller do end end + context 'when user is unconfirmed and unapproved' do + let!(:user) { Fabricate(:user, confirmation_token: 'foobar', confirmed_at: nil, approved: false) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to login' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when user is already confirmed' do + let!(:user) { Fabricate(:user) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + sign_in(user, scope: :user) + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to root path' do + expect(response).to redirect_to(root_path) + end + end + + context 'when user is already confirmed but unapproved' do + let!(:user) { Fabricate(:user, approved: false) } + + before do + allow(BootstrapTimelineWorker).to receive(:perform_async) + @request.env['devise.mapping'] = Devise.mappings[:user] + user.approved = false + user.save! + sign_in(user, scope: :user) + get :show, params: { confirmation_token: 'foobar' } + end + + it 'redirects to settings' do + expect(response).to redirect_to(edit_user_registration_path) + end + end + context 'when user is updating email' do let!(:user) { Fabricate(:user, confirmation_token: 'foobar', unconfirmed_email: 'new-email@example.com') } -- cgit From 566fc909134586d1746ad60ee455832dec6bc61a Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 May 2021 14:22:54 +0200 Subject: Add Ruby 3.0 support (#16046) * Fix issues with POSIX::Spawn, Terrapin and Ruby 3.0 Also improve the Terrapin monkey-patch for the stderr/stdout issue. * Fix keyword argument handling throughout the codebase * Monkey-patch Paperclip to fix keyword arguments handling in validators * Change validation_extensions to please CodeClimate * Bump microformats from 4.2.1 to 4.3.1 * Allow Ruby 3.0 * Add Ruby 3.0 test target to CircleCI * Add test for admin dashboard warnings * Fix admin dashboard warnings on Ruby 3.0 --- .circleci/config.yml | 27 +++++++ Gemfile | 2 +- Gemfile.lock | 6 +- app/controllers/activitypub/outboxes_controller.rb | 2 +- app/controllers/api/v1/accounts_controller.rb | 4 +- .../api/v1/follow_requests_controller.rb | 2 +- app/models/session_activation.rb | 2 +- app/models/user.rb | 19 +++-- app/views/admin/dashboard/index.html.haml | 2 +- app/workers/import/relationship_worker.rb | 6 +- config/application.rb | 1 + config/initializers/session_store.rb | 5 +- lib/paperclip/validation_extensions.rb | 58 +++++++++++++++ lib/terrapin/multi_pipe_extensions.rb | 87 +++++++++++----------- .../controllers/admin/dashboard_controller_spec.rb | 12 ++- spec/mailers/notification_mailer_spec.rb | 4 +- spec/mailers/user_mailer_spec.rb | 4 +- spec/models/session_activation_spec.rb | 6 +- .../account_relationships_presenter_spec.rb | 2 +- 19 files changed, 177 insertions(+), 74 deletions(-) create mode 100644 lib/paperclip/validation_extensions.rb (limited to 'spec') diff --git a/.circleci/config.yml b/.circleci/config.yml index 862fa126b..2f3860d7c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -129,6 +129,13 @@ jobs: environment: *ruby_environment <<: *install_ruby_dependencies + install-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + <<: *install_ruby_dependencies + build: <<: *defaults steps: @@ -187,6 +194,18 @@ jobs: - image: circleci/redis:5-alpine <<: *test_steps + test-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + - image: circleci/postgres:12.2 + environment: + POSTGRES_USER: root + POSTGRES_HOST_AUTH_METHOD: trust + - image: circleci/redis:5-alpine + <<: *test_steps + test-webui: <<: *defaults docker: @@ -227,6 +246,10 @@ workflows: requires: - install - install-ruby2.7 + - install-ruby3.0: + requires: + - install + - install-ruby2.7 - build: requires: - install-ruby2.7 @@ -241,6 +264,10 @@ workflows: requires: - install-ruby2.6 - build + - test-ruby3.0: + requires: + - install-ruby3.0 + - build - test-webui: requires: - install diff --git a/Gemfile b/Gemfile index 6ca0a81de..5a55d6b04 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ # frozen_string_literal: true source 'https://rubygems.org' -ruby '>= 2.5.0', '< 3.0.0' +ruby '>= 2.5.0', '< 3.1.0' gem 'pkg-config', '~> 1.4' diff --git a/Gemfile.lock b/Gemfile.lock index b1ae4fd22..980750b63 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -292,7 +292,7 @@ GEM ipaddress (0.8.3) iso-639 (0.3.5) jmespath (1.4.0) - json (2.3.1) + json (2.5.1) json-canonicalization (0.2.1) json-ld (3.1.9) htmlentities (~> 4.3) @@ -344,7 +344,7 @@ GEM redis (>= 3.0.5) memory_profiler (1.0.0) method_source (1.0.0) - microformats (4.2.1) + microformats (4.3.1) json (~> 2.2) nokogiri (~> 1.10) mime-types (3.3.1) @@ -354,7 +354,7 @@ GEM nokogiri (~> 1) rake mini_mime (1.0.3) - mini_portile2 (2.5.0) + mini_portile2 (2.5.1) minitest (5.14.4) msgpack (1.4.2) multi_json (1.15.0) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 5fd735ad6..111285036 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -20,7 +20,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def outbox_presenter if page_requested? ActivityPub::CollectionPresenter.new( - id: outbox_url(page_params), + id: outbox_url(**page_params), type: :ordered, part_of: outbox_url, prev: prev_page, diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 996f1b79b..95869f554 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -35,7 +35,7 @@ class Api::V1::AccountsController < Api::BaseController follow = FollowService.new.call(current_user.account, @account, reblogs: params.key?(:reblogs) ? truthy_param?(:reblogs) : nil, notify: params.key?(:notify) ? truthy_param?(:notify) : nil, with_rate_limit: true) options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: follow.show_reblogs?, notify: follow.notify? } }, requested_map: { @account.id => false } } - render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) + render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(**options) end def block @@ -70,7 +70,7 @@ class Api::V1::AccountsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([@account.id], current_user.account_id, options) + AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) end def account_params diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index b34c76f29..f4b2a74d0 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -29,7 +29,7 @@ class Api::V1::FollowRequestsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, options) + AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, **options) end def load_accounts diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index b0ce9d112..3a59bad93 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -44,7 +44,7 @@ class SessionActivation < ApplicationRecord end def activate(**options) - activation = create!(options) + activation = create!(**options) purge_old activation end diff --git a/app/models/user.rb b/app/models/user.rb index 0440627c5..4973c68b6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -370,15 +370,20 @@ class User < ApplicationRecord protected - def send_devise_notification(notification, *args) + def send_devise_notification(notification, *args, **kwargs) # This method can be called in `after_update` and `after_commit` hooks, # but we must make sure the mailer is actually called *after* commit, # otherwise it may work on stale data. To do this, figure out if we are # within a transaction. + + # It seems like devise sends keyword arguments as a hash in the last + # positional argument + kwargs = args.pop if args.last.is_a?(Hash) && kwargs.empty? + if ActiveRecord::Base.connection.current_transaction.try(:records)&.include?(self) - pending_devise_notifications << [notification, args] + pending_devise_notifications << [notification, args, kwargs] else - render_and_send_devise_message(notification, *args) + render_and_send_devise_message(notification, *args, **kwargs) end end @@ -389,8 +394,8 @@ class User < ApplicationRecord end def send_pending_devise_notifications - pending_devise_notifications.each do |notification, args| - render_and_send_devise_message(notification, *args) + pending_devise_notifications.each do |notification, args, kwargs| + render_and_send_devise_message(notification, *args, **kwargs) end # Empty the pending notifications array because the @@ -403,8 +408,8 @@ class User < ApplicationRecord @pending_devise_notifications ||= [] end - def render_and_send_devise_message(notification, *args) - devise_mailer.send(notification, self, *args).deliver_later + def render_and_send_devise_message(notification, *args, **kwargs) + devise_mailer.send(notification, self, *args, **kwargs).deliver_later end def set_approved diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 57a753e6b..e8a2b46fd 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -5,7 +5,7 @@ .flash-message-stack - @system_checks.each do |message| .flash-message.warning - = t("admin.system_checks.#{message.key}.message_html", message.value ? { value: content_tag(:strong, message.value) } : {}) + = t("admin.system_checks.#{message.key}.message_html", value: message.value ? content_tag(:strong, message.value) : nil) - if message.action = link_to t("admin.system_checks.#{message.key}.action"), message.action diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb index 4a7100435..6791b15c3 100644 --- a/app/workers/import/relationship_worker.rb +++ b/app/workers/import/relationship_worker.rb @@ -5,7 +5,7 @@ class Import::RelationshipWorker sidekiq_options queue: 'pull', retry: 8, dead: false - def perform(account_id, target_account_uri, relationship, options = {}) + def perform(account_id, target_account_uri, relationship, options) from_account = Account.find(account_id) target_domain = domain(target_account_uri) target_account = stoplight_wrap_request(target_domain) { ResolveAccountService.new.call(target_account_uri, { check_delivery_availability: true }) } @@ -16,7 +16,7 @@ class Import::RelationshipWorker case relationship when 'follow' begin - FollowService.new.call(from_account, target_account, options) + FollowService.new.call(from_account, target_account, **options) rescue ActiveRecord::RecordInvalid raise if FollowLimitValidator.limit_for_account(from_account) < from_account.following_count end @@ -27,7 +27,7 @@ class Import::RelationshipWorker when 'unblock' UnblockService.new.call(from_account, target_account) when 'mute' - MuteService.new.call(from_account, target_account, options) + MuteService.new.call(from_account, target_account, **options) when 'unmute' UnmuteService.new.call(from_account, target_account) end diff --git a/config/application.rb b/config/application.rb index 37a996224..08a4e4c97 100644 --- a/config/application.rb +++ b/config/application.rb @@ -10,6 +10,7 @@ require_relative '../lib/exceptions' require_relative '../lib/enumerable' require_relative '../lib/sanitize_ext/sanitize_config' require_relative '../lib/redis/namespace_extensions' +require_relative '../lib/paperclip/validation_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index e5d1be4c6..3d9bf96fd 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,7 +1,6 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.session_store :cookie_store, { +Rails.application.config.session_store :cookie_store, key: '_mastodon_session', secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'), - same_site: :lax, -} + same_site: :lax diff --git a/lib/paperclip/validation_extensions.rb b/lib/paperclip/validation_extensions.rb new file mode 100644 index 000000000..0df0434f6 --- /dev/null +++ b/lib/paperclip/validation_extensions.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# Monkey-patch various Paperclip validators for Ruby 3.0 compatibility + +module Paperclip + module Validators + module AttachmentSizeValidatorExtensions + def validate_each(record, attr_name, _value) + base_attr_name = attr_name + attr_name = "#{attr_name}_file_size".to_sym + value = record.send(:read_attribute_for_validation, attr_name) + + if value.present? + options.slice(*Paperclip::Validators::AttachmentSizeValidator::AVAILABLE_CHECKS).each do |option, option_value| + option_value = option_value.call(record) if option_value.is_a?(Proc) + option_value = extract_option_value(option, option_value) + + next if value.send(Paperclip::Validators::AttachmentSizeValidator::CHECKS[option], option_value) + + error_message_key = options[:in] ? :in_between : option + [attr_name, base_attr_name].each do |error_attr_name| + record.errors.add(error_attr_name, error_message_key, **filtered_options(value).merge( + min: min_value_in_human_size(record), + max: max_value_in_human_size(record), + count: human_size(option_value) + )) + end + end + end + end + end + + module AttachmentContentTypeValidatorExtensions + def mark_invalid(record, attribute, types) + record.errors.add attribute, :invalid, **options.merge({ types: types.join(', ') }) + end + end + + module AttachmentPresenceValidatorExtensions + def validate_each(record, attribute, _value) + if record.send("#{attribute}_file_name").blank? + record.errors.add(attribute, :blank, **options) + end + end + end + + module AttachmentFileNameValidatorExtensions + def mark_invalid(record, attribute, patterns) + record.errors.add attribute, :invalid, options.merge({ names: patterns.join(', ') }) + end + end + end +end + +Paperclip::Validators::AttachmentSizeValidator.prepend(Paperclip::Validators::AttachmentSizeValidatorExtensions) +Paperclip::Validators::AttachmentContentTypeValidator.prepend(Paperclip::Validators::AttachmentContentTypeValidatorExtensions) +Paperclip::Validators::AttachmentPresenceValidator.prepend(Paperclip::Validators::AttachmentPresenceValidatorExtensions) +Paperclip::Validators::AttachmentFileNameValidator.prepend(Paperclip::Validators::AttachmentFileNameValidatorExtensions) diff --git a/lib/terrapin/multi_pipe_extensions.rb b/lib/terrapin/multi_pipe_extensions.rb index 51d7de37c..209f4ad6c 100644 --- a/lib/terrapin/multi_pipe_extensions.rb +++ b/lib/terrapin/multi_pipe_extensions.rb @@ -1,61 +1,64 @@ # frozen_string_literal: false -# Fix adapted from https://github.com/thoughtbot/terrapin/pull/5 + +require 'fcntl' module Terrapin module MultiPipeExtensions - def read - read_streams(@stdout_in, @stderr_in) - end + def initialize + @stdout_in, @stdout_out = IO.pipe + @stderr_in, @stderr_out = IO.pipe - def close_read - begin - @stdout_in.close - rescue IOError - # Do nothing - end - - begin - @stderr_in.close - rescue IOError - # Do nothing - end + clear_nonblocking_flags! end - def read_streams(output, error) - @stdout_output = '' - @stderr_output = '' + def pipe_options + # Add some flags to explicitly close the other end of the pipes + { out: @stdout_out, err: @stderr_out, @stdout_in => :close, @stderr_in => :close } + end - read_fds = [output, error] + def read + # While we are patching Terrapin, fix child process potentially getting stuck on writing + # to stderr. - until read_fds.empty? - to_read, = IO.select(read_fds) + @stdout_output = +'' + @stderr_output = +'' - if to_read.include?(output) - @stdout_output << read_stream(output) - read_fds.delete(output) if output.closed? - end + fds_to_read = [@stdout_in, @stderr_in] + until fds_to_read.empty? + rs, = IO.select(fds_to_read) - if to_read.include?(error) - @stderr_output << read_stream(error) - read_fds.delete(error) if error.closed? - end + read_nonblocking!(@stdout_in, @stdout_output, fds_to_read) if rs.include?(@stdout_in) + read_nonblocking!(@stderr_in, @stderr_output, fds_to_read) if rs.include?(@stderr_in) end end - def read_stream(io) - result = '' - - begin - while (partial_result = io.read_nonblock(8192)) - result << partial_result - end - rescue EOFError, Errno::EPIPE - io.close - rescue Errno::EINTR, Errno::EWOULDBLOCK, Errno::EAGAIN - # Do nothing + private + + # @param [IO] io IO Stream to read until there is nothing to read + # @param [String] result Mutable string to which read values will be appended to + # @param [Array] fds_to_read Mutable array from which `io` should be removed on EOF + def read_nonblocking!(io, result, fds_to_read) + while (partial_result = io.read_nonblock(8192)) + result << partial_result end + rescue IO::WaitReadable + # Do nothing + rescue EOFError + fds_to_read.delete(io) + end + + def clear_nonblocking_flags! + # Ruby 3.0 sets pipes to non-blocking mode, and resets the flags as + # needed when calling fork/exec-related syscalls, but posix-spawn does + # not currently do that, so we need to do it manually for the time being + # so that the child process do not error out when the buffers are full. + stdout_flags = @stdout_out.fcntl(Fcntl::F_GETFL) + @stdout_out.fcntl(Fcntl::F_SETFL, stdout_flags & ~Fcntl::O_NONBLOCK) if stdout_flags & Fcntl::O_NONBLOCK - result + stderr_flags = @stderr_out.fcntl(Fcntl::F_GETFL) + @stderr_out.fcntl(Fcntl::F_SETFL, stderr_flags & ~Fcntl::O_NONBLOCK) if stderr_flags & Fcntl::O_NONBLOCK + rescue NameError, NotImplementedError, Errno::EINVAL + # Probably on windows, where pipes are blocking by default end end end diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index 73b50e721..7824854f9 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -3,9 +3,19 @@ require 'rails_helper' describe Admin::DashboardController, type: :controller do + render_views + describe 'GET #index' do - it 'returns 200' do + before do + allow(Admin::SystemCheck).to receive(:perform).and_return([ + Admin::SystemCheck::Message.new(:database_schema_check), + Admin::SystemCheck::Message.new(:rules_check, nil, admin_rules_path), + Admin::SystemCheck::Message.new(:sidekiq_process_check, 'foo, bar'), + ]) sign_in Fabricate(:user, admin: true) + end + + it 'returns 200' do get :index expect(response).to have_http_status(200) diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 3ae106218..9b645bad8 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -10,12 +10,12 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = %i(de en).sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6b430b505..9c866788f 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -9,12 +9,12 @@ describe UserMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = I18n.available_locales.sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/models/session_activation_spec.rb b/spec/models/session_activation_spec.rb index 2aa695037..450dc1399 100644 --- a/spec/models/session_activation_spec.rb +++ b/spec/models/session_activation_spec.rb @@ -74,13 +74,13 @@ RSpec.describe SessionActivation, type: :model do let(:options) { { user: Fabricate(:user), session_id: '1' } } it 'calls create! and purge_old' do - expect(described_class).to receive(:create!).with(options) + expect(described_class).to receive(:create!).with(**options) expect(described_class).to receive(:purge_old) - described_class.activate(options) + described_class.activate(**options) end it 'returns an instance of SessionActivation' do - expect(described_class.activate(options)).to be_kind_of SessionActivation + expect(described_class.activate(**options)).to be_kind_of SessionActivation end end diff --git a/spec/presenters/account_relationships_presenter_spec.rb b/spec/presenters/account_relationships_presenter_spec.rb index f8b048d38..edfbbb354 100644 --- a/spec/presenters/account_relationships_presenter_spec.rb +++ b/spec/presenters/account_relationships_presenter_spec.rb @@ -13,7 +13,7 @@ RSpec.describe AccountRelationshipsPresenter do allow(Account).to receive(:domain_blocking_map).with(account_ids, current_account_id).and_return(default_map) end - let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, options) } + let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, **options) } let(:current_account_id) { Fabricate(:account).id } let(:account_ids) { [Fabricate(:account).id] } let(:default_map) { { 1 => true } } -- cgit From 2c77d97e0d59e045a9b04fccc83f0f24d190d8d8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 14:33:19 +0200 Subject: Add joined date to profiles in web UI (#16169) --- .../mastodon/features/account/components/header.js | 2 ++ app/javascript/mastodon/locales/defaultMessages.json | 14 +++++++++----- app/javascript/styles/mastodon/components.scss | 11 +++++++++++ app/serializers/activitypub/actor_serializer.rb | 6 +++++- app/serializers/rest/account_serializer.rb | 4 ++++ app/services/activitypub/process_account_service.rb | 3 ++- spec/lib/activitypub/activity/update_spec.rb | 2 +- 7 files changed, 34 insertions(+), 8 deletions(-) (limited to 'spec') diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 8e49486bd..20641121f 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -326,6 +326,8 @@ class Header extends ImmutablePureComponent { {account.get('id') !== me && !suspended && } {account.get('note').length > 0 && account.get('note') !== '

' &&
} + +
{!suspended && ( diff --git a/app/javascript/mastodon/locales/defaultMessages.json b/app/javascript/mastodon/locales/defaultMessages.json index 86cf83403..172fae81f 100644 --- a/app/javascript/mastodon/locales/defaultMessages.json +++ b/app/javascript/mastodon/locales/defaultMessages.json @@ -909,6 +909,10 @@ { "defaultMessage": "Group", "id": "account.badges.group" + }, + { + "defaultMessage": "Joined {date}", + "id": "account.joined" } ], "path": "app/javascript/mastodon/features/account/components/header.json" @@ -1919,12 +1923,12 @@ "id": "home.hide_announcements" }, { - "defaultMessage": "Your home timeline is empty! Visit {public} or use search to get started and meet other users.", + "defaultMessage": "Your home timeline is empty! Follow more people to fill it up. {suggestions}", "id": "empty_column.home" }, { - "defaultMessage": "the public timeline", - "id": "empty_column.home.public_timeline" + "defaultMessage": "See some suggestions", + "id": "empty_column.home.suggestions" } ], "path": "app/javascript/mastodon/features/home_timeline/index.json" @@ -2417,7 +2421,7 @@ "id": "notifications.mark_as_read" }, { - "defaultMessage": "You don't have any notifications yet. Interact with others to start the conversation.", + "defaultMessage": "You don't have any notifications yet. When other people interact with you, you will see it here.", "id": "empty_column.notifications" } ], @@ -3249,4 +3253,4 @@ ], "path": "app/javascript/mastodon/features/video/index.json" } -] \ No newline at end of file +] diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 74a181603..d3dd1af60 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -6769,6 +6769,17 @@ noscript { } } + .account__header__joined { + font-size: 14px; + padding: 5px 15px; + color: $darker-text-color; + + .columns-area--mobile & { + padding-left: 20px; + padding-right: 20px; + } + } + .account__header__fields { margin: 0; border-top: 1px solid lighten($ui-base-color, 12%); diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 759ef30f9..d92aae7b3 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -13,7 +13,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer :inbox, :outbox, :featured, :featured_tags, :preferred_username, :name, :summary, :url, :manually_approves_followers, - :discoverable + :discoverable, :published has_one :public_key, serializer: ActivityPub::PublicKeySerializer @@ -158,6 +158,10 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer !object.suspended? && !object.also_known_as.empty? end + def published + object.created_at.midnight.iso8601 + end + class CustomEmojiSerializer < ActivityPub::EmojiSerializer end diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index 189a62d0e..219f8075a 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -55,6 +55,10 @@ class REST::AccountSerializer < ActiveModel::Serializer full_asset_url(object.suspended? ? object.header.default_url : object.header_static_url) end + def created_at + object.created_at.midnight.iso8601 + end + def last_status_at object.last_status_at&.to_date&.iso8601 end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 6afeb92d6..bb2e8f665 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -87,6 +87,7 @@ class ActivityPub::ProcessAccountService < BaseService @account.url = url || @uri @account.uri = @uri @account.actor_type = actor_type + @account.created_at = @json['published'] if @json['published'].present? end def set_immediate_attributes! @@ -101,7 +102,7 @@ class ActivityPub::ProcessAccountService < BaseService end def set_fetchable_key! - @account.public_key = public_key || '' + @account.public_key = public_key || '' end def set_fetchable_attributes! diff --git a/spec/lib/activitypub/activity/update_spec.rb b/spec/lib/activitypub/activity/update_spec.rb index 42da29860..1c9bcf43b 100644 --- a/spec/lib/activitypub/activity/update_spec.rb +++ b/spec/lib/activitypub/activity/update_spec.rb @@ -13,7 +13,7 @@ RSpec.describe ActivityPub::Activity::Update do end let(:modified_sender) do - sender.dup.tap do |modified_sender| + sender.tap do |modified_sender| modified_sender.display_name = 'Totally modified now' end end -- cgit From 74081433d0078784b7c2139f6caaa812740632b2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 7 May 2021 14:33:43 +0200 Subject: Change trending hashtags to be affected be reblogs (#16164) If a status with a hashtag becomes very popular, it stands to reason that the hashtag should have a chance at trending Fix no stats being recorded for hashtags that are not allowed to trend, and stop ignoring bots Remove references to hashtags in profile directory from the code and the admin UI --- app/controllers/directories_controller.rb | 10 ------ app/lib/activitypub/activity/announce.rb | 4 +++ app/lib/activitypub/activity/create.rb | 2 +- app/models/account.rb | 11 ++----- app/models/account_tag_stat.rb | 24 -------------- app/models/tag.rb | 38 +++++----------------- app/models/tag_filter.rb | 2 -- app/models/trending_tags.rb | 12 ++++--- app/services/process_hashtags_service.rb | 3 +- app/services/reblog_service.rb | 11 +++++++ app/views/admin/tags/_tag.html.haml | 2 -- app/views/admin/tags/index.html.haml | 9 ++--- app/views/admin/tags/show.html.haml | 13 ++------ config/locales/en.yml | 7 ++-- config/locales/simple_form.en.yml | 2 +- config/routes.rb | 2 -- .../20210502233513_drop_account_tag_stats.rb | 13 ++++++++ db/schema.rb | 10 ------ spec/models/account_tag_stat_spec.rb | 38 ---------------------- spec/models/trending_tags_spec.rb | 6 ++-- 20 files changed, 59 insertions(+), 160 deletions(-) delete mode 100644 app/models/account_tag_stat.rb create mode 100644 db/post_migrate/20210502233513_drop_account_tag_stats.rb delete mode 100644 spec/models/account_tag_stat_spec.rb (limited to 'spec') diff --git a/app/controllers/directories_controller.rb b/app/controllers/directories_controller.rb index f198ad5ba..f28c5b2af 100644 --- a/app/controllers/directories_controller.rb +++ b/app/controllers/directories_controller.rb @@ -6,7 +6,6 @@ class DirectoriesController < ApplicationController before_action :authenticate_user!, if: :whitelist_mode? before_action :require_enabled! before_action :set_instance_presenter - before_action :set_tag, only: :show before_action :set_accounts skip_before_action :require_functional!, unless: :whitelist_mode? @@ -15,23 +14,14 @@ class DirectoriesController < ApplicationController render :index end - def show - render :index - end - private def require_enabled! return not_found unless Setting.profile_directory end - def set_tag - @tag = Tag.discoverable.find_normalized!(params[:id]) - end - def set_accounts @accounts = Account.local.discoverable.by_recent_status.page(params[:page]).per(20).tap do |query| - query.merge!(Account.tagged_with(@tag.id)) if @tag query.merge!(Account.not_excluded_by_account(current_account)) if current_account end end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index a1081522e..9f778ffb9 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -22,6 +22,10 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity visibility: visibility_from_audience ) + original_status.tags.each do |tag| + tag.use!(@account) + end + distribute(@status) end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index c7a655d9d..e46361c14 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -164,7 +164,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def attach_tags(status) @tags.each do |tag| status.tags << tag - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(@account, status: status, at_time: status.created_at) if status.public_visibility? end @mentions.each do |mention| diff --git a/app/models/account.rb b/app/models/account.rb index a573365de..994459338 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -111,7 +111,6 @@ class Account < ApplicationRecord scope :searchable, -> { without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } - 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, accounts.id desc')) } scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) } scope :popular, -> { order('account_stats.followers_count desc') } @@ -279,19 +278,13 @@ class Account < ApplicationRecord if hashtags_map.key?(tag.name) hashtags_map.delete(tag.name) else - transaction do - tags.delete(tag) - tag.decrement_count!(:accounts_count) - end + tags.delete(tag) end end # Add hashtags that were so far missing hashtags_map.each_value do |tag| - transaction do - tags << tag - tag.increment_count!(:accounts_count) - end + tags << tag end end diff --git a/app/models/account_tag_stat.rb b/app/models/account_tag_stat.rb deleted file mode 100644 index 3c36c155a..000000000 --- a/app/models/account_tag_stat.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true -# == Schema Information -# -# Table name: account_tag_stats -# -# id :bigint(8) not null, primary key -# tag_id :bigint(8) not null -# accounts_count :bigint(8) default(0), not null -# hidden :boolean default(FALSE), not null -# created_at :datetime not null -# updated_at :datetime not null -# - -class AccountTagStat < ApplicationRecord - belongs_to :tag, inverse_of: :account_tag_stat - - def increment_count!(key) - update(key => public_send(key) + 1) - end - - def decrement_count!(key) - update(key => [public_send(key) - 1, 0].max) - end -end diff --git a/app/models/tag.rb b/app/models/tag.rb index efffc7eee..735c30608 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -20,10 +20,8 @@ class Tag < ApplicationRecord has_and_belongs_to_many :statuses has_and_belongs_to_many :accounts - has_and_belongs_to_many :sample_accounts, -> { local.discoverable.popular.limit(3) }, class_name: 'Account' has_many :featured_tags, dependent: :destroy, inverse_of: :tag - has_one :account_tag_stat, dependent: :destroy HASHTAG_SEPARATORS = "_\u00B7\u200c" HASHTAG_NAME_RE = "([[:word:]_][[:word:]#{HASHTAG_SEPARATORS}]*[[:alpha:]#{HASHTAG_SEPARATORS}][[:word:]#{HASHTAG_SEPARATORS}]*[[:word:]_])|([[:word:]_]*[[:alpha:]][[:word:]_]*)" @@ -38,29 +36,11 @@ class Tag < ApplicationRecord scope :usable, -> { where(usable: [true, nil]) } scope :listable, -> { where(listable: [true, nil]) } scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } - scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } - # Search with case-sensitive to use B-tree index. - scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } - - delegate :accounts_count, - :accounts_count=, - :increment_count!, - :decrement_count!, - to: :account_tag_stat - - after_save :save_account_tag_stat + scope :matches_name, ->(term) { where(arel_table[:name].lower.matches(arel_table.lower("#{sanitize_sql_like(Tag.normalize(term))}%"), nil, true)) } # Search with case-sensitive to use B-tree index update_index('tags#tag', :self) - def account_tag_stat - super || build_account_tag_stat - end - - def cached_sample_accounts - Rails.cache.fetch("#{cache_key}/sample_accounts", expires_in: 12.hours) { sample_accounts } - end - def to_param name end @@ -95,6 +75,10 @@ class Tag < ApplicationRecord requested_review_at.present? end + def use!(account, status: nil, at_time: Time.now.utc) + TrendingTags.record_use!(self, account, status: status, at_time: at_time) + end + def trending? TrendingTags.trending?(self) end @@ -127,9 +111,10 @@ class Tag < ApplicationRecord end def search_for(term, limit = 5, offset = 0, options = {}) - striped_term = term.strip - query = Tag.listable.matches_name(striped_term) - query = query.merge(matching_name(striped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] + stripped_term = term.strip + + query = Tag.listable.matches_name(stripped_term) + query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed] query.order(Arel.sql('length(name) ASC, name ASC')) .limit(limit) @@ -161,11 +146,6 @@ class Tag < ApplicationRecord private - def save_account_tag_stat - return unless account_tag_stat&.changed? - account_tag_stat.save - end - def validate_name_change errors.add(:name, I18n.t('tags.does_not_match_previous_name')) unless name_was.mb_chars.casecmp(name.mb_chars).zero? end diff --git a/app/models/tag_filter.rb b/app/models/tag_filter.rb index a9ff5b703..85bfcbea5 100644 --- a/app/models/tag_filter.rb +++ b/app/models/tag_filter.rb @@ -33,8 +33,6 @@ class TagFilter def scope_for(key, value) case key.to_s - when 'directory' - Tag.discoverable when 'reviewed' Tag.reviewed.order(reviewed_at: :desc) when 'unreviewed' diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb index 9c2aa0ee8..31890b082 100644 --- a/app/models/trending_tags.rb +++ b/app/models/trending_tags.rb @@ -13,19 +13,23 @@ class TrendingTags class << self include Redisable - def record_use!(tag, account, at_time = Time.now.utc) - return if account.silenced? || account.bot? || !tag.usable? || !(tag.trendable? || tag.requires_review?) + def record_use!(tag, account, status: nil, at_time: Time.now.utc) + return unless tag.usable? && !account.silenced? + # Even if a tag is not allowed to trend, we still need to + # record the stats since they can be displayed in other places increment_historical_use!(tag.id, at_time) increment_unique_use!(tag.id, account.id, at_time) increment_use!(tag.id, at_time) - tag.update(last_status_at: Time.now.utc) if tag.last_status_at.nil? || tag.last_status_at < 12.hours.ago + # Only update when the tag was last used once every 12 hours + # and only if a status is given (lets use ignore reblogs) + tag.update(last_status_at: at_time) if status.present? && (tag.last_status_at.nil? || (tag.last_status_at < at_time && tag.last_status_at < 12.hours.ago)) end def update!(at_time = Time.now.utc) tag_ids = redis.smembers("#{KEY}:used:#{at_time.beginning_of_day.to_i}") + redis.zrange(KEY, 0, -1) - tags = Tag.where(id: tag_ids.uniq) + tags = Tag.trendable.where(id: tag_ids.uniq) # First pass to calculate scores and update the set diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index e8e139b05..c42b79db8 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -8,8 +8,7 @@ class ProcessHashtagsService < BaseService Tag.find_or_create_by_names(tags) do |tag| status.tags << tag records << tag - - TrendingTags.record_use!(tag, status.account, status.created_at) if status.public_visibility? + tag.use!(status.account, status: status, at_time: status.created_at) if status.public_visibility? end return unless status.distributable? diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 5032397b3..744bdf567 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -35,6 +35,7 @@ class ReblogService < BaseService create_notification(reblog) bump_potential_friendship(account, reblog) + record_use(account, reblog) reblog end @@ -59,6 +60,16 @@ class ReblogService < BaseService PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog) end + def record_use(account, reblog) + return unless reblog.public_visibility? + + original_status = reblog.reblog + + original_status.tags.each do |tag| + tag.use!(account) + end + end + def build_json(reblog) Oj.dump(serialize_payload(ActivityPub::ActivityPresenter.from_status(reblog), ActivityPub::ActivitySerializer, signer: reblog.account)) end diff --git a/app/views/admin/tags/_tag.html.haml b/app/views/admin/tags/_tag.html.haml index 287d28e53..adf4ca7b2 100644 --- a/app/views/admin/tags/_tag.html.haml +++ b/app/views/admin/tags/_tag.html.haml @@ -10,8 +10,6 @@ = tag.name %small - = t('admin.tags.in_directory', count: tag.accounts_count) - • = t('admin.tags.unique_uses_today', count: tag.history.first[:accounts]) - if tag.trending? diff --git a/app/views/admin/tags/index.html.haml b/app/views/admin/tags/index.html.haml index d7719d45d..d78f3c6d1 100644 --- a/app/views/admin/tags/index.html.haml +++ b/app/views/admin/tags/index.html.haml @@ -5,12 +5,6 @@ = javascript_pack_tag 'admin', async: true, crossorigin: 'anonymous' .filters - .filter-subset - %strong= t('admin.tags.context') - %ul - %li= filter_link_to t('generic.all'), directory: nil - %li= filter_link_to t('admin.tags.directory'), directory: '1' - .filter-subset %strong= t('admin.tags.review') %ul @@ -23,8 +17,9 @@ %strong= t('generic.order_by') %ul %li= filter_link_to t('admin.tags.most_recent'), popular: nil, active: nil - %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil %li= filter_link_to t('admin.tags.last_active'), active: '1', popular: nil + %li= filter_link_to t('admin.tags.most_popular'), popular: '1', active: nil + = form_tag admin_tags_url, method: 'GET', class: 'simple_form' do .fields-group diff --git a/app/views/admin/tags/show.html.haml b/app/views/admin/tags/show.html.haml index c9a147587..c4caffda1 100644 --- a/app/views/admin/tags/show.html.haml +++ b/app/views/admin/tags/show.html.haml @@ -10,15 +10,6 @@ %div .dashboard__counters__num= number_with_delimiter @accounts_week .dashboard__counters__label= t 'admin.tags.accounts_week' - %div - - if @tag.accounts_count > 0 - = link_to explore_hashtag_path(@tag) do - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' - - else - %div - .dashboard__counters__num= number_with_delimiter @tag.accounts_count - .dashboard__counters__label= t 'admin.tags.directory' %hr.spacer/ @@ -30,8 +21,8 @@ .fields-group = f.input :usable, as: :boolean, wrapper: :with_label - = f.input :trendable, as: :boolean, wrapper: :with_label, disabled: !Setting.trends - = f.input :listable, as: :boolean, wrapper: :with_label, disabled: !Setting.profile_directory + = f.input :trendable, as: :boolean, wrapper: :with_label + = f.input :listable, as: :boolean, wrapper: :with_label .actions = f.button :button, t('generic.save_changes'), type: :submit diff --git a/config/locales/en.yml b/config/locales/en.yml index bfa489817..d8ad5bd84 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -698,12 +698,9 @@ en: accounts_today: Unique uses today accounts_week: Unique uses this week breakdown: Breakdown of today's usage by source - context: Context - directory: In directory - in_directory: "%{count} in directory" - last_active: Last active + last_active: Recently used most_popular: Most popular - most_recent: Most recent + most_recent: Recently created name: Hashtag review: Review status reviewed: Reviewed diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 8ff880ebc..c4388ffc5 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -208,7 +208,7 @@ en: rule: text: Rule tag: - listable: Allow this hashtag to appear in searches and on the profile directory + listable: Allow this hashtag to appear in searches and suggestions name: Hashtag trendable: Allow this hashtag to appear under trends usable: Allow posts to use this hashtag diff --git a/config/routes.rb b/config/routes.rb index 8ca7fccdd..2373d8a51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -97,8 +97,6 @@ Rails.application.routes.draw do post '/interact/:id', to: 'remote_interaction#create' get '/explore', to: 'directories#index', as: :explore - get '/explore/:id', to: 'directories#show', as: :explore_hashtag - get '/settings', to: redirect('/settings/profile') namespace :settings do diff --git a/db/post_migrate/20210502233513_drop_account_tag_stats.rb b/db/post_migrate/20210502233513_drop_account_tag_stats.rb new file mode 100644 index 000000000..80adadcab --- /dev/null +++ b/db/post_migrate/20210502233513_drop_account_tag_stats.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DropAccountTagStats < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + drop_table :account_tag_stats + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index 88e906079..19b1afe00 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -115,15 +115,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true end - create_table "account_tag_stats", force: :cascade do |t| - t.bigint "tag_id", null: false - t.bigint "accounts_count", default: 0, null: false - t.boolean "hidden", default: false, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["tag_id"], name: "index_account_tag_stats_on_tag_id", unique: true - end - create_table "account_warning_presets", force: :cascade do |t| t.text "text", default: "", null: false t.datetime "created_at", null: false @@ -985,7 +976,6 @@ ActiveRecord::Schema.define(version: 2021_05_05_174616) do add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade - add_foreign_key "account_tag_stats", "tags", on_delete: :cascade add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_warnings", "accounts", on_delete: :nullify add_foreign_key "accounts", "accounts", column: "moved_to_account_id", on_delete: :nullify diff --git a/spec/models/account_tag_stat_spec.rb b/spec/models/account_tag_stat_spec.rb deleted file mode 100644 index 6d3057f35..000000000 --- a/spec/models/account_tag_stat_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe AccountTagStat, type: :model do - key = 'accounts_count' - let(:account_tag_stat) { Fabricate(:tag).account_tag_stat } - - describe '#increment_count!' do - it 'calls #update' do - args = { key => account_tag_stat.public_send(key) + 1 } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.increment_count!(key) - end - - it 'increments value by 1' do - expect do - account_tag_stat.increment_count!(key) - end.to change { account_tag_stat.accounts_count }.by(1) - end - end - - describe '#decrement_count!' do - it 'calls #update' do - args = { key => [account_tag_stat.public_send(key) - 1, 0].max } - expect(account_tag_stat).to receive(:update).with(args) - account_tag_stat.decrement_count!(key) - end - - it 'decrements value by 1' do - account_tag_stat.update(key => 1) - - expect do - account_tag_stat.decrement_count!(key) - end.to change { account_tag_stat.accounts_count }.by(-1) - end - end -end diff --git a/spec/models/trending_tags_spec.rb b/spec/models/trending_tags_spec.rb index b6122c994..dfbc7d6f8 100644 --- a/spec/models/trending_tags_spec.rb +++ b/spec/models/trending_tags_spec.rb @@ -7,9 +7,9 @@ RSpec.describe TrendingTags do describe '.update!' do let!(:at_time) { Time.now.utc } - let!(:tag1) { Fabricate(:tag, name: 'Catstodon') } - let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon') } - let!(:tag3) { Fabricate(:tag, name: 'OCs') } + let!(:tag1) { Fabricate(:tag, name: 'Catstodon', trendable: true) } + let!(:tag2) { Fabricate(:tag, name: 'DogsOfMastodon', trendable: true) } + let!(:tag3) { Fabricate(:tag, name: 'OCs', trendable: true) } before do allow(Redis.current).to receive(:pfcount) do |key| -- cgit