From c789fba629dad8c616e8052733456bfabaebc3d0 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 4 Oct 2021 13:27:05 -0500 Subject: [PATCH] Notifications: use Markers for unread count This works consistently across all backends --- app/soapbox/actions/markers.js | 32 ++-- app/soapbox/actions/notifications.js | 33 ++-- app/soapbox/features/ui/index.js | 26 +-- .../reducers/__tests__/notifications-test.js | 157 ++++++++++++++---- app/soapbox/reducers/notifications.js | 49 ++++-- 5 files changed, 205 insertions(+), 92 deletions(-) diff --git a/app/soapbox/actions/markers.js b/app/soapbox/actions/markers.js index 3bf623fde..3d7d12c7e 100644 --- a/app/soapbox/actions/markers.js +++ b/app/soapbox/actions/markers.js @@ -1,33 +1,33 @@ import api from '../api'; -export const FETCH_MARKERS_REQUEST = 'FETCH_MARKERS_REQUEST'; -export const FETCH_MARKERS_SUCCESS = 'FETCH_MARKERS_SUCCESS'; -export const FETCH_MARKERS_FAIL = 'FETCH_MARKERS_FAIL'; +export const MARKER_FETCH_REQUEST = 'MARKER_FETCH_REQUEST'; +export const MARKER_FETCH_SUCCESS = 'MARKER_FETCH_SUCCESS'; +export const MARKER_FETCH_FAIL = 'MARKER_FETCH_FAIL'; -export const SAVE_MARKERS_REQUEST = 'SAVE_MARKERS_REQUEST'; -export const SAVE_MARKERS_SUCCESS = 'SAVE_MARKERS_SUCCESS'; -export const SAVE_MARKERS_FAIL = 'SAVE_MARKERS_FAIL'; +export const MARKER_SAVE_REQUEST = 'MARKER_SAVE_REQUEST'; +export const MARKER_SAVE_SUCCESS = 'MARKER_SAVE_SUCCESS'; +export const MARKER_SAVE_FAIL = 'MARKER_SAVE_FAIL'; -export function fetchMarkers(timeline) { +export function fetchMarker(timeline) { return (dispatch, getState) => { - dispatch({ type: FETCH_MARKERS_REQUEST }); + dispatch({ type: MARKER_FETCH_REQUEST }); return api(getState).get('/api/v1/markers', { params: { timeline }, - }).then(response => { - dispatch({ type: FETCH_MARKERS_SUCCESS, markers: response.data }); + }).then(({ data: marker }) => { + dispatch({ type: MARKER_FETCH_SUCCESS, marker }); }).catch(error => { - dispatch({ type: FETCH_MARKERS_FAIL, error }); + dispatch({ type: MARKER_FETCH_FAIL, error }); }); }; } -export function saveMarkers(params) { +export function saveMarker(marker) { return (dispatch, getState) => { - dispatch({ type: SAVE_MARKERS_REQUEST }); - return api(getState).post('/api/v1/markers', params).then(response => { - dispatch({ type: SAVE_MARKERS_SUCCESS, markers: response.data }); + dispatch({ type: MARKER_SAVE_REQUEST, marker }); + return api(getState).post('/api/v1/markers', marker).then(({ data: marker }) => { + dispatch({ type: MARKER_SAVE_SUCCESS, marker }); }).catch(error => { - dispatch({ type: SAVE_MARKERS_FAIL, error }); + dispatch({ type: MARKER_SAVE_FAIL, error }); }); }; } diff --git a/app/soapbox/actions/notifications.js b/app/soapbox/actions/notifications.js index 18f31b810..0c8f8ab83 100644 --- a/app/soapbox/actions/notifications.js +++ b/app/soapbox/actions/notifications.js @@ -8,6 +8,7 @@ import { importFetchedStatus, importFetchedStatuses, } from './importer'; +import { saveMarker } from './markers'; import { getSettings, saveSettings } from './settings'; import { defineMessages } from 'react-intl'; import { @@ -168,7 +169,7 @@ const noOp = () => {}; export function expandNotifications({ maxId } = {}, done = noOp) { return (dispatch, getState) => { - if (!isLoggedIn(getState)) return; + if (!isLoggedIn(getState)) return dispatch(noOp); const activeFilter = getSettings(getState()).getIn(['notifications', 'quickFilter', 'active']); const notifications = getState().get('notifications'); @@ -176,7 +177,7 @@ export function expandNotifications({ maxId } = {}, done = noOp) { if (notifications.get('isLoading')) { done(); - return; + return dispatch(noOp); } const params = { @@ -192,7 +193,7 @@ export function expandNotifications({ maxId } = {}, done = noOp) { dispatch(expandNotificationsRequest(isLoadingMore)); - api(getState).get('/api/v1/notifications', { params }).then(response => { + return api(getState).get('/api/v1/notifications', { params }).then(response => { const next = getLinks(response).refs.find(link => link.rel === 'next'); const entries = response.data.reduce((acc, item) => { @@ -288,25 +289,17 @@ export function markReadNotifications() { if (!isLoggedIn(getState)) return; const state = getState(); - const topNotification = state.getIn(['notifications', 'items'], ImmutableOrderedMap()).first(ImmutableMap()).get('id'); - const lastRead = state.getIn(['notifications', 'lastRead']); + const topNotificationId = state.getIn(['notifications', 'items'], ImmutableOrderedMap()).first(ImmutableMap()).get('id'); + const lastReadId = state.getIn(['notifications', 'lastRead']); - if (!(topNotification && topNotification > lastRead)) return; + if (!(topNotificationId && topNotificationId > lastReadId)) return; - dispatch({ - type: NOTIFICATIONS_MARK_READ_REQUEST, - lastRead: topNotification, - }); + const marker = { + notifications: { + last_read_id: topNotificationId, + }, + }; - api(getState).post('/api/v1/pleroma/notifications/read', { - max_id: topNotification, - }).then(response => { - dispatch({ - type: NOTIFICATIONS_MARK_READ_SUCCESS, - notifications: response.data, - }); - }).catch(e => { - dispatch({ type: NOTIFICATIONS_MARK_READ_FAIL }); - }); + dispatch(saveMarker(marker)); }; } diff --git a/app/soapbox/features/ui/index.js b/app/soapbox/features/ui/index.js index 00c418136..77f7bf2a3 100644 --- a/app/soapbox/features/ui/index.js +++ b/app/soapbox/features/ui/index.js @@ -13,6 +13,7 @@ import { debounce } from 'lodash'; import { uploadCompose, resetCompose } from '../../actions/compose'; import { expandHomeTimeline } from '../../actions/timelines'; import { expandNotifications } from '../../actions/notifications'; +import { fetchMarker } from 'soapbox/actions/markers'; import { fetchReports, fetchUsers, fetchConfig } from '../../actions/admin'; import { fetchFilters } from '../../actions/filters'; import { fetchChats } from 'soapbox/actions/chats'; @@ -445,7 +446,7 @@ class UI extends React.PureComponent { }); componentDidMount() { - const { account, features } = this.props; + const { account, features, dispatch } = this.props; if (!account) return; window.addEventListener('resize', this.handleResize, { passive: true }); @@ -464,32 +465,35 @@ class UI extends React.PureComponent { } if (account) { - this.props.dispatch(expandHomeTimeline()); - this.props.dispatch(expandNotifications()); + dispatch(expandHomeTimeline()); + + dispatch(expandNotifications()) + .then(() => dispatch(fetchMarker(['notifications']))) + .catch(console.error); if (features.chats) { - this.props.dispatch(fetchChats()); + dispatch(fetchChats()); } if (isStaff(account)) { - this.props.dispatch(fetchReports({ state: 'open' })); - this.props.dispatch(fetchUsers(['local', 'need_approval'])); + dispatch(fetchReports({ state: 'open' })); + dispatch(fetchUsers(['local', 'need_approval'])); } if (isAdmin(account)) { - this.props.dispatch(fetchConfig()); + dispatch(fetchConfig()); } - setTimeout(() => this.props.dispatch(fetchFilters()), 500); + setTimeout(() => dispatch(fetchFilters()), 500); if (account.get('locked')) { - setTimeout(() => this.props.dispatch(fetchFollowRequests()), 700); + setTimeout(() => dispatch(fetchFollowRequests()), 700); } - setTimeout(() => this.props.dispatch(fetchScheduledStatuses()), 900); + setTimeout(() => dispatch(fetchScheduledStatuses()), 900); } - this.props.dispatch(fetchCustomEmojis()); + dispatch(fetchCustomEmojis()); this.connectStreaming(); } diff --git a/app/soapbox/reducers/__tests__/notifications-test.js b/app/soapbox/reducers/__tests__/notifications-test.js index 2eab85f40..581c4b38e 100644 --- a/app/soapbox/reducers/__tests__/notifications-test.js +++ b/app/soapbox/reducers/__tests__/notifications-test.js @@ -8,6 +8,11 @@ import notification from 'soapbox/__fixtures__/notification.json'; import intlMessages from 'soapbox/__fixtures__/intlMessages.json'; import relationship from 'soapbox/__fixtures__/relationship.json'; import { TIMELINE_DELETE } from 'soapbox/actions/timelines'; +import { + MARKER_FETCH_SUCCESS, + MARKER_SAVE_REQUEST, + MARKER_SAVE_SUCCESS, +} from 'soapbox/actions/markers'; describe('notifications reducer', () => { it('should return the initial state', () => { @@ -42,7 +47,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -53,7 +57,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -64,12 +67,11 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), hasMore: false, top: false, - unread: 1, + unread: 0, isLoading: false, queuedNotifications: ImmutableOrderedMap(), totalQueuedNotificationsCount: 0, @@ -113,7 +115,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -124,7 +125,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -135,7 +135,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), hasMore: false, @@ -210,7 +209,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), top: false, @@ -269,7 +267,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ]), unread: 1, @@ -292,7 +289,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -303,7 +299,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -314,7 +309,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10734', ImmutableMap({ id: '10734', @@ -325,7 +319,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ]), unread: 1, @@ -357,7 +350,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -368,7 +360,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -379,7 +370,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), unread: 1, @@ -400,7 +390,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -411,7 +400,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -422,7 +410,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), }); @@ -441,7 +428,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -452,7 +438,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), })); @@ -470,7 +455,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -481,7 +465,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -492,7 +475,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), }); @@ -511,7 +493,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -522,7 +503,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), })); @@ -568,7 +548,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: '😢', chat_message: undefined, - is_seen: false, })], ['10743', ImmutableMap({ id: '10743', @@ -579,7 +558,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ['10741', ImmutableMap({ id: '10741', @@ -590,7 +568,6 @@ describe('notifications reducer', () => { status: '9vvNxoo5EFbbnfdXQu', emoji: undefined, chat_message: undefined, - is_seen: true, })], ]), }); @@ -617,7 +594,6 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: '😢', // chat_message: undefined, - // is_seen: false, // }), // ImmutableMap({ // id: '10743', @@ -627,7 +603,6 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: undefined, // chat_message: undefined, - // is_seen: true, // }), // ImmutableMap({ // id: '10741', @@ -637,7 +612,6 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: undefined, // chat_message: undefined, - // is_seen: true, // }), // ]), // }); @@ -656,7 +630,6 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: '😢', // chat_message: undefined, - // is_seen: false, // }), // ImmutableMap({ // id: '10743', @@ -666,7 +639,6 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: undefined, // chat_message: undefined, - // is_seen: true, // }), // ImmutableMap({ // id: '10741', @@ -676,10 +648,125 @@ describe('notifications reducer', () => { // status: '9vvNxoo5EFbbnfdXQu', // emoji: undefined, // chat_message: undefined, - // is_seen: true, // }), // ]), // })); // }); + describe('MARKER_FETCH_SUCCESS', () => { + it('sets lastRead', () => { + const action = { + type: MARKER_FETCH_SUCCESS, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '1234', + }, + }, + }; + + expect(reducer(undefined, action).get('lastRead')).toEqual('1234'); + }); + + it('updates the unread count', () => { + const action = { + type: MARKER_FETCH_SUCCESS, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '5678', + }, + }, + }; + + const state = ImmutableMap({ + items: ImmutableOrderedMap({ + '9012': ImmutableMap({ id: '9012' }), + '5678': ImmutableMap({ id: '5678' }), + '1234': ImmutableMap({ id: '1234' }), + }), + unread: 3, + }); + + expect(reducer(state, action).get('unread')).toEqual(1); + }); + }); + + describe('MARKER_SAVE_REQUEST', () => { + it('sets lastRead', () => { + const action = { + type: MARKER_SAVE_REQUEST, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '1234', + }, + }, + }; + + expect(reducer(undefined, action).get('lastRead')).toEqual('1234'); + }); + + it('updates the unread count', () => { + const action = { + type: MARKER_SAVE_REQUEST, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '5678', + }, + }, + }; + + const state = ImmutableMap({ + items: ImmutableOrderedMap({ + '9012': ImmutableMap({ id: '9012' }), + '5678': ImmutableMap({ id: '5678' }), + '1234': ImmutableMap({ id: '1234' }), + }), + unread: 3, + }); + + expect(reducer(state, action).get('unread')).toEqual(1); + }); + }); + + describe('MARKER_SAVE_SUCCESS', () => { + it('sets lastRead', () => { + const action = { + type: MARKER_SAVE_SUCCESS, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '5678', + }, + }, + }; + + expect(reducer(undefined, action).get('lastRead')).toEqual('5678'); + }); + + it('updates the unread count', () => { + const action = { + type: MARKER_SAVE_SUCCESS, + timeline: ['notifications'], + marker: { + notifications: { + last_read_id: '9012', + }, + }, + }; + + const state = ImmutableMap({ + items: ImmutableOrderedMap({ + '9012': ImmutableMap({ id: '9012' }), + '5678': ImmutableMap({ id: '5678' }), + '1234': ImmutableMap({ id: '1234' }), + }), + unread: 3, + }); + + expect(reducer(state, action).get('unread')).toEqual(0); + }); + }); }); diff --git a/app/soapbox/reducers/notifications.js b/app/soapbox/reducers/notifications.js index 416fae9c0..149523c7e 100644 --- a/app/soapbox/reducers/notifications.js +++ b/app/soapbox/reducers/notifications.js @@ -18,8 +18,12 @@ import { FOLLOW_REQUEST_REJECT_SUCCESS, } from '../actions/accounts'; import { TIMELINE_DELETE } from '../actions/timelines'; -import { Map as ImmutableMap, OrderedMap as ImmutableOrderedMap } from 'immutable'; -import { get } from 'lodash'; +import { + MARKER_FETCH_SUCCESS, + MARKER_SAVE_REQUEST, + MARKER_SAVE_SUCCESS, +} from 'soapbox/actions/markers'; +import { Map as ImmutableMap, OrderedMap as ImmutableOrderedMap, fromJS } from 'immutable'; const initialState = ImmutableMap({ items: ImmutableOrderedMap(), @@ -32,9 +36,11 @@ const initialState = ImmutableMap({ lastRead: -1, }); +const parseId = id => parseInt(id, 10); + // For sorting the notifications const comparator = (a, b) => { - const parse = m => parseInt(m.get('id'), 10); + const parse = m => parseId(m.get('id')); if (parse(a) < parse(b)) return 1; if (parse(a) > parse(b)) return -1; return 0; @@ -49,14 +55,21 @@ const notificationToMap = notification => ImmutableMap({ status: notification.status ? notification.status.id : null, emoji: notification.emoji, chat_message: notification.chat_message, - is_seen: get(notification, ['pleroma', 'is_seen'], true), }); // https://gitlab.com/soapbox-pub/soapbox-fe/-/issues/424 const isValid = notification => Boolean(notification.account.id); -const countUnseen = notifications => notifications.reduce((acc, cur) => - get(cur, ['pleroma', 'is_seen'], true) === false ? acc + 1 : acc, 0); +// Count how many notifications appear after the given ID (for unread count) +const countFuture = (notifications, lastId) => { + return notifications.reduce((acc, notification) => { + if (parseId(notification.get('id')) > parseId(lastId)) { + return acc + 1; + } else { + return acc; + } + }, 0); +}; const normalizeNotification = (state, notification) => { const top = state.get('top'); @@ -81,16 +94,12 @@ const processRawNotifications = notifications => ( const expandNormalizedNotifications = (state, notifications, next) => { const items = processRawNotifications(notifications); - const unread = state.get('unread'); - const legacyUnread = countUnseen(notifications); return state.withMutations(mutable => { mutable.update('items', map => map.merge(items).sort(comparator)); if (!next) mutable.set('hasMore', false); mutable.set('isLoading', false); - - mutable.set('unread', Math.max(legacyUnread, unread)); }); }; @@ -134,6 +143,22 @@ const updateNotificationsQueue = (state, notification, intlMessages, intlLocale) }); }; +const importMarker = (state, marker) => { + const lastReadId = marker.getIn(['notifications', 'last_read_id'], -1); + + if (!lastReadId) { + return state; + } + + return state.withMutations(state => { + const notifications = state.get('items'); + const unread = countFuture(notifications, lastReadId); + + state.set('unread', unread); + state.set('lastRead', lastReadId); + }); +}; + export default function notifications(state = initialState, action) { switch(action.type) { case NOTIFICATIONS_EXPAND_REQUEST: @@ -166,6 +191,10 @@ export default function notifications(state = initialState, action) { return state.set('items', ImmutableOrderedMap()).set('hasMore', false); case NOTIFICATIONS_MARK_READ_REQUEST: return state.set('lastRead', action.lastRead); + case MARKER_FETCH_SUCCESS: + case MARKER_SAVE_REQUEST: + case MARKER_SAVE_SUCCESS: + return importMarker(state, fromJS(action.marker)); case TIMELINE_DELETE: return deleteByStatus(state, action.id);