refactor: refactored useTaskRuns.ts

Author: anwesha-palit-redhat

src/components/hooks/useTaskRuns.ts

@@ -69,6 +69,8 @@ export type UseTaskRunsOptions = {
   pipelineRunFinished?: boolean;
   pipelineRunManagedBy?: string;
   skipFetch?: boolean;
+  name?: string /* used for fetching a single task run by metadata.name */;
+  limit?: number /* used for fetching a limited number of task runs */;
 };
 
 export const useTaskRuns = (
@@ -78,7 +80,7 @@ export const useTaskRuns = (
   pipelineRunUid?: string,
   options?: UseTaskRunsOptions,
 ): [TaskRunKind[], boolean, boolean, Error | undefined, boolean, boolean] => {
-  const { pipelineRunFinished, pipelineRunManagedBy, skipFetch } =
+  const { pipelineRunFinished, pipelineRunManagedBy, skipFetch, name, limit } =
     options || {};
   const selector: Selector = useMemo(() => {
     if (pipelineRunName && pipelineRunUid) {
@@ -106,9 +108,10 @@ export const useTaskRuns = (
     error,
     pendingAdmission,
     proxyUnavailable,
-  ] = useTaskRuns2(
+  ] = useRuns<TaskRunKind>(
+    TASK_RUN_GVK,
     namespace,
-    { selector, skipFetch },
+    { selector, skipFetch, name, limit },
     pipelineRunFinished,
     pipelineRunManagedBy,
   );
@@ -150,24 +153,6 @@ export const useTaskRuns = (
   );
 };
 
-export const useTaskRuns2 = (
-  namespace: string,
-  options?: {
-    selector?: Selector;
-    limit?: number;
-    skipFetch?: boolean;
-  },
-  pipelineRunFinished?: boolean,
-  pipelineRunManagedBy?: string,
-): [TaskRunKind[], boolean, boolean, Error | undefined, boolean, boolean] =>
-  useRuns<TaskRunKind>(
-    TASK_RUN_GVK,
-    namespace,
-    options,
-    pipelineRunFinished,
-    pipelineRunManagedBy,
-  );
-
 export const useApprovalTasks = (
   namespace: string,
   pipelineRunName?: string,
@@ -205,32 +190,14 @@ export const usePipelineRuns = (
   options?: {
     selector?: Selector;
     limit?: number;
+    name?: string;
   },
 ): [PipelineRunKind[], boolean, boolean, Error | undefined, boolean, boolean] =>
-  useRuns<PipelineRunKind>(PIPELINE_RUN_GVK, namespace, options);
-
-export const usePipelineRun = (
-  namespace: string,
-  pipelineRunName: string,
-): [PipelineRunKind, boolean, boolean, Error | undefined] => {
-  const result = usePipelineRuns(
-    namespace,
-    useMemo(
-      () => ({
-        name: pipelineRunName,
-        limit: 1,
-      }),
-      [pipelineRunName],
-    ),
-  ) as unknown as [PipelineRunKind[], boolean, boolean, Error | undefined];
-
-  return [
-    result[0]?.[0],
-    result[1],
-    result[2],
-    result[0]?.[0] ? undefined : result[3],
-  ];
-};
+  useRuns<PipelineRunKind>(PIPELINE_RUN_GVK, namespace, {
+    selector: options?.selector,
+    limit: options?.limit /* similar to one present in UseTaskRunsOptions */,
+    name: options?.name /* similar to one present in UseTaskRunsOptions */,
+  });
 
 export const useRuns = <Kind extends K8sResourceKind>(
   groupVersionKind: K8sGroupVersionKind,
@@ -380,84 +347,45 @@ export const useRuns = <Kind extends K8sResourceKind>(
     optionsMemo?.skipFetch,
   );
 
-  return useMemo(() => {
-    // dedupe PLR by name since UIDs differ between hub and spoke clusters; for other cases(TR) dedupe by UID
+  // dedupe PLR by name since UIDs differ between hub and spoke clusters; for other cases(TR) dedupe by UID
 
-    const rResources: Kind[] =
-      runs && trResources
-        ? !isTaskRunQuery
-          ? uniqBy([...runs, ...trResources], (r) => r.metadata.name)
-          : uniqBy([...runs, ...trResources], (r) => r.metadata.uid)
-        : runs || trResources;
+  const rResources: Kind[] =
+    runs && trResources
+      ? !isTaskRunQuery
+        ? uniqBy([...runs, ...trResources], (r) => r.metadata.name)
+        : uniqBy([...runs, ...trResources], (r) => r.metadata.uid)
+      : runs || trResources;
 
-    /* Refactoring the nesting as it is causing cognitive damage */
-    let resolvedError: Error | undefined = undefined;
+  /* Refactoring the nesting as it is causing cognitive damage */
+  let resolvedError: Error | undefined = undefined;
 
-    if (namespace) {
-      if (queryTr) {
-        if (isList) {
-          resolvedError = trError && effectiveError;
-        } else {
-          // when searching by name, return an error if we have no result
-          if (trError && trLoaded && !trResources?.length) {
-            resolvedError = effectiveError;
-          }
-        }
+  if (namespace) {
+    if (queryTr) {
+      if (isList) {
+        resolvedError = trError && effectiveError;
       } else {
-        resolvedError = effectiveError;
+        // when searching by name, return an error if we have no result
+        if (trError && trLoaded && !trResources?.length) {
+          resolvedError = effectiveError;
+        }
       }
-    } else if (effectiveError) {
+    } else {
       resolvedError = effectiveError;
     }
+  } else if (effectiveError) {
+    resolvedError = effectiveError;
+  }
 
-    const pendingAdmission = shouldUseMultiCluster ? mcPendingAdmission : false;
-    const proxyUnavailable = shouldUseMultiCluster ? mcProxyUnavailable : false;
-
-    const isTrLoaded = trLoaded || !!trError;
-    return [
-      rResources,
-      effectiveLoaded,
-      isTrLoaded,
-      resolvedError,
-      pendingAdmission,
-      proxyUnavailable,
-    ];
-  }, [
-    runs,
-    trResources,
-    loaded,
-    trLoaded,
-    isTektonResultEnabled,
-    namespace,
-    queryTr,
-    isList,
-    trError,
-    error,
-  ]);
-};
-
-export const useTaskRun = (
-  namespace: string,
-  taskRunName: string,
-): [TaskRunKind, boolean, boolean, string] => {
-  const result = useTaskRuns2(
-    namespace,
-    useMemo(
-      () => ({
-        name: taskRunName,
-        limit: 1,
-      }),
-      [taskRunName],
-    ),
-  ) as unknown as [TaskRunKind[], boolean, boolean, string];
+  const pendingAdmission = shouldUseMultiCluster ? mcPendingAdmission : false;
+  const proxyUnavailable = shouldUseMultiCluster ? mcProxyUnavailable : false;
 
-  return useMemo(
-    () => [
-      result[0]?.[0],
-      result[1],
-      result[2],
-      result[0]?.[0] ? undefined : result[3],
-    ],
-    [result],
-  );
+  const isTrLoaded = trLoaded || !!trError;
+  return [
+    rResources,
+    effectiveLoaded,
+    isTrLoaded,
+    resolvedError,
+    pendingAdmission,
+    proxyUnavailable,
+  ];
 };

src/components/pipelineRuns-details/PipelineRunDetailsPage.tsx

@@ -31,7 +31,7 @@ import {
 import PipelineRunParametersForm from './PipelineRunParametersForm';
 import { PipelineRunLogsWithActiveTask } from './PipelineRunLogs';
 import PipelineRunEvents from './PipelineRunEvents';
-import { usePipelineRun } from '../hooks/useTaskRuns';
+import { usePipelineRuns } from '../hooks/useTaskRuns';
 import { getReferenceForModel } from '../pipelines-overview/utils';
 import { LazyActionMenu } from '@openshift-console/dynamic-plugin-sdk-internal';
 import { ActionMenuVariant } from '@openshift-console/dynamic-plugin-sdk-internal/lib/api/internal-types';
@@ -46,7 +46,11 @@ const PipelineRunDetailsPage: FC<PipelineRunDetailsPageProps> = ({
   namespace,
 }) => {
   const { t } = useTranslation('plugin__pipelines-console-plugin');
-  const [pipelineRun, k8sLoaded, trLoaded] = usePipelineRun(namespace, name);
+  const [pipelineRuns, k8sLoaded, trLoaded] = usePipelineRuns(namespace, {
+    name,
+    limit: 1,
+  });
+  const pipelineRun = pipelineRuns?.[0];
   /* this needs decoupling */
   const pipelineRunLoaded = k8sLoaded || trLoaded;
 

src/components/pipelineRuns-details/PipelineRunVisualization.tsx

@@ -23,7 +23,7 @@ const PipelineRunVisualization: FC<PipelineRunVisualizationProps> = ({
     plrStatus !== ComputedStatus.Running &&
     plrStatus !== ComputedStatus.Pending &&
     plrStatus !== ComputedStatus.Cancelling;
-  const [taskRuns, k8sLoaded, trLoaded, pendingAdmission, proxyUnavailable] =
+  const [taskRuns, k8sLoaded, trLoaded, , pendingAdmission, proxyUnavailable] =
     useTaskRuns(
       pipelineRun?.metadata?.namespace,
       pipelineRun?.metadata?.name,

src/components/pipelines-overview/utils.ts

@@ -4,11 +4,15 @@ import {
   K8sResourceKindReference,
   PrometheusResponse,
 } from '@openshift-console/dynamic-plugin-sdk';
-import { useState, useCallback, useEffect } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { useTranslation } from 'react-i18next';
-import { useLocation, useNavigate } from 'react-router';
 import { ALL_NAMESPACES_KEY } from '../../consts';
 import { adjustToStartOfWeek } from '../pipelines-metrics/utils';
+import {
+  getQueryArgument,
+  removeQueryArgument,
+  setQueryArgument,
+} from '../utils/router';
 
 export const alphanumericCompare = (a: string, b: string): number => {
   return a.localeCompare(b, undefined, {
@@ -277,73 +281,24 @@ export const getFilter = (date, parentName, kind): string => {
 };
 
 export const useQueryParams = (param) => {
-  const {
-    key,
-    setValue,
-    defaultValue,
-    options,
-    displayFormat,
-    loadFormat,
-    value,
-  } = param;
-  const [isLoaded, setIsLoaded] = useState(0);
-  const navigate = useNavigate();
-  const location = useLocation();
-  const queryParams = {};
-  location.search
-    .substring(1)
-    ?.split('&')
-    .forEach((_) => {
-      const [key, value] = _.split('=');
-      if (key) queryParams[key] = value;
-    });
-
-  function setQueryParams(key?: string, value?: string) {
-    const path = location.pathname;
-
-    if (key && value) queryParams[key] = value;
-    navigate(
-      `${path}?${Object.keys(queryParams)
-        .map((k) => {
-          const v = queryParams[k];
-          if (k) return k + '=' + v;
-        })
-        .join('&')}`,
-    );
-  }
+  const { key, setValue, options, displayFormat, loadFormat, value } = param;
 
-  //Loads Url Params Data
-  useEffect(() => {
-    if (queryParams[key]) {
-      const paramValue = queryParams[key];
-      if (!options || options[paramValue])
-        setValue(loadFormat ? loadFormat(paramValue) : paramValue);
-    }
-  }, []);
-  //If Url Params doesn't contain a key, initializes with defaultValue
   useEffect(() => {
-    if (isLoaded >= 0) {
-      if (!queryParams[key]) {
-        const newValue = displayFormat
-          ? displayFormat(defaultValue)
-          : defaultValue;
-        if (newValue) {
-          setQueryParams(key, newValue);
-          setIsLoaded(isLoaded + 1);
-        }
-      } else {
-        setIsLoaded(-1);
+    const urlValue = getQueryArgument(key);
+    if (urlValue) {
+      if (!options || options[urlValue]) {
+        setValue(loadFormat ? loadFormat(urlValue) : urlValue);
       }
     }
-  }, [isLoaded]);
-  //Updating Url Params when values of filter changes
+    return () => removeQueryArgument(key);
+  }, []);
+
   useEffect(() => {
-    const newValue = displayFormat ? displayFormat(value) : value;
-    if (newValue) {
-      setQueryParams(key, newValue);
-    } else if (queryParams[key]) {
-      delete queryParams[key];
-      setQueryParams();
+    const formatted = displayFormat ? displayFormat(value) : value;
+    if (formatted) {
+      setQueryArgument(key, String(formatted));
+    } else {
+      removeQueryArgument(key);
     }
   }, [value]);
 };

src/components/pipelines-tasks/tasks-details-pages/TaskRunDetailsPage.tsx

@@ -16,7 +16,7 @@ import { useTranslation } from 'react-i18next';
 import TaskRunDetails from './TaskRunDetails';
 import TaskRunLogsTab from './TaskRunLogsTab';
 import TaskRunEvents from './events/TaskRunEvents';
-import { useTaskRun } from '../../hooks/useTaskRuns';
+import { useTaskRuns } from '../../hooks/useTaskRuns';
 import { navFactory } from '../../utils/horizontal-nav';
 import ResourceYAMLEditorViewOnly from '../../yaml-editor/ResourceYAMLEditorViewOnly';
 import {
@@ -31,7 +31,15 @@ const TaskRunDetailsPage = () => {
   const { t } = useTranslation('plugin__pipelines-console-plugin');
   const params = useParams();
   const { name, ns: namespace } = params;
-  const [data, k8sLoaded, trLoaded] = useTaskRun(namespace, name);
+  /* Returning 1 taskrun */
+  const [taskRuns, k8sLoaded, trLoaded] = useTaskRuns(
+    namespace,
+    undefined, // pipelineRunName
+    undefined, // taskName — this is a label filter so can't be used for fetching 1 item without some refactoring
+    undefined, // pipelineRunUid
+    { name, limit: 1 },
+  );
+  const data = taskRuns?.[0];
   const loaded = k8sLoaded || trLoaded;
   const trStatus = useMemo(
     () => loaded && data && taskRunStatus(data),