Fix storage table page-size persistence

Author: chen21019

app/services/prefs.js

@@ -1,6 +1,28 @@
 import Ember from 'ember';
 import C from 'ui/utils/constants';
 
+function pageSizePreference(preference, options, fallback) {
+  function normalize(value) {
+    let parsed = parseInt(value, 10);
+
+    return options.indexOf(parsed) === -1 ? fallback : parsed;
+  }
+
+  return Ember.computed(preference, {
+    get() {
+      return normalize(this.get(preference));
+    },
+
+    set(key, value) {
+      let normalized = normalize(value);
+
+      this.set(preference, normalized);
+
+      return normalized;
+    },
+  });
+}
+
 export default Ember.Service.extend({
   userStore: Ember.inject.service('user-store'),
 
@@ -79,32 +101,21 @@ export default Ember.Service.extend({
     this.endPropertyChanges();
   },
 
-  tablePerPage: Ember.computed(`${C.PREFS.TABLE_COUNT}`, function() {
-    let out = this.get(`${C.PREFS.TABLE_COUNT}`);
-    if ( C.TABLES.PAGE_SIZES.indexOf(out) === -1 ) {
-      out = C.TABLES.DEFAULT_COUNT;
-    }
-
-    return out;
-  }),
-
-  statsTablePerPage: Ember.computed(`${C.PREFS.STATS_TABLE_COUNT}`, function() {
-    let out = this.get(`${C.PREFS.STATS_TABLE_COUNT}`);
-
-    if ( C.TABLES.STATS_PAGE_SIZES.indexOf(out) === -1 ) {
-      out = C.TABLES.DEFAULT_STATS_COUNT;
-    }
-
-    return out;
-  }),
-
-  storageTablePerPage: Ember.computed(`${C.PREFS.STORAGE_TABLE_COUNT}`, function() {
-    let out = this.get(`${C.PREFS.STORAGE_TABLE_COUNT}`);
-
-    if ( C.TABLES.STORAGE_PAGE_SIZES.indexOf(out) === -1 ) {
-      out = C.TABLES.DEFAULT_STORAGE_COUNT;
-    }
-
-    return out;
-  }),
+  tablePerPage: pageSizePreference(
+    C.PREFS.TABLE_COUNT,
+    C.TABLES.PAGE_SIZES,
+    C.TABLES.DEFAULT_COUNT
+  ),
+
+  statsTablePerPage: pageSizePreference(
+    C.PREFS.STATS_TABLE_COUNT,
+    C.TABLES.STATS_PAGE_SIZES,
+    C.TABLES.DEFAULT_STATS_COUNT
+  ),
+
+  storageTablePerPage: pageSizePreference(
+    C.PREFS.STORAGE_TABLE_COUNT,
+    C.TABLES.STORAGE_PAGE_SIZES,
+    C.TABLES.DEFAULT_STORAGE_COUNT
+  ),
 });

docs/storage-bulk-remove.md

@@ -26,14 +26,17 @@ sent. Removal uses the public resource action with at most four concurrent
 requests and never mutates a database directly. Progress and per-item failures
 remain visible until the operator closes the result.
 
-The page-size selector treats its invocation value as read-only. The table
-keeps a separate internal effective page size, so choosing All stores the
-semantic preference value `0` without attempting to write through the
-caller's computed preference. This prevents an Ember property-setter failure
-and preserves the same 10, 25, 50, and All behavior for every shared table.
+The page-size selector keeps a separate internal effective page size and the
+preference service exposes normalized read/write computed properties. Choosing
+All stores the semantic preference value `0`; a legacy two-way component
+binding can safely write that value back without reaching an undefined Ember
+setter. Unsupported values normalize to the documented default. This preserves
+the same 10, 25, 50, and All behavior for every shared table.
 
 After each successful remove response, the corresponding row is removed from
 the current model, filtered result, and selected-item list immediately. A
 revision-backed recomputation keeps the visible table and selected count in
-sync while the remaining requests continue. Failed rows stay visible and
-selected so the operator can inspect or retry them.
+sync while the remaining requests continue. If removal empties the current
+page, the table moves to the last valid page before it renders the remaining
+rows. Failed rows stay visible and selected so the operator can inspect or
+retry them.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@pasturestack/web-console",
-  "version": "1.6.56-pasturestack.54",
+  "version": "1.6.56-pasturestack.55",
   "private": true,
   "description": "PastureStack browser console for the compatible control platform.",
   "repository": {

package.json

@@ -41,8 +41,8 @@ with open('package.json', encoding='utf-8') as f:
     print(json.load(f).get('version', ''))
 PY
 )
-if [[ "$version" != "1.6.56-pasturestack.54" ]]; then
-  echo "UNEXPECTED_UI_ARTIFACT_VERSION version=$version expected=1.6.56-pasturestack.54"
+if [[ "$version" != "1.6.56-pasturestack.55" ]]; then
+  echo "UNEXPECTED_UI_ARTIFACT_VERSION version=$version expected=1.6.56-pasturestack.55"
   failures=$((failures + 1))
 fi
 

scripts/check-modernization-blockers

@@ -141,4 +141,4 @@ if [[ -n ${PASTURESTACK_PRIVATE_MARKER:-} ]] && grep -RInF -- "$PASTURESTACK_PRI
 fi
 
 printf 'UI_CONSOLE_WORKSPACE_OK version=%s persistence=%s cross_tab=%s\n' \
-  1.6.56-pasturestack.54 browser-session broker-broadcast
+  1.6.56-pasturestack.55 browser-session broker-broadcast

scripts/check-ui-console-workspace

@@ -7,11 +7,15 @@ const componentPath = 'vendor/lacsso/addon/components/sortable-table.js';
 const testPath = 'tests/unit/components/sortable-table-test.js';
 const hostRoutePath = 'app/host/containers/route.js';
 const hostRouteTestPath = 'tests/unit/host/containers/route-test.js';
+const prefsPath = 'app/services/prefs.js';
+const prefsTestPath = 'tests/unit/services/prefs-test.js';
 const ciPath = 'scripts/ci';
 const component = fs.readFileSync(componentPath, 'utf8');
 const test = fs.readFileSync(testPath, 'utf8');
 const hostRoute = fs.readFileSync(hostRoutePath, 'utf8');
 const hostRouteTest = fs.readFileSync(hostRouteTestPath, 'utf8');
+const prefs = fs.readFileSync(prefsPath, 'utf8');
+const prefsTest = fs.readFileSync(prefsTestPath, 'utf8');
 const ci = fs.readFileSync(ciPath, 'utf8');
 const failures = [];
 
@@ -31,6 +35,8 @@ for (const marker of [
   "paged.set('page', page);",
   "paged.set('perPage', perPage);",
   'this._syncPagedContent(out);',
+  'this.clampPageToContentLength(out.length);',
+  'clampPageToContentLength(length) {',
   'Ember.run.throttle(this, this._updateFiltered, 100, false)',
   'Ember.run.debounce(this, this._updateFiltered, 100, false)',
 ]) {
@@ -39,6 +45,29 @@ for (const marker of [
   }
 }
 
+for (const marker of [
+  'function pageSizePreference(preference, options, fallback)',
+  'return Ember.computed(preference, {',
+  'set(key, value) {',
+  'this.set(preference, normalized);',
+  'storageTablePerPage: pageSizePreference(',
+]) {
+  if (!prefs.includes(marker)) {
+    failures.push('PAGE_SIZE_PREFERENCE_CONTRACT_MISSING=' + marker);
+  }
+}
+
+for (const marker of [
+  'page-size computed properties accept legacy two-way binding writes',
+  "service.set('storageTablePerPage', 0)",
+  "assert.equal(service.get('storageTablePerPage'), 0",
+  "records.findBy('name', 'storageTableCount').get('value'), '0'",
+]) {
+  if (!prefsTest.includes(marker)) {
+    failures.push('PAGE_SIZE_PREFERENCE_TEST_MISSING=' + marker);
+  }
+}
+
 for (const marker of [
   "host.followLink('instances')",
   'Ember.Object.create({',
@@ -93,6 +122,7 @@ for (const marker of [
   "component.set('page', 2)",
   'component.setProperties({page: 1, perPage: 2})',
   'keeps the invocation page size read-only while selecting all rows',
+  'clamps the current page immediately when live rows are removed',
   "component.send('changePerPage', '0')",
   "assert.equal(component.get('perPage'), 1, 'does not write through the caller-owned input')",
   "assert.equal(prefs.get('storageTableCount'), 0, 'persists the semantic All preference')",

scripts/check-ui-sortable-table-refresh.js

@@ -204,3 +204,43 @@ test('it keeps the invocation page size read-only while selecting all rows', fun
 
   destroyOwned(component);
 });
+
+test('it clamps the current page immediately when live rows are removed', function(assert) {
+  assert.expect(4);
+
+  let body = Ember.A([
+    Ember.Object.create({id: '1', name: 'Volume 1'}),
+    Ember.Object.create({id: '2', name: 'Volume 2'}),
+    Ember.Object.create({id: '3', name: 'Volume 3'}),
+  ]);
+  let component;
+
+  Ember.run(() => {
+    component = createOwned(SortableTableComponent, {
+      renderer: inertRenderer(),
+      prefs: Ember.Object.create(),
+      body,
+      headers: Ember.A([
+        Ember.Object.create({name: 'name', searchField: 'name'}),
+      ]),
+      sortBy: 'name',
+      perPage: 1,
+      paging: true,
+    }, 'component');
+    component.didReceiveAttrs();
+    component.set('page', 3);
+  });
+
+  assert.equal(component.get('page'), 3, 'starts on the last page');
+  assert.deepEqual(component.get('pagedContent').mapBy('name'), ['Volume 3'], 'renders the last row');
+
+  Ember.run(() => {
+    body.removeObjects(body.slice(1));
+    component.didReceiveAttrs();
+  });
+
+  assert.equal(component.get('page'), 1, 'moves to the last valid page synchronously');
+  assert.deepEqual(component.get('pagedContent').mapBy('name'), ['Volume 1'], 'renders the remaining row immediately');
+
+  destroyOwned(component);
+});

tests/unit/components/sortable-table-test.js

@@ -0,0 +1,53 @@
+import Ember from 'ember';
+import { module, test } from 'qunit';
+
+import PrefsService from 'ui/services/prefs';
+
+module('Unit | Service | prefs');
+
+function preferenceRecord(name, value) {
+  return Ember.Object.create({
+    id: `1up-${name}`,
+    name,
+    value: JSON.stringify(value),
+    save() {
+      return Ember.RSVP.resolve(this);
+    },
+  });
+}
+
+test('page-size computed properties accept legacy two-way binding writes', function(assert) {
+  assert.expect(7);
+
+  let records = Ember.A([
+    preferenceRecord('tableCount', 50),
+    preferenceRecord('statsTableCount', 10),
+    preferenceRecord('storageTableCount', 25),
+  ]);
+  let service = PrefsService.create({
+    userStore: Ember.Object.create({
+      all() {
+        return records;
+      },
+    }),
+  });
+
+  assert.equal(service.get('storageTablePerPage'), 25, 'reads the stored storage page size');
+
+  Ember.run(() => service.set('storageTablePerPage', 0));
+
+  assert.equal(service.get('storageTablePerPage'), 0, 'accepts the semantic All value');
+  assert.equal(records.findBy('name', 'storageTableCount').get('value'), '0', 'persists All through the base preference');
+
+  Ember.run(() => service.set('storageTablePerPage', 999));
+
+  assert.equal(service.get('storageTablePerPage'), 25, 'normalizes unsupported storage sizes');
+  assert.equal(records.findBy('name', 'storageTableCount').get('value'), '25', 'persists the normalized storage size');
+
+  Ember.run(() => service.set('tablePerPage', 0));
+
+  assert.equal(service.get('tablePerPage'), 50, 'does not allow All on ordinary tables');
+  assert.equal(records.findBy('name', 'tableCount').get('value'), '50', 'persists the ordinary-table fallback');
+
+  Ember.run(() => service.destroy());
+});

tests/unit/services/prefs-test.js

@@ -391,10 +391,20 @@ export default Ember.Component.extend(Sortable, StickyHeader, {
       }
     }
 
+    this.clampPageToContentLength(out.length);
     this.set('filtered', out);
     this._syncPagedContent(out);
   },
 
+  clampPageToContentLength(length) {
+    let perPage = this.get('effectivePerPage') || 1;
+    let lastPage = Math.max(1, Math.ceil(length / perPage));
+
+    if ( this.get('page') > lastPage ) {
+      this.set('page', lastPage);
+    }
+  },
+
   pagedContentChanged: Ember.observer('pagedContent.[]', function() {
     // Remove selected items not in the current content
     let content = this.get('pagedContent');
@@ -519,15 +529,8 @@ export default Ember.Component.extend(Sortable, StickyHeader, {
   }),
 
   pageCountChanged: Ember.observer('indexFrom', 'filtered.length', function() {
-    // Go to the last page if we end up past the last page
-    let from = this.get('indexFrom');
-    let last = this.get('filtered.length');
-    var perPage = this.get('effectivePerPage');
-
-    if ( this.get('page') > 1 && from > last) {
-      let page = Math.ceil(last/perPage);
-      this.set('page', page);
-    }
+    // Keep the current page valid when live rows are removed.
+    this.clampPageToContentLength(this.get('filtered.length') || 0);
   }),
 
   sortKeyChanged: Ember.observer('sortBy', function() {