Merge pull request #1366 from sgratch/release-2.7

[v2.7.3] Update hasObjectChangedInGivenFields to ignore predefined fields

packages/forklift-console-plugin/src/modules/Plans/views/details/utils/index.ts

@@ -3,7 +3,6 @@ export * from './canDeleteAndPatchPlan';
 export * from './constants';
 export * from './getInventoryApiUrl';
 export * from './getValueByJsonPath';
-export * from './hasObjectChangedInGivenFields';
 export * from './hasPipelineCompleted';
 export * from './hasPlanEditable';
 export * from './hasPlanMappingsChanged';

packages/forklift-console-plugin/src/modules/Providers/hooks/useProviderInventory.ts

@@ -5,14 +5,14 @@ import { consoleFetchJSON } from '@openshift-console/dynamic-plugin-sdk';
 
 import { getInventoryApiUrl, hasObjectChangedInGivenFields } from '../utils/helpers';
 
-import { DEFAULT_FIELDS_TO_COMPARE } from './utils';
+import { DEFAULT_FIELDS_TO_AVOID_COMPARING } from './utils';
 
 /**
  * @typedef {Object} UseProviderInventoryParams
  *
  * @property {V1beta1Provider} provider - The provider from which the inventory is fetched.
  * @property {string} [subPath] - The sub path to be used in the inventory fetch API URL.
- * @property {string[]} [fieldsToCompare] - The fields to compare to check if the inventory has changed.
+ * @property {string[]} [fieldsToAvoidComparing] - The fields to ignore comparing when checking if the inventory has changed.
  * @property {number} [interval] - The polling interval in milliseconds.
  * @property {number} [fetchTimeout] - The fetch timeout in milliseconds.
  * @property {number} [cacheExpiryDuration] - Duration in milliseconds till the cache remains valid.
@@ -21,7 +21,7 @@ import { DEFAULT_FIELDS_TO_COMPARE } from './utils';
 export interface UseProviderInventoryParams {
   provider: V1beta1Provider;
   subPath?: string;
-  fieldsToCompare?: string[];
+  fieldsToAvoidComparing?: string[];
   interval?: number;
   fetchTimeout?: number;
   disabled?: boolean;
@@ -43,13 +43,13 @@ interface UseProviderInventoryResult<T> {
 /**
  * A React hook to fetch and cache inventory data from a provider.
  * It fetches new data on mount and then at the specified interval.
- * If the new data is the same as the old data (compared using the specified fields),
+ * If the new data is the same as the old data (compared ignoring specified fields, if exist),
  * it does not update the state to prevent unnecessary re-renders.
  *
  * @param {Object} useProviderInventoryParams Configuration parameters for the hook
  * @param {Object} useProviderInventoryParams.provider Provider object to get inventory data from
  * @param {string} [useProviderInventoryParams.subPath=''] Sub-path to append to the provider API URL
- * @param {Array} [useProviderInventoryParams.fieldsToCompare=DEFAULT_FIELDS_TO_COMPARE] Fields to use for comparing new data with old data
+ * @param {Array} [useProviderInventoryParams.fieldsToAvoidComparing=DEFAULT_FIELDS_TO_AVOID_COMPARING] Fields to ignore when comparing new data with old data
  * @param {number} [useProviderInventoryParams.interval=10000] Interval (in milliseconds) to fetch new data at
  * @param {boolean} [useProviderInventoryParams.disabled=false] Prevent query execution.
  *
@@ -61,7 +61,7 @@ interface UseProviderInventoryResult<T> {
 export const useProviderInventory = <T>({
   provider,
   subPath = '',
-  fieldsToCompare = DEFAULT_FIELDS_TO_COMPARE,
+  fieldsToAvoidComparing = DEFAULT_FIELDS_TO_AVOID_COMPARING,
   interval = 20000,
   fetchTimeout,
   disabled = false,
@@ -102,7 +102,7 @@ export const useProviderInventory = <T>({
           fetchTimeout,
         );
 
-        updateInventoryIfChanged(newInventory, fieldsToCompare);
+        updateInventoryIfChanged(newInventory, fieldsToAvoidComparing);
         handleError(null);
       } catch (e) {
         handleError(e);
@@ -145,13 +145,13 @@ export const useProviderInventory = <T>({
    * Checks if the inventory data has changed and updates the inventory state if it has.
    * Also updates the loading state.
    * @param {T} newInventory - The new inventory data.
-   * @param {string[]} fieldsToCompare - The fields to compare to check if the inventory data has changed.
+   * @param {string[]} fieldsToAvoidComparing - The fields to ignore comparing when checking if the inventory data has changed.
    */
-  function updateInventoryIfChanged(newInventory: T, fieldsToCompare: string[]): void {
+  function updateInventoryIfChanged(newInventory: T, fieldsToAvoidComparing: string[]): void {
     const needReRender = hasObjectChangedInGivenFields({
       oldObject: oldDataRef.current?.inventory,
       newObject: newInventory,
-      fieldsToCompare: fieldsToCompare,
+      fieldsToAvoidComparing: fieldsToAvoidComparing,
     });
 
     if (needReRender) {

packages/forklift-console-plugin/src/modules/Providers/hooks/useProvidersInventoryList.ts

@@ -10,7 +10,7 @@ import { consoleFetchJSON, k8sGet, useFlag } from '@openshift-console/dynamic-pl
 
 import { getInventoryApiUrl, hasObjectChangedInGivenFields } from '../utils/helpers';
 
-import { DEFAULT_FIELDS_TO_COMPARE } from './utils';
+import { DEFAULT_FIELDS_TO_AVOID_COMPARING } from './utils';
 
 const INVENTORY_TYPES: string[] = ['openshift', 'openstack', 'ovirt', 'vsphere', 'ova'];
 
@@ -68,7 +68,7 @@ export const useProvidersInventoryList = ({
           ? await consoleFetchJSON(getInventoryApiUrl(`providers?detail=1`)) // Fetch all providers
           : await getInventoryByNamespace(namespace); // Fetch single namespace's providers
 
-        updateInventoryIfChanged(newInventory, DEFAULT_FIELDS_TO_COMPARE);
+        updateInventoryIfChanged(newInventory, DEFAULT_FIELDS_TO_AVOID_COMPARING);
         handleError(null);
       } catch (e) {
         handleError(e);
@@ -163,14 +163,13 @@ export const useProvidersInventoryList = ({
    * and updates the reference to the old data.
    *
    * @param newInventoryList - The new inventory list.
-   * @param fieldsToCompare - The fields to compare in order to determine
-   *                          if an inventory item has changed.
+   * @param fieldsToAvoidComparing - The fields to ignore comparing in order to determine if an inventory item has changed.
    *
    * @returns {void}
    */
   function updateInventoryIfChanged(
     newInventoryList: ProvidersInventoryList,
-    fieldsToCompare: string[],
+    fieldsToAvoidComparing: string[],
   ): void {
     // Calculate total lengths of old and new inventories.
     const oldTotalLength = INVENTORY_TYPES.reduce(
@@ -204,7 +203,11 @@ export const useProvidersInventoryList = ({
         // If a matching item is not found in the new list, or the item has changed, we need to re-render.
         if (
           !newItem ||
-          hasObjectChangedInGivenFields({ oldObject: oldItem, newObject: newItem, fieldsToCompare })
+          hasObjectChangedInGivenFields({
+            oldObject: oldItem,
+            newObject: newItem,
+            fieldsToAvoidComparing,
+          })
         ) {
           needReRender = true;
           break;

packages/forklift-console-plugin/src/modules/Providers/hooks/utils/constants.ts

@@ -1,15 +1,11 @@
-export const DEFAULT_FIELDS_TO_COMPARE = [
-  'vmCount',
-  'networkCount',
-  'storageClassCount',
-  'regionCount',
-  'projectCount',
-  'imageCount',
-  'volumeCount',
-  'volumeTypeCount',
-  'datacenterCount',
-  'clusterCount',
-  'hostCount',
-  'storageDomainCount',
-  'datastoreCount',
-];
+/**
+ * List of fields to ignore/skip comparing when checking if an inventory's object has changed and required
+ * re-rendering.
+ *
+ * Note: The fields included in this list have values which are changed occasionally in inventory
+ * while not displayed in UI and therefore if not ignored it will
+ * generate frequent unnecessary re-rendering.
+ *
+ * Need to add more fields to this if required.
+ */
+export const DEFAULT_FIELDS_TO_AVOID_COMPARING = ['revision', 'revisionValidated', 'storageUsed'];

packages/forklift-console-plugin/src/modules/Providers/utils/helpers/hasObjectChangedInGivenFields.ts

@@ -1,20 +1,24 @@
 type FieldsComparisonArgs<T> = {
   oldObject?: T;
   newObject?: T;
-  fieldsToCompare: string[];
+  fieldsToAvoidComparing: string[];
 };
 
 /**
  * Checks whether the specified fields have changed in two objects.
  *
- * @param params - An object containing the old object, new object, and fields to be compared.
+ * @param params - An object containing the old object, new object, and fields to avoid comparing.
  * @returns A boolean indicating whether any of the specified fields have changed.
  */
 export function hasObjectChangedInGivenFields<T>(params: FieldsComparisonArgs<T>): boolean {
-  return !isEqual(params.newObject, params.oldObject);
+  return !isEqual(params.newObject, params.oldObject, params.fieldsToAvoidComparing);
 }
 
-function isEqual(obj1: unknown, obj2: unknown): boolean {
+function isEqual(obj1: unknown, obj2: unknown, fieldsToAvoidComparing: string[]): boolean {
+  const isFieldToCompare = (key: string) => !fieldsToAvoidComparing.includes(key);
+  const isFieldChanged = (key: string, keys2: string[], fieldsToAvoidComparing: string[]) =>
+    !keys2.includes(key) || !isEqual(obj1[key], obj2[key], fieldsToAvoidComparing);
+
   // Check the object types
   if (typeof obj1 !== 'object' || typeof obj2 !== 'object' || !obj1 || !obj2) {
     return obj1 === obj2;
@@ -39,7 +43,7 @@ function isEqual(obj1: unknown, obj2: unknown): boolean {
       return false;
     }
     for (let i = 0; i < obj1.length; i++) {
-      if (!isEqual(obj1[i], obj2[i])) {
+      if (!isEqual(obj1[i], obj2[i], fieldsToAvoidComparing)) {
         return false;
       }
     }
@@ -55,7 +59,7 @@ function isEqual(obj1: unknown, obj2: unknown): boolean {
 
   // Check each key in obj1 to see if they are equal
   for (const key of keys1) {
-    if (!keys2.includes(key) || !isEqual(obj1[key], obj2[key])) {
+    if (isFieldToCompare(key) && isFieldChanged(key, keys2, fieldsToAvoidComparing)) {
       return false;
     }
   }