Update hasObjectChangedInGivenFields to ignore predefined fields

Reference: https://issues.redhat.com/browse/MTV-1546

- Check if the inventory data has changed for re-loading the data to UI
and re-rendering only for specified fields by ignoring a pre-defined list of fields.
- Remove unused hasObjectChangedInGivenFields duplicate method
  implementation.

Up till now, all fields have been compared to check if something was
changed for reloading. But since there are few fields that are changed occasionally in inventory,
while not being displayed in UI, we can ignore them while checking for new data, for less requent unnecessary re-rendering.

Signed-off-by: Sharon Gratch <[email protected]>

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;
     }
   }