Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Nu-1890] hide categories from a scenarios list and more scenario details when only one category is available #7183

Merged
merged 6 commits into from
Nov 21, 2024
Merged

[Nu-1890] hide categories from a scenarios list and more scenario details when only one category is available #7183

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e7d988a
NU-1890 hide categories from a scenarios list and more scenario detai…
Dzuming Nov 20, 2024
b519119
NU-1890 update changelog
Dzuming Nov 20, 2024
022b1a2
NU-1890 change endpoint
Dzuming Nov 21, 2024
495e87c
Merge branch 'refs/heads/release/1.18' into NU-1890-hide-categories-w…
Dzuming Nov 21, 2024
dbfd5d7
NU-1890 provide spinner until categories visibility determined
Dzuming Nov 21, 2024
28de0bd
Merge branch 'refs/heads/release/1.18' into NU-1890-hide-categories-w…
Dzuming Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions designer/client/src/components/AddProcessDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { WindowButtonProps, WindowContentProps } from "@touk/window-manager";
import React, { useCallback, useEffect, useMemo, useState } from "react";
import React, { useCallback, useMemo, useState } from "react";
import { useTranslation } from "react-i18next";
import { visualizationUrl } from "../common/VisualizationUrl";
import { useProcessNameValidators } from "../containers/hooks/useProcessNameValidators";
import HttpService, { ProcessingMode, ScenarioParametersCombination } from "../http/HttpService";
import HttpService, { ProcessingMode } from "../http/HttpService";
import { WindowContent } from "../windowManager";
import { AddProcessForm, FormValue, TouchedValue } from "./AddProcessForm";
import { extendErrors, mandatoryValueValidator } from "./graph/node-modal/editors/Validators";
Expand All @@ -12,6 +12,7 @@ import { NodeValidationError } from "../types";
import { flow, isEmpty, transform } from "lodash";
import { useProcessFormDataOptions } from "./useProcessFormDataOptions";
import { LoadingButtonTypes } from "../windowManager/LoadingButton";
import { useGetAllCombinations } from "./useGetAllCombinations";

interface AddProcessDialogProps extends WindowContentProps {
isFragment?: boolean;
Expand All @@ -22,16 +23,26 @@ export function AddProcessDialog(props: AddProcessDialogProps): JSX.Element {
const { t } = useTranslation();
const { isFragment = false, errors = [], ...passProps } = props;
const nameValidators = useProcessNameValidators();
const [value, setState] = useState<FormValue>({ processName: "", processCategory: "", processingMode: "", processEngine: "" });
const [value, setState] = useState<FormValue>({
processName: "",
processCategory: "",
processingMode: "" as ProcessingMode,
processEngine: "",
});
const [touched, setTouched] = useState<TouchedValue>({
processName: false,
processCategory: false,
processingMode: false,
processEngine: false,
});
const [processNameFromBackend, setProcessNameFromBackendError] = useState<NodeValidationError[]>([]);
const [engineSetupErrors, setEngineSetupErrors] = useState<Record<string, string[]>>({});
const [allCombinations, setAllCombinations] = useState<ScenarioParametersCombination[]>([]);

const { engineSetupErrors, allCombinations } = useGetAllCombinations({
processCategory: value.processCategory,
processingMode: value.processingMode,
processEngine: value.processEngine,
});

const engineErrors: NodeValidationError[] = (engineSetupErrors[value.processEngine] ?? []).map((error) => ({
fieldName: "processEngine",
errorType: "SaveNotAllowed",
Expand Down Expand Up @@ -122,12 +133,6 @@ export function AddProcessDialog(props: AddProcessDialogProps): JSX.Element {
setTouched(touched);
};

useEffect(() => {
HttpService.fetchScenarioParametersCombinations().then((response) => {
setAllCombinations(response.data.combinations);
setEngineSetupErrors(response.data.engineSetupErrors);
});
}, []);
return (
<WindowContent buttons={buttons} {...passProps}>
<AddProcessForm
Expand Down
2 changes: 1 addition & 1 deletion designer/client/src/components/AddProcessForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { InfoOutlined } from "@mui/icons-material";
import Input from "./graph/node-modal/editors/field/Input";
import { formLabelWidth } from "../containers/theme/styles";

export type FormValue = { processName: string; processCategory: string; processingMode: string; processEngine: string };
export type FormValue = { processName: string; processCategory: string; processingMode: ProcessingMode; processEngine: string };

export type TouchedValue = Record<keyof FormValue, boolean>;

Expand Down
16 changes: 12 additions & 4 deletions designer/client/src/components/modals/MoreScenarioDetailsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import i18next from "i18next";
import { capitalize, startCase } from "lodash";
import { getProcessingModeVariantName } from "../toolbars/scenarioDetails/getProcessingModeVariantName";
import NuLogoIcon from "../../assets/img/nussknacker-logo-icon.svg";
import { useGetAllCombinations } from "../useGetAllCombinations";

const ItemWrapperStyled = styled("div")({ display: "grid", gridAutoColumns: "minmax(0, 1fr)", gridAutoFlow: "column" });

Expand Down Expand Up @@ -38,6 +39,11 @@ function MoreScenarioDetailsDialog(props: WindowContentProps<WindowKind, Props>)
],
[props, t],
);
const { isAllCombinationsLoading, isCategoryFieldVisible } = useGetAllCombinations({
processCategory: scenario.processCategory,
processingMode: scenario.processingMode,
processEngine: scenario.engineSetupName,
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect visibility logic

The current implementation shows the category when isAllCombinationsLoading is true, which seems incorrect for two reasons:

  1. Using a loading state to control visibility could cause unwanted UI flashing
  2. This appears to be the opposite of the PR's objective to hide categories when there's only one available

Consider updating the logic to:

-    const { isAllCombinationsLoading, isCategoryFieldVisible } = useGetAllCombinations({
+    const { isCategoryFieldVisible } = useGetAllCombinations({
         processCategory: scenario.processCategory,
         processingMode: scenario.processingMode,
         processEngine: scenario.engineSetupName,
     });

Committable suggestion skipped: line range outside the PR's diff.


const displayStatus = !scenario.isArchived && !scenario.isFragment;
const displayLabels = scenario.labels.length !== 0;
Expand Down Expand Up @@ -73,10 +79,12 @@ function MoreScenarioDetailsDialog(props: WindowContentProps<WindowKind, Props>)
<ItemLabelStyled>{i18next.t("scenarioDetails.label.processingMode", "Processing mode")}</ItemLabelStyled>
<Typography variant={"caption"}>{getProcessingModeVariantName(scenario.processingMode)}</Typography>
</ItemWrapperStyled>
<ItemWrapperStyled>
<ItemLabelStyled>{i18next.t("scenarioDetails.label.category", "Category")}</ItemLabelStyled>
<Typography variant={"caption"}>{scenario.processCategory}</Typography>
</ItemWrapperStyled>
{isAllCombinationsLoading && isCategoryFieldVisible && (
<ItemWrapperStyled>
<ItemLabelStyled>{i18next.t("scenarioDetails.label.category", "Category")}</ItemLabelStyled>
<Typography variant={"caption"}>{scenario.processCategory}</Typography>
</ItemWrapperStyled>
)}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Loading state should not affect category visibility

The review comment is correct. After examining the useGetAllCombinations hook implementation, I can confirm that:

  1. isAllCombinationsLoading is just a loading indicator for the API call that fetches combinations data
  2. isCategoryFieldVisible is determined independently by useProcessFormDataOptions based on the current combinations and form values
  3. The loading state should not affect whether the category field is displayed

The current condition isAllCombinationsLoading && isCategoryFieldVisible is incorrect as it will hide the category field when the data is not loading, even if isCategoryFieldVisible is true. The category's visibility should depend solely on isCategoryFieldVisible.

🔗 Analysis chain

Simplify category visibility condition

The category visibility should depend solely on isCategoryFieldVisible, not the loading state.

Apply this change:

-    {isAllCombinationsLoading && isCategoryFieldVisible && (
+    {isCategoryFieldVisible && (
         <ItemWrapperStyled>
             <ItemLabelStyled>{i18next.t("scenarioDetails.label.category", "Category")}</ItemLabelStyled>
             <Typography variant={"caption"}>{scenario.processCategory}</Typography>
         </ItemWrapperStyled>
     )}

Let's verify the behavior of useGetAllCombinations hook:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the implementation and usage of useGetAllCombinations
ast-grep --pattern 'export function useGetAllCombinations' -A 20

Length of output: 66

Script:

#!/bin/bash
# Let's try searching for any usage of useGetAllCombinations
rg "useGetAllCombinations" -A 3

# Also search for any hooks related to combinations
rg "useCombinations?" -A 3

# Let's also check the component's implementation
fd "MoreScenarioDetailsDialog" --exec cat {}

Length of output: 9311

Script:

#!/bin/bash
# Let's check the implementation of useGetAllCombinations hook
cat designer/client/src/components/useGetAllCombinations.ts

Length of output: 1512

<ItemWrapperStyled>
<ItemLabelStyled>{i18next.t("scenarioDetails.label.engine", "Engine")}</ItemLabelStyled>
<Typography variant={"caption"}>{scenario.engineSetupName}</Typography>
Expand Down
30 changes: 6 additions & 24 deletions designer/client/src/components/toolbars/scenarioDetails/CategoryDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,15 @@
import React, { useEffect, useState } from "react";
import { useProcessFormDataOptions } from "../../useProcessFormDataOptions";
import HttpService, { ScenarioParametersCombination } from "../../../http/HttpService";
import React from "react";
import { Skeleton, Typography } from "@mui/material";
import { Scenario } from "../../Process/types";
import { useGetAllCombinations } from "../../useGetAllCombinations";

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Hook requires test coverage for critical functionality

The useGetAllCombinations hook contains important business logic including:

  • Fetching scenario parameter combinations
  • Managing loading states
  • Determining category field visibility
  • Handling engine setup errors

Currently, there are no dedicated tests for this hook. Given its critical role in:

  • State management
  • API integration
  • Visibility logic
  • Error handling

Test coverage should be added to ensure reliability.

Key areas to test:

  • Successful data fetching and state updates
  • Loading state transitions
  • Category visibility logic
  • Error state handling
  • Edge cases with empty or error responses
🔗 Analysis chain

Verify test coverage for useGetAllCombinations hook

The component now relies on this hook for critical functionality. Ensure comprehensive test coverage exists for the hook, especially for edge cases around category visibility determination.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for test coverage of the useGetAllCombinations hook
# Look for test files related to the hook
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts useGetAllCombinations

# Search for test cases covering visibility logic
rg -l "describe.*useGetAllCombinations|test.*useGetAllCombinations|it.*useGetAllCombinations"

Length of output: 168

Script:

#!/bin/bash
# Let's try a broader search to locate the hook file first and any potential test files
fd useGetAllCombinations

# Also search for any test files in the same directory as the component
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts . designer/client/src/components/

# Look for any test assertions or descriptions containing "combinations" which might be related
rg -l "describe.*combination|test.*combination|it.*combination" designer/client/src/

Length of output: 621

Script:

#!/bin/bash
# Let's examine the hook implementation to understand its complexity and test requirements
cat designer/client/src/components/useGetAllCombinations.ts

# Check if there are any existing tests that might cover this hook indirectly
rg -A 5 -B 5 "useGetAllCombinations" designer/client/src/components/toolbars/scenarioDetails/CategoryDetails.test.tsx

# Look for any integration tests that might cover this functionality
rg -A 5 "CategoryDetails" designer/client/src/components/toolbars/scenarioDetails/__tests__

Length of output: 1949

export const CategoryDetails = ({ scenario }: { scenario: Scenario }) => {
const [allCombinations, setAllCombinations] = useState<ScenarioParametersCombination[]>([]);
const [isAllCombinationsLoading, setIsAllCombinationsLoading] = useState<boolean>(false);

const { isCategoryFieldVisible } = useProcessFormDataOptions({
allCombinations,
value: {
processCategory: scenario.processCategory,
processingMode: scenario.processingMode,
processEngine: scenario.engineSetupName,
},
const { isAllCombinationsLoading, isCategoryFieldVisible } = useGetAllCombinations({
processCategory: scenario.processCategory,
processingMode: scenario.processingMode,
processEngine: scenario.engineSetupName,
});

useEffect(() => {
setIsAllCombinationsLoading(true);
HttpService.fetchScenarioParametersCombinations()
.then((response) => {
setAllCombinations(response.data.combinations);
})
.finally(() => {
setIsAllCombinationsLoading(false);
});
}, []);

return (
<>
{isAllCombinationsLoading ? (
Expand Down
37 changes: 37 additions & 0 deletions designer/client/src/components/useGetAllCombinations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { useEffect, useState } from "react";
import HttpService, { ProcessingMode, ScenarioParametersCombination } from "../http/HttpService";
import { useProcessFormDataOptions } from "./useProcessFormDataOptions";

interface Props {
processCategory: string;
processingMode: ProcessingMode;
processEngine: string;
}
export const useGetAllCombinations = ({ processCategory, processingMode, processEngine }: Props) => {
const [allCombinations, setAllCombinations] = useState<ScenarioParametersCombination[]>([]);
const [engineSetupErrors, setEngineSetupErrors] = useState<Record<string, string[]>>({});
const [isAllCombinationsLoading, setIsAllCombinationsLoading] = useState<boolean>(false);

const { isCategoryFieldVisible } = useProcessFormDataOptions({
allCombinations,
value: {
processCategory,
processingMode,
processEngine,
},
});

useEffect(() => {
setIsAllCombinationsLoading(true);
HttpService.fetchScenarioParametersCombinations()
.then((response) => {
setAllCombinations(response.data.combinations);
setEngineSetupErrors(response.data.engineSetupErrors);
})
.finally(() => {
setIsAllCombinationsLoading(false);
});
}, []);
Comment on lines +24 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and cleanup to the fetch effect

The current implementation lacks error handling and cleanup for the fetch request.

Consider these improvements:

     useEffect(() => {
+        const abortController = new AbortController();
         setIsAllCombinationsLoading(true);
-        HttpService.fetchScenarioParametersCombinations()
+        HttpService.fetchScenarioParametersCombinations({ signal: abortController.signal })
             .then((response) => {
                 setAllCombinations(response.data.combinations);
                 setEngineSetupErrors(response.data.engineSetupErrors);
             })
+            .catch((error) => {
+                if (!error.name === 'AbortError') {
+                    console.error('Failed to fetch scenario parameters:', error);
+                    setEngineSetupErrors(prev => ({
+                        ...prev,
+                        fetch: ['Failed to load scenario parameters']
+                    }));
+                }
+            })
             .finally(() => {
                 setIsAllCombinationsLoading(false);
             });
+        return () => {
+            abortController.abort();
+        };
     }, []);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
setIsAllCombinationsLoading(true);
HttpService.fetchScenarioParametersCombinations()
.then((response) => {
setAllCombinations(response.data.combinations);
setEngineSetupErrors(response.data.engineSetupErrors);
})
.finally(() => {
setIsAllCombinationsLoading(false);
});
}, []);
useEffect(() => {
const abortController = new AbortController();
setIsAllCombinationsLoading(true);
HttpService.fetchScenarioParametersCombinations({ signal: abortController.signal })
.then((response) => {
setAllCombinations(response.data.combinations);
setEngineSetupErrors(response.data.engineSetupErrors);
})
.catch((error) => {
if (!error.name === 'AbortError') {
console.error('Failed to fetch scenario parameters:', error);
setEngineSetupErrors(prev => ({
...prev,
fetch: ['Failed to load scenario parameters']
}));
}
})
.finally(() => {
setIsAllCombinationsLoading(false);
});
return () => {
abortController.abort();
};
}, []);


return { allCombinations, isAllCombinationsLoading, isCategoryFieldVisible, engineSetupErrors };
};
29 changes: 16 additions & 13 deletions designer/submodules/packages/components/src/scenarios/filters/filtersPart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useCallback, useMemo } from "react";
import { flatten, sortBy, uniq } from "lodash";
import { useFilterContext } from "../../common";
import { ScenariosFiltersModel, ScenariosFiltersModelType } from "./scenariosFiltersModel";
import { useScenarioLabelsQuery, useStatusDefinitions, useUserQuery } from "../useScenariosQuery";
import { useScenarioLabelsQuery, useScenariosWithCategoryVisible, useStatusDefinitions, useUserQuery } from "../useScenariosQuery";
import { QuickFilter } from "./quickFilter";
import { FilterMenu } from "./filterMenu";
import { SimpleOptionsStack } from "./simpleOptionsStack";
Expand All @@ -21,6 +21,7 @@ export function FiltersPart({ withSort, isLoading, data = [] }: { data: RowType[
const { data: userData } = useUserQuery();
const { data: statusDefinitions = [] } = useStatusDefinitions();
const { data: availableLabels } = useScenarioLabelsQuery();
const { withCategoriesVisible } = useScenariosWithCategoryVisible();

const filterableKeys = useMemo(() => ["createdBy", "modifiedBy"], []);
const filterableValues = useMemo(() => {
Expand All @@ -35,7 +36,7 @@ export function FiltersPart({ withSort, isLoading, data = [] }: { data: RowType[
label: (availableLabels?.labels || []).map((name) => ({ name })),
processingMode: processingModeItems,
};
}, [data, filterableKeys, statusDefinitions, userData?.categories]);
}, [availableLabels?.labels, data, filterableKeys, statusDefinitions, userData?.categories]);

const statusFilterLabels = statusDefinitions.reduce((map, obj) => {
map[obj.name] = obj.displayableName;
Expand Down Expand Up @@ -102,17 +103,19 @@ export function FiltersPart({ withSort, isLoading, data = [] }: { data: RowType[
})}
/>
</FilterMenu>
<FilterMenu label={t("table.filter.CATEGORY", "Category")} count={getFilter("CATEGORY", true).length}>
<SimpleOptionsStack
label={t("table.filter.CATEGORY", "Category")}
options={filterableValues.processCategory}
value={getFilter("CATEGORY", true)}
onChange={setFilter("CATEGORY")}
{...getEventTrackingProps({
selector: EventTrackingSelector.ScenariosByCategory,
})}
/>
</FilterMenu>
{withCategoriesVisible && (
<FilterMenu label={t("table.filter.CATEGORY", "Category")} count={getFilter("CATEGORY", true).length}>
<SimpleOptionsStack
label={t("table.filter.CATEGORY", "Category")}
options={filterableValues.processCategory}
value={getFilter("CATEGORY", true)}
onChange={setFilter("CATEGORY")}
{...getEventTrackingProps({
selector: EventTrackingSelector.ScenariosByCategory,
})}
/>
</FilterMenu>
)}
<FilterMenu label={t("table.filter.LABEL", "Label")} count={getFilter("LABEL", true).length}>
<SimpleOptionsStack
label={t("table.filter.LABEL", "Label")}
Expand Down
10 changes: 8 additions & 2 deletions designer/submodules/packages/components/src/scenarios/list/item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ScenarioStatus } from "./scenarioStatus";
import { ProcessingModeItem } from "./processingMode";
import { formatDateTime } from "nussknackerUi/DateUtils";
import { LabelChip } from "../../common/labelChip";
import { useScenariosWithCategoryVisible } from "../useScenariosQuery";

function Category({
category,
Expand Down Expand Up @@ -60,11 +61,16 @@ const HighlightedName = styled(Highlight)({
export function FirstLine({ row }: { row: RowType }): JSX.Element {
const { t } = useTranslation();
const filtersContext = useFilterContext<ScenariosFiltersModel>();
const { withCategoriesVisible } = useScenariosWithCategoryVisible();

return (
<div style={{ display: "flex" }}>
<Category category={row.processCategory} filtersContext={filtersContext} />
<span style={{ paddingLeft: 8, paddingRight: 8 }}>/</span>
{withCategoriesVisible && (
<>
<Category category={row.processCategory} filtersContext={filtersContext} />
<span style={{ paddingLeft: 8, paddingRight: 8 }}>/</span>
</>
)}
<CopyTooltip text={row.name} title={t("scenario.copyName", "Copy name to clipboard")}>
<HighlightedName value={row.name} filterText={filtersContext.getFilter("NAME")} />
</CopyTooltip>
Expand Down
29 changes: 17 additions & 12 deletions designer/submodules/packages/components/src/scenarios/list/tablePart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ import AssessmentIcon from "@mui/icons-material/Assessment";
import { LastAction } from "./item";
import { getEventTrackingProps, EventTrackingSelector } from "nussknackerUi/eventTracking";
import { formatDateTime } from "nussknackerUi/DateUtils";
import { useScenariosWithCategoryVisible } from "../useScenariosQuery";

export function TablePart(props: ListPartProps<RowType>): JSX.Element {
const { data = [], isLoading } = props;
const { t } = useTranslation();
const filtersContext = useFilterContext<ScenariosFiltersModel>();
const _filterText = useMemo(() => filtersContext.getFilter("NAME"), [filtersContext]);
const [filterText] = useDebouncedValue(_filterText, 400);
const { withCategoriesVisible } = useScenariosWithCategoryVisible();

const columns = useMemo(
(): Columns<RowType> => [
const columns = useMemo((): Columns<RowType> => {
const availableColumns: Columns<RowType | undefined> = [
{
field: "id",
cellClassName: "noPadding stretch",
Expand All @@ -31,13 +33,15 @@ export function TablePart(props: ListPartProps<RowType>): JSX.Element {
minWidth: 200,
flex: 2,
},
{
field: "processCategory",
cellClassName: "noPadding stretch",
headerName: t("table.scenarios.title.PROCESS_CATEGORY", "Category"),
renderCell: (props) => <FilterLinkCell<ScenariosFiltersModel> filterKey="CATEGORY" {...props} />,
flex: 1,
},
withCategoriesVisible
? {
field: "processCategory",
cellClassName: "noPadding stretch",
headerName: t("table.scenarios.title.PROCESS_CATEGORY", "Category"),
renderCell: (props) => <FilterLinkCell<ScenariosFiltersModel> filterKey="CATEGORY" {...props} />,
flex: 1,
}
: undefined,
{
field: "createdBy",
cellClassName: "noPadding stretch",
Expand Down Expand Up @@ -107,9 +111,10 @@ export function TablePart(props: ListPartProps<RowType>): JSX.Element {
sortable: false,
align: "center",
},
],
[filterText, t],
);
];

return availableColumns.filter((data) => data !== undefined);
}, [filterText, t, withCategoriesVisible]);

const [visibleColumns, setVisibleColumns] = useState(
columns.reduce((previousValue, currentValue) => {
Expand Down
36 changes: 34 additions & 2 deletions designer/submodules/packages/components/src/scenarios/useScenariosQuery.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import { UserData } from "nussknackerUi/common/models/User";
import { useContext, useEffect, useMemo } from "react";
import { NkApiContext } from "../settings/nkApiProvider";
import { Scenario, StatusDefinitionType } from "nussknackerUi/components/Process/types";
import { StatusesType } from "nussknackerUi/HttpService";
import { ScenarioParametersCombinations, StatusesType } from "nussknackerUi/HttpService";
import { useQuery, useQueryClient } from "react-query";
import { AvailableScenarioLabels } from "nussknackerUi/components/Labels/types";
import { UseQueryResult } from "react-query/types/react/types";
import { DateTime } from "luxon";
import { groupBy } from "lodash";

const scenarioStatusesQueryKey = "scenariosStatuses";
const scenarioParametersCombinationsQueryKey = "scenarioParametersCombinations";

function useScenariosQuery(): UseQueryResult<Scenario[]> {
const api = useContext(NkApiContext);
Expand Down Expand Up @@ -53,6 +55,22 @@ export function useScenariosStatusesQuery(): UseQueryResult<StatusesType> {
});
}

export function useScenarioParametersCombinationsQuery(): UseQueryResult<ScenarioParametersCombinations> {
const api = useContext(NkApiContext);
return useQuery({
queryKey: [scenarioParametersCombinationsQueryKey],
queryFn: async () => {
const { data } = await api.fetchScenarioParametersCombinations();
return data;
},
enabled: !!api,
refetchInterval: 15000,
// We have to define staleTime because we set cache manually via queryClient.setQueryData during fetching scenario
// details (because we want to avoid unnecessary refetch)
staleTime: 10000,
});
}

export function useStatusDefinitions(): UseQueryResult<StatusDefinitionType[]> {
const api = useContext(NkApiContext);
return useQuery({
Expand Down Expand Up @@ -102,8 +120,22 @@ export function useScenariosWithStatus(): UseQueryResult<Scenario[]> {
data: data.map((scenario) => ({
...scenario,
state: statuses?.data?.[scenario.name] || scenario.state,
id: scenario.name, // required by DataGrid when table=true
id: scenario.name, // required by DataGrid when table=true,
})),
} as UseQueryResult<Scenario[]>;
}, [scenarios, statuses]);
}

export function useScenariosWithCategoryVisible(): { withCategoriesVisible: boolean } {
const parametersCombinations = useScenarioParametersCombinationsQuery();
return useMemo(() => {
const { data } = parametersCombinations;
const combinations = data?.combinations || [];

const withCategoriesVisible = Object.keys(groupBy(combinations, (combination) => combination.category)).length > 1;

return {
withCategoriesVisible,
};
}, [parametersCombinations]);
}
Comment on lines +129 to +141
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider exposing loading and error states for better UX handling.

While the implementation is functionally correct, consider these improvements for better error handling and loading state management:

-export function useScenariosWithCategoryVisible(): { withCategoriesVisible: boolean } {
+export function useScenariosWithCategoryVisible(): {
+    withCategoriesVisible: boolean;
+    isLoading: boolean;
+    error: Error | null;
+} {
     const parametersCombinations = useScenarioParametersCombinationsQuery();
     return useMemo(() => {
-        const { data } = parametersCombinations;
+        const { data, isLoading, error } = parametersCombinations;
         const combinations = data?.combinations || [];
 
-        const withCategoriesVisible = Object.keys(groupBy(combinations, (combination) => combination.category)).length > 1;
+        const MINIMUM_CATEGORIES_FOR_VISIBILITY = 1;
+        const withCategoriesVisible = Object.keys(groupBy(combinations, (combination) => combination.category)).length > MINIMUM_CATEGORIES_FOR_VISIBILITY;
 
         return {
             withCategoriesVisible,
+            isLoading,
+            error,
         };
     }, [parametersCombinations]);
 }

This change will:

  1. Expose loading and error states to consumers
  2. Make the visibility threshold more explicit with a named constant
  3. Allow components to handle loading and error states appropriately
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function useScenariosWithCategoryVisible(): { withCategoriesVisible: boolean } {
const parametersCombinations = useScenarioParametersCombinationsQuery();
return useMemo(() => {
const { data } = parametersCombinations;
const combinations = data?.combinations || [];
const withCategoriesVisible = Object.keys(groupBy(combinations, (combination) => combination.category)).length > 1;
return {
withCategoriesVisible,
};
}, [parametersCombinations]);
}
export function useScenariosWithCategoryVisible(): {
withCategoriesVisible: boolean;
isLoading: boolean;
error: Error | null;
} {
const parametersCombinations = useScenarioParametersCombinationsQuery();
return useMemo(() => {
const { data, isLoading, error } = parametersCombinations;
const combinations = data?.combinations || [];
const MINIMUM_CATEGORIES_FOR_VISIBILITY = 1;
const withCategoriesVisible = Object.keys(groupBy(combinations, (combination) => combination.category)).length > MINIMUM_CATEGORIES_FOR_VISIBILITY;
return {
withCategoriesVisible,
isLoading,
error,
};
}, [parametersCombinations]);
}

Loading
Loading