Merge branch 'notifications-refactor' into 'develop'

Notifications: refactor with OrderedSet instead of List

See merge request soapbox-pub/soapbox-fe!245
merge-requests/246/head
Alex Gleason 2020-09-22 18:55:47 +00:00
commit 3b8eed17b6
4 zmienionych plików z 83 dodań i 82 usunięć

Wyświetl plik

@ -10,7 +10,11 @@ import {
} from './importer';
import { getSettings, saveSettings } from './settings';
import { defineMessages } from 'react-intl';
import { List as ImmutableList } from 'immutable';
import {
List as ImmutableList,
Map as ImmutableMap,
OrderedSet as ImmutableOrderedSet,
} from 'immutable';
import { unescapeHTML } from '../utils/html';
import { getFilters, regexFromFilters } from '../selectors';
@ -121,7 +125,7 @@ export function updateNotificationsQueue(notification, intlMessages, intlLocale,
export function dequeueNotifications() {
return (dispatch, getState) => {
const queuedNotifications = getState().getIn(['notifications', 'queuedNotifications'], ImmutableList());
const queuedNotifications = getState().getIn(['notifications', 'queuedNotifications'], ImmutableOrderedSet());
const totalQueuedNotificationsCount = getState().getIn(['notifications', 'totalQueuedNotificationsCount'], 0);
if (totalQueuedNotificationsCount === 0) {
@ -252,9 +256,12 @@ export function setFilter(filterType) {
export function markReadNotifications() {
return (dispatch, getState) => {
if (!getState().get('me')) return;
const topNotification = parseInt(getState().getIn(['notifications', 'items', 0, 'id']));
const lastRead = getState().getIn(['notifications', 'lastRead']);
const state = getState();
if (!state.get('me')) return;
const topNotification = state.getIn(['notifications', 'items'], ImmutableOrderedSet()).first(ImmutableMap()).get('id');
const lastRead = state.getIn(['notifications', 'lastRead']);
if (!(topNotification && topNotification > lastRead)) return;
dispatch({

Wyświetl plik

@ -55,7 +55,7 @@ export default @connect(mapStateToProps)
class Notifications extends React.PureComponent {
static propTypes = {
notifications: ImmutablePropTypes.list.isRequired,
notifications: ImmutablePropTypes.orderedSet.isRequired,
showFilterBar: PropTypes.bool.isRequired,
dispatch: PropTypes.func.isRequired,
intl: PropTypes.object.isRequired,
@ -142,6 +142,8 @@ class Notifications extends React.PureComponent {
} else if (notifications.size > 0 || hasMore) {
scrollableContent = notifications.map((item, index) => item === null ? (
<LoadGap
// FIXME: This won't work, can't get OrderedSet by index
// https://github.com/immutable-js/immutable-js/issues/1790
key={'gap:' + notifications.getIn([index + 1, 'id'])}
disabled={isLoading}
maxId={index > 0 ? notifications.getIn([index - 1, 'id']) : null}

Wyświetl plik

@ -1,7 +1,7 @@
import * as actions from 'soapbox/actions/notifications';
import reducer from '../notifications';
import notifications from 'soapbox/__fixtures__/notifications.json';
import { Map as ImmutableMap, List as ImmutableList } from 'immutable';
import { Map as ImmutableMap, OrderedSet as ImmutableOrderedSet } from 'immutable';
import { take } from 'lodash';
import { ACCOUNT_BLOCK_SUCCESS, ACCOUNT_MUTE_SUCCESS } from 'soapbox/actions/accounts';
import notification from 'soapbox/__fixtures__/notification.json';
@ -12,12 +12,12 @@ import { TIMELINE_DELETE, TIMELINE_DISCONNECT } from 'soapbox/actions/timelines'
describe('notifications reducer', () => {
it('should return the initial state', () => {
expect(reducer(undefined, {})).toEqual(ImmutableMap({
items: ImmutableList(),
items: ImmutableOrderedSet(),
hasMore: true,
top: false,
unread: 0,
isLoading: false,
queuedNotifications: ImmutableList(),
queuedNotifications: ImmutableOrderedSet(),
totalQueuedNotificationsCount: 0,
lastRead: -1,
}));
@ -32,7 +32,7 @@ describe('notifications reducer', () => {
skipLoading: true,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -68,7 +68,7 @@ describe('notifications reducer', () => {
top: false,
unread: 1,
isLoading: false,
queuedNotifications: ImmutableList(),
queuedNotifications: ImmutableOrderedSet(),
totalQueuedNotificationsCount: 0,
lastRead: -1,
}));
@ -100,7 +100,7 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_FILTER_SET', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -136,7 +136,7 @@ describe('notifications reducer', () => {
top: false,
unread: 1,
isLoading: false,
queuedNotifications: ImmutableList(),
queuedNotifications: ImmutableOrderedSet(),
totalQueuedNotificationsCount: 0,
lastRead: -1,
});
@ -144,12 +144,12 @@ describe('notifications reducer', () => {
type: actions.NOTIFICATIONS_FILTER_SET,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList(),
items: ImmutableOrderedSet(),
hasMore: true,
top: false,
unread: 1,
isLoading: false,
queuedNotifications: ImmutableList(),
queuedNotifications: ImmutableOrderedSet(),
totalQueuedNotificationsCount: 0,
lastRead: -1,
}));
@ -185,7 +185,7 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_UPDATE, when top = false, increment unread', () => {
const state = ImmutableMap({
items: ImmutableList(),
items: ImmutableOrderedSet(),
top: false,
unread: 1,
});
@ -194,7 +194,7 @@ describe('notifications reducer', () => {
notification: notification,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10743',
type: 'favourite',
@ -213,8 +213,8 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_UPDATE_QUEUE', () => {
const state = ImmutableMap({
items: ImmutableList([]),
queuedNotifications: ImmutableList([]),
items: ImmutableOrderedSet([]),
queuedNotifications: ImmutableOrderedSet([]),
totalQueuedNotificationsCount: 0,
});
const action = {
@ -224,8 +224,8 @@ describe('notifications reducer', () => {
intlLocale: 'en',
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([]),
queuedNotifications: ImmutableList([{
items: ImmutableOrderedSet([]),
queuedNotifications: ImmutableOrderedSet([{
notification: notification,
intlMessages: intlMessages,
intlLocale: 'en',
@ -236,7 +236,7 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_DEQUEUE', () => {
const state = ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
queuedNotifications: take(notifications, 1),
totalQueuedNotificationsCount: 1,
});
@ -244,15 +244,15 @@ describe('notifications reducer', () => {
type: actions.NOTIFICATIONS_DEQUEUE,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([]),
queuedNotifications: ImmutableList([]),
items: ImmutableOrderedSet([]),
queuedNotifications: ImmutableOrderedSet([]),
totalQueuedNotificationsCount: 0,
}));
});
it('should handle NOTIFICATIONS_EXPAND_SUCCESS with non-empty items and next set true', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10734',
type: 'pleroma:emoji_reaction',
@ -274,7 +274,7 @@ describe('notifications reducer', () => {
next: true,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -324,7 +324,7 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_EXPAND_SUCCESS with empty items and next set true', () => {
const state = ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
unread: 1,
hasMore: true,
isLoading: false,
@ -335,7 +335,7 @@ describe('notifications reducer', () => {
next: true,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -375,7 +375,7 @@ describe('notifications reducer', () => {
it('should handle ACCOUNT_BLOCK_SUCCESS', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -413,7 +413,7 @@ describe('notifications reducer', () => {
relationship: relationship,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10743',
type: 'favourite',
@ -440,7 +440,7 @@ describe('notifications reducer', () => {
it('should handle ACCOUNT_MUTE_SUCCESS', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -478,7 +478,7 @@ describe('notifications reducer', () => {
relationship: relationship,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10743',
type: 'favourite',
@ -505,35 +505,35 @@ describe('notifications reducer', () => {
it('should handle NOTIFICATIONS_CLEAR', () => {
const state = ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
hasMore: true,
});
const action = {
type: actions.NOTIFICATIONS_CLEAR,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
hasMore: false,
}));
});
it('should handle NOTIFICATIONS_MARK_READ_REQUEST', () => {
const state = ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
});
const action = {
type: actions.NOTIFICATIONS_MARK_READ_REQUEST,
lastRead: 35098814,
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
lastRead: 35098814,
}));
});
it('should handle TIMELINE_DELETE', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -571,13 +571,13 @@ describe('notifications reducer', () => {
id: '9vvNxoo5EFbbnfdXQu',
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([]),
items: ImmutableOrderedSet([]),
}));
});
it('should handle TIMELINE_DISCONNECT', () => {
const state = ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
ImmutableMap({
id: '10744',
type: 'pleroma:emoji_reaction',
@ -615,7 +615,7 @@ describe('notifications reducer', () => {
timeline: 'home',
};
expect(reducer(state, action)).toEqual(ImmutableMap({
items: ImmutableList([
items: ImmutableOrderedSet([
null,
ImmutableMap({
id: '10744',

Wyświetl plik

@ -16,21 +16,27 @@ import {
ACCOUNT_MUTE_SUCCESS,
} from '../actions/accounts';
import { TIMELINE_DELETE, TIMELINE_DISCONNECT } from '../actions/timelines';
import { Map as ImmutableMap, List as ImmutableList } from 'immutable';
import compareId from '../compare_id';
import { Map as ImmutableMap, OrderedSet as ImmutableOrderedSet } from 'immutable';
import { get } from 'lodash';
const initialState = ImmutableMap({
items: ImmutableList(),
items: ImmutableOrderedSet(),
hasMore: true,
top: false,
unread: 0,
isLoading: false,
queuedNotifications: ImmutableList(), //max = MAX_QUEUED_NOTIFICATIONS
queuedNotifications: ImmutableOrderedSet(), //max = MAX_QUEUED_NOTIFICATIONS
totalQueuedNotificationsCount: 0, //used for queuedItems overflow for MAX_QUEUED_NOTIFICATIONS+
lastRead: -1,
});
// For sorting the notifications
const comparator = (a, b) => {
if (a.get('id') < b.get('id')) return 1;
if (a.get('id') > b.get('id')) return -1;
return 0;
};
const notificationToMap = notification => ImmutableMap({
id: notification.id,
type: notification.type,
@ -42,49 +48,36 @@ const notificationToMap = notification => ImmutableMap({
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 normalizeNotification = (state, notification) => {
const top = state.get('top');
if (!top) {
state = state.update('unread', unread => unread + 1);
}
if (!top) state = state.update('unread', unread => unread + 1);
return state.update('items', list => {
if (top && list.size > 40) {
list = list.take(20);
}
return list.unshift(notificationToMap(notification));
return list.add(notificationToMap(notification)).sort(comparator);
});
};
const expandNormalizedNotifications = (state, notifications, next) => {
let items = ImmutableList();
const processRawNotifications = notifications => (
ImmutableOrderedSet(
notifications
.filter(isValid)
.map(notificationToMap)));
notifications.forEach((n) => {
if (!n.account.id) return;
items = items.push(notificationToMap(n));
});
const expandNormalizedNotifications = (state, notifications, next) => {
const items = processRawNotifications(notifications);
return state.withMutations(mutable => {
if (!items.isEmpty()) {
mutable.update('items', list => {
const lastIndex = 1 + list.findLastIndex(
item => item !== null && (compareId(item.get('id'), items.last().get('id')) > 0 || item.get('id') === items.last().get('id'))
);
const firstIndex = 1 + list.take(lastIndex).findLastIndex(
item => item !== null && compareId(item.get('id'), items.first().get('id')) > 0
);
return list.take(firstIndex).concat(items, list.skip(lastIndex));
});
}
if (!next) {
mutable.set('hasMore', false);
}
mutable.update('items', list => list.union(items).sort(comparator));
if (!next) mutable.set('hasMore', false);
mutable.set('isLoading', false);
});
};
@ -94,10 +87,7 @@ const filterNotifications = (state, relationship) => {
};
const updateTop = (state, top) => {
if (top) {
state = state.set('unread', 0);
}
if (top) state = state.set('unread', 0);
return state.set('top', top);
};
@ -106,8 +96,8 @@ const deleteByStatus = (state, statusId) => {
};
const updateNotificationsQueue = (state, notification, intlMessages, intlLocale) => {
const queuedNotifications = state.getIn(['queuedNotifications'], ImmutableList());
const listedNotifications = state.getIn(['items'], ImmutableList());
const queuedNotifications = state.getIn(['queuedNotifications'], ImmutableOrderedSet());
const listedNotifications = state.getIn(['items'], ImmutableOrderedSet());
const totalQueuedNotificationsCount = state.getIn(['totalQueuedNotificationsCount'], 0);
let alreadyExists = queuedNotifications.find(existingQueuedNotification => existingQueuedNotification.id === notification.id);
@ -121,7 +111,7 @@ const updateNotificationsQueue = (state, notification, intlMessages, intlLocale)
return state.withMutations(mutable => {
if (totalQueuedNotificationsCount <= MAX_QUEUED_NOTIFICATIONS) {
mutable.set('queuedNotifications', newQueuedNotifications.push({
mutable.set('queuedNotifications', newQueuedNotifications.add({
notification,
intlMessages,
intlLocale,
@ -138,7 +128,7 @@ export default function notifications(state = initialState, action) {
case NOTIFICATIONS_EXPAND_FAIL:
return state.set('isLoading', false);
case NOTIFICATIONS_FILTER_SET:
return state.set('items', ImmutableList()).set('hasMore', true);
return state.set('items', ImmutableOrderedSet()).set('hasMore', true);
case NOTIFICATIONS_SCROLL_TOP:
return updateTop(state, action.top);
case NOTIFICATIONS_UPDATE:
@ -147,7 +137,7 @@ export default function notifications(state = initialState, action) {
return updateNotificationsQueue(state, action.notification, action.intlMessages, action.intlLocale);
case NOTIFICATIONS_DEQUEUE:
return state.withMutations(mutable => {
mutable.set('queuedNotifications', ImmutableList());
mutable.set('queuedNotifications', ImmutableOrderedSet());
mutable.set('totalQueuedNotificationsCount', 0);
});
case NOTIFICATIONS_EXPAND_SUCCESS:
@ -160,14 +150,16 @@ export default function notifications(state = initialState, action) {
case ACCOUNT_MUTE_SUCCESS:
return action.relationship.muting_notifications ? filterNotifications(state, action.relationship) : state;
case NOTIFICATIONS_CLEAR:
return state.set('items', ImmutableList()).set('hasMore', false);
return state.set('items', ImmutableOrderedSet()).set('hasMore', false);
case NOTIFICATIONS_MARK_READ_REQUEST:
return state.set('lastRead', action.lastRead);
case TIMELINE_DELETE:
return deleteByStatus(state, action.id);
case TIMELINE_DISCONNECT:
// This is kind of a hack - `null` renders a LoadGap in the component
// https://github.com/tootsuite/mastodon/pull/6886
return action.timeline === 'home' ?
state.update('items', items => items.first() ? items.unshift(null) : items) :
state.update('items', items => items.first() ? ImmutableOrderedSet([null]).union(items) : items) :
state;
default:
return state;