Include settings checkbox to drive legacycid_in_change_table preference

The legacycid_in_change_table preference predates the change_table
column preference and is thus persisted in separate field. With this
change, a checkbox to drive this preference is added to the change table
editor component as though it were a change_table preference.

Feature: Issue 5333
Change-Id: Ia055ef23d7eec341455680b67bb6f3b0bdcbf531

polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html

@@ -49,6 +49,17 @@ limitations under the License.
           </tr>
         </thead>
         <tbody>
+          <tr>
+            <td>Number</td>
+            <td
+                class="checkboxContainer"
+                on-tap="_handleTargetTap">
+              <input
+                  type="checkbox"
+                  name="number"
+                  checked$="[[showNumber]]">
+            </td>
+          </tr>
           <template is="dom-repeat" items="[[columnNames]]">
             <tr>
               <td>[[item]]</td>

polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js

@@ -22,6 +22,10 @@
         type: Array,
         notify: true,
       },
+      showNumber: {
+        type: Boolean,
+        notify: true,
+      },
     },
 
     behaviors: [
@@ -53,6 +57,12 @@
         // The target is the checkbox itself.
         checkbox = Polymer.dom(e).rootTarget;
       }
+
+      if (checkbox.name === 'number') {
+        this.showNumber = checkbox.checked;
+        return;
+      }
+
       this.set('displayedColumns',
           this._updateDisplayedColumns(
               this.displayedColumns, checkbox.name, checkbox.checked));

polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html

@@ -62,15 +62,17 @@ limitations under the License.
       const rows = element.$$('tbody').querySelectorAll('tr');
       let tds;
 
-      assert.equal(rows.length, element.columnNames.length);
+      // The `+ 1` is for the number column, which isn't included in the change
+      // table behavior's list.
+      assert.equal(rows.length, element.columnNames.length + 1);
       for (let i = 0; i < columns.length; i++) {
-        tds = rows[i].querySelectorAll('td');
+        tds = rows[i + 1].querySelectorAll('td');
         assert.equal(tds[0].textContent, columns[i]);
       }
     });
 
     test('hide item', () => {
-      const checkbox = element.$$('table input');
+      const checkbox = element.$$('table tr:nth-child(2) input');
       const isChecked = checkbox.checked;
       const displayedLength = element.displayedColumns.length;
       assert.isTrue(isChecked);
@@ -78,8 +80,7 @@ limitations under the License.
       MockInteractions.tap(checkbox);
       flushAsynchronousOperations();
 
-      assert.equal(element.displayedColumns.length,
+      assert.equal(element.displayedColumns.length, displayedLength - 1);
-          displayedLength - 1);
     });
 
     test('show item', () => {
@@ -91,7 +92,7 @@ limitations under the License.
         'Updated',
       ]);
       flushAsynchronousOperations();
-      const checkbox = element.$$('table input');
+      const checkbox = element.$$('table tr:nth-child(2) input');
       const isChecked = checkbox.checked;
       const displayedLength = element.displayedColumns.length;
       assert.isFalse(isChecked);
@@ -105,9 +106,9 @@ limitations under the License.
     });
 
     test('_handleTargetTap', () => {
-      const checkbox = element.$$('table input');
+      const checkbox = element.$$('table tr:nth-child(2) input');
       let originalDisplayedColumns = element.displayedColumns;
-      const td = element.$$('table .checkboxContainer');
+      const td = element.$$('table tr:nth-child(2) .checkboxContainer');
       const displayedColumnStub =
           sandbox.stub(element, '_updateDisplayedColumns');
 
@@ -125,6 +126,20 @@ limitations under the License.
           checkbox.checked));
     });
 
+    test('_handleTargetTap on number', () => {
+      element.showNumber = false;
+      const checkbox = element.$$('table tr:nth-child(1) input');
+      const displayedColumnStub =
+          sandbox.stub(element, '_updateDisplayedColumns');
+
+      MockInteractions.tap(checkbox);
+      assert.isFalse(displayedColumnStub.called);
+      assert.isTrue(element.showNumber);
+
+      MockInteractions.tap(checkbox);
+      assert.isFalse(element.showNumber);
+    });
+
     test('_updateDisplayedColumns', () => {
       let name = 'Subject';
       let checked = false;

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html

@@ -321,6 +321,7 @@ limitations under the License.
         </h2>
         <fieldset id="changeTableColumns">
           <gr-change-table-editor
+              show-number="{{_showNumber}}"
               displayed-columns="{{_localChangeTableColumns}}">
           </gr-change-table-editor>
           <gr-button

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js

@@ -121,6 +121,8 @@
        * For testing purposes.
        */
       _loadingPromise: Object,
+
+      _showNumber: Boolean,
     },
 
     behaviors: [
@@ -132,7 +134,7 @@
       '_handlePrefsChanged(_localPrefs.*)',
       '_handleDiffPrefsChanged(_diffPrefs.*)',
       '_handleMenuChanged(_localMenu.splices)',
-      '_handleChangeTableChanged(_localChangeTableColumns)',
+      '_handleChangeTableChanged(_localChangeTableColumns, _showNumber)',
     ],
 
     attached() {
@@ -147,6 +149,7 @@
 
       promises.push(this.$.restAPI.getPreferences().then(prefs => {
         this.prefs = prefs;
+        this._showNumber = !!prefs.legacycid_in_change_table;
         this._copyPrefs('_localPrefs', 'prefs');
         this._cloneMenu();
         this._cloneChangeTableColumns();
@@ -303,6 +306,7 @@
 
     _handleSaveChangeTable() {
       this.set('prefs.change_table', this._localChangeTableColumns);
+      this.set('prefs.legacycid_in_change_table', this._showNumber);
       this._cloneChangeTableColumns();
       return this.$.restAPI.savePreferences(this.prefs).then(() => {
         this._changeTableChanged = false;

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html

@@ -386,6 +386,25 @@ limitations under the License.
       assert.isTrue(element.$.emailEditor.loadData.calledOnce);
     });
 
+    test('_handleSaveChangeTable', () => {
+      let newColumns = ['Owner', 'Project', 'Branch'];
+      element._localChangeTableColumns = newColumns.slice(0);
+      element._showNumber = false;
+      const cloneStub = sandbox.stub(element, '_cloneChangeTableColumns');
+      element._handleSaveChangeTable();
+      assert.isTrue(cloneStub.calledOnce);
+      assert.deepEqual(element.prefs.change_table, newColumns);
+      assert.isNotOk(element.prefs.legacycid_in_change_table);
+
+      newColumns = ['Size'];
+      element._localChangeTableColumns = newColumns;
+      element._showNumber = true;
+      element._handleSaveChangeTable();
+      assert.isTrue(cloneStub.calledTwice);
+      assert.deepEqual(element.prefs.change_table, newColumns);
+      assert.isTrue(element.prefs.legacycid_in_change_table);
+    });
+
     suite('_getFilterDocsLink', () => {
       test('with http: docs base URL', () => {
         const base = 'http://example.com/';