Move Discussion List State into its own class (#2150)

Extract discussion list state

framework/core/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;
   }
 
   /**

framework/core/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));
   }

framework/core/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() {
-    /**
+    this.state = this.props.state;
-     * 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();
   }
 
   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 <div className="DiscussionList">{Placeholder.component({ text })}</div>;
     }
 
     return (
-      <div className={'DiscussionList' + (this.props.params.q ? ' DiscussionList--searchResults' : '')}>
+      <div className={'DiscussionList' + (state.isSearchResults() ? ' DiscussionList--searchResults' : '')}>
         <ul className="DiscussionList-discussions">
-          {this.discussions.map((discussion) => {
+          {state.discussions.map((discussion) => {
             return (
               <li key={discussion.id()} data-id={discussion.id()}>
                 {DiscussionListItem.component({ discussion, params })}
@@ -72,140 +54,4 @@ export default class DiscussionList extends Component {
       </div>
     );
   }
-
-  /**
-   * 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);
-  }
 }

framework/core/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 (
       <div className="DiscussionPage">
-        {app.cache.discussionList ? (
+        {app.discussions.hasDiscussions() ? (
           <div className="DiscussionPage-list" config={this.configPane.bind(this)}>
-            {!$('.App-navigation').is(':visible') ? app.cache.discussionList.render() : ''}
+            {!$('.App-navigation').is(':visible') && <DiscussionList state={app.discussions} />}
           </div>
         ) : (
           ''

framework/core/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();
+    app.discussions.refreshParams(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.history.push('index', app.translator.trans('core.forum.header.back_to_index_tooltip'));
 
@@ -81,7 +64,7 @@ export default class IndexPage extends Page {
                 <ul className="IndexPage-toolbar-view">{listItems(this.viewItems().toArray())}</ul>
                 <ul className="IndexPage-toolbar-action">{listItems(this.actionItems().toArray())}</ul>
               </div>
-              {app.cache.discussionList.render()}
+              <DiscussionList state={app.discussions} />
             </div>
           </div>
         </div>
@@ -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();

framework/core/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();
+  }
+}

framework/core/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;

framework/core/js/src/forum/utils/DiscussionControls.js

@@ -229,13 +229,7 @@ export default {
         app.history.back();
       }
 
-      return this.delete().then(() => {
+      return this.delete().then(() => app.discussions.removeDiscussion(this));
-        // If there is a discussion list in the cache, remove this discussion.
-        if (app.cache.discussionList) {
-          app.cache.discussionList.removeDiscussion(this);
-          m.redraw();
-        }
-      });
     }
   },
 

framework/core/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.
+          app.discussions.removeDiscussion(discussion);
-          if (app.cache.discussionList) {
-            app.cache.discussionList.removeDiscussion(discussion);
-          }
 
           if (app.viewingDiscussion(discussion)) {
             app.history.back();