Do not allow state updates with deep equal state objects

Most places use `select()`, which contains `distinctUntilChanged`, but
we also have some subscribers that do not explicitly pipe using this
operator. Instead of checking this at all call or subscription sites,
it seems most efficient to just prevent state updates that are not
actually an update.

This change will be needed in change 347036, which subscibes to view
model state changes, but really only wants to listen on actual changes.

Had to fix bulk-actions-model, which was relying on the array in state
being the same array as in the `updateState()` call, even if the state
did not need an update at all.

Release-Notes: skip
Change-Id: Id9c9960840ff712a12506113da3096fe5894eb83

3 files changed, 7 insertions, 3 deletions

diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
index f58ad1499f..f706712690 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -26,6 +26,7 @@ import {
 } from '../../types/common';
 import {getUserId} from '../../utils/account-util';
 import {getChangeNumber} from '../../utils/change-util';
+import {deepEqual} from '../../utils/deep-util';
 
 export const bulkActionsModelToken =
   define<BulkActionsModel>('bulk-actions-model');
@@ -258,7 +259,9 @@ export class BulkActionsModel
       );
     currentState = this.getState();
     // Return early if sync has been called again since starting the load.
-    if (selectableChangeNums !== currentState.selectableChangeNums) return;
+    if (!deepEqual(selectableChangeNums, currentState.selectableChangeNums)) {
+      return;
+    }
     const allDetailedChanges: Map<NumericChangeId, ChangeInfo> = new Map();
     for (const detailedChange of changeDetails ?? []) {
       allDetailedChanges.set(detailedChange._number, detailedChange);
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
index 2192246bf4..e2f75f72dd 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
@@ -500,7 +500,7 @@ suite('bulk actions model test', () => {
     const getChangesStub = stubRestApi(
       'getDetailedChangesWithActions'
     ).callsFake(() => promise);
-    bulkActionsModel.sync([c1, c2]);
+    bulkActionsModel.sync([c1]);
     assert.strictEqual(getChangesStub.callCount, 1);
     await waitUntilObserved(
       bulkActionsModel.loadingState$,
@@ -534,7 +534,6 @@ suite('bulk actions model test', () => {
     // Resolve the old promise.
     responsePromise1.resolve([
       {...createChange(), _number: 1, subject: 'Subject 1-old'},
-      {...createChange(), _number: 2, subject: 'Subject 2-old'},
     ] as ChangeInfo[]);
     await waitEventLoop();
     const model2 = bulkActionsModel.getState();
diff --git a/polygerrit-ui/app/models/model.ts b/polygerrit-ui/app/models/model.ts
index 7cd8b2a9a5..2e90a5e6b4 100644
--- a/polygerrit-ui/app/models/model.ts
+++ b/polygerrit-ui/app/models/model.ts
@@ -5,6 +5,7 @@
  */
 import {BehaviorSubject, Observable} from 'rxjs';
 import {Finalizable} from '../services/registry';
+import {deepEqual} from '../utils/deep-util';
 
 /**
  * A Model stores a value <T> and controls changes to that value via `subject$`
@@ -35,6 +36,7 @@ export abstract class Model<T> implements Finalizable {
   }
 
   setState(state: T) {
+    if (deepEqual(state, this.getState())) return;
     this.subject$.next(state);
   }