From 974335e4144eab90a8c5eb91da55d2f8385c68c6 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 3 Jun 2024 10:35:59 +0200 Subject: [PATCH] Add experimental server-side notification grouping (#29889) --- .../api/v2_alpha/notifications_controller.rb | 91 ++++++++++ app/models/notification.rb | 62 +++++++ app/models/notification_group.rb | 29 ++++ .../rest/notification_group_serializer.rb | 45 +++++ app/services/notify_service.rb | 22 +++ config/application.rb | 2 + config/routes/api.rb | 12 ++ ...13095755_add_group_key_to_notifications.rb | 7 + ...tifications_on_account_id_and_group_key.rb | 9 + db/schema.rb | 2 + lib/active_record/with_recursive.rb | 65 +++++++ lib/arel/union_parenthesizing.rb | 51 ++++++ spec/models/notification_spec.rb | 60 +++++++ .../api/v2_alpha/notifications_spec.rb | 161 ++++++++++++++++++ 14 files changed, 618 insertions(+) create mode 100644 app/controllers/api/v2_alpha/notifications_controller.rb create mode 100644 app/models/notification_group.rb create mode 100644 app/serializers/rest/notification_group_serializer.rb create mode 100644 db/migrate/20240513095755_add_group_key_to_notifications.rb create mode 100644 db/migrate/20240513123807_add_index_notifications_on_account_id_and_group_key.rb create mode 100644 lib/active_record/with_recursive.rb create mode 100644 lib/arel/union_parenthesizing.rb create mode 100644 spec/requests/api/v2_alpha/notifications_spec.rb diff --git a/app/controllers/api/v2_alpha/notifications_controller.rb b/app/controllers/api/v2_alpha/notifications_controller.rb new file mode 100644 index 00000000000..19d3ac9018f --- /dev/null +++ b/app/controllers/api/v2_alpha/notifications_controller.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +class Api::V2Alpha::NotificationsController < Api::BaseController + before_action -> { doorkeeper_authorize! :read, :'read:notifications' }, except: [:clear, :dismiss] + before_action -> { doorkeeper_authorize! :write, :'write:notifications' }, only: [:clear, :dismiss] + before_action :require_user! + after_action :insert_pagination_headers, only: :index + + DEFAULT_NOTIFICATIONS_LIMIT = 40 + + def index + with_read_replica do + @notifications = load_notifications + @group_metadata = load_group_metadata + @relationships = StatusRelationshipsPresenter.new(target_statuses_from_notifications, current_user&.account_id) + end + + render json: @notifications.map { |notification| NotificationGroup.from_notification(notification) }, each_serializer: REST::NotificationGroupSerializer, relationships: @relationships, group_metadata: @group_metadata + end + + def show + @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:id]) + render json: NotificationGroup.from_notification(@notification), serializer: REST::NotificationGroupSerializer + end + + def clear + current_account.notifications.delete_all + render_empty + end + + def dismiss + current_account.notifications.where(group_key: params[:id]).destroy_all + render_empty + end + + private + + def load_notifications + notifications = browserable_account_notifications.includes(from_account: [:account_stat, :user]).to_a_grouped_paginated_by_id( + limit_param(DEFAULT_NOTIFICATIONS_LIMIT), + params_slice(:max_id, :since_id, :min_id) + ) + + Notification.preload_cache_collection_target_statuses(notifications) do |target_statuses| + preload_collection(target_statuses, Status) + end + end + + def load_group_metadata + return {} if @notifications.empty? + + browserable_account_notifications + .where(group_key: @notifications.filter_map(&:group_key)) + .where(id: (@notifications.last.id)..(@notifications.first.id)) + .group(:group_key) + .pluck(:group_key, 'min(notifications.id) as min_id', 'max(notifications.id) as max_id', 'max(notifications.created_at) as latest_notification_at') + .to_h { |group_key, min_id, max_id, latest_notification_at| [group_key, { min_id: min_id, max_id: max_id, latest_notification_at: latest_notification_at }] } + end + + def browserable_account_notifications + current_account.notifications.without_suspended.browserable( + types: Array(browserable_params[:types]), + exclude_types: Array(browserable_params[:exclude_types]), + include_filtered: truthy_param?(:include_filtered) + ) + end + + def target_statuses_from_notifications + @notifications.filter_map(&:target_status) + end + + def next_path + api_v2_alpha_notifications_url pagination_params(max_id: pagination_max_id) unless @notifications.empty? + end + + def prev_path + api_v2_alpha_notifications_url pagination_params(min_id: pagination_since_id) unless @notifications.empty? + end + + def pagination_collection + @notifications + end + + def browserable_params + params.permit(:include_filtered, types: [], exclude_types: []) + end + + def pagination_params(core_params) + params.slice(:limit, :types, :exclude_types, :include_filtered).permit(:limit, :include_filtered, types: [], exclude_types: []).merge(core_params) + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 7cbab4dc8c3..e3deaa5348b 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -13,6 +13,7 @@ # from_account_id :bigint(8) not null # type :string # filtered :boolean default(FALSE), not null +# group_key :string # class Notification < ApplicationRecord @@ -136,6 +137,67 @@ class Notification < ApplicationRecord end end + # This returns notifications from the request page, but with at most one notification per group. + # Notifications that have no `group_key` each count as a separate group. + def paginate_groups_by_max_id(limit, max_id: nil, since_id: nil) + query = reorder(id: :desc) + query = query.where(id: ...max_id) if max_id.present? + query = query.where(id: (since_id + 1)...) if since_id.present? + + unscoped + .with_recursive( + grouped_notifications: [ + query + .select('notifications.*', "ARRAY[COALESCE(notifications.group_key, 'ungrouped-' || notifications.id)] groups") + .limit(1), + query + .joins('CROSS JOIN grouped_notifications') + .where('notifications.id < grouped_notifications.id') + .where.not("COALESCE(notifications.group_key, 'ungrouped-' || notifications.id) = ANY(grouped_notifications.groups)") + .select('notifications.*', "array_append(grouped_notifications.groups, COALESCE(notifications.group_key, 'ungrouped-' || notifications.id))") + .limit(1), + ] + ) + .from('grouped_notifications AS notifications') + .order(id: :desc) + .limit(limit) + end + + # Differs from :paginate_groups_by_max_id in that it gives the results immediately following min_id, + # whereas since_id gives the items with largest id, but with since_id as a cutoff. + # Results will be in ascending order by id. + def paginate_groups_by_min_id(limit, max_id: nil, min_id: nil) + query = reorder(id: :asc) + query = query.where(id: (min_id + 1)...) if min_id.present? + query = query.where(id: ...max_id) if max_id.present? + + unscoped + .with_recursive( + grouped_notifications: [ + query + .select('notifications.*', "ARRAY[COALESCE(notifications.group_key, 'ungrouped-' || notifications.id)] groups") + .limit(1), + query + .joins('CROSS JOIN grouped_notifications') + .where('notifications.id > grouped_notifications.id') + .where.not("COALESCE(notifications.group_key, 'ungrouped-' || notifications.id) = ANY(grouped_notifications.groups)") + .select('notifications.*', "array_append(grouped_notifications.groups, COALESCE(notifications.group_key, 'ungrouped-' || notifications.id))") + .limit(1), + ] + ) + .from('grouped_notifications AS notifications') + .order(id: :asc) + .limit(limit) + end + + def to_a_grouped_paginated_by_id(limit, options = {}) + if options[:min_id].present? + paginate_groups_by_min_id(limit, min_id: options[:min_id], max_id: options[:max_id]).reverse + else + paginate_groups_by_max_id(limit, max_id: options[:max_id], since_id: options[:since_id]).to_a + 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/app/models/notification_group.rb b/app/models/notification_group.rb new file mode 100644 index 00000000000..07967f9dcbc --- /dev/null +++ b/app/models/notification_group.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class NotificationGroup < ActiveModelSerializers::Model + attributes :group_key, :sample_accounts, :notifications_count, :notification + + def self.from_notification(notification) + if notification.group_key.present? + # TODO: caching and preloading + sample_accounts = notification.account.notifications.where(group_key: notification.group_key).order(id: :desc).limit(3).map(&:from_account) + notifications_count = notification.account.notifications.where(group_key: notification.group_key).count + else + sample_accounts = [notification.from_account] + notifications_count = 1 + end + + NotificationGroup.new( + notification: notification, + group_key: notification.group_key || "ungrouped-#{notification.id}", + sample_accounts: sample_accounts, + notifications_count: notifications_count + ) + end + + delegate :type, + :target_status, + :report, + :account_relationship_severance_event, + to: :notification, prefix: false +end diff --git a/app/serializers/rest/notification_group_serializer.rb b/app/serializers/rest/notification_group_serializer.rb new file mode 100644 index 00000000000..6c1d2465d25 --- /dev/null +++ b/app/serializers/rest/notification_group_serializer.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class REST::NotificationGroupSerializer < ActiveModel::Serializer + attributes :group_key, :notifications_count, :type + + attribute :page_min_id, if: :paginated? + attribute :page_max_id, if: :paginated? + attribute :latest_page_notification_at, if: :paginated? + + has_many :sample_accounts, serializer: REST::AccountSerializer + belongs_to :target_status, key: :status, if: :status_type?, serializer: REST::StatusSerializer + belongs_to :report, if: :report_type?, serializer: REST::ReportSerializer + belongs_to :account_relationship_severance_event, key: :event, if: :relationship_severance_event?, serializer: REST::AccountRelationshipSeveranceEventSerializer + + def status_type? + [:favourite, :reblog, :status, :mention, :poll, :update].include?(object.type) + end + + def report_type? + object.type == :'admin.report' + end + + def relationship_severance_event? + object.type == :severed_relationships + end + + def page_min_id + range = instance_options[:group_metadata][object.group_key] + range.present? ? range[:min_id].to_s : object.notification.id.to_s + end + + def page_max_id + range = instance_options[:group_metadata][object.group_key] + range.present? ? range[:max_id].to_s : object.notification.id.to_s + end + + def latest_page_notification_at + range = instance_options[:group_metadata][object.group_key] + range.present? ? range[:latest_notification_at] : object.notification.created_at + end + + def paginated? + instance_options[:group_metadata].present? + end +end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index 1f01c2d48e0..d69b5af1412 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -3,6 +3,9 @@ class NotifyService < BaseService include Redisable + MAXIMUM_GROUP_SPAN_HOURS = 12 + MAXIMUM_GROUP_GAP_TIME = 4.hours.to_i + NON_EMAIL_TYPES = %i( admin.report admin.sign_up @@ -183,6 +186,7 @@ class NotifyService < BaseService return if dismiss? @notification.filtered = filter? + @notification.group_key = notification_group_key @notification.save! # It's possible the underlying activity has been deleted @@ -202,6 +206,24 @@ class NotifyService < BaseService private + def notification_group_key + return nil if @notification.filtered || %i(favourite reblog).exclude?(@notification.type) + + type_prefix = "#{@notification.type}-#{@notification.target_status.id}" + redis_key = "notif-group/#{@recipient.id}/#{type_prefix}" + hour_bucket = @notification.activity.created_at.utc.to_i / 1.hour.to_i + + # Reuse previous group if it does not span too large an amount of time + previous_bucket = redis.get(redis_key).to_i + hour_bucket = previous_bucket if hour_bucket < previous_bucket + MAXIMUM_GROUP_SPAN_HOURS + + # Do not track groups past a given inactivity time + # We do not concern ourselves with race conditions since we use hour buckets + redis.set(redis_key, hour_bucket, ex: MAXIMUM_GROUP_GAP_TIME) + + "#{type_prefix}-#{hour_bucket}" + end + def dismiss? DismissCondition.new(@notification).dismiss? end diff --git a/config/application.rb b/config/application.rb index 6d6e91a5cc7..a8e313069d9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -51,6 +51,8 @@ require_relative '../lib/rails/engine_extensions' require_relative '../lib/action_dispatch/remote_ip_extensions' require_relative '../lib/active_record/database_tasks_extensions' require_relative '../lib/active_record/batches' +require_relative '../lib/active_record/with_recursive' +require_relative '../lib/arel/union_parenthesizing' require_relative '../lib/simple_navigation/item_extensions' Bundler.require(:pam_authentication) if ENV['PAM_ENABLED'] == 'true' diff --git a/config/routes/api.rb b/config/routes/api.rb index bf3cee0c10e..135a19a0a7b 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -327,6 +327,18 @@ namespace :api, format: false do end end + namespace :v2_alpha do + resources :notifications, only: [:index, :show] do + collection do + post :clear + end + + member do + post :dismiss + end + end + end + namespace :web do resource :settings, only: [:update] resources :embeds, only: [:show] diff --git a/db/migrate/20240513095755_add_group_key_to_notifications.rb b/db/migrate/20240513095755_add_group_key_to_notifications.rb new file mode 100644 index 00000000000..2e2a302fffd --- /dev/null +++ b/db/migrate/20240513095755_add_group_key_to_notifications.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddGroupKeyToNotifications < ActiveRecord::Migration[7.1] + def change + add_column :notifications, :group_key, :string + end +end diff --git a/db/migrate/20240513123807_add_index_notifications_on_account_id_and_group_key.rb b/db/migrate/20240513123807_add_index_notifications_on_account_id_and_group_key.rb new file mode 100644 index 00000000000..66874418b2f --- /dev/null +++ b/db/migrate/20240513123807_add_index_notifications_on_account_id_and_group_key.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddIndexNotificationsOnAccountIdAndGroupKey < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def change + add_index :notifications, [:account_id, :group_key], algorithm: :concurrently, where: 'group_key IS NOT NULL' + end +end diff --git a/db/schema.rb b/db/schema.rb index 3a47522d267..73f6b464e47 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -724,6 +724,8 @@ ActiveRecord::Schema[7.1].define(version: 2024_05_22_041528) do t.bigint "from_account_id", null: false t.string "type" t.boolean "filtered", default: false, null: false + t.string "group_key" + t.index ["account_id", "group_key"], name: "index_notifications_on_account_id_and_group_key", where: "(group_key IS NOT NULL)" t.index ["account_id", "id", "type"], name: "index_notifications_on_account_id_and_id_and_type", order: { id: :desc } t.index ["account_id", "id", "type"], name: "index_notifications_on_filtered", order: { id: :desc }, where: "(filtered = false)" t.index ["activity_id", "activity_type"], name: "index_notifications_on_activity_id_and_activity_type" diff --git a/lib/active_record/with_recursive.rb b/lib/active_record/with_recursive.rb new file mode 100644 index 00000000000..4bd3e81eedc --- /dev/null +++ b/lib/active_record/with_recursive.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +# Add support for writing recursive CTEs in ActiveRecord + +# Initially from Lorin Thwaits (https://github.com/lorint) as per comment: +# https://github.com/vlado/activerecord-cte/issues/16#issuecomment-1433043310 + +# Modified from the above code to change the signature to +# `with_recursive(hash)` and extending CTE hash values to also includes arrays +# of values that get turned into UNION ALL expressions. + +# This implementation has been merged in Rails: https://github.com/rails/rails/pull/51601 + +module ActiveRecord + module QueryMethodsExtensions + def with_recursive(*args) + @with_is_recursive = true + check_if_method_has_arguments!(__callee__, args) + spawn.with_recursive!(*args) + end + + # Like #with_recursive but modifies the relation in place. + def with_recursive!(*args) # :nodoc: + self.with_values += args + @with_is_recursive = true + self + end + + private + + def build_with(arel) + return if with_values.empty? + + with_statements = with_values.map do |with_value| + raise ArgumentError, "Unsupported argument type: #{with_value} #{with_value.class}" unless with_value.is_a?(Hash) + + build_with_value_from_hash(with_value) + end + + # Was: arel.with(with_statements) + @with_is_recursive ? arel.with(:recursive, with_statements) : arel.with(with_statements) + end + + def build_with_value_from_hash(hash) + hash.map do |name, value| + Arel::Nodes::TableAlias.new(build_with_expression_from_value(value), name) + end + end + + def build_with_expression_from_value(value) + case value + when Arel::Nodes::SqlLiteral then Arel::Nodes::Grouping.new(value) + when ActiveRecord::Relation then value.arel + when Arel::SelectManager then value + when Array then value.map { |e| build_with_expression_from_value(e) }.reduce { |result, value| Arel::Nodes::UnionAll.new(result, value) } + else + raise ArgumentError, "Unsupported argument type: `#{value}` #{value.class}" + end + end + end +end + +ActiveSupport.on_load(:active_record) do + ActiveRecord::QueryMethods.prepend(ActiveRecord::QueryMethodsExtensions) +end diff --git a/lib/arel/union_parenthesizing.rb b/lib/arel/union_parenthesizing.rb new file mode 100644 index 00000000000..852d8e92d8a --- /dev/null +++ b/lib/arel/union_parenthesizing.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +# Fix an issue with `LIMIT` ocurring on the left side of a `UNION` causing syntax errors. +# See https://github.com/rails/rails/issues/40181 + +# The fix has been merged in ActiveRecord: https://github.com/rails/rails/pull/51549 +# TODO: drop this when available in ActiveRecord + +# rubocop:disable all -- This is a mostly vendored file + +module Arel + module Visitors + class ToSql + private + + def infix_value_with_paren(o, collector, value, suppress_parens = false) + collector << "( " unless suppress_parens + collector = if o.left.class == o.class + infix_value_with_paren(o.left, collector, value, true) + else + select_parentheses o.left, collector, false # Changed from `visit o.left, collector` + end + collector << value + collector = if o.right.class == o.class + infix_value_with_paren(o.right, collector, value, true) + else + select_parentheses o.right, collector, false # Changed from `visit o.right, collector` + end + collector << " )" unless suppress_parens + collector + end + + def select_parentheses(o, collector, always_wrap_selects = true) + if o.is_a?(Nodes::SelectStatement) && (always_wrap_selects || require_parentheses?(o)) + collector << "(" + visit o, collector + collector << ")" + collector + else + visit o, collector + end + end + + def require_parentheses?(o) + !o.orders.empty? || o.limit || o.offset + end + end + end +end + +# rubocop:enable all diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 3c7d51ae1a3..d498ee02a55 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -151,6 +151,66 @@ RSpec.describe Notification do end end + describe '.paginate_groups_by_max_id' do + let(:account) { Fabricate(:account) } + + let!(:notifications) do + ['group-1', 'group-1', nil, 'group-2', nil, 'group-1', 'group-2', 'group-1'] + .map { |group_key| Fabricate(:notification, account: account, group_key: group_key) } + end + + context 'without since_id or max_id' do + it 'returns the most recent notifications, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_max_id(4).pluck(:id)) + .to eq [notifications[7], notifications[6], notifications[4], notifications[2]].pluck(:id) + end + end + + context 'with since_id' do + it 'returns the most recent notifications, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_max_id(4, since_id: notifications[4].id).pluck(:id)) + .to eq [notifications[7], notifications[6]].pluck(:id) + end + end + + context 'with max_id' do + it 'returns the most recent notifications after max_id, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_max_id(4, max_id: notifications[7].id).pluck(:id)) + .to eq [notifications[6], notifications[5], notifications[4], notifications[2]].pluck(:id) + end + end + end + + describe '.paginate_groups_by_min_id' do + let(:account) { Fabricate(:account) } + + let!(:notifications) do + ['group-1', 'group-1', nil, 'group-2', nil, 'group-1', 'group-2', 'group-1'] + .map { |group_key| Fabricate(:notification, account: account, group_key: group_key) } + end + + context 'without min_id or max_id' do + it 'returns the oldest notifications, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_min_id(4).pluck(:id)) + .to eq [notifications[0], notifications[2], notifications[3], notifications[4]].pluck(:id) + end + end + + context 'with max_id' do + it 'returns the oldest notifications, stopping at max_id, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_min_id(4, max_id: notifications[4].id).pluck(:id)) + .to eq [notifications[0], notifications[2], notifications[3]].pluck(:id) + end + end + + context 'with min_id' do + it 'returns the most oldest notifications after min_id, only keeping one notification per group' do + expect(described_class.without_suspended.paginate_groups_by_min_id(4, min_id: notifications[0].id).pluck(:id)) + .to eq [notifications[1], notifications[2], notifications[3], notifications[4]].pluck(:id) + end + end + end + describe '.preload_cache_collection_target_statuses' do subject do described_class.preload_cache_collection_target_statuses(notifications) do |target_statuses| diff --git a/spec/requests/api/v2_alpha/notifications_spec.rb b/spec/requests/api/v2_alpha/notifications_spec.rb new file mode 100644 index 00000000000..9bd1a32e9bf --- /dev/null +++ b/spec/requests/api/v2_alpha/notifications_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Notifications' do + let(:user) { Fabricate(:user, account_attributes: { username: 'alice' }) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:scopes) { 'read:notifications write:notifications' } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + describe 'GET /api/v2_alpha/notifications', :sidekiq_inline do + subject do + get '/api/v2_alpha/notifications', headers: headers, params: params + end + + let(:bob) { Fabricate(:user) } + let(:tom) { Fabricate(:user) } + let(:params) { {} } + + before do + first_status = PostStatusService.new.call(user.account, text: 'Test') + ReblogService.new.call(bob.account, first_status) + mentioning_status = PostStatusService.new.call(bob.account, text: 'Hello @alice') + mentioning_status.mentions.first + FavouriteService.new.call(bob.account, first_status) + FavouriteService.new.call(tom.account, first_status) + FollowService.new.call(bob.account, user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + context 'with no options' do + it 'returns expected notification types', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_json_types).to include('reblog', 'mention', 'favourite', 'follow') + end + end + + context 'with exclude_types param' do + let(:params) { { exclude_types: %w(mention) } } + + it 'returns everything but excluded type', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json.size).to_not eq 0 + expect(body_json_types.uniq).to_not include 'mention' + end + end + + context 'with types param' do + let(:params) { { types: %w(mention) } } + + it 'returns only requested type', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_json_types.uniq).to eq ['mention'] + end + end + + context 'with limit param' do + let(:params) { { limit: 3 } } + + it 'returns the requested number of notifications paginated', :aggregate_failures do + subject + + notifications = user.account.notifications + + expect(body_as_json.size) + .to eq(params[:limit]) + + expect(response) + .to include_pagination_headers( + prev: api_v2_alpha_notifications_url(limit: params[:limit], min_id: notifications.last.id), + # TODO: one downside of the current approach is that we return the first ID matching the group, + # not the last that has been skipped, so pagination is very likely to give overlap + next: api_v2_alpha_notifications_url(limit: params[:limit], max_id: notifications[1].id) + ) + end + end + + def body_json_types + body_as_json.pluck(:type) + end + end + + describe 'GET /api/v2_alpha/notifications/:id' do + subject do + get "/api/v2_alpha/notifications/#{notification.group_key}", headers: headers + end + + let(:notification) { Fabricate(:notification, account: user.account, group_key: 'foobar') } + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + it 'returns http success' do + subject + + expect(response).to have_http_status(200) + end + + context 'when notification belongs to someone else' do + let(:notification) { Fabricate(:notification, group_key: 'foobar') } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v2_alpha/notifications/:id/dismiss' do + subject do + post "/api/v2_alpha/notifications/#{notification.group_key}/dismiss", headers: headers + end + + let!(:notification) { Fabricate(:notification, account: user.account, group_key: 'foobar') } + + it_behaves_like 'forbidden for wrong scope', 'read read:notifications' + + it 'destroys the notification' do + subject + + expect(response).to have_http_status(200) + expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'when notification belongs to someone else' do + let(:notification) { Fabricate(:notification) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v2_alpha/notifications/clear' do + subject do + post '/api/v2_alpha/notifications/clear', headers: headers + end + + before do + Fabricate(:notification, account: user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'read read:notifications' + + it 'clears notifications for the account' do + subject + + expect(user.account.reload.notifications).to be_empty + expect(response).to have_http_status(200) + end + end +end