From 47bf7a8047ce59b899d147e4483168f9852eeb7c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 11 Jun 2017 17:07:35 +0200 Subject: Fix #3665 - Refactor timelines reducer (#3686) * Move ancestors/descendants out of timelines reducer * Refactor timelines reducer All types of timelines now have a flat structure and use the same reducer functions and actions * Reintroduce some missing behaviours * Fix wrong import in reports * Fix includes typo * Fix issue related to "next" pagination in timelines and notifications * Fix bug with timeline's initial state, expandNotifications --- app/javascript/mastodon/reducers/accounts.js | 4 - .../mastodon/reducers/accounts_counters.js | 4 - app/javascript/mastodon/reducers/contexts.js | 43 +++ app/javascript/mastodon/reducers/index.js | 2 + app/javascript/mastodon/reducers/statuses.js | 8 - app/javascript/mastodon/reducers/timelines.js | 314 ++++----------------- 6 files changed, 103 insertions(+), 272 deletions(-) create mode 100644 app/javascript/mastodon/reducers/contexts.js (limited to 'app/javascript/mastodon/reducers') diff --git a/app/javascript/mastodon/reducers/accounts.js b/app/javascript/mastodon/reducers/accounts.js index d4d9ad62e..7b7074317 100644 --- a/app/javascript/mastodon/reducers/accounts.js +++ b/app/javascript/mastodon/reducers/accounts.js @@ -4,8 +4,6 @@ import { FOLLOWERS_EXPAND_SUCCESS, FOLLOWING_FETCH_SUCCESS, FOLLOWING_EXPAND_SUCCESS, - ACCOUNT_TIMELINE_FETCH_SUCCESS, - ACCOUNT_TIMELINE_EXPAND_SUCCESS, FOLLOW_REQUESTS_FETCH_SUCCESS, FOLLOW_REQUESTS_EXPAND_SUCCESS, } from '../actions/accounts'; @@ -113,8 +111,6 @@ export default function accounts(state = initialState, action) { return normalizeAccountsFromStatuses(normalizeAccounts(state, action.accounts), action.statuses); case TIMELINE_REFRESH_SUCCESS: case TIMELINE_EXPAND_SUCCESS: - case ACCOUNT_TIMELINE_FETCH_SUCCESS: - case ACCOUNT_TIMELINE_EXPAND_SUCCESS: case CONTEXT_FETCH_SUCCESS: case FAVOURITED_STATUSES_FETCH_SUCCESS: case FAVOURITED_STATUSES_EXPAND_SUCCESS: diff --git a/app/javascript/mastodon/reducers/accounts_counters.js b/app/javascript/mastodon/reducers/accounts_counters.js index ea631ceca..eb8a4f83d 100644 --- a/app/javascript/mastodon/reducers/accounts_counters.js +++ b/app/javascript/mastodon/reducers/accounts_counters.js @@ -4,8 +4,6 @@ import { FOLLOWERS_EXPAND_SUCCESS, FOLLOWING_FETCH_SUCCESS, FOLLOWING_EXPAND_SUCCESS, - ACCOUNT_TIMELINE_FETCH_SUCCESS, - ACCOUNT_TIMELINE_EXPAND_SUCCESS, FOLLOW_REQUESTS_FETCH_SUCCESS, FOLLOW_REQUESTS_EXPAND_SUCCESS, ACCOUNT_FOLLOW_SUCCESS, @@ -115,8 +113,6 @@ export default function accountsCounters(state = initialState, action) { return normalizeAccountsFromStatuses(normalizeAccounts(state, action.accounts), action.statuses); case TIMELINE_REFRESH_SUCCESS: case TIMELINE_EXPAND_SUCCESS: - case ACCOUNT_TIMELINE_FETCH_SUCCESS: - case ACCOUNT_TIMELINE_EXPAND_SUCCESS: case CONTEXT_FETCH_SUCCESS: case FAVOURITED_STATUSES_FETCH_SUCCESS: case FAVOURITED_STATUSES_EXPAND_SUCCESS: diff --git a/app/javascript/mastodon/reducers/contexts.js b/app/javascript/mastodon/reducers/contexts.js new file mode 100644 index 000000000..8a24f5f7a --- /dev/null +++ b/app/javascript/mastodon/reducers/contexts.js @@ -0,0 +1,43 @@ +import { CONTEXT_FETCH_SUCCESS } from '../actions/statuses'; +import { TIMELINE_DELETE } from '../actions/timelines'; +import Immutable from 'immutable'; + +const initialState = Immutable.Map({ + ancestors: Immutable.Map(), + descendants: Immutable.Map(), +}); + +const normalizeContext = (state, id, ancestors, descendants) => { + const ancestorsIds = ancestors.map(ancestor => ancestor.get('id')); + const descendantsIds = descendants.map(descendant => descendant.get('id')); + + return state.withMutations(map => { + map.setIn(['ancestors', id], ancestorsIds); + map.setIn(['descendants', id], descendantsIds); + }); +}; + +const deleteFromContexts = (state, id) => { + state.getIn(['descendants', id], Immutable.List()).forEach(descendantId => { + state = state.updateIn(['ancestors', descendantId], Immutable.List(), list => list.filterNot(itemId => itemId === id)); + }); + + state.getIn(['ancestors', id], Immutable.List()).forEach(ancestorId => { + state = state.updateIn(['descendants', ancestorId], Immutable.List(), list => list.filterNot(itemId => itemId === id)); + }); + + state = state.deleteIn(['descendants', id]).deleteIn(['ancestors', id]); + + return state; +}; + +export default function contexts(state = initialState, action) { + switch(action.type) { + case CONTEXT_FETCH_SUCCESS: + return normalizeContext(state, action.id, Immutable.fromJS(action.ancestors), Immutable.fromJS(action.descendants)); + case TIMELINE_DELETE: + return deleteFromContexts(state, action.id); + default: + return state; + } +}; diff --git a/app/javascript/mastodon/reducers/index.js b/app/javascript/mastodon/reducers/index.js index c4fe28ea7..be402a16b 100644 --- a/app/javascript/mastodon/reducers/index.js +++ b/app/javascript/mastodon/reducers/index.js @@ -17,6 +17,7 @@ import settings from './settings'; import status_lists from './status_lists'; import cards from './cards'; import reports from './reports'; +import contexts from './contexts'; export default combineReducers({ timelines, @@ -37,4 +38,5 @@ export default combineReducers({ settings, cards, reports, + contexts, }); diff --git a/app/javascript/mastodon/reducers/statuses.js b/app/javascript/mastodon/reducers/statuses.js index 7bc3710c4..691135ff7 100644 --- a/app/javascript/mastodon/reducers/statuses.js +++ b/app/javascript/mastodon/reducers/statuses.js @@ -21,10 +21,6 @@ import { TIMELINE_EXPAND_SUCCESS, } from '../actions/timelines'; import { - ACCOUNT_TIMELINE_FETCH_SUCCESS, - ACCOUNT_TIMELINE_EXPAND_SUCCESS, - ACCOUNT_MEDIA_TIMELINE_FETCH_SUCCESS, - ACCOUNT_MEDIA_TIMELINE_EXPAND_SUCCESS, ACCOUNT_BLOCK_SUCCESS, } from '../actions/accounts'; import { @@ -113,10 +109,6 @@ export default function statuses(state = initialState, action) { return state.setIn([action.id, 'muted'], false); case TIMELINE_REFRESH_SUCCESS: case TIMELINE_EXPAND_SUCCESS: - case ACCOUNT_TIMELINE_FETCH_SUCCESS: - case ACCOUNT_TIMELINE_EXPAND_SUCCESS: - case ACCOUNT_MEDIA_TIMELINE_FETCH_SUCCESS: - case ACCOUNT_MEDIA_TIMELINE_EXPAND_SUCCESS: case CONTEXT_FETCH_SUCCESS: case NOTIFICATIONS_REFRESH_SUCCESS: case NOTIFICATIONS_EXPAND_SUCCESS: diff --git a/app/javascript/mastodon/reducers/timelines.js b/app/javascript/mastodon/reducers/timelines.js index ab756b854..2bc1c8050 100644 --- a/app/javascript/mastodon/reducers/timelines.js +++ b/app/javascript/mastodon/reducers/timelines.js @@ -18,228 +18,79 @@ import { UNFAVOURITE_SUCCESS, } from '../actions/interactions'; import { - ACCOUNT_TIMELINE_FETCH_REQUEST, - ACCOUNT_TIMELINE_FETCH_SUCCESS, - ACCOUNT_TIMELINE_FETCH_FAIL, - ACCOUNT_TIMELINE_EXPAND_REQUEST, - ACCOUNT_TIMELINE_EXPAND_SUCCESS, - ACCOUNT_TIMELINE_EXPAND_FAIL, - ACCOUNT_MEDIA_TIMELINE_FETCH_REQUEST, - ACCOUNT_MEDIA_TIMELINE_FETCH_SUCCESS, - ACCOUNT_MEDIA_TIMELINE_FETCH_FAIL, - ACCOUNT_MEDIA_TIMELINE_EXPAND_REQUEST, - ACCOUNT_MEDIA_TIMELINE_EXPAND_SUCCESS, - ACCOUNT_MEDIA_TIMELINE_EXPAND_FAIL, ACCOUNT_BLOCK_SUCCESS, ACCOUNT_MUTE_SUCCESS, } from '../actions/accounts'; -import { - CONTEXT_FETCH_SUCCESS, -} from '../actions/statuses'; import Immutable from 'immutable'; -const initialState = Immutable.Map({ - home: Immutable.Map({ - path: () => '/api/v1/timelines/home', - next: null, - isLoading: false, - online: false, - loaded: false, - top: true, - unread: 0, - items: Immutable.List(), - }), - - public: Immutable.Map({ - path: () => '/api/v1/timelines/public', - next: null, - isLoading: false, - online: false, - loaded: false, - top: true, - unread: 0, - items: Immutable.List(), - }), - - community: Immutable.Map({ - path: () => '/api/v1/timelines/public', - next: null, - params: { local: true }, - isLoading: false, - online: false, - loaded: false, - top: true, - unread: 0, - items: Immutable.List(), - }), +const initialState = Immutable.Map(); - tag: Immutable.Map({ - path: (id) => `/api/v1/timelines/tag/${id}`, - next: null, - isLoading: false, - id: null, - loaded: false, - top: true, - unread: 0, - items: Immutable.List(), - }), - - accounts_timelines: Immutable.Map(), - accounts_media_timelines: Immutable.Map(), - ancestors: Immutable.Map(), - descendants: Immutable.Map(), +const initialTimeline = Immutable.Map({ + unread: 0, + online: false, + top: true, + loaded: false, + isLoading: false, + next: false, + items: Immutable.List(), }); -const normalizeStatus = (state, status) => { - return state; -}; - const normalizeTimeline = (state, timeline, statuses, next) => { - let ids = Immutable.List(); - const loaded = state.getIn([timeline, 'loaded']); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - ids = ids.set(i, status.get('id')); - }); - - state = state.setIn([timeline, 'loaded'], true); - state = state.setIn([timeline, 'isLoading'], false); - - if (state.getIn([timeline, 'next']) === null) { - state = state.setIn([timeline, 'next'], next); - } - - return state.updateIn([timeline, 'items'], Immutable.List(), list => (loaded ? ids.concat(list) : ids)); + const ids = Immutable.List(statuses.map(status => status.get('id'))); + const wasLoaded = state.getIn([timeline, 'loaded']); + const hadNext = state.getIn([timeline, 'next']); + const oldIds = state.getIn([timeline, 'items'], Immutable.List()); + + return state.update(timeline, initialTimeline, map => map.withMutations(mMap => { + mMap.set('loaded', true); + mMap.set('isLoading', false); + if (!hadNext) mMap.set('next', next); + mMap.set('items', wasLoaded ? ids.concat(oldIds) : ids); + })); }; const appendNormalizedTimeline = (state, timeline, statuses, next) => { - let moreIds = Immutable.List(); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - moreIds = moreIds.set(i, status.get('id')); - }); - - state = state.setIn([timeline, 'isLoading'], false); - state = state.setIn([timeline, 'next'], next); - - return state.updateIn([timeline, 'items'], Immutable.List(), list => list.concat(moreIds)); -}; - -const normalizeAccountTimeline = (state, accountId, statuses, replace, next) => { - let ids = Immutable.List(); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - ids = ids.set(i, status.get('id')); - }); - - return state.updateIn(['accounts_timelines', accountId], Immutable.Map(), map => map - .set('isLoading', false) - .set('loaded', true) - .update('next', null, v => replace ? next : v) - .update('items', Immutable.List(), list => (replace ? ids : ids.concat(list)))); -}; - -const normalizeAccountMediaTimeline = (state, accountId, statuses, replace, next) => { - let ids = Immutable.List(); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - ids = ids.set(i, status.get('id')); - }); - - return state.updateIn(['accounts_media_timelines', accountId], Immutable.Map(), map => map - .set('isLoading', false) - .update('next', null, v => replace ? next : v) - .update('items', Immutable.List(), list => (replace ? ids : ids.concat(list)))); -}; - -const appendNormalizedAccountTimeline = (state, accountId, statuses, next) => { - let moreIds = Immutable.List([]); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - moreIds = moreIds.set(i, status.get('id')); - }); - - return state.updateIn(['accounts_timelines', accountId], Immutable.Map(), map => map - .set('isLoading', false) - .set('next', next) - .update('items', list => list.concat(moreIds))); -}; - -const appendNormalizedAccountMediaTimeline = (state, accountId, statuses, next) => { - let moreIds = Immutable.List([]); - - statuses.forEach((status, i) => { - state = normalizeStatus(state, status); - moreIds = moreIds.set(i, status.get('id')); - }); - - return state.updateIn(['accounts_media_timelines', accountId], Immutable.Map(), map => map - .set('isLoading', false) - .set('next', next) - .update('items', list => list.concat(moreIds))); + const ids = Immutable.List(statuses.map(status => status.get('id'))); + const oldIds = state.getIn([timeline, 'items'], Immutable.List()); + + return state.update(timeline, initialTimeline, map => map.withMutations(mMap => { + mMap.set('isLoading', false); + mMap.set('next', next); + mMap.set('items', oldIds.concat(ids)); + })); }; const updateTimeline = (state, timeline, status, references) => { - const top = state.getIn([timeline, 'top']); + const top = state.getIn([timeline, 'top']); + const ids = state.getIn([timeline, 'items'], Immutable.List()); + const includesId = ids.includes(status.get('id')); + const unread = state.getIn([timeline, 'unread'], 0); - state = normalizeStatus(state, status); - - if (!top) { - state = state.updateIn([timeline, 'unread'], unread => unread + 1); + if (includesId) { + return state; } - state = state.updateIn([timeline, 'items'], Immutable.List(), list => { - if (top && list.size > 40) { - list = list.take(20); - } - - if (list.includes(status.get('id'))) { - return list; - } - - const reblogOfId = status.getIn(['reblog', 'id'], null); + let newIds = ids; - if (reblogOfId !== null) { - list = list.filterNot(itemId => references.includes(itemId)); - } - - return list.unshift(status.get('id')); - }); - - return state; + return state.update(timeline, initialTimeline, map => map.withMutations(mMap => { + if (!top) mMap.set('unread', unread + 1); + if (top && ids.size > 40) newIds = newIds.take(20); + if (status.getIn(['reblog', 'id'], null) !== null) newIds = newIds.filterNot(item => references.includes(item)); + mMap.set('items', newIds.unshift(status.get('id'))); + })); }; const deleteStatus = (state, id, accountId, references, reblogOf) => { - if (reblogOf) { - // If we are deleting a reblog, just replace reblog with its original - return state.updateIn(['home', 'items'], list => list.map(item => item === id ? reblogOf : item)); - } - - // Remove references from timelines - ['home', 'public', 'community', 'tag'].forEach(function (timeline) { - state = state.updateIn([timeline, 'items'], list => list.filterNot(item => item === id)); + state.keySeq().forEach(timeline => { + state = state.updateIn([timeline, 'items'], list => { + if (reblogOf && !list.includes(reblogOf)) { + return list.map(item => item === id ? reblogOf : item); + } else { + return list.filterNot(item => item === id); + } + }); }); - // Remove references from account timelines - state = state.updateIn(['accounts_timelines', accountId, 'items'], Immutable.List([]), list => list.filterNot(item => item === id)); - state = state.updateIn(['accounts_media_timelines', accountId, 'items'], Immutable.List([]), list => list.filterNot(item => item === id)); - - // Remove references from context - state.getIn(['descendants', id], Immutable.List()).forEach(descendantId => { - state = state.updateIn(['ancestors', descendantId], Immutable.List(), list => list.filterNot(itemId => itemId === id)); - }); - - state.getIn(['ancestors', id], Immutable.List()).forEach(ancestorId => { - state = state.updateIn(['descendants', ancestorId], Immutable.List(), list => list.filterNot(itemId => itemId === id)); - }); - - state = state.deleteIn(['descendants', id]).deleteIn(['ancestors', id]); - // Remove reblogs of deleted status references.forEach(ref => { state = deleteStatus(state, ref[0], ref[1], []); @@ -257,54 +108,27 @@ const filterTimelines = (state, relationship, statuses) => { } references = statuses.filter(item => item.get('reblog') === status.get('id')).map(item => [item.get('id'), item.get('account')]); - state = deleteStatus(state, status.get('id'), status.get('account'), references); - }); - - return state; -}; - -const normalizeContext = (state, id, ancestors, descendants) => { - const ancestorsIds = ancestors.map(ancestor => ancestor.get('id')); - const descendantsIds = descendants.map(descendant => descendant.get('id')); - - return state.withMutations(map => { - map.setIn(['ancestors', id], ancestorsIds); - map.setIn(['descendants', id], descendantsIds); + state = deleteStatus(state, status.get('id'), status.get('account'), references); }); -}; - -const resetTimeline = (state, timeline, id) => { - if (timeline === 'tag' && typeof id !== 'undefined' && state.getIn([timeline, 'id']) !== id) { - state = state.update(timeline, map => map - .set('id', id) - .set('isLoading', true) - .set('loaded', false) - .set('next', null) - .set('top', true) - .update('items', list => list.clear())); - } else { - state = state.setIn([timeline, 'isLoading'], true); - } return state; }; const updateTop = (state, timeline, top) => { - if (top) { - state = state.setIn([timeline, 'unread'], 0); - } - - return state.setIn([timeline, 'top'], top); + return state.update(timeline, initialTimeline, map => map.withMutations(mMap => { + if (top) mMap.set('unread', 0); + mMap.set('top', top); + })); }; export default function timelines(state = initialState, action) { switch(action.type) { case TIMELINE_REFRESH_REQUEST: case TIMELINE_EXPAND_REQUEST: - return resetTimeline(state, action.timeline, action.id); + return state.update(action.timeline, initialTimeline, map => map.set('isLoading', true)); case TIMELINE_REFRESH_FAIL: case TIMELINE_EXPAND_FAIL: - return state.setIn([action.timeline, 'isLoading'], false); + return state.update(action.timeline, initialTimeline, map => map.set('isLoading', false)); case TIMELINE_REFRESH_SUCCESS: return normalizeTimeline(state, action.timeline, Immutable.fromJS(action.statuses), action.next); case TIMELINE_EXPAND_SUCCESS: @@ -313,37 +137,15 @@ export default function timelines(state = initialState, action) { return updateTimeline(state, action.timeline, Immutable.fromJS(action.status), action.references); case TIMELINE_DELETE: return deleteStatus(state, action.id, action.accountId, action.references, action.reblogOf); - case CONTEXT_FETCH_SUCCESS: - return normalizeContext(state, action.id, Immutable.fromJS(action.ancestors), Immutable.fromJS(action.descendants)); - case ACCOUNT_TIMELINE_FETCH_REQUEST: - case ACCOUNT_TIMELINE_EXPAND_REQUEST: - return state.updateIn(['accounts_timelines', action.id], Immutable.Map(), map => map.set('isLoading', true)); - case ACCOUNT_TIMELINE_FETCH_FAIL: - case ACCOUNT_TIMELINE_EXPAND_FAIL: - return state.updateIn(['accounts_timelines', action.id], Immutable.Map(), map => map.set('isLoading', false)); - case ACCOUNT_TIMELINE_FETCH_SUCCESS: - return normalizeAccountTimeline(state, action.id, Immutable.fromJS(action.statuses), action.replace, action.next); - case ACCOUNT_TIMELINE_EXPAND_SUCCESS: - return appendNormalizedAccountTimeline(state, action.id, Immutable.fromJS(action.statuses), action.next); - case ACCOUNT_MEDIA_TIMELINE_FETCH_REQUEST: - case ACCOUNT_MEDIA_TIMELINE_EXPAND_REQUEST: - return state.updateIn(['accounts_media_timelines', action.id], Immutable.Map(), map => map.set('isLoading', true)); - case ACCOUNT_MEDIA_TIMELINE_FETCH_FAIL: - case ACCOUNT_MEDIA_TIMELINE_EXPAND_FAIL: - return state.updateIn(['accounts_media_timelines', action.id], Immutable.Map(), map => map.set('isLoading', false)); - case ACCOUNT_MEDIA_TIMELINE_FETCH_SUCCESS: - return normalizeAccountMediaTimeline(state, action.id, Immutable.fromJS(action.statuses), action.replace, action.next); - case ACCOUNT_MEDIA_TIMELINE_EXPAND_SUCCESS: - return appendNormalizedAccountMediaTimeline(state, action.id, Immutable.fromJS(action.statuses), action.next); case ACCOUNT_BLOCK_SUCCESS: case ACCOUNT_MUTE_SUCCESS: return filterTimelines(state, action.relationship, action.statuses); case TIMELINE_SCROLL_TOP: return updateTop(state, action.timeline, action.top); case TIMELINE_CONNECT: - return state.setIn([action.timeline, 'online'], true); + return state.update(action.timeline, initialTimeline, map => map.set('online', true)); case TIMELINE_DISCONNECT: - return state.setIn([action.timeline, 'online'], false); + return state.update(action.timeline, initialTimeline, map => map.set('online', false)); default: return state; } -- cgit