Clean up change-list columns

The COMMENTS column and the STATUS column are obsolete by introducing
the submit requirements column.

The COMMENTS column could just be removed, no complexity involved.

Replacing the STATUS column by the new STATUS2 column was a bit
trickier, because there was some magic replacing in place. We have
moved the canonical rewriting of configured columns into user-model,
so this is easier to consume by components.

Columns are not enabled anymore based on the server config or flags,
so this logic could be simplified a bit.

Refactored `computeIsColumnHidden()` to only take one argument. The
second was always `this.visibleChangeTableColumns`, so no need to pass
that in.

Tried using the `ColumnNames` enum in a few more places instead of
using strings.

Release-Notes: skip
Google-Bug-Id: b/297159286
Change-Id: I2e42743365d332851a666c56e07fd69c03c28d7f

11 files changed, 58 insertions, 267 deletions

diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index 25d25b0827..d28f33f26d 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -94,16 +94,13 @@ export enum ProgressStatus {
 
 export enum ColumnNames {
   SUBJECT = 'Subject',
-  // TODO(milutin) - remove once Submit Requirements are rolled out.
-  STATUS = 'Status',
   OWNER = 'Owner',
   REVIEWERS = 'Reviewers',
-  COMMENTS = 'Comments',
   REPO = 'Repo',
   BRANCH = 'Branch',
   UPDATED = 'Updated',
   SIZE = 'Size',
-  STATUS2 = ' Status ', // spaces to differentiate from old 'Status'
+  STATUS = 'Status',
 }
 
 /**
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index e6564043af..1d481a7b13 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -5,7 +5,6 @@
  */
 import '../../shared/gr-account-label/gr-account-label';
 import '../../shared/gr-change-star/gr-change-star';
-import '../../shared/gr-change-status/gr-change-status';
 import '../../shared/gr-date-formatter/gr-date-formatter';
 import '../../shared/gr-icon/gr-icon';
 import '../../shared/gr-limited-text/gr-limited-text';
@@ -19,7 +18,6 @@ import {navigationToken} from '../../core/gr-navigation/gr-navigation';
 import {getDisplayName} from '../../../utils/display-name-util';
 import {getAppContext} from '../../../services/app-context';
 import {truncatePath} from '../../../utils/path-list-util';
-import {changeStatuses} from '../../../utils/change-util';
 import {isSelf, isServiceUser} from '../../../utils/account-util';
 import {
   ChangeInfo,
@@ -59,7 +57,6 @@ export enum LabelCategory {
   APPROVED = 'APPROVED',
   POSITIVE = 'POSITIVE',
   NEUTRAL = 'NEUTRAL',
-  UNRESOLVED_COMMENTS = 'UNRESOLVED_COMMENTS',
   NEGATIVE = 'NEGATIVE',
   REJECTED = 'REJECTED',
 }
@@ -237,7 +234,6 @@ export class GrChangeListItem extends LitElement {
           white-space: nowrap;
           width: 100%;
         }
-        .comments,
         .reviewers,
         .requirements {
           white-space: nowrap;
@@ -249,17 +245,6 @@ export class GrChangeListItem extends LitElement {
           height: 0;
           overflow: hidden;
         }
-        .status {
-          align-items: center;
-          display: inline-flex;
-        }
-        .status .comma {
-          padding-right: var(--spacing-xs);
-        }
-        /* Used to hide the leading separator comma for statuses. */
-        .status .comma:first-of-type {
-          display: none;
-        }
         .size gr-tooltip-content {
           margin: -0.4rem -0.6rem;
           max-width: 2.5rem;
@@ -342,8 +327,7 @@ export class GrChangeListItem extends LitElement {
       <td aria-hidden="true" class="cell leftPadding"></td>
       ${this.renderCellSelectionBox()} ${this.renderCellStar()}
       ${this.renderCellNumber(changeUrl)} ${this.renderCellSubject(changeUrl)}
-      ${this.renderCellStatus()} ${this.renderCellOwner()}
-      ${this.renderCellReviewers()} ${this.renderCellComments()}
+      ${this.renderCellOwner()} ${this.renderCellReviewers()}
       ${this.renderCellRepo()} ${this.renderCellBranch()}
       ${this.renderCellUpdated()} ${this.renderCellSubmitted()}
       ${this.renderCellWaiting()} ${this.renderCellSize()}
@@ -398,13 +382,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellSubject(changeUrl: string) {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.SUBJECT,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.SUBJECT)) return;
 
     return html`
       <td class="cell subject">
@@ -430,39 +408,8 @@ export class GrChangeListItem extends LitElement {
     `;
   }
 
-  private renderCellStatus() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.STATUS,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
-
-    return html` <td class="cell status">${this.renderChangeStatus()}</td> `;
-  }
-
-  private renderChangeStatus() {
-    if (!this.changeStatuses().length) {
-      return html`<span class="placeholder">--</span>`;
-    }
-
-    return this.changeStatuses().map(
-      status => html`
-        <div class="comma">,</div>
-        <gr-change-status flat .status=${status}></gr-change-status>
-      `
-    );
-  }
-
   private renderCellOwner() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.OWNER,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.OWNER)) return;
 
     return html`
       <td class="cell owner">
@@ -477,13 +424,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellReviewers() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.REVIEWERS,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.REVIEWERS)) return;
 
     return html`
       <td class="cell reviewers">
@@ -517,29 +458,8 @@ export class GrChangeListItem extends LitElement {
     `;
   }
 
-  private renderCellComments() {
-    if (this.computeIsColumnHidden('Comments', this.visibleChangeTableColumns))
-      return;
-
-    return html`
-      <td class="cell comments">
-        ${this.change?.unresolved_comment_count
-          ? html`<gr-icon icon="mode_comment" filled></gr-icon>`
-          : ''}
-        <span
-          >${this.computeComments(this.change?.unresolved_comment_count)}</span
-        >
-      </td>
-    `;
-  }
-
   private renderCellRepo() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.REPO,
-        this.visibleChangeTableColumns
-      )
-    ) {
+    if (this.computeIsColumnHidden(ColumnNames.REPO)) {
       return;
     }
 
@@ -555,13 +475,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellBranch() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.BRANCH,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.BRANCH)) return;
 
     return html`
       <td class="cell branch">
@@ -586,8 +500,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellUpdated() {
-    if (this.computeIsColumnHidden('Updated', this.visibleChangeTableColumns))
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.UPDATED)) return;
 
     return html`
       <td class="cell updated">
@@ -600,8 +513,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellSubmitted() {
-    if (this.computeIsColumnHidden('Submitted', this.visibleChangeTableColumns))
-      return;
+    if (this.computeIsColumnHidden('Submitted')) return;
 
     return html`
       <td class="cell submitted">
@@ -614,8 +526,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellWaiting() {
-    if (this.computeIsColumnHidden(WAITING, this.visibleChangeTableColumns))
-      return;
+    if (this.computeIsColumnHidden(WAITING)) return;
 
     return html`
       <td class="cell waiting">
@@ -630,8 +541,7 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellSize() {
-    if (this.computeIsColumnHidden('Size', this.visibleChangeTableColumns))
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.SIZE)) return;
 
     return html`
       <td class="cell size">
@@ -652,16 +562,10 @@ export class GrChangeListItem extends LitElement {
   }
 
   private renderCellRequirements() {
-    if (
-      this.computeIsColumnHidden(
-        ColumnNames.STATUS2,
-        this.visibleChangeTableColumns
-      )
-    )
-      return;
+    if (this.computeIsColumnHidden(ColumnNames.STATUS)) return;
 
     return html`
-      <td class="cell requirements">
+      <td class="cell status requirements">
         <gr-change-list-column-requirements-summary .change=${this.change}>
         </gr-change-list-column-requirements-summary>
       </td>
@@ -701,11 +605,6 @@ export class GrChangeListItem extends LitElement {
     }
   };
 
-  private changeStatuses() {
-    if (!this.change) return [];
-    return changeStatuses(this.change);
-  }
-
   private computeChangeURL() {
     if (!this.change) return '';
     return createChangeUrl({change: this.change, usp: this.usp});
@@ -801,11 +700,6 @@ export class GrChangeListItem extends LitElement {
       .join(', ');
   }
 
-  private computeComments(unresolved_comment_count?: number) {
-    if (!unresolved_comment_count || unresolved_comment_count < 1) return '';
-    return `${unresolved_comment_count} unresolved`;
-  }
-
   /**
    * TShirt sizing is based on the following paper:
    * http://dirkriehle.com/wp-content/uploads/2008/09/hicss-42-csdistr-final-web.pdf
@@ -843,14 +737,12 @@ export class GrChangeListItem extends LitElement {
     return this.change?.attention_set[userId]?.last_update;
   }
 
-  private computeIsColumnHidden(
-    columnToCheck?: string,
-    columnsToDisplay?: string[]
-  ) {
-    if (!columnsToDisplay || !columnToCheck) {
-      return false;
-    }
-    return !columnsToDisplay.includes(columnToCheck);
+  private computeIsColumnHidden(columnToCheck?: string) {
+    if (!columnToCheck) return false;
+    const columnsToDisplay = this.visibleChangeTableColumns ?? [];
+    return (
+      columnsToDisplay.length > 0 && !columnsToDisplay.includes(columnToCheck)
+    );
   }
 
   private formatDate(date: Timestamp | undefined): string | undefined {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index 25e97af0f9..4f5a88852a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -82,15 +82,13 @@ suite('gr-change-list-item tests', () => {
   test('no hidden columns', async () => {
     element.visibleChangeTableColumns = [
       ColumnNames.SUBJECT,
-      ColumnNames.STATUS,
       ColumnNames.OWNER,
       ColumnNames.REVIEWERS,
-      ColumnNames.COMMENTS,
       ColumnNames.REPO,
       ColumnNames.BRANCH,
       ColumnNames.UPDATED,
       ColumnNames.SIZE,
-      ColumnNames.STATUS2,
+      ColumnNames.STATUS,
     ];
 
     await element.updateComplete;
@@ -214,14 +212,12 @@ suite('gr-change-list-item tests', () => {
   test('repo column hidden', async () => {
     element.visibleChangeTableColumns = [
       ColumnNames.SUBJECT,
-      ColumnNames.STATUS,
       ColumnNames.OWNER,
       ColumnNames.REVIEWERS,
-      ColumnNames.COMMENTS,
       ColumnNames.BRANCH,
       ColumnNames.UPDATED,
       ColumnNames.SIZE,
-      ColumnNames.STATUS2,
+      ColumnNames.STATUS,
     ];
 
     await element.updateComplete;
@@ -410,14 +406,12 @@ suite('gr-change-list-item tests', () => {
             <span></span>
           </div>
         </a>
-        <span class="placeholder"> -- </span>
         <gr-account-label
           deselected=""
           clickable=""
           highlightattention=""
         ></gr-account-label>
         <div></div>
-        <span></span>
         <a class="fullRepo" href="/q/project:test-project+status:open">
           test-project
         </a>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
index 4ec8491407..694d953e75 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
@@ -75,7 +75,7 @@ suite('gr-change-list section', () => {
         <input class="selection-checkbox" type="checkbox"/>
       </td>
       #
-              SubjectStatusOwnerReviewersCommentsRepoBranchUpdatedSize Status
+              SubjectOwnerReviewersRepoBranchUpdatedSizeStatus
       <gr-change-list-item
         aria-label="Test subject, section: test"
         role="button"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 38ee01ad19..17f4acfbda 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -8,7 +8,6 @@ import '../gr-change-list-item/gr-change-list-item';
 import '../gr-change-list-section/gr-change-list-section';
 import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
 import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
-import {getAppContext} from '../../../services/app-context';
 import {navigationToken} from '../../core/gr-navigation/gr-navigation';
 import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
 import {
@@ -30,13 +29,15 @@ import {customElement, property, state} from 'lit/decorators.js';
 import {Shortcut, ShortcutController} from '../../lit/shortcut-controller';
 import {queryAll} from '../../../utils/common-util';
 import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
-import {Execution} from '../../../constants/reporting';
 import {ValueChangedEvent} from '../../../types/events';
 import {resolve} from '../../../models/dependency';
 import {createChangeUrl} from '../../../models/views/change';
 import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
 import {subscribe} from '../../lit/subscription-controller';
-import {userModelToken} from '../../../models/user/user-model';
+import {
+  changeTablePrefs,
+  userModelToken,
+} from '../../../models/user/user-model';
 import {configModelToken} from '../../../models/config/config-model';
 
 export interface ChangeListSection {
@@ -131,10 +132,6 @@ export class GrChangeList extends LitElement {
   // private but used in test
   @state() config?: ServerInfo;
 
-  private readonly flagsService = getAppContext().flagsService;
-
-  private readonly reporting = getAppContext().reportingService;
-
   private readonly shortcuts = new ShortcutController(this);
 
   private readonly getConfigModel = resolve(this, configModelToken);
@@ -345,46 +342,17 @@ export class GrChangeList extends LitElement {
 
     this.changeTableColumns = Object.values(ColumnNames);
     this.showNumber = false;
-    this.visibleChangeTableColumns = this.changeTableColumns.filter(col =>
-      this.isColumnEnabled(col, this.config)
-    );
+    this.visibleChangeTableColumns = Object.values(ColumnNames);
     if (this.loggedInUser && this.preferences) {
       this.showNumber = !!this.preferences?.legacycid_in_change_table;
-      if (
-        this.preferences?.change_table &&
-        this.preferences.change_table.length > 0
-      ) {
-        const prefColumns = this.preferences.change_table
-          .map(column => (column === 'Project' ? ColumnNames.REPO : column))
-          .map(column =>
-            column === ColumnNames.STATUS ? ColumnNames.STATUS2 : column
-          );
-        this.reporting.reportExecution(Execution.USER_PREFERENCES_COLUMNS, {
-          statusColumn: prefColumns.includes(ColumnNames.STATUS2),
-        });
-        // Order visible column names by columnNames, filter only one that
-        // are in prefColumns and enabled by config
-        this.visibleChangeTableColumns = Object.values(ColumnNames)
-          .filter(col => prefColumns.includes(col))
-          .filter(col => this.isColumnEnabled(col, this.config));
-      }
+      const prefColumns = changeTablePrefs(this.preferences);
+      // This is for sorting `prefColumns` as in `ColumnNames`:
+      this.visibleChangeTableColumns = Object.values(ColumnNames).filter(col =>
+        prefColumns.includes(col)
+      );
     }
   }
 
-  /**
-   * Is the column disabled by a server config or experiment?
-   */
-  isColumnEnabled(column: string, config?: ServerInfo) {
-    if (!Object.values(ColumnNames).includes(column as unknown as ColumnNames))
-      return false;
-    if (!config || !config.change) return true;
-    if (column === 'Comments')
-      return this.flagsService.isEnabled('comments-column');
-    if (column === 'Status') return false;
-    if (column === ColumnNames.STATUS2) return true;
-    return true;
-  }
-
   // private but used in test
   computeLabelNames(sections: ChangeListSection[]) {
     if (!sections) return [];
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index 62a4f7d671..c53f813364 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -17,7 +17,6 @@ import {
 } from '../../../test/test-utils';
 import {Key} from '../../../utils/dom-util';
 import {
-  ColumnNames,
   createDefaultPreferences,
   TimeFormat,
 } from '../../../constants/constants';
@@ -456,12 +455,10 @@ suite('gr-change-list basic tests', () => {
           'Status',
           'Owner',
           'Reviewers',
-          'Comments',
           'Repo',
           'Branch',
           'Updated',
           'Size',
-          ColumnNames.STATUS2,
         ],
       };
       element.config = createServerInfo();
@@ -495,11 +492,9 @@ suite('gr-change-list basic tests', () => {
           'Status',
           'Owner',
           'Reviewers',
-          'Comments',
           'Branch',
           'Updated',
           'Size',
-          ColumnNames.STATUS2,
         ],
       };
       element.config = createServerInfo();
@@ -531,10 +526,6 @@ suite('gr-change-list basic tests', () => {
     });
   });
 
-  test('obsolete column in preferences not visible', () => {
-    assert.isTrue(element.isColumnEnabled('Subject'));
-  });
-
   test('loggedIn and showNumber', async () => {
     element.sections = [{results: [{...createChange()}], name: 'a'}];
     element.loggedInUser = {_account_id: 1001 as AccountId};
@@ -546,11 +537,9 @@ suite('gr-change-list basic tests', () => {
         'Status',
         'Owner',
         'Reviewers',
-        'Comments',
         'Branch',
         'Updated',
         'Size',
-        ColumnNames.STATUS2,
       ],
     };
     element.config = createServerInfo();
@@ -596,26 +585,4 @@ suite('gr-change-list basic tests', () => {
 
     assert.isNotOk(query<HTMLElement>(element, '.bad'));
   });
-
-  test('Show new status with feature flag', async () => {
-    stubFlags('isEnabled').returns(true);
-    const element: GrChangeList = await fixture(
-      html`<gr-change-list></gr-change-list>`
-    );
-    element.sections = [{results: [{...createChange()}]}];
-    element.loggedInUser = {_account_id: 1001 as AccountId};
-    element.preferences = {
-      change_table: [
-        'Status', // old status
-      ],
-    };
-    element.config = createServerInfo();
-    await element.updateComplete;
-    assert.isTrue(
-      element.visibleChangeTableColumns?.includes(ColumnNames.STATUS2),
-      'Show new status'
-    );
-    const section = queryAndAssert(element, 'gr-change-list-section');
-    queryAndAssert<HTMLElement>(section, '.status');
-  });
 });
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
index 568e7c31f5..4e5c8b8346 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.ts
@@ -5,12 +5,10 @@
  */
 import '../../shared/gr-button/gr-button';
 import {ServerInfo} from '../../../types/common';
-import {getAppContext} from '../../../services/app-context';
 import {LitElement, css, html} from 'lit';
 import {customElement, property, state} from 'lit/decorators.js';
 import {sharedStyles} from '../../../styles/shared-styles';
 import {formStyles} from '../../../styles/gr-form-styles';
-import {PropertyValues} from 'lit';
 import {fire} from '../../../utils/event-util';
 import {ValueChangedEvent} from '../../../types/events';
 import {ColumnNames} from '../../../constants/constants';
@@ -27,13 +25,11 @@ export class GrChangeTableEditor extends LitElement {
   showNumber?: boolean;
 
   @property({type: Array})
-  defaultColumns: string[] = [];
+  defaultColumns: string[] = Object.values(ColumnNames);
 
   @state()
   serverConfig?: ServerInfo;
 
-  private readonly flagsService = getAppContext().flagsService;
-
   private readonly getConfigModel = resolve(this, configModelToken);
 
   static override get styles() {
@@ -118,34 +114,6 @@ export class GrChangeTableEditor extends LitElement {
     </tr>`;
   }
 
-  override willUpdate(changedProperties: PropertyValues) {
-    if (changedProperties.has('serverConfig')) {
-      this.configChanged();
-    }
-  }
-
-  private configChanged() {
-    this.defaultColumns = Object.values(ColumnNames).filter(column =>
-      this.isColumnEnabled(column)
-    );
-    if (!this.displayedColumns) return;
-    this.displayedColumns = this.displayedColumns.filter(column =>
-      this.isColumnEnabled(column)
-    );
-  }
-
-  /**
-   * Is the column disabled by a server config or experiment?
-   * private but used in test
-   */
-  isColumnEnabled(column: string) {
-    if (!this.serverConfig?.change) return true;
-    if (column === ColumnNames.COMMENTS)
-      return this.flagsService.isEnabled('comments-column');
-    if (column === ColumnNames.STATUS) return false;
-    return true;
-  }
-
   /**
    * Get the list of enabled column names from whichever checkboxes are
    * checked (excluding the number checkbox).
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
index 4d3d3a1eed..9efebefe32 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.ts
@@ -21,14 +21,14 @@ suite('gr-change-table-editor tests', () => {
     );
 
     columns = [
-      'Subject',
-      'Status',
-      'Owner',
-      'Reviewers',
-      'Comments',
-      'Repo',
+      ColumnNames.SUBJECT,
+      ColumnNames.OWNER,
+      ColumnNames.REVIEWERS,
+      ColumnNames.REPO,
       ColumnNames.BRANCH,
       ColumnNames.UPDATED,
+      ColumnNames.SIZE,
+      // ColumnNames.STATUS omitted for testing
     ];
 
     element.displayedColumns = columns;
@@ -99,13 +99,13 @@ suite('gr-change-table-editor tests', () => {
             <tr>
               <td><label for="Size"> Size </label></td>
               <td class="checkboxContainer">
-                <input id="Size" name="Size" type="checkbox" />
+                <input checked="" id="Size" name="Size" type="checkbox" />
               </td>
             </tr>
             <tr>
-              <td><label for=" Status "> Status </label></td>
+              <td><label for="Status"> Status </label></td>
               <td class="checkboxContainer">
-                <input id=" Status " name=" Status " type="checkbox" />
+                <input id="Status" name="Status" type="checkbox" />
               </td>
             </tr>
           </tbody>
@@ -170,9 +170,7 @@ suite('gr-change-table-editor tests', () => {
   });
 
   test('getDisplayedColumns', () => {
-    const enabledColumns = columns.filter(column =>
-      element.isColumnEnabled(column)
-    );
+    const enabledColumns = columns;
     assert.deepEqual(element.getDisplayedColumns(), enabledColumns);
     const input = queryAndAssert<HTMLInputElement>(
       element,
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 5fcafc3ff8..892429b8a9 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -40,7 +40,6 @@ import {GrEmailEditor} from '../gr-email-editor/gr-email-editor';
 import {fireAlert, fireTitleChange} from '../../../utils/event-util';
 import {getAppContext} from '../../../services/app-context';
 import {
-  ColumnNames,
   DateFormat,
   DefaultBase,
   DiffViewMode,
@@ -64,7 +63,10 @@ import {subscribe} from '../../lit/subscription-controller';
 import {resolve} from '../../../models/dependency';
 import {settingsViewModelToken} from '../../../models/views/settings';
 import {areNotificationsEnabled} from '../../../utils/worker-util';
-import {userModelToken} from '../../../models/user/user-model';
+import {
+  changeTablePrefs,
+  userModelToken,
+} from '../../../models/user/user-model';
 import {modalStyles} from '../../../styles/gr-modal-styles';
 import {navigationToken} from '../../core/gr-navigation/gr-navigation';
 import {rootUrl} from '../../../utils/url-util';
@@ -242,12 +244,7 @@ export class GrSettingsView extends LitElement {
         this.showNumber = !!prefs.legacycid_in_change_table;
         this.copyPrefs(CopyPrefsDirection.PrefsToLocalPrefs);
         this.prefsChanged = false;
-        this.localChangeTableColumns =
-          prefs.change_table.length === 0
-            ? Object.values(ColumnNames)
-            : prefs.change_table.map(column =>
-                column === 'Project' ? 'Repo' : column
-              );
+        this.localChangeTableColumns = changeTablePrefs(prefs);
       }
     );
   }
diff --git a/polygerrit-ui/app/models/user/user-model.ts b/polygerrit-ui/app/models/user/user-model.ts
index 97f90fa4a6..7752f0ced0 100644
--- a/polygerrit-ui/app/models/user/user-model.ts
+++ b/polygerrit-ui/app/models/user/user-model.ts
@@ -20,6 +20,7 @@ import {
   createDefaultDiffPrefs,
   createDefaultEditPrefs,
   AppTheme,
+  ColumnNames,
 } from '../../constants/constants';
 import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
 import {DiffPreferencesInfo} from '../../types/diff';
@@ -28,6 +29,14 @@ import {define} from '../dependency';
 import {Model} from '../model';
 import {isDefined} from '../../types/types';
 
+export function changeTablePrefs(prefs: Partial<PreferencesInfo>) {
+  const cols = prefs.change_table ?? [];
+  if (cols.length === 0) return Object.values(ColumnNames);
+  return cols
+    .map(column => (column === 'Project' ? ColumnNames.REPO : column))
+    .map(column => (column === ' Status ' ? ColumnNames.STATUS : column));
+}
+
 export interface UserState {
   /**
    * Keeps being defined even when credentials have expired.
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index b23fc7e475..88bb6b9072 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1317,6 +1317,7 @@ export interface PreferencesInfo {
   mute_common_path_prefixes?: boolean;
   signed_off_by?: boolean;
   my: TopMenuItemInfo[];
+  // Do not use directly, but use changeTablePrefs() in user model to map/filter legacy columns.
   change_table: string[];
   email_strategy: EmailStrategy;
   default_base_for_merges: DefaultBase;