Create Pagination util & make DiscussionListState and DiscussionList use it

I'm 100% sure that this code can be improved by a ton. Just pushing this so we can potentially work off of it.
Some stuff taken from PR #1829.
Does *not* have changing URL query parameter - this code is already a disaster.

.gitignore

@@ -7,3 +7,4 @@ Thumbs.db
 /tests/integration/tmp
 .vagrant
 .idea/*
+.vscode

js/src/common/utils/Pagination.ts

@@ -0,0 +1,80 @@
+export default class Pagination<T> {
+  private readonly loadFunction: (page: number) => Promise<any>;
+
+  public loading = {
+    prev: false,
+    next: false,
+  };
+
+  public page: number;
+
+  public data: { [page: number]: T } = {};
+
+  public pages: {
+    first: number;
+    last: number;
+  };
+
+  constructor(load: (page: number) => Promise<any>, page: number = 1) {
+    this.loadFunction = load;
+    this.page = page;
+
+    this.pages = {
+      first: page,
+      last: page,
+    };
+  }
+
+  clear() {
+    this.data = {};
+  }
+
+  refresh(page: number) {
+    this.clear();
+
+    this.page = page;
+    this.pages.last = page - 1;
+    this.pages.first = page;
+
+    return this.loadNext();
+  }
+
+  loadNext() {
+    this.loading.next = true;
+    const page = this.pages.last + 1;
+
+    return this.load(
+      page,
+      () => (this.loading.next = false),
+      () => (this.pages.last = this.page = page)
+    );
+  }
+
+  loadPrev() {
+    this.loading.prev = true;
+    const page = this.pages.first - 1;
+
+    return this.load(
+      page,
+      () => (this.loading.prev = false),
+      () => (this.pages.first = this.page = page)
+    );
+  }
+
+  private load(page, done, success) {
+    return this.loadFunction(page)
+      .then((out) => {
+        done();
+        success();
+
+        this.data[this.page] = out;
+
+        return out;
+      })
+      .catch((err) => {
+        done();
+
+        return Promise.reject(err);
+      });
+  }
+}

js/src/forum/components/Composer.js

@@ -265,7 +265,7 @@ export default class Composer extends Component {
     this.animateHeightChange().then(() => this.focus());
 
     if (app.screen() === 'phone') {
-      this.$().css('top', $(window).scrollTop());
+      this.$().css('top', 0);
       this.showBackdrop();
     }
   }

js/src/forum/components/DiscussionList.js

@@ -21,13 +21,7 @@ export default class DiscussionList extends Component {
     if (state.isLoading()) {
       loading = LoadingIndicator.component();
     } else if (state.moreResults) {
-      loading = Button.component(
-        {
-          className: 'Button',
-          onclick: state.loadMore.bind(state),
-        },
-        app.translator.trans('core.forum.discussion_list.load_more_button')
-      );
+      loading = this.getLoadButton('more', state.loadMore.bind(state));
     }
 
     if (state.empty()) {
@@ -35,8 +29,18 @@ export default class DiscussionList extends Component {
       return <div className="DiscussionList">{Placeholder.component({ text })}</div>;
     }
 
+    console.log(state);
+
     return (
       <div className={'DiscussionList' + (state.isSearchResults() ? ' DiscussionList--searchResults' : '')}>
+        {state.isLoadingPrev() ? (
+          <LoadingIndicator />
+        ) : state.pagination.pages.first !== 1 ? (
+          <div className="DiscussionList-loadMore">{this.getLoadButton('prev', state.loadPrev.bind(state))}</div>
+        ) : (
+          ''
+        )}
+
         <ul className="DiscussionList-discussions">
           {state.discussions.map((discussion) => {
             return (
@@ -46,8 +50,17 @@ export default class DiscussionList extends Component {
             );
           })}
         </ul>
+
         <div className="DiscussionList-loadMore">{loading}</div>
       </div>
     );
   }
+
+  getLoadButton(key, onclick) {
+    return (
+      <Button className="Button" onclick={onclick}>
+        {app.translator.trans(`core.forum.discussion_list.load_${key}_button`)}
+      </Button>
+    );
+  }
 }

js/src/forum/states/DiscussionListState.js

@@ -1,4 +1,8 @@
+import Pagination from '../../common/utils/Pagination';
+
 export default class DiscussionListState {
+  static DISCUSSIONS_PER_PAGE = 20;
+
   constructor(params = {}, app = window.app) {
     this.params = params;
 
@@ -8,7 +12,7 @@ export default class DiscussionListState {
 
     this.moreResults = false;
 
-    this.loading = false;
+    this.pagination = new Pagination(this.load.bind(this));
   }
 
   /**
@@ -82,33 +86,16 @@ export default class DiscussionListState {
    * This can be used to refresh discussions without loading animations.
    */
   refresh({ deferClear = false } = {}) {
-    this.loading = true;
+    this.pagination.loading.next = true;
 
     if (!deferClear) {
       this.clear();
     }
 
-    return this.loadResults().then(
-      (results) => {
-        // This ensures that any changes made while waiting on this request
-        // are ignored. Otherwise, we could get duplicate discussions.
-        // We don't use `this.clear()` to avoid an unnecessary redraw.
-        this.discussions = [];
-        this.parseResults(results);
-      },
-      () => {
-        this.loading = false;
-        m.redraw();
-      }
-    );
+    return this.pagination.refresh(Number(m.route.param('page')) || 1).then(this.parse.bind(this));
   }
 
-  /**
-   * Load a new page of discussion results.
-   *
-   * @param offset The index to start the page at.
-   */
-  loadResults(offset) {
+  load(page) {
     const preloadedDiscussions = this.app.preloadedApiDocument();
 
     if (preloadedDiscussions) {
@@ -116,44 +103,54 @@ export default class DiscussionListState {
     }
 
     const params = this.requestParams();
-    params.page = { offset };
+    params.page = { offset: DiscussionListState.DISCUSSIONS_PER_PAGE * (page - 1) };
     params.include = params.include.join(',');
 
     return this.app.store.find('discussions', params);
   }
 
-  /**
-   * Load the next page of discussion results.
-   */
-  loadMore() {
-    this.loading = true;
+  loadPrev() {
+    return this.pagination.loadPrev().then(this.parse.bind(this));
+  }
 
-    this.loadResults(this.discussions.length).then(this.parseResults.bind(this));
+  loadMore() {
+    return this.pagination.loadNext().then(this.parse.bind(this));
   }
 
   /**
    * Parse results and append them to the discussion list.
    */
-  parseResults(results) {
-    this.discussions.push(...results);
+  parse() {
+    const discussions = [];
+    const { first, last } = this.pagination.pages;
 
-    this.loading = false;
-    this.moreResults = !!results.payload.links && !!results.payload.links.next;
+    for (let page = first; page <= last; page++) {
+      const results = this.pagination.data[page];
+
+      if (Array.isArray(results)) discussions.push(...results);
+    }
+
+    this.discussions = discussions;
+
+    const results = this.pagination.data[last];
+    this.moreResults = !!results.payload.links.next;
 
     m.redraw();
 
-    return results;
+    return discussions;
   }
 
   /**
    * Remove a discussion from the list if it is present.
    */
   removeDiscussion(discussion) {
-    const index = this.discussions.indexOf(discussion);
+    Object.keys(this.pagination.data).forEach((key) => {
+      const index = this.pagination.data[key].indexOf(discussion);
 
-    if (index !== -1) {
-      this.discussions.splice(index, 1);
-    }
+      this.pagination.data[key].splice(index, 1);
+    });
+
+    this.parse();
 
     m.redraw();
   }
@@ -177,7 +174,11 @@ export default class DiscussionListState {
    * Are discussions currently being loaded?
    */
   isLoading() {
-    return this.loading;
+    return this.pagination.loading.next;
+  }
+
+  isLoadingPrev() {
+    return this.pagination.loading.prev;
   }
 
   /**

less/common/mixins/header-background.less

@@ -6,7 +6,6 @@
   right: 0;
   z-index: @zindex-header;
   border-bottom: 1px solid @control-bg;
-  .translate3d(0, 0, 0);
   .transition(~"box-shadow 0.2s, -webkit-transform 0.2s");
 
   @media @phone {

less/forum/Composer.less

@@ -114,7 +114,7 @@
     background: @body-bg;
 
     &:not(.minimized) {
-      position: absolute;
+      position: fixed;
       top: 0;
       height: 350px !important;
       padding-top: @header-height-phone;
@@ -219,7 +219,6 @@
   .Composer {
     border-radius: @border-radius @border-radius 0 0;
     background: fade(@body-bg, 95%);
-    transform: translateZ(0); // Fix for Chrome bug where a transparent white background is actually gray
     position: relative;
     height: 300px;
     .transition(~"background 0.2s, box-shadow 0.2s");

src/Extend/Settings.php

@@ -26,11 +26,12 @@ class Settings implements ExtenderInterface
      * @param string $attributeName: The attribute name to be used in the ForumSerializer attributes array.
      * @param string $key: The key of the setting.
      * @param string|callable|null $callback: Optional callback to modify the value before serialization.
+     * @param mixed $default: Optional default serialized value. Will be run through the optional callback.
      * @return $this
      */
-    public function serializeToForum(string $attributeName, string $key, $callback = null)
+    public function serializeToForum(string $attributeName, string $key, $callback = null, $default = null)
     {
-        $this->settings[$key] = compact('attributeName', 'callback');
+        $this->settings[$key] = compact('attributeName', 'callback', 'default');
 
         return $this;
     }
@@ -45,7 +46,7 @@ class Settings implements ExtenderInterface
                     $attributes = [];
 
                     foreach ($this->settings as $key => $setting) {
-                        $value = $settings->get($key, null);
+                        $value = $settings->get($key, $setting['default']);
 
                         if (isset($setting['callback'])) {
                             $callback = ContainerUtil::wrapCallback($setting['callback'], $container);

src/User/UserMetadataUpdater.php

@@ -65,7 +65,7 @@ class UserMetadataUpdater
     /**
      * @param \Flarum\User\User $user
      */
-    private function updateCommentsCount(User $user)
+    private function updateCommentsCount(?User $user)
     {
         if ($user && $user->exists) {
             $user->refreshCommentCount()->save();

tests/integration/extenders/SettingsTest.php

@@ -122,6 +122,56 @@ class SettingsTest extends TestCase
         $this->assertArrayHasKey('customPrefix.customSetting2', $payload['data']['attributes']);
         $this->assertEquals('customValueModifiedByInvokable', $payload['data']['attributes']['customPrefix.customSetting2']);
     }
+
+    /**
+     * @test
+     */
+    public function custom_setting_falls_back_to_default()
+    {
+        $this->extend(
+            (new Extend\Settings())
+                ->serializeToForum('customPrefix.noCustomSetting', 'custom-prefix.no_custom_setting', null, 'customDefault')
+        );
+
+        $this->prepDb();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody(), true);
+
+        $this->assertArrayHasKey('customPrefix.noCustomSetting', $payload['data']['attributes']);
+        $this->assertEquals('customDefault', $payload['data']['attributes']['customPrefix.noCustomSetting']);
+    }
+
+    /**
+     * @test
+     */
+    public function custom_setting_default_passed_to_callback()
+    {
+        $this->extend(
+            (new Extend\Settings())
+                ->serializeToForum('customPrefix.noCustomSetting', 'custom-prefix.no_custom_setting', function ($value) {
+                    return $value.'Modified2';
+                }, 'customDefault')
+        );
+
+        $this->prepDb();
+
+        $response = $this->send(
+            $this->request('GET', '/api', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $payload = json_decode($response->getBody(), true);
+
+        $this->assertArrayHasKey('customPrefix.noCustomSetting', $payload['data']['attributes']);
+        $this->assertEquals('customDefaultModified2', $payload['data']['attributes']['customPrefix.noCustomSetting']);
+    }
 }
 
 class CustomInvokableClass