From 8dd46cf04e749c3de879b91d2cbbe598be08fa7b Mon Sep 17 00:00:00 2001
From: Matthew Irish <meirish@users.noreply.github.com>
Date: Wed, 28 Nov 2018 17:14:42 -0600
Subject: [PATCH] UI - fix policy pagination bug (#5866)

* centralize page size setting, and default to page size of 5 in dev

* remove size arg when using lazyPaginatedQuery so that the app uses the config default size

* move list-pagination component out of the loop

* update store tests
---
 .../cluster/access/identity/aliases/index.js  |  1 -
 .../vault/cluster/access/identity/index.js    |  1 -
 .../vault/cluster/access/leases/list.js       |  1 -
 ui/app/routes/vault/cluster/policies/index.js |  1 -
 .../vault/cluster/secrets/backend/list.js     |  1 -
 ui/app/services/store.js                      |  7 ++++++-
 .../vault/cluster/policies/index.hbs          | 14 ++++++-------
 ui/config/environment.js                      |  3 +++
 ui/tests/unit/services/store-test.js          | 20 +++++++++++--------
 9 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/ui/app/routes/vault/cluster/access/identity/aliases/index.js b/ui/app/routes/vault/cluster/access/identity/aliases/index.js
index 46b134329f..abee732e5b 100644
--- a/ui/app/routes/vault/cluster/access/identity/aliases/index.js
+++ b/ui/app/routes/vault/cluster/access/identity/aliases/index.js
@@ -10,7 +10,6 @@ export default Route.extend(ListRoute, {
         responsePath: 'data.keys',
         page: params.page,
         pageFilter: params.pageFilter,
-        size: 100,
       })
       .catch(err => {
         if (err.httpStatus === 404) {
diff --git a/ui/app/routes/vault/cluster/access/identity/index.js b/ui/app/routes/vault/cluster/access/identity/index.js
index 2d453df151..a9a983ecdd 100644
--- a/ui/app/routes/vault/cluster/access/identity/index.js
+++ b/ui/app/routes/vault/cluster/access/identity/index.js
@@ -10,7 +10,6 @@ export default Route.extend(ListRoute, {
         responsePath: 'data.keys',
         page: params.page,
         pageFilter: params.pageFilter,
-        size: 100,
       })
       .catch(err => {
         if (err.httpStatus === 404) {
diff --git a/ui/app/routes/vault/cluster/access/leases/list.js b/ui/app/routes/vault/cluster/access/leases/list.js
index 7c417c778c..e56f23d497 100644
--- a/ui/app/routes/vault/cluster/access/leases/list.js
+++ b/ui/app/routes/vault/cluster/access/leases/list.js
@@ -24,7 +24,6 @@ export default Route.extend({
             responsePath: 'data.keys',
             page: params.page,
             pageFilter: params.pageFilter,
-            size: 100,
           })
           .then(model => {
             this.set('has404', false);
diff --git a/ui/app/routes/vault/cluster/policies/index.js b/ui/app/routes/vault/cluster/policies/index.js
index 831d7725db..a1aac6911f 100644
--- a/ui/app/routes/vault/cluster/policies/index.js
+++ b/ui/app/routes/vault/cluster/policies/index.js
@@ -27,7 +27,6 @@ export default Route.extend(ClusterRoute, ListRoute, {
         page: params.page,
         pageFilter: params.pageFilter,
         responsePath: 'data.keys',
-        size: 100,
       })
       .catch(err => {
         // acls will never be empty, but sentinel policies can be
diff --git a/ui/app/routes/vault/cluster/secrets/backend/list.js b/ui/app/routes/vault/cluster/secrets/backend/list.js
index 3171d1249e..2e861d4ee5 100644
--- a/ui/app/routes/vault/cluster/secrets/backend/list.js
+++ b/ui/app/routes/vault/cluster/secrets/backend/list.js
@@ -63,7 +63,6 @@ export default Route.extend({
           responsePath: 'data.keys',
           page: params.page,
           pageFilter: params.pageFilter,
-          size: 100,
         })
         .then(model => {
           this.set('has404', false);
diff --git a/ui/app/services/store.js b/ui/app/services/store.js
index c525474223..e9aa211079 100644
--- a/ui/app/services/store.js
+++ b/ui/app/services/store.js
@@ -6,6 +6,9 @@ import { assert } from '@ember/debug';
 import { set, get, computed } from '@ember/object';
 import DS from 'ember-data';
 import clamp from 'vault/utils/clamp';
+import config from 'vault/config/environment';
+
+const { DEFAULT_PAGE_SIZE } = config.APP;
 
 export function normalizeModelName(modelName) {
   return dasherize(modelName);
@@ -69,7 +72,9 @@ export default DS.Store.extend({
     const responsePath = query.responsePath;
     assert('responsePath is required', responsePath);
     assert('page is required', typeof query.page === 'number');
-    assert('size is required', query.size);
+    if (!query.size) {
+      query.size = DEFAULT_PAGE_SIZE;
+    }
 
     if (dataCache) {
       return resolve(this.fetchPage(modelName, query));
diff --git a/ui/app/templates/vault/cluster/policies/index.hbs b/ui/app/templates/vault/cluster/policies/index.hbs
index da7b5bc80b..c8f254c61e 100644
--- a/ui/app/templates/vault/cluster/policies/index.hbs
+++ b/ui/app/templates/vault/cluster/policies/index.hbs
@@ -139,18 +139,18 @@
           </div>
         {{/linked-block}}
       {{/if}}
-      {{#if (gt model.meta.lastPage 1) }}
-        {{list-pagination
-          page=model.meta.currentPage
-          lastPage=model.meta.lastPage
-          link="vault.cluster.policies.index"
-        }}
-      {{/if}}
     {{else}}
       <EmptyState
         @title="No policies matching &quot;{{pageFilter}}&quot;"
       />
     {{/each}}
+    {{#if (gt model.meta.lastPage 1) }}
+      {{list-pagination
+        page=model.meta.currentPage
+        lastPage=model.meta.lastPage
+        link="vault.cluster.policies.index"
+      }}
+    {{/if}}
   {{else}}
     <EmptyState
       @title="No {{uppercase policyType}} policies yet"
diff --git a/ui/config/environment.js b/ui/config/environment.js
index b0d4b92ba7..43558df1ad 100644
--- a/ui/config/environment.js
+++ b/ui/config/environment.js
@@ -24,6 +24,8 @@ module.exports = function(environment) {
       // endpoints that UI uses to determine the cluster state
       // calls to these endpoints will always go to the root namespace
       NAMESPACE_ROOT_URLS: ['sys/health', 'sys/seal-status', 'sys/license/features'],
+      // number of records to show on a single page by default - this is used by the client-side pagination
+      DEFAULT_PAGE_SIZE: 100,
     },
     flashMessageDefaults: {
       timeout: 7000,
@@ -60,6 +62,7 @@ module.exports = function(environment) {
     ENV.flashMessageDefaults.timeout = 50;
   }
   if (environment !== 'production') {
+    ENV.APP.DEFAULT_PAGE_SIZE = 5;
     ENV.contentSecurityPolicyHeader = 'Content-Security-Policy';
     ENV.contentSecurityPolicyMeta = true;
     ENV.contentSecurityPolicy = {
diff --git a/ui/tests/unit/services/store-test.js b/ui/tests/unit/services/store-test.js
index 1a5ba7a8d1..88b7f5fa39 100644
--- a/ui/tests/unit/services/store-test.js
+++ b/ui/tests/unit/services/store-test.js
@@ -4,6 +4,9 @@ import { module, test } from 'qunit';
 import { setupTest } from 'ember-qunit';
 import { normalizeModelName, keyForCache } from 'vault/services/store';
 import clamp from 'vault/utils/clamp';
+import config from 'vault/config/environment';
+
+const { DEFAULT_PAGE_SIZE } = config.APP;
 
 module('Unit | Service | store', function(hooks) {
   setupTest(hooks);
@@ -197,10 +200,12 @@ module('Unit | Service | store', function(hooks) {
     let response = {
       data: ['foo'],
     };
+    let queryArgs;
     const store = this.owner.factoryFor('service:store').create({
       adapterFor() {
         return {
-          query() {
+          query(store, modelName, query) {
+            queryArgs = query;
             return resolve(response);
           },
         };
@@ -217,6 +222,12 @@ module('Unit | Service | store', function(hooks) {
       { response: { data: null }, dataset: ['foo'] },
       'stores returned dataset'
     );
+
+    run(function() {
+      store.lazyPaginatedQuery('secret', { page: 1, responsePath: 'data' });
+    });
+    assert.equal(queryArgs.size, DEFAULT_PAGE_SIZE, 'calls query with DEFAULT_PAGE_SIZE');
+
     assert.throws(
       () => {
         store.lazyPaginatedQuery('transit-key', {});
@@ -231,12 +242,5 @@ module('Unit | Service | store', function(hooks) {
       /page is required/,
       'requires page'
     );
-    assert.throws(
-      () => {
-        store.lazyPaginatedQuery('transit-key', { responsePath: 'foo', page: 1 });
-      },
-      /size is required/,
-      'requires size'
-    );
   });
 });