gr-ajax cleanup (gr-change-list-view)

Bug: Issue 3988 Change-Id: I1f312eba0e561f0c04296f42e8f8da5779077402
2016-05-03 18:11:00 -04:00
parent 4833af6503
commit 61bcdbc3db
6 changed files with 78 additions and 91 deletions
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
@@ -15,8 +15,7 @@ limitations under the License.
 -->

 <link rel="import" href="../../../bower_components/polymer/polymer.html">
-<link rel="import" href="../../../behaviors/rest-client-behavior.html">
-<link rel="import" href="../../shared/gr-ajax/gr-ajax.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../gr-change-list/gr-change-list.html">

 <dom-module id="gr-change-list-view">
@@ -26,15 +25,9 @@ limitations under the License.
        background-color: var(--view-background-color);
        display: block;
      }
-      .loading,
-      .error {
-        padding: 1em var(--default-horizontal-margin);
-      }
      .loading {
        color: #666;
-      }
-      .error {
-        color: #D32F2F;
+        padding: 1em var(--default-horizontal-margin);
      }
      gr-change-list {
        width: 100%;
@@ -56,30 +49,21 @@ limitations under the License.
        }
      }
    </style>
-    <gr-ajax
-        auto
-        url="/changes/"
-        params="[[_computeQueryParams(_query, _offset, changesPerPage)]]"
-        last-response="{{_changes}}"
-        last-error="{{_lastError}}"
-        loading="{{_loading}}"></gr-ajax>
-    <div class="loading" hidden$="[[!_loading]]" hidden>Loading...</div>
-    <div class="error" hidden$="[[_computeErrorHidden(_loading, _lastError)]]" hidden>
-      [[_lastError.request.xhr.responseText]]
-    </div>
-    <div hidden$="[[_computeListHidden(_loading, _lastError)]]" hidden>
+    <div class="loading" hidden$="[[!_loading]]">Loading...</div>
+    <div hidden$="[[_loading]]">
      <gr-change-list
          changes="{{_changes}}"
          selected-index="{{viewState.selectedChangeIndex}}"
          show-star="[[loggedIn]]"></gr-change-list>
      <nav>
-        <a href$="[[_computeNavLink(_query, _offset, -1, changesPerPage)]]"
+        <a href$="[[_computeNavLink(_query, _offset, -1, _changesPerPage)]]"
           hidden$="[[_hidePrevArrow(_offset)]]">&larr; Prev</a>
-        <a href$="[[_computeNavLink(_query, _offset, 1, changesPerPage)]]"
-           hidden$="[[_hideNextArrow(_changes.length, changesPerPage)]]">
+        <a href$="[[_computeNavLink(_query, _offset, 1, _changesPerPage)]]"
+           hidden$="[[_hideNextArrow(_changes.length, _changesPerPage)]]">
          Next &rarr;</a>
      </nav>
    </div>
+    <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
  </template>
  <script src="gr-change-list-view.js"></script>
 </dom-module>
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
@@ -49,7 +49,7 @@
        value: function() { return {}; },
      },

-      changesPerPage: Number,
+      _changesPerPage: Number,

      /**
       * Currently active query.
@@ -66,11 +66,6 @@
       */
      _changes: Array,

-      /**
-       * Contains error of last request (in case of change loading error).
-       */
-      _lastError: Object,
-
      /**
       * For showing a "loading..." string during ajax requests.
       */
@@ -80,10 +75,6 @@
      },
    },

-    behaviors: [
-      Gerrit.RESTClientBehavior,
-    ],
-
    attached: function() {
      this.fire('title-change', {title: this._query});
    },
@@ -91,6 +82,7 @@
    _paramsChanged: function(value) {
      if (value.view != this.tagName.toLowerCase()) { return; }

+      this._loading = true;
      this._query = value.query;
      this._offset = value.offset || 0;
      if (this.viewState.query != this._query ||
@@ -101,22 +93,23 @@
      }

      this.fire('title-change', {title: this._query});
+
+      this._getPreferences().then(function(prefs) {
+        this._changesPerPage = prefs.changes_per_page;
+        return this._getChanges();
+      }.bind(this)).then(function(changes) {
+        this._changes = changes;
+        this._loading = false;
+      }.bind(this));
    },

-    _computeQueryParams: function(query, offset, changesPerPage) {
-      var options = this.listChangesOptionsToHex(
-          this.ListChangesOption.LABELS,
-          this.ListChangesOption.DETAILED_ACCOUNTS
-      );
-      var obj = {
-        n: changesPerPage,
-        O: options,
-        S: offset || 0,
-      };
-      if (query && query.length > 0) {
-        obj.q = query;
-      }
-      return obj;
+    _getChanges: function() {
+      return this.$.restAPI.getChanges(this._changesPerPage, this._query,
+          this._offset);
+    },
+
+    _getPreferences: function() {
+      return this.$.restAPI.getPreferences();
    },

    _computeNavLink: function(query, offset, direction, changesPerPage) {
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -121,29 +121,7 @@
    },

    _getDiffPreferences: function() {
-      return this._getLoggedIn().then(function(loggedIn) {
-        if (!loggedIn) {
-          // These defaults should match the defaults in
-          // gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java
-          // NOTE: There are some settings that don't apply to PolyGerrit
-          // (Render mode being at least one of them).
-          return Promise.resolve({
-            auto_hide_diff_table_header: true,
-            context: 10,
-            cursor_blink_rate: 0,
-            ignore_whitespace: 'IGNORE_NONE',
-            intraline_difference: true,
-            line_length: 100,
-            show_line_endings: true,
-            show_tabs: true,
-            show_whitespace_errors: true,
-            syntax_highlighting: true,
-            tab_size: 8,
-            theme: 'DEFAULT',
-          });
-        }
-        return this.$.restAPI.getDiffPreferences();
-      }.bind(this));
+      return this.$.restAPI.getDiffPreferences();
    },

    _handleReviewedChange: function(e) {
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -89,7 +89,6 @@ limitations under the License.
        <gr-change-list-view
            params="[[params]]"
            view-state="{{_viewState.changeListView}}"
-            changes-per-page="[[_preferences.changes_per_page]]"
            logged-in="[[_computeLoggedIn(_account)]]"></gr-change-list-view>
      </template>
      <template is="dom-if" if="[[_showDashboardView]]" restamp="true">
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -30,7 +30,6 @@
      },
      _serverConfig: Object,
      _version: String,
-      _preferences: Object,
      _showChangeListView: Boolean,
      _showDashboardView: Boolean,
      _showChangeView: Boolean,
@@ -52,10 +51,6 @@
      Gerrit.KeyboardShortcutBehavior,
    ],

-    get loggedIn() {
-      return !!(this._account && Object.keys(this._account).length > 0);
-    },
-
    attached: function() {
      this.$.restAPI.getAccount().then(function(account) {
        this._account = account;
@@ -88,17 +83,9 @@
    },

    _accountChanged: function(account) {
-      if (this.loggedIn) {
-        this.$.restAPI.getPreferences().then(function(preferences) {
-          this._preferences = preferences;
-        }.bind(this));
-        // Diff preferences are cached; warm it before a diff is rendered.
-        this.$.restAPI.getDiffPreferences();
-      } else {
-        this._preferences = {
-          changes_per_page: 25,
-        };
-      }
+      // Preferences are cached when a user is logged in; warm them.
+      this.$.restAPI.getPreferences();
+      this.$.restAPI.getDiffPreferences();
    },

    _viewChanged: function(view) {
@@ -117,7 +104,7 @@

    // Argument used for binding update only.
    _computeLoggedIn: function(account) {
-      return this.loggedIn;
+      return !!(account && Object.keys(account).length > 0);
    },

    _handlePageError: function(e) {
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -153,7 +153,29 @@
    },

    getDiffPreferences: function() {
-      return this._fetchSharedCacheURL('/accounts/self/preferences.diff');
+      return this.getLoggedIn().then(function(loggedIn) {
+        if (loggedIn) {
+          return this._fetchSharedCacheURL('/accounts/self/preferences.diff');
+        }
+        // These defaults should match the defaults in
+        // gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java
+        // NOTE: There are some settings that don't apply to PolyGerrit
+        // (Render mode being at least one of them).
+        return Promise.resolve({
+          auto_hide_diff_table_header: true,
+          context: 10,
+          cursor_blink_rate: 0,
+          ignore_whitespace: 'IGNORE_NONE',
+          intraline_difference: true,
+          line_length: 100,
+          show_line_endings: true,
+          show_tabs: true,
+          show_whitespace_errors: true,
+          syntax_highlighting: true,
+          tab_size: 8,
+          theme: 'DEFAULT',
+        });
+      }.bind(this));
    },

    saveDiffPreferences: function(prefs, opt_errFn, opt_ctx) {
@@ -172,7 +194,15 @@
    },

    getPreferences: function() {
-      return this._fetchSharedCacheURL('/accounts/self/preferences');
+      return this.getLoggedIn().then(function(loggedIn) {
+        if (loggedIn) {
+          return this._fetchSharedCacheURL('/accounts/self/preferences');
+        }
+
+        return Promise.resolve({
+          changes_per_page: 25,
+        });
+      }.bind(this));
    },

    _fetchSharedCacheURL: function(url) {
@@ -197,6 +227,22 @@
      return this._sharedFetchPromises[url];
    },

+    getChanges: function(changesPerPage, opt_query, opt_offset) {
+      var options = this._listChangesOptionsToHex(
+          ListChangesOption.LABELS,
+          ListChangesOption.DETAILED_ACCOUNTS
+      );
+      var params = {
+        n: changesPerPage,
+        O: options,
+        S: opt_offset || 0,
+      };
+      if (opt_query && opt_query.length > 0) {
+        params.q = opt_query;
+      }
+      return this.fetchJSON('/changes/', null, null, params);
+    },
+
    getChangeActionURL: function(changeNum, opt_patchNum, endpoint) {
      return this._changeBaseURL(changeNum, opt_patchNum) + endpoint;
    },