From 465ee7792ff48905088efdf3df6f718081b5e244 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 7 Apr 2022 18:06:15 +0200 Subject: Fix pagination header on empty trends responses in REST API (#17986) --- app/controllers/api/v1/trends/links_controller.rb | 6 +++++- app/controllers/api/v1/trends/statuses_controller.rb | 6 +++++- app/controllers/api/v1/trends/tags_controller.rb | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/trends/links_controller.rb b/app/controllers/api/v1/trends/links_controller.rb index b1cde5a4b..2385fe438 100644 --- a/app/controllers/api/v1/trends/links_controller.rb +++ b/app/controllers/api/v1/trends/links_controller.rb @@ -36,13 +36,17 @@ class Api::V1::Trends::LinksController < Api::BaseController end def next_path - api_v1_trends_links_url pagination_params(offset: offset_param + limit_param(DEFAULT_LINKS_LIMIT)) + api_v1_trends_links_url pagination_params(offset: offset_param + limit_param(DEFAULT_LINKS_LIMIT)) if records_continue? end def prev_path api_v1_trends_links_url pagination_params(offset: offset_param - limit_param(DEFAULT_LINKS_LIMIT)) if offset_param > limit_param(DEFAULT_LINKS_LIMIT) end + def records_continue? + @links.size == limit_param(DEFAULT_LINKS_LIMIT) + end + def offset_param params[:offset].to_i end diff --git a/app/controllers/api/v1/trends/statuses_controller.rb b/app/controllers/api/v1/trends/statuses_controller.rb index 4977803fb..1f2fff582 100644 --- a/app/controllers/api/v1/trends/statuses_controller.rb +++ b/app/controllers/api/v1/trends/statuses_controller.rb @@ -36,7 +36,7 @@ class Api::V1::Trends::StatusesController < Api::BaseController end def next_path - api_v1_trends_statuses_url pagination_params(offset: offset_param + limit_param(DEFAULT_STATUSES_LIMIT)) + api_v1_trends_statuses_url pagination_params(offset: offset_param + limit_param(DEFAULT_STATUSES_LIMIT)) if records_continue? end def prev_path @@ -46,4 +46,8 @@ class Api::V1::Trends::StatusesController < Api::BaseController def offset_param params[:offset].to_i end + + def records_continue? + @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) + end end diff --git a/app/controllers/api/v1/trends/tags_controller.rb b/app/controllers/api/v1/trends/tags_controller.rb index 329ef5ae7..38003f599 100644 --- a/app/controllers/api/v1/trends/tags_controller.rb +++ b/app/controllers/api/v1/trends/tags_controller.rb @@ -32,7 +32,7 @@ class Api::V1::Trends::TagsController < Api::BaseController end def next_path - api_v1_trends_tags_url pagination_params(offset: offset_param + limit_param(DEFAULT_TAGS_LIMIT)) + api_v1_trends_tags_url pagination_params(offset: offset_param + limit_param(DEFAULT_TAGS_LIMIT)) if records_continue? end def prev_path @@ -42,4 +42,8 @@ class Api::V1::Trends::TagsController < Api::BaseController def offset_param params[:offset].to_i end + + def records_continue? + @tags.size == limit_param(DEFAULT_TAGS_LIMIT) + end end -- cgit From 8e20e16cf030fef48850df4764bbf13a317f6b0c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 8 Apr 2022 18:03:31 +0200 Subject: Change e-mail notifications to only be sent when recipient is offline (#17984) * Change e-mail notifications to only be sent when recipient is offline Change the default for follow and mention notifications back on * Add preference to always send e-mail notifications * Change wording --- app/controllers/settings/preferences_controller.rb | 3 +- app/lib/user_settings_decorator.rb | 5 ++ app/models/user.rb | 2 +- app/services/notify_service.rb | 63 ++++++++++++++-------- .../preferences/notifications/show.html.haml | 3 ++ config/locales/simple_form.en.yml | 2 + config/settings.yml | 5 +- 7 files changed, 58 insertions(+), 25 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb index c7492700c..bfe651bc6 100644 --- a/app/controllers/settings/preferences_controller.rb +++ b/app/controllers/settings/preferences_controller.rb @@ -54,7 +54,8 @@ class Settings::PreferencesController < Settings::BaseController :setting_use_pending_items, :setting_trends, :setting_crop_images, - notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account trending_tag), + :setting_always_send_emails, + notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account trending_tag appeal), interactions: %i(must_be_follower must_be_following must_be_following_dm) ) end diff --git a/app/lib/user_settings_decorator.rb b/app/lib/user_settings_decorator.rb index de054e403..5fb7655a9 100644 --- a/app/lib/user_settings_decorator.rb +++ b/app/lib/user_settings_decorator.rb @@ -38,6 +38,7 @@ class UserSettingsDecorator user.settings['use_pending_items'] = use_pending_items_preference if change?('setting_use_pending_items') user.settings['trends'] = trends_preference if change?('setting_trends') user.settings['crop_images'] = crop_images_preference if change?('setting_crop_images') + user.settings['always_send_emails'] = always_send_emails_preference if change?('setting_always_send_emails') end def merged_notification_emails @@ -132,6 +133,10 @@ class UserSettingsDecorator boolean_cast_setting 'setting_crop_images' end + def always_send_emails_preference + boolean_cast_setting 'setting_always_send_emails' + end + def boolean_cast_setting(key) ActiveModel::Type::Boolean.new.cast(settings[key]) end diff --git a/app/models/user.rb b/app/models/user.rb index d19fe2c92..9da7b2639 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -130,7 +130,7 @@ class User < ApplicationRecord :reduce_motion, :system_font_ui, :noindex, :theme, :display_media, :expand_spoilers, :default_language, :aggregate_reblogs, :show_application, :advanced_layout, :use_blurhash, :use_pending_items, :trends, :crop_images, - :disable_swiping, + :disable_swiping, :always_send_emails, to: :settings, prefix: :setting, allow_nil: false attr_reader :invite_code diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index a90f17cfd..d30b33876 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class NotifyService < BaseService + include Redisable + def call(recipient, type, activity) @recipient = recipient @activity = activity @@ -8,10 +10,15 @@ class NotifyService < BaseService return if recipient.user.nil? || blocked? - create_notification! + @notification.save! + + # It's possible the underlying activity has been deleted + # between the save call and now + return if @notification.activity.nil? + push_notification! push_to_conversation! if direct_message? - send_email! if email_enabled? + send_email! if email_needed? rescue ActiveRecord::RecordInvalid nil end @@ -92,8 +99,8 @@ class NotifyService < BaseService end def blocked? - blocked = @recipient.suspended? # Skip if the recipient account is suspended anyway - blocked ||= from_self? && @notification.type != :poll # Skip for interactions with self + blocked = @recipient.suspended? + blocked ||= from_self? && @notification.type != :poll return blocked if message? && from_staff? @@ -117,38 +124,52 @@ class NotifyService < BaseService end end - def create_notification! - @notification.save! + def push_notification! + push_to_streaming_api! if subscribed_to_streaming_api? + push_to_web_push_subscriptions! end - def push_notification! - return if @notification.activity.nil? + def push_to_streaming_api! + redis.publish("timeline:#{@recipient.id}:notifications", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification))) + end - Redis.current.publish("timeline:#{@recipient.id}:notifications", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification))) - send_push_notifications! + def subscribed_to_streaming_api? + redis.exists?("subscribed:timeline:#{@recipient.id}") || redis.exists?("subscribed:timeline:#{@recipient.id}:notifications") end def push_to_conversation! - return if @notification.activity.nil? AccountConversation.add_status(@recipient, @notification.target_status) end - def send_push_notifications! - subscriptions_ids = ::Web::PushSubscription.where(user_id: @recipient.user.id) - .select { |subscription| subscription.pushable?(@notification) } - .map(&:id) + def push_to_web_push_subscriptions! + ::Web::PushNotificationWorker.push_bulk(web_push_subscriptions.select { |subscription| subscription.pushable?(@notification) }) { |subscription| [subscription.id, @notification.id] } + end - ::Web::PushNotificationWorker.push_bulk(subscriptions_ids) do |subscription_id| - [subscription_id, @notification.id] - end + def web_push_subscriptions + @web_push_subscriptions ||= ::Web::PushSubscription.where(user_id: @recipient.user.id).to_a + end + + def subscribed_to_web_push? + web_push_subscriptions.any? end def send_email! - return if @notification.activity.nil? - NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes) + NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes) if NotificationMailer.respond_to?(@notification.type) + end + + def email_needed? + (!recipient_online? || always_send_emails?) && send_email_for_notification_type? + end + + def recipient_online? + subscribed_to_streaming_api? || subscribed_to_web_push? + end + + def always_send_emails? + @recipient.user.settings.always_send_emails end - def email_enabled? + def send_email_for_notification_type? @recipient.user.settings.notification_emails[@notification.type.to_s] end end diff --git a/app/views/settings/preferences/notifications/show.html.haml b/app/views/settings/preferences/notifications/show.html.haml index 223e5d740..42754a852 100644 --- a/app/views/settings/preferences/notifications/show.html.haml +++ b/app/views/settings/preferences/notifications/show.html.haml @@ -25,6 +25,9 @@ = ff.input :pending_account, as: :boolean, wrapper: :with_label = ff.input :trending_tag, as: :boolean, wrapper: :with_label + .fields-group + = f.input :setting_always_send_emails, as: :boolean, wrapper: :with_label + .fields-group = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff| = ff.input :digest, as: :boolean, wrapper: :with_label diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index b19b7891f..b784b1da7 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -49,6 +49,7 @@ en: phrase: Will be matched regardless of casing in text or content warning of a post scopes: Which APIs the application will be allowed to access. If you select a top-level scope, you don't need to select individual ones. setting_aggregate_reblogs: Do not show new boosts for posts that have been recently boosted (only affects newly-received boosts) + setting_always_send_emails: Normally e-mail notifications won't be sent when you are actively using Mastodon setting_default_sensitive: Sensitive media is hidden by default and can be revealed with a click setting_display_media_default: Hide media marked as sensitive setting_display_media_hide_all: Always hide media @@ -151,6 +152,7 @@ en: phrase: Keyword or phrase setting_advanced_layout: Enable advanced web interface setting_aggregate_reblogs: Group boosts in timelines + setting_always_send_emails: Always send e-mail notifications setting_auto_play_gif: Auto-play animated GIFs setting_boost_modal: Show confirmation dialog before boosting setting_crop_images: Crop images in non-expanded posts to 16x9 diff --git a/config/settings.yml b/config/settings.yml index 06dd2b3f3..eaa05071e 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -38,16 +38,17 @@ defaults: &defaults trendable_by_default: false crop_images: true notification_emails: - follow: false + follow: true reblog: false favourite: false - mention: false + mention: true follow_request: true digest: true report: true pending_account: true trending_tag: true appeal: true + always_send_emails: false interactions: must_be_follower: false must_be_following: false -- cgit From 3906dd67ed84f963238f9bdccef63fe3fd3a05aa Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 8 Apr 2022 19:17:37 +0200 Subject: Fix extremely rare race condition when deleting a toot or account (#17994) --- app/controllers/api/v1/admin/accounts_controller.rb | 3 ++- app/controllers/api/v1/statuses_controller.rb | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb index dc9d3402f..65ed69f7b 100644 --- a/app/controllers/api/v1/admin/accounts_controller.rb +++ b/app/controllers/api/v1/admin/accounts_controller.rb @@ -65,8 +65,9 @@ class Api::V1::Admin::AccountsController < Api::BaseController def destroy authorize @account, :destroy? + json = render_to_body json: @account, serializer: REST::Admin::AccountSerializer Admin::AccountDeletionWorker.perform_async(@account.id) - render json: @account, serializer: REST::Admin::AccountSerializer + render json: json end def unsensitive diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 3fe137bfd..9270117da 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -77,10 +77,12 @@ class Api::V1::StatusesController < Api::BaseController authorize @status, :destroy? @status.discard - RemovalWorker.perform_async(@status.id, { 'redraft' => true }) @status.account.statuses_count = @status.account.statuses_count - 1 + json = render_to_body json: @status, serializer: REST::StatusSerializer, source_requested: true + + RemovalWorker.perform_async(@status.id, { 'redraft' => true }) - render json: @status, serializer: REST::StatusSerializer, source_requested: true + render json: json end private -- cgit From 012537452a1b9087ea085253e8d42fe4129cea42 Mon Sep 17 00:00:00 2001 From: 0x2019 <34298117+single-right-quote@users.noreply.github.com> Date: Fri, 8 Apr 2022 19:21:49 +0000 Subject: Fix error resposes for `from` search prefix (#17963) * Fix error responses in `from` search prefix (addresses mastodon/mastodon#17941) Using unsupported prefixes now reports a 422; searching for posts from an account the instance is not aware of reports a 404. TODO: The UI for this on the front end is abysmal. Searching `from:username@domain` now succeeds when `domain` is the local domain; searching `from:@username(@domain)?` now works as expected. * Remove unused methods on new Error classes as they are not being used Currently when `raise`d there are error messages being supplied, but this is not actually being used. The associated `raise`s have been edited accordingly. * Remove needless comments * Satisfy rubocop * Try fixing tests being unable to find AccountFindingConcern methods * Satisfy rubocop * Simplify `from` prefix logic This incorporates @ClearlyClaire's suggestion (see https://github.com/mastodon/mastodon/pull/17963#pullrequestreview-933986737). Accepctable account strings in `from:` clauses are more lenient than before this commit; for example, `from:@user@example.org@asnteo +cat` will not error, and return posts by @user@example.org containing the word "cat". This is more consistent with how Mastodon matches mentions in statuses. In addition, `from` clauses will not be checked for syntatically invalid usernames or domain names, simply 404ing when `Account.find_remote!` raises ActiveRecord::NotFound. New code for this PR that is no longer used has been removed. --- app/controllers/api/v2/search_controller.rb | 4 ++++ app/lib/search_query_transformer.rb | 8 ++++---- lib/exceptions.rb | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'app/controllers') diff --git a/app/controllers/api/v2/search_controller.rb b/app/controllers/api/v2/search_controller.rb index f17431dd1..a30560133 100644 --- a/app/controllers/api/v2/search_controller.rb +++ b/app/controllers/api/v2/search_controller.rb @@ -11,6 +11,10 @@ class Api::V2::SearchController < Api::BaseController def index @search = Search.new(search_results) render json: @search, serializer: REST::SearchSerializer + rescue Mastodon::SyntaxError + unprocessable_entity + rescue ActiveRecord::RecordNotFound + not_found end private diff --git a/app/lib/search_query_transformer.rb b/app/lib/search_query_transformer.rb index c685d7b6f..aef05e9d9 100644 --- a/app/lib/search_query_transformer.rb +++ b/app/lib/search_query_transformer.rb @@ -88,14 +88,14 @@ class SearchQueryTransformer < Parslet::Transform case prefix when 'from' @filter = :account_id - username, domain = term.split('@') - account = Account.find_remote(username, domain) - raise "Account not found: #{term}" unless account + username, domain = term.gsub(/\A@/, '').split('@') + domain = nil if TagManager.instance.local_domain?(domain) + account = Account.find_remote!(username, domain) @term = account.id else - raise "Unknown prefix: #{prefix}" + raise Mastodon::SyntaxError end end end diff --git a/lib/exceptions.rb b/lib/exceptions.rb index eb472abaa..0c677b660 100644 --- a/lib/exceptions.rb +++ b/lib/exceptions.rb @@ -10,6 +10,7 @@ module Mastodon class StreamValidationError < ValidationError; end class RaceConditionError < Error; end class RateLimitExceededError < Error; end + class SyntaxError < Error; end class UnexpectedResponseError < Error attr_reader :response -- cgit