From 47584180d8ed03666d45f423107d3113cc4a6230 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 1 Sep 2019 19:44:05 +0200 Subject: Fix wrong percentages in admin UI for hashtag usage breakdown (#11714) --- app/controllers/admin/tags_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app/controllers') diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 39aca2a4b..25d9b7d3d 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -37,7 +37,8 @@ module Admin def set_usage_by_domain @usage_by_domain = @tag.statuses - .where(visibility: :public) + .with_public_visibility + .excluding_silenced_accounts .where(Status.arel_table[:id].gteq(Mastodon::Snowflake.id_at(Time.now.utc.beginning_of_day))) .joins(:account) .group('accounts.domain') -- cgit From 70ddef2654a931827ce5e4323e3042365f6078f2 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 2 Sep 2019 18:11:13 +0200 Subject: Change trending hashtags to not disappear instantly after midnight (#11712) --- app/controllers/admin/tags_controller.rb | 2 +- app/lib/feed_manager.rb | 2 +- app/models/tag.rb | 4 +- app/models/trending_tags.rb | 102 +++++++++++++++------ app/workers/scheduler/trending_tags_scheduler.rb | 11 +++ config/sidekiq.yml | 3 + db/migrate/20190901035623_add_max_score_to_tags.rb | 6 ++ .../20190901040524_remove_score_from_tags.rb | 12 +++ db/schema.rb | 6 +- spec/models/trending_tags_spec.rb | 68 ++++++++++++++ 10 files changed, 179 insertions(+), 37 deletions(-) create mode 100644 app/workers/scheduler/trending_tags_scheduler.rb create mode 100644 db/migrate/20190901035623_add_max_score_to_tags.rb create mode 100644 db/post_migrate/20190901040524_remove_score_from_tags.rb create mode 100644 spec/models/trending_tags_spec.rb (limited to 'app/controllers') diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 25d9b7d3d..8bd4e5f8b 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/app/controllers/admin/tags_controller.rb @@ -57,7 +57,7 @@ module Admin scope = scope.unreviewed if filter_params[:review] == 'unreviewed' scope = scope.reviewed.order(reviewed_at: :desc) if filter_params[:review] == 'reviewed' scope = scope.pending_review.order(requested_review_at: :desc) if filter_params[:review] == 'pending_review' - scope.order(score: :desc) + scope.order(max_score: :desc) end def filter_params diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index ca3d890a8..871ec5c19 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -63,7 +63,7 @@ class FeedManager reblog_key = key(type, account_id, 'reblogs') # Remove any items past the MAX_ITEMS'th entry in our feed - redis.zremrangebyrank(timeline_key, '0', (-(FeedManager::MAX_ITEMS + 1)).to_s) + redis.zremrangebyrank(timeline_key, 0, -(FeedManager::MAX_ITEMS + 1)) # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop # tracking anything after it for deduplication purposes. diff --git a/app/models/tag.rb b/app/models/tag.rb index 945e3a3c6..135e0a030 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -7,14 +7,14 @@ # name :string default(""), not null # created_at :datetime not null # updated_at :datetime not null -# score :integer # usable :boolean # trendable :boolean # listable :boolean # reviewed_at :datetime # requested_review_at :datetime # last_status_at :datetime -# last_trend_at :datetime +# max_score :float +# max_score_at :datetime # class Tag < ApplicationRecord diff --git a/app/models/trending_tags.rb b/app/models/trending_tags.rb index e4ce988c1..e1b92b175 100644 --- a/app/models/trending_tags.rb +++ b/app/models/trending_tags.rb @@ -7,6 +7,8 @@ class TrendingTags THRESHOLD = 5 LIMIT = 10 REVIEW_THRESHOLD = 3 + MAX_SCORE_COOLDOWN = 3.days.freeze + MAX_SCORE_HALFLIFE = 6.hours.freeze class << self include Redisable @@ -16,14 +18,75 @@ class TrendingTags increment_historical_use!(tag.id, at_time) increment_unique_use!(tag.id, account.id, at_time) - increment_vote!(tag, 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 - tag.update(last_trend_at: Time.now.utc) if trending?(tag) && (tag.last_trend_at.nil? || tag.last_trend_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) + + # First pass to calculate scores and update the set + + tags.each do |tag| + expected = redis.pfcount("activity:tags:#{tag.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts").to_f + expected = 1.0 if expected.zero? + observed = redis.pfcount("activity:tags:#{tag.id}:#{at_time.beginning_of_day.to_i}:accounts").to_f + max_time = tag.max_score_at + max_score = tag.max_score + max_score = 0 if max_time.nil? || max_time < (at_time - MAX_SCORE_COOLDOWN) + + score = begin + if expected > observed || observed < THRESHOLD + 0 + else + ((observed - expected)**2) / expected + end + end + + if score > max_score + max_score = score + max_time = at_time + + # Not interested in triggering any callbacks for this + tag.update_columns(max_score: max_score, max_score_at: max_time) + end + + decaying_score = max_score * (0.5**((at_time.to_f - max_time.to_f) / MAX_SCORE_HALFLIFE.to_f)) + + if decaying_score.zero? + redis.zrem(KEY, tag.id) + else + redis.zadd(KEY, decaying_score, tag.id) + end + end + + users_for_review = User.staff.includes(:account).to_a.select(&:allows_trending_tag_emails?) + + # Second pass to notify about previously unreviewed trends + + tags.each do |tag| + current_rank = redis.zrevrank(KEY, tag.id) + needs_review_notification = tag.requires_review? && !tag.requested_review? + rank_passes_threshold = current_rank.present? && current_rank <= REVIEW_THRESHOLD + + next unless !tag.trendable? && rank_passes_threshold && needs_review_notification + + tag.touch(:requested_review_at) + + users_for_review.each do |user| + AdminMailer.new_trending_tag(user.account, tag).deliver_later! + end + end + + # Trim older items + + redis.zremrangebyrank(KEY, 0, -(LIMIT + 1)) end def get(limit, filtered: true) - tag_ids = redis.zrevrange("#{KEY}:#{Time.now.utc.beginning_of_day.to_i}", 0, LIMIT - 1).map(&:to_i) + tag_ids = redis.zrevrange(KEY, 0, LIMIT - 1).map(&:to_i) tags = Tag.where(id: tag_ids) tags = tags.where(trendable: true) if filtered @@ -33,8 +96,8 @@ class TrendingTags end def trending?(tag) - rank = redis.zrevrank("#{KEY}:#{Time.now.utc.beginning_of_day.to_i}", tag.id) - rank.present? && rank <= LIMIT + rank = redis.zrevrank(KEY, tag.id) + rank.present? && rank < LIMIT end private @@ -51,31 +114,10 @@ class TrendingTags redis.expire(key, EXPIRE_HISTORY_AFTER) end - def increment_vote!(tag, at_time) - key = "#{KEY}:#{at_time.beginning_of_day.to_i}" - expected = redis.pfcount("activity:tags:#{tag.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts").to_f - expected = 1.0 if expected.zero? - observed = redis.pfcount("activity:tags:#{tag.id}:#{at_time.beginning_of_day.to_i}:accounts").to_f - - if expected > observed || observed < THRESHOLD - redis.zrem(key, tag.id) - else - score = ((observed - expected)**2) / expected - old_rank = redis.zrevrank(key, tag.id) - - redis.zadd(key, score, tag.id) - request_review!(tag) if (old_rank.nil? || old_rank > REVIEW_THRESHOLD) && redis.zrevrank(key, tag.id) <= REVIEW_THRESHOLD && !tag.trendable? && tag.requires_review? && !tag.requested_review? - end - - redis.expire(key, EXPIRE_TRENDS_AFTER) - end - - def request_review!(tag) - return unless Setting.trends - - tag.touch(:requested_review_at) - - User.staff.includes(:account).find_each { |u| AdminMailer.new_trending_tag(u.account, tag).deliver_later! if u.allows_trending_tag_emails? } + def increment_use!(tag_id, at_time) + key = "#{KEY}:used:#{at_time.beginning_of_day.to_i}" + redis.sadd(key, tag_id) + redis.expire(key, EXPIRE_HISTORY_AFTER) end end end diff --git a/app/workers/scheduler/trending_tags_scheduler.rb b/app/workers/scheduler/trending_tags_scheduler.rb new file mode 100644 index 000000000..77f0d5747 --- /dev/null +++ b/app/workers/scheduler/trending_tags_scheduler.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class Scheduler::TrendingTagsScheduler + include Sidekiq::Worker + + sidekiq_options unique: :until_executed, retry: 0 + + def perform + TrendingTags.update! if Setting.trends + end +end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 6ebe450b0..5de25de23 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -9,6 +9,9 @@ scheduled_statuses_scheduler: every: '5m' class: Scheduler::ScheduledStatusesScheduler + trending_tags_scheduler: + every: '5m' + class: Scheduler::TrendingTagsScheduler media_cleanup_scheduler: cron: '<%= Random.rand(0..59) %> <%= Random.rand(3..5) %> * * *' class: Scheduler::MediaCleanupScheduler diff --git a/db/migrate/20190901035623_add_max_score_to_tags.rb b/db/migrate/20190901035623_add_max_score_to_tags.rb new file mode 100644 index 000000000..f936e9871 --- /dev/null +++ b/db/migrate/20190901035623_add_max_score_to_tags.rb @@ -0,0 +1,6 @@ +class AddMaxScoreToTags < ActiveRecord::Migration[5.2] + def change + add_column :tags, :max_score, :float + add_column :tags, :max_score_at, :datetime + end +end diff --git a/db/post_migrate/20190901040524_remove_score_from_tags.rb b/db/post_migrate/20190901040524_remove_score_from_tags.rb new file mode 100644 index 000000000..a1112700b --- /dev/null +++ b/db/post_migrate/20190901040524_remove_score_from_tags.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class RemoveScoreFromTags < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + safety_assured do + remove_column :tags, :score, :int + remove_column :tags, :last_trend_at, :datetime + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 482bca367..5576f70bf 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: 2019_08_23_221802) do +ActiveRecord::Schema.define(version: 2019_09_01_040524) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -664,14 +664,14 @@ ActiveRecord::Schema.define(version: 2019_08_23_221802) do t.string "name", default: "", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.integer "score" t.boolean "usable" t.boolean "trendable" t.boolean "listable" t.datetime "reviewed_at" t.datetime "requested_review_at" t.datetime "last_status_at" - t.datetime "last_trend_at" + t.float "max_score" + t.datetime "max_score_at" t.index "lower((name)::text)", name: "index_tags_on_name_lower", unique: true end diff --git a/spec/models/trending_tags_spec.rb b/spec/models/trending_tags_spec.rb new file mode 100644 index 000000000..b6122c994 --- /dev/null +++ b/spec/models/trending_tags_spec.rb @@ -0,0 +1,68 @@ +require 'rails_helper' + +RSpec.describe TrendingTags do + describe '.record_use!' do + pending + end + + 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') } + + before do + allow(Redis.current).to receive(:pfcount) do |key| + case key + when "activity:tags:#{tag1.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts" + 2 + when "activity:tags:#{tag1.id}:#{at_time.beginning_of_day.to_i}:accounts" + 16 + when "activity:tags:#{tag2.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts" + 0 + when "activity:tags:#{tag2.id}:#{at_time.beginning_of_day.to_i}:accounts" + 4 + when "activity:tags:#{tag3.id}:#{(at_time - 1.day).beginning_of_day.to_i}:accounts" + 13 + end + end + + Redis.current.zadd('trending_tags', 0.9, tag3.id) + Redis.current.sadd("trending_tags:used:#{at_time.beginning_of_day.to_i}", [tag1.id, tag2.id]) + + tag3.update(max_score: 0.9, max_score_at: (at_time - 1.day).beginning_of_day + 12.hours) + + described_class.update!(at_time) + end + + it 'calculates and re-calculates scores' do + expect(described_class.get(10, filtered: false)).to eq [tag1, tag3] + end + + it 'omits hashtags below threshold' do + expect(described_class.get(10, filtered: false)).to_not include(tag2) + end + + it 'decays scores' do + expect(Redis.current.zscore('trending_tags', tag3.id)).to be < 0.9 + end + end + + describe '.trending?' do + let(:tag) { Fabricate(:tag) } + + before do + 10.times { |i| Redis.current.zadd('trending_tags', i + 1, Fabricate(:tag).id) } + end + + it 'returns true if the hashtag is within limit' do + Redis.current.zadd('trending_tags', 11, tag.id) + expect(described_class.trending?(tag)).to be true + end + + it 'returns false if the hashtag is outside the limit' do + Redis.current.zadd('trending_tags', 0, tag.id) + expect(described_class.trending?(tag)).to be false + end + end +end -- cgit From 43f56f12917f154fbb70cbc305daba9e2fd364ed Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 4 Sep 2019 04:13:54 +0200 Subject: Change account deletion page to have better explanations (#11753) Fix deletion of unconfirmed account not freeing up the username Add prefill of logged-in user's email in the reconfirmation form --- app/controllers/auth/confirmations_controller.rb | 23 +++++++++++++++++++++++ app/javascript/styles/mastodon/forms.scss | 9 +++++++++ app/services/suspend_account_service.rb | 1 + app/views/auth/setup/show.html.haml | 5 +---- app/views/auth/shared/_links.html.haml | 22 ++++++++++++++-------- app/views/settings/deletes/show.html.haml | 24 +++++++++++++++++------- config/locales/en.yml | 16 ++++++++++++---- 7 files changed, 77 insertions(+), 23 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 0d7c6e7c2..3e419eb96 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -4,15 +4,38 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController layout 'auth' before_action :set_body_classes + before_action :require_unconfirmed! skip_before_action :require_functional! + def new + super + + resource.email = current_user.unconfirmed_email || current_user.email if user_signed_in? + end + private + def require_unconfirmed! + redirect_to edit_user_registration_path if user_signed_in? && current_user.confirmed? && current_user.unconfirmed_email.blank? + end + def set_body_classes @body_classes = 'lighter' end + def after_resending_confirmation_instructions_path_for(_resource_name) + if user_signed_in? + if user.confirmed? && user.approved? + edit_user_registration_path + else + auth_setup_path + end + else + new_user_session_path + end + end + def after_confirmation_path_for(_resource_name, user) if user.created_by_application && truthy_param?(:redirect_to_app) user.created_by_application.redirect_uri diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index ac99124ea..16352340b 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -112,6 +112,15 @@ code { padding: 0.2em 0.4em; background: darken($ui-base-color, 12%); } + + li { + list-style: disc; + margin-left: 18px; + } + } + + ul.hint { + margin-bottom: 15px; } span.hint { diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 902af376c..85da7e921 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -61,6 +61,7 @@ class SuspendAccountService < BaseService return if !@account.local? || @account.user.nil? if @options[:including_user] + @options[:destroy] = true if !@account.user_confirmed? || @account.user_pending? @account.user.destroy else @account.user.disable! diff --git a/app/views/auth/setup/show.html.haml b/app/views/auth/setup/show.html.haml index 8bb44ca7f..c14fed56f 100644 --- a/app/views/auth/setup/show.html.haml +++ b/app/views/auth/setup/show.html.haml @@ -17,7 +17,4 @@ .simple_form %p.hint= t('auth.setup.email_settings_hint_html', email: content_tag(:strong, @user.email)) -.form-footer - %ul.no-list - %li= link_to t('settings.account_settings'), edit_user_registration_path - %li= link_to t('auth.logout'), destroy_user_session_path, data: { method: :delete } +.form-footer= render 'auth/shared/links' diff --git a/app/views/auth/shared/_links.html.haml b/app/views/auth/shared/_links.html.haml index 3c68ccd22..e6c3f7cca 100644 --- a/app/views/auth/shared/_links.html.haml +++ b/app/views/auth/shared/_links.html.haml @@ -1,12 +1,18 @@ %ul.no-list - - if controller_name != 'sessions' - %li= link_to t('auth.login'), new_session_path(resource_name) + - if user_signed_in? + %li= link_to t('settings.account_settings'), edit_user_registration_path + - else + - if controller_name != 'sessions' + %li= link_to t('auth.login'), new_user_session_path - - if devise_mapping.registerable? && controller_name != 'registrations' - %li= link_to t('auth.register'), available_sign_up_path + - if controller_name != 'registrations' + %li= link_to t('auth.register'), available_sign_up_path - - if devise_mapping.recoverable? && controller_name != 'passwords' && controller_name != 'registrations' - %li= link_to t('auth.forgot_password'), new_password_path(resource_name) + - if controller_name != 'passwords' && controller_name != 'registrations' + %li= link_to t('auth.forgot_password'), new_user_password_path - - if devise_mapping.confirmable? && controller_name != 'confirmations' - %li= link_to t('auth.didnt_get_confirmation'), new_confirmation_path(resource_name) + - if controller_name != 'confirmations' + %li= link_to t('auth.didnt_get_confirmation'), new_user_confirmation_path + + - if user_signed_in? && controller_name != 'setup' + %li= link_to t('auth.logout'), destroy_user_session_path, data: { method: :delete } diff --git a/app/views/settings/deletes/show.html.haml b/app/views/settings/deletes/show.html.haml index b246f83a1..6e2ff31c5 100644 --- a/app/views/settings/deletes/show.html.haml +++ b/app/views/settings/deletes/show.html.haml @@ -2,15 +2,25 @@ = t('settings.delete') = simple_form_for @confirmation, url: settings_delete_path, method: :delete do |f| - .warning - %strong - = fa_icon('warning') - = t('deletes.warning_title') - = t('deletes.warning_html') + %p.hint= t('deletes.warning.before') - %p.hint= t('deletes.description_html') + %ul.hint + - if current_user.confirmed? && current_user.approved? + %li.warning-hint= t('deletes.warning.irreversible') + %li.warning-hint= t('deletes.warning.username_unavailable') + %li.warning-hint= t('deletes.warning.data_removal') + %li.warning-hint= t('deletes.warning.caches') + - else + %li.positive-hint= t('deletes.warning.email_change_html', path: edit_user_registration_path) + %li.positive-hint= t('deletes.warning.email_reconfirmation_html', path: new_user_confirmation_path) + %li.positive-hint= t('deletes.warning.email_contact_html', email: Setting.site_contact_email) + %li.positive-hint= t('deletes.warning.username_available') - = f.input :password, placeholder: t('simple_form.labels.defaults.current_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password'), :autocomplete => 'off' }, hint: t('deletes.confirm_password') + %p.hint= t('deletes.warning.more_details_html', terms_path: terms_path) + + %hr.spacer/ + + = f.input :password, wrapper: :with_block_label, input_html: { :autocomplete => 'off' }, hint: t('deletes.confirm_password') .actions = f.button :button, t('deletes.proceed'), type: :submit, class: 'negative' diff --git a/config/locales/en.yml b/config/locales/en.yml index ad29e0a74..687f5f2a0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -626,13 +626,21 @@ en: x_months: "%{count}mo" x_seconds: "%{count}s" deletes: - bad_password_msg: Nice try, hackers! Incorrect password + bad_password_msg: The password you entered was incorrect confirm_password: Enter your current password to verify your identity - description_html: This will permanently, irreversibly remove content from your account and deactivate it. Your username will remain reserved to prevent future impersonations. proceed: Delete account success_msg: Your account was successfully deleted - warning_html: Only deletion of content from this particular server is guaranteed. Content that has been widely shared is likely to leave traces. Offline servers and servers that have unsubscribed from your updates will not update their databases. - warning_title: Disseminated content availability + warning: + before: 'Before proceeding, please read these notes carefully:' + caches: Content that has been cached by other servers may persist + data_removal: Your posts and other data will be permanently removed + email_change_html: You can change your e-mail address without deleting your account + email_contact_html: If it still doesn't arrive, you can e-mail %{email} for help + email_reconfirmation_html: If you are not receiving the confirmation e-mail, you can request it again + irreversible: You will not be able to restore or reactivate your account + more_details_html: For more details, see the privacy policy. + username_available: Your username will become available again + username_unavailable: Your username will remain unavailable directories: directory: Profile directory explanation: Discover users based on their interests -- cgit From 58755439ac758f173cf6c8afb49d62b450782d14 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 5 Sep 2019 06:13:50 +0200 Subject: Fix wrong variable regression from #11753 (#11763) --- app/controllers/auth/confirmations_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers') diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 3e419eb96..898525269 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -26,7 +26,7 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController def after_resending_confirmation_instructions_path_for(_resource_name) if user_signed_in? - if user.confirmed? && user.approved? + if current_user.confirmed? && current_user.approved? edit_user_registration_path else auth_setup_path -- cgit