From eebafe24a8000ac77bb54660170f5b47bfc5b3a3 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Mar 2022 04:11:13 +0100 Subject: Fix statuses not being referenced in strike when category is spam (#17786) --- app/models/admin/account_action.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/admin/account_action.rb b/app/models/admin/account_action.rb index d3be4be3f..850ea6d82 100644 --- a/app/models/admin/account_action.rb +++ b/app/models/admin/account_action.rb @@ -22,6 +22,16 @@ class Admin::AccountAction attr_reader :warning, :send_email_notification, :include_statuses + alias send_email_notification? send_email_notification + alias include_statuses? include_statuses + + def initialize(attributes = {}) + @send_email_notification = true + @include_statuses = true + + super + end + def send_email_notification=(value) @send_email_notification = ActiveModel::Type::Boolean.new.cast(value) end @@ -141,11 +151,11 @@ class Admin::AccountAction end def warnable? - send_email_notification && target_account.local? + send_email_notification? && target_account.local? end def status_ids - report.status_ids if with_report? && include_statuses + report.status_ids if with_report? && include_statuses? end def reports -- cgit From e6ffbfb5e7684880e9252f72f9cb00b151c28d10 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Mar 2022 04:11:29 +0100 Subject: Add `types` param to `GET /api/v1/notifications` in REST API (#17767) * Add `types` param to `GET /api/v1/notifications` in REST API * Improve tests --- app/controllers/api/v1/notifications_controller.rb | 19 +++--- app/models/notification.rb | 24 +++++-- .../api/v1/notifications_controller_spec.rb | 79 +++++++--------------- 3 files changed, 49 insertions(+), 73 deletions(-) (limited to 'app/models') diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index f9d780839..6d464997e 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -35,13 +35,18 @@ class Api::V1::NotificationsController < Api::BaseController limit_param(DEFAULT_NOTIFICATIONS_LIMIT), params_slice(:max_id, :since_id, :min_id) ) + Notification.preload_cache_collection_target_statuses(notifications) do |target_statuses| cache_collection(target_statuses, Status) end end def browserable_account_notifications - current_account.notifications.without_suspended.browserable(exclude_types, from_account) + current_account.notifications.without_suspended.browserable( + types: Array(browserable_params[:types]), + exclude_types: Array(browserable_params[:exclude_types]), + from_account_id: browserable_params[:account_id] + ) end def target_statuses_from_notifications @@ -72,17 +77,11 @@ class Api::V1::NotificationsController < Api::BaseController @notifications.first.id end - def exclude_types - val = params.permit(exclude_types: [])[:exclude_types] || [] - val = [val] unless val.is_a?(Enumerable) - val - end - - def from_account - params[:account_id] + def browserable_params + params.permit(:account_id, types: [], exclude_types: []) end def pagination_params(core_params) - params.slice(:limit, :exclude_types).permit(:limit, exclude_types: []).merge(core_params) + params.slice(:limit, :account_id, :types, :exclude_types).permit(:limit, :account_id, types: [], exclude_types: []).merge(core_params) end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 9bf296386..ba94b54d1 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -63,13 +63,6 @@ class Notification < ApplicationRecord scope :without_suspended, -> { joins(:from_account).merge(Account.without_suspended) } - scope :browserable, ->(exclude_types = [], account_id = nil) { - scope = all - scope = where(from_account_id: account_id) if account_id.present? - scope = scope.where(type: TYPES - exclude_types.map(&:to_sym)) unless exclude_types.empty? - scope - } - def type @type ||= (super || LEGACY_TYPE_CLASS_MAP[activity_type]).to_sym end @@ -90,6 +83,23 @@ class Notification < ApplicationRecord end class << self + def browserable(types: [], exclude_types: [], from_account_id: nil) + requested_types = begin + if types.empty? + TYPES + else + types.map(&:to_sym) & TYPES + end + end + + requested_types -= exclude_types.map(&:to_sym) + + all.tap do |scope| + scope.merge!(where(from_account_id: from_account_id)) if from_account_id.present? + scope.merge!(where(type: requested_types)) unless requested_types.size == TYPES.size + end + end + def preload_cache_collection_target_statuses(notifications, &_block) notifications.group_by(&:type).each do |type, grouped_notifications| associations = TARGET_STATUS_INCLUDES_BY_TYPE[type] diff --git a/spec/controllers/api/v1/notifications_controller_spec.rb b/spec/controllers/api/v1/notifications_controller_spec.rb index f8df6589f..46e177c0e 100644 --- a/spec/controllers/api/v1/notifications_controller_spec.rb +++ b/spec/controllers/api/v1/notifications_controller_spec.rb @@ -70,23 +70,23 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do end it 'includes reblog' do - expect(assigns(:notifications).map(&:activity)).to include(@reblog_of_first_status) + expect(body_as_json.map { |x| x[:type] }).to include 'reblog' end it 'includes mention' do - expect(assigns(:notifications).map(&:activity)).to include(@mention_from_status) + expect(body_as_json.map { |x| x[:type] }).to include 'mention' end it 'includes favourite' do - expect(assigns(:notifications).map(&:activity)).to include(@favourite) + expect(body_as_json.map { |x| x[:type] }).to include 'favourite' end it 'includes follow' do - expect(assigns(:notifications).map(&:activity)).to include(@follow) + expect(body_as_json.map { |x| x[:type] }).to include 'follow' end end - describe 'from specified user' do + describe 'with account_id param' do before do get :index, params: { account_id: third.account.id } end @@ -95,28 +95,12 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do expect(response).to have_http_status(200) end - it 'includes favourite' do - expect(assigns(:notifications).map(&:activity)).to include(@second_favourite) - end - - it 'excludes favourite' do - expect(assigns(:notifications).map(&:activity)).to_not include(@favourite) - end - - it 'excludes mention' do - expect(assigns(:notifications).map(&:activity)).to_not include(@mention_from_status) - end - - it 'excludes reblog' do - expect(assigns(:notifications).map(&:activity)).to_not include(@reblog_of_first_status) - end - - it 'excludes follow' do - expect(assigns(:notifications).map(&:activity)).to_not include(@follow) + it 'returns only notifications from specified user' do + expect(body_as_json.map { |x| x[:account][:id] }.uniq).to eq [third.account.id.to_s] end end - describe 'from nonexistent user' do + describe 'with invalid account_id param' do before do get :index, params: { account_id: 'foo' } end @@ -125,54 +109,37 @@ RSpec.describe Api::V1::NotificationsController, type: :controller do expect(response).to have_http_status(200) end - it 'excludes favourite' do - expect(assigns(:notifications).map(&:activity)).to_not include(@favourite) - end - - it 'excludes second favourite' do - expect(assigns(:notifications).map(&:activity)).to_not include(@second_favourite) - end - - it 'excludes mention' do - expect(assigns(:notifications).map(&:activity)).to_not include(@mention_from_status) - end - - it 'excludes reblog' do - expect(assigns(:notifications).map(&:activity)).to_not include(@reblog_of_first_status) - end - - it 'excludes follow' do - expect(assigns(:notifications).map(&:activity)).to_not include(@follow) + it 'returns nothing' do + expect(body_as_json.size).to eq 0 end end - describe 'with excluded mentions' do + describe 'with excluded_types param' do before do - get :index, params: { exclude_types: ['mention'] } + get :index, params: { exclude_types: %w(mention) } end it 'returns http success' do expect(response).to have_http_status(200) end - it 'includes reblog' do - expect(assigns(:notifications).map(&:activity)).to include(@reblog_of_first_status) - end - - it 'excludes mention' do - expect(assigns(:notifications).map(&:activity)).to_not include(@mention_from_status) + it 'returns everything but excluded type' do + expect(body_as_json.size).to_not eq 0 + expect(body_as_json.map { |x| x[:type] }.uniq).to_not include 'mention' end + end - it 'includes favourite' do - expect(assigns(:notifications).map(&:activity)).to include(@favourite) + describe 'with types param' do + before do + get :index, params: { types: %w(mention) } end - it 'includes third favourite' do - expect(assigns(:notifications).map(&:activity)).to include(@second_favourite) + it 'returns http success' do + expect(response).to have_http_status(200) end - it 'includes follow' do - expect(assigns(:notifications).map(&:activity)).to include(@follow) + it 'returns only requested type' do + expect(body_as_json.map { |x| x[:type] }.uniq).to eq ['mention'] end end end -- cgit From a7941176791ec8ad56e45efbb449b9d09a580a6b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 15 Mar 2022 07:51:55 +0100 Subject: Fix individually approved/rejected statuses/links showing as pending review (#17787) --- app/models/preview_card.rb | 4 ++++ app/models/status.rb | 4 ++++ app/views/admin/trends/links/_preview_card.html.haml | 4 ++-- app/views/admin/trends/statuses/_status.html.haml | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) (limited to 'app/models') diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index 0f9e23fa1..0d2c32c35 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -80,6 +80,10 @@ class PreviewCard < ApplicationRecord end end + def requires_review? + attributes['trendable'].nil? && (provider.nil? || provider.requires_review?) + end + def requires_review_notification? attributes['trendable'].nil? && (provider.nil? || provider.requires_review_notification?) end diff --git a/app/models/status.rb b/app/models/status.rb index f2c55090b..5b984543e 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -294,6 +294,10 @@ class Status < ApplicationRecord end end + def requires_review? + attributes['trendable'].nil? && account.requires_review? + end + def requires_review_notification? attributes['trendable'].nil? && account.requires_review_notification? end diff --git a/app/views/admin/trends/links/_preview_card.html.haml b/app/views/admin/trends/links/_preview_card.html.haml index d88e06bfd..2e6a0c62f 100644 --- a/app/views/admin/trends/links/_preview_card.html.haml +++ b/app/views/admin/trends/links/_preview_card.html.haml @@ -1,4 +1,4 @@ -.batch-table__row{ class: [preview_card.provider&.requires_review? && 'batch-table__row--attention', !preview_card.provider&.requires_review? && !preview_card.trendable? && 'batch-table__row--muted'] } +.batch-table__row{ class: [preview_card.requires_review? && 'batch-table__row--attention', !preview_card.requires_review? && !preview_card.trendable? && 'batch-table__row--muted'] } %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox = f.check_box :preview_card_ids, { multiple: true, include_hidden: false }, preview_card.id @@ -25,6 +25,6 @@ - if preview_card.decaying? • = t('admin.trends.tags.peaked_on_and_decaying', date: l(preview_card.max_score_at.to_date, format: :short)) - - elsif preview_card.provider&.requires_review? + - elsif preview_card.requires_review? • = t('admin.trends.pending_review') diff --git a/app/views/admin/trends/statuses/_status.html.haml b/app/views/admin/trends/statuses/_status.html.haml index 50a855349..0c463c6b1 100644 --- a/app/views/admin/trends/statuses/_status.html.haml +++ b/app/views/admin/trends/statuses/_status.html.haml @@ -1,4 +1,4 @@ -.batch-table__row{ class: [status.account.requires_review? && 'batch-table__row--attention', !status.account.requires_review? && !status.trendable? && 'batch-table__row--muted'] } +.batch-table__row{ class: [status.requires_review? && 'batch-table__row--attention', !status.requires_review? && !status.trendable? && 'batch-table__row--muted'] } %label.batch-table__row__select.batch-table__row__select--aligned.batch-checkbox = f.check_box :status_ids, { multiple: true, include_hidden: false }, status.id @@ -28,6 +28,6 @@ - if status.trendable? && (rank = Trends.statuses.rank(status.id)) • %abbr{ title: t('admin.trends.tags.current_score', score: Trends.statuses.score(status.id)) }= t('admin.trends.tags.trending_rank', rank: rank + 1) - - elsif status.account.requires_review? + - elsif status.requires_review? • = t('admin.trends.pending_review') -- cgit