From 5c1663d8f145bdb9bab1101241e67fbd28e9f4ca Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Thu, 18 Jun 2020 19:53:40 -0400 Subject: [PATCH] Move Discussion List State into its own class (#2150) Extract discussion list state --- js/src/forum/ForumApplication.js | 14 ++ js/src/forum/components/DiscussionComposer.js | 2 +- js/src/forum/components/DiscussionList.js | 176 +--------------- js/src/forum/components/DiscussionPage.js | 10 +- js/src/forum/components/IndexPage.js | 27 +-- js/src/forum/state/DiscussionListState.js | 190 ++++++++++++++++++ js/src/forum/states/GlobalSearchState.js | 2 +- js/src/forum/utils/DiscussionControls.js | 8 +- js/src/forum/utils/PostControls.js | 5 +- 9 files changed, 230 insertions(+), 204 deletions(-) create mode 100644 js/src/forum/state/DiscussionListState.js diff --git a/js/src/forum/ForumApplication.js b/js/src/forum/ForumApplication.js index c1ab8a5eee..a07e91906c 100644 --- a/js/src/forum/ForumApplication.js +++ b/js/src/forum/ForumApplication.js @@ -15,6 +15,7 @@ import Application from '../common/Application'; import Navigation from '../common/components/Navigation'; import NotificationListState from './states/NotificationListState'; import GlobalSearchState from './states/GlobalSearchState'; +import DiscussionListState from './state/DiscussionListState'; export default class ForumApplication extends Application { /** @@ -76,6 +77,19 @@ export default class ForumApplication extends Application { super(); routes(this); + + /** + * An object which controls the state of the cached discussion list, which + * is used in the index page and the slideout pane. + * + * @type {DiscussionListState} + */ + this.discussions = new DiscussionListState({ forumApp: this }); + + /** + * @deprecated beta 14, remove in beta 15. + */ + this.cache.discussionList = this.discussions; } /** diff --git a/js/src/forum/components/DiscussionComposer.js b/js/src/forum/components/DiscussionComposer.js index 76c4155de9..f8123d625c 100644 --- a/js/src/forum/components/DiscussionComposer.js +++ b/js/src/forum/components/DiscussionComposer.js @@ -98,7 +98,7 @@ export default class DiscussionComposer extends ComposerBody { .save(data) .then((discussion) => { app.composer.hide(); - app.cache.discussionList.refresh(); + app.discussions.refresh(); m.route(app.route.discussion(discussion)); }, this.loaded.bind(this)); } diff --git a/js/src/forum/components/DiscussionList.js b/js/src/forum/components/DiscussionList.js index 6c065c4177..1d16fa1c6f 100644 --- a/js/src/forum/components/DiscussionList.js +++ b/js/src/forum/components/DiscussionList.js @@ -11,56 +11,38 @@ import Placeholder from '../../common/components/Placeholder'; * * - `params` A map of parameters used to construct a refined parameter object * to send along in the API request to get discussion results. + * - `state` A DiscussionListState object that represents the discussion lists's state. */ export default class DiscussionList extends Component { init() { - /** - * Whether or not discussion results are loading. - * - * @type {Boolean} - */ - this.loading = true; - - /** - * Whether or not there are more results that can be loaded. - * - * @type {Boolean} - */ - this.moreResults = false; - - /** - * The discussions in the discussion list. - * - * @type {Discussion[]} - */ - this.discussions = []; - - this.refresh(); + this.state = this.props.state; } view() { - const params = this.props.params; + const state = this.state; + + const params = state.getParams(); let loading; - if (this.loading) { + if (state.isLoading()) { loading = LoadingIndicator.component(); - } else if (this.moreResults) { + } else if (state.moreResults) { loading = Button.component({ children: app.translator.trans('core.forum.discussion_list.load_more_button'), className: 'Button', - onclick: this.loadMore.bind(this), + onclick: state.loadMore.bind(state), }); } - if (this.discussions.length === 0 && !this.loading) { + if (state.empty()) { const text = app.translator.trans('core.forum.discussion_list.empty_text'); return
{Placeholder.component({ text })}
; } return ( -
+
); } - - /** - * Get the parameters that should be passed in the API request to get - * discussion results. - * - * @return {Object} - * @api - */ - requestParams() { - const params = { include: ['user', 'lastPostedUser'], filter: {} }; - - params.sort = this.sortMap()[this.props.params.sort]; - - if (this.props.params.q) { - params.filter.q = this.props.params.q; - - params.include.push('mostRelevantPost', 'mostRelevantPost.user'); - } - - return params; - } - - /** - * Get a map of sort keys (which appear in the URL, and are used for - * translation) to the API sort value that they represent. - * - * @return {Object} - */ - sortMap() { - const map = {}; - - if (this.props.params.q) { - map.relevance = ''; - } - map.latest = '-lastPostedAt'; - map.top = '-commentCount'; - map.newest = '-createdAt'; - map.oldest = 'createdAt'; - - return map; - } - - /** - * Clear and reload the discussion list. - * - * @public - */ - refresh(clear = true) { - if (clear) { - this.loading = true; - this.discussions = []; - } - - return this.loadResults().then( - (results) => { - this.discussions = []; - this.parseResults(results); - }, - () => { - this.loading = false; - m.redraw(); - } - ); - } - - /** - * Load a new page of discussion results. - * - * @param {Integer} offset The index to start the page at. - * @return {Promise} - */ - loadResults(offset) { - const preloadedDiscussions = app.preloadedApiDocument(); - - if (preloadedDiscussions) { - return m.deferred().resolve(preloadedDiscussions).promise; - } - - const params = this.requestParams(); - params.page = { offset }; - params.include = params.include.join(','); - - return app.store.find('discussions', params); - } - - /** - * Load the next page of discussion results. - * - * @public - */ - loadMore() { - this.loading = true; - - this.loadResults(this.discussions.length).then(this.parseResults.bind(this)); - } - - /** - * Parse results and append them to the discussion list. - * - * @param {Discussion[]} results - * @return {Discussion[]} - */ - parseResults(results) { - [].push.apply(this.discussions, results); - - this.loading = false; - this.moreResults = !!results.payload.links.next; - - m.lazyRedraw(); - - return results; - } - - /** - * Remove a discussion from the list if it is present. - * - * @param {Discussion} discussion - * @public - */ - removeDiscussion(discussion) { - const index = this.discussions.indexOf(discussion); - - if (index !== -1) { - this.discussions.splice(index, 1); - } - } - - /** - * Add a discussion to the top of the list. - * - * @param {Discussion} discussion - * @public - */ - addDiscussion(discussion) { - this.discussions.unshift(discussion); - } } diff --git a/js/src/forum/components/DiscussionPage.js b/js/src/forum/components/DiscussionPage.js index aa686f7c98..5f8f73cdd9 100644 --- a/js/src/forum/components/DiscussionPage.js +++ b/js/src/forum/components/DiscussionPage.js @@ -7,6 +7,7 @@ import LoadingIndicator from '../../common/components/LoadingIndicator'; import SplitDropdown from '../../common/components/SplitDropdown'; import listItems from '../../common/helpers/listItems'; import DiscussionControls from '../utils/DiscussionControls'; +import DiscussionList from './DiscussionList'; /** * The `DiscussionPage` component displays a whole discussion page, including @@ -35,9 +36,9 @@ export default class DiscussionPage extends Page { // If the discussion list has been loaded, then we'll enable the pane (and // hide it by default). Also, if we've just come from another discussion // page, then we don't want Mithril to redraw the whole page – if it did, - // then the pane would which would be slow and would cause problems with + // then the pane would redraw which would be slow and would cause problems with // event handlers. - if (app.cache.discussionList) { + if (app.discussions.hasDiscussions()) { app.pane.enable(); app.pane.hide(); @@ -49,6 +50,7 @@ export default class DiscussionPage extends Page { app.history.push('discussion'); this.bodyClass = 'App--discussion'; + this.discussionListClass = DiscussionList; } onunload(e) { @@ -90,9 +92,9 @@ export default class DiscussionPage extends Page { return (
- {app.cache.discussionList ? ( + {app.discussions.hasDiscussions() ? (
- {!$('.App-navigation').is(':visible') ? app.cache.discussionList.render() : ''} + {!$('.App-navigation').is(':visible') && }
) : ( '' diff --git a/js/src/forum/components/IndexPage.js b/js/src/forum/components/IndexPage.js index 76a2e83c8d..ee692083fb 100644 --- a/js/src/forum/components/IndexPage.js +++ b/js/src/forum/components/IndexPage.js @@ -34,27 +34,10 @@ export default class IndexPage extends Page { // probably want to refresh the results. We will clear the discussion list // cache so that results are reloaded. if (app.previous instanceof IndexPage) { - app.cache.discussionList = null; + app.discussions.clear(); } - const params = app.search.params(); - - if (app.cache.discussionList) { - // Compare the requested parameters (sort, search query) to the ones that - // are currently present in the cached discussion list. If they differ, we - // will clear the cache and set up a new discussion list component with - // the new parameters. - Object.keys(params).some((key) => { - if (app.cache.discussionList.props.params[key] !== params[key]) { - app.cache.discussionList = null; - return true; - } - }); - } - - if (!app.cache.discussionList) { - app.cache.discussionList = new DiscussionList({ params }); - } + app.discussions.refreshParams(app.search.params()); app.history.push('index', app.translator.trans('core.forum.header.back_to_index_tooltip')); @@ -81,7 +64,7 @@ export default class IndexPage extends Page {
- {app.cache.discussionList.render()} +
@@ -212,7 +195,7 @@ export default class IndexPage extends Page { */ viewItems() { const items = new ItemList(); - const sortMap = app.cache.discussionList.sortMap(); + const sortMap = app.discussions.sortMap(); const sortOptions = {}; for (const i in sortMap) { @@ -257,7 +240,7 @@ export default class IndexPage extends Page { icon: 'fas fa-sync', className: 'Button Button--icon', onclick: () => { - app.cache.discussionList.refresh(); + app.discussions.refresh(); if (app.session.user) { app.store.find('users', app.session.user.id()); m.redraw(); diff --git a/js/src/forum/state/DiscussionListState.js b/js/src/forum/state/DiscussionListState.js new file mode 100644 index 0000000000..38a87bf3d6 --- /dev/null +++ b/js/src/forum/state/DiscussionListState.js @@ -0,0 +1,190 @@ +export default class DiscussionListState { + constructor({ params = {}, forumApp = app } = {}) { + this.params = params; + + this.app = forumApp; + + this.discussions = []; + + this.moreResults = false; + + this.loading = false; + } + + /** + * Get the parameters that should be passed in the API request to get + * discussion results. + * + * @api + */ + requestParams() { + const params = { include: ['user', 'lastPostedUser'], filter: {} }; + + params.sort = this.sortMap()[this.params.sort]; + + if (this.params.q) { + params.filter.q = this.params.q; + + params.include.push('mostRelevantPost', 'mostRelevantPost.user'); + } + + return params; + } + + /** + * Get a map of sort keys (which appear in the URL, and are used for + * translation) to the API sort value that they represent. + */ + sortMap() { + const map = {}; + + if (this.params.q) { + map.relevance = ''; + } + map.latest = '-lastPostedAt'; + map.top = '-commentCount'; + map.newest = '-createdAt'; + map.oldest = 'createdAt'; + + return map; + } + + /** + * Get the search parameters. + */ + getParams() { + return this.params; + } + + /** + * Clear cached discussions. + */ + clear() { + this.discussions = []; + m.redraw(); + } + + /** + * If there are no cached discussions or the new params differ from the + * old ones, update params and refresh the discussion list from the database. + */ + refreshParams(newParams) { + if (!this.hasDiscussions() || Object.keys(newParams).some((key) => this.getParams()[key] !== newParams[key])) { + this.params = newParams; + + this.refresh(); + } + } + + /** + * Clear and reload the discussion list. + */ + refresh({ clear = true } = {}) { + this.loading = true; + + if (clear) { + this.clear(); + } + + return this.loadResults().then( + (results) => { + this.parseResults(results); + }, + () => { + this.loading = false; + m.redraw(); + } + ); + } + + /** + * Load a new page of discussion results. + * + * @param offset The index to start the page at. + */ + loadResults(offset) { + const preloadedDiscussions = this.app.preloadedApiDocument(); + + if (preloadedDiscussions) { + return Promise.resolve(preloadedDiscussions); + } + + const params = this.requestParams(); + params.page = { offset }; + params.include = params.include.join(','); + + return this.app.store.find('discussions', params); + } + + /** + * Load the next page of discussion results. + */ + loadMore() { + this.loading = true; + + this.loadResults(this.discussions.length).then(this.parseResults.bind(this)); + } + + /** + * Parse results and append them to the discussion list. + */ + parseResults(results) { + this.discussions.push(...results); + + this.loading = false; + this.moreResults = !!results.payload.links && !!results.payload.links.next; + + m.redraw(); + + return results; + } + + /** + * Remove a discussion from the list if it is present. + */ + removeDiscussion(discussion) { + const index = this.discussions.indexOf(discussion); + + if (index !== -1) { + this.discussions.splice(index, 1); + } + + m.redraw(); + } + + /** + * Add a discussion to the top of the list. + */ + addDiscussion(discussion) { + this.discussions.unshift(discussion); + m.redraw(); + } + + /** + * Are there discussions stored in the discussion list state? + */ + hasDiscussions() { + return this.discussions.length > 0; + } + + /** + * Are discussions currently being loaded? + */ + isLoading() { + return this.loading; + } + + /** + * In the last request, has the user searched for a discussion? + */ + isSearchResults() { + return !!this.params.q; + } + + /** + * Have the search results come up empty? + */ + empty() { + return !this.hasDiscussions() && !this.isLoading(); + } +} diff --git a/js/src/forum/states/GlobalSearchState.js b/js/src/forum/states/GlobalSearchState.js index a068008e38..ec3f9538ac 100644 --- a/js/src/forum/states/GlobalSearchState.js +++ b/js/src/forum/states/GlobalSearchState.js @@ -60,7 +60,7 @@ export default class GlobalSearchState extends SearchState { changeSort(sort) { const params = this.params(); - if (sort === Object.keys(app.cache.discussionList.sortMap())[0]) { + if (sort === Object.keys(app.discussions.sortMap())[0]) { delete params.sort; } else { params.sort = sort; diff --git a/js/src/forum/utils/DiscussionControls.js b/js/src/forum/utils/DiscussionControls.js index a29a09213b..ebdf7e9d0f 100644 --- a/js/src/forum/utils/DiscussionControls.js +++ b/js/src/forum/utils/DiscussionControls.js @@ -229,13 +229,7 @@ export default { app.history.back(); } - return this.delete().then(() => { - // If there is a discussion list in the cache, remove this discussion. - if (app.cache.discussionList) { - app.cache.discussionList.removeDiscussion(this); - m.redraw(); - } - }); + return this.delete().then(() => app.discussions.removeDiscussion(this)); } }, diff --git a/js/src/forum/utils/PostControls.js b/js/src/forum/utils/PostControls.js index 25adb8a0d1..4c36ed4bc3 100644 --- a/js/src/forum/utils/PostControls.js +++ b/js/src/forum/utils/PostControls.js @@ -181,10 +181,7 @@ export default { // If this was the last post in the discussion, then we will assume that // the whole discussion was deleted too. if (!discussion.postIds().length) { - // If there is a discussion list in the cache, remove this discussion. - if (app.cache.discussionList) { - app.cache.discussionList.removeDiscussion(discussion); - } + app.discussions.removeDiscussion(discussion); if (app.viewingDiscussion(discussion)) { app.history.back();