diff --git a/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/hasObjectChangedInGivenFields.ts b/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/hasObjectChangedInGivenFields.ts deleted file mode 100644 index 0e37cffb7..000000000 --- a/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/hasObjectChangedInGivenFields.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { getValueByJsonPath } from './getValueByJsonPath'; - -type FieldsComparisonArgs = { - oldObject?: T; - newObject?: T; - fieldsToCompare: 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. - * @returns A boolean indicating whether any of the specified fields have changed. - */ -export function hasObjectChangedInGivenFields(params: FieldsComparisonArgs): boolean { - if (!params?.oldObject && !params?.newObject) { - return false; - } - - if (!params?.oldObject || !params?.newObject) { - return true; - } - - return params.fieldsToCompare.some( - (field) => - getValueByJsonPath(params.oldObject, field) !== getValueByJsonPath(params.newObject, field), - ); -} diff --git a/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/index.ts b/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/index.ts index 7756ce20f..0af5f24d7 100644 --- a/packages/forklift-console-plugin/src/modules/Plans/views/details/utils/index.ts +++ b/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'; diff --git a/packages/forklift-console-plugin/src/modules/Providers/hooks/useProviderInventory.ts b/packages/forklift-console-plugin/src/modules/Providers/hooks/useProviderInventory.ts index ce4b4982d..42abdb40b 100644 --- a/packages/forklift-console-plugin/src/modules/Providers/hooks/useProviderInventory.ts +++ b/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 { /** * 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 { export const useProviderInventory = ({ 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 = ({ fetchTimeout, ); - updateInventoryIfChanged(newInventory, fieldsToCompare); + updateInventoryIfChanged(newInventory, fieldsToAvoidComparing); handleError(null); } catch (e) { handleError(e); @@ -145,13 +145,13 @@ export const useProviderInventory = ({ * 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) { diff --git a/packages/forklift-console-plugin/src/modules/Providers/hooks/useProvidersInventoryList.ts b/packages/forklift-console-plugin/src/modules/Providers/hooks/useProvidersInventoryList.ts index 956461caa..a77dacb30 100644 --- a/packages/forklift-console-plugin/src/modules/Providers/hooks/useProvidersInventoryList.ts +++ b/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; diff --git a/packages/forklift-console-plugin/src/modules/Providers/hooks/utils/constants.ts b/packages/forklift-console-plugin/src/modules/Providers/hooks/utils/constants.ts index ec44453a3..8d472fe10 100644 --- a/packages/forklift-console-plugin/src/modules/Providers/hooks/utils/constants.ts +++ b/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']; diff --git a/packages/forklift-console-plugin/src/modules/Providers/utils/helpers/hasObjectChangedInGivenFields.ts b/packages/forklift-console-plugin/src/modules/Providers/utils/helpers/hasObjectChangedInGivenFields.ts index b31963a85..93dc7f956 100644 --- a/packages/forklift-console-plugin/src/modules/Providers/utils/helpers/hasObjectChangedInGivenFields.ts +++ b/packages/forklift-console-plugin/src/modules/Providers/utils/helpers/hasObjectChangedInGivenFields.ts @@ -1,20 +1,24 @@ type FieldsComparisonArgs = { 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(params: FieldsComparisonArgs): 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; } }