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 all 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
23 changes: 18 additions & 5 deletions designer/client/src/components/modals/MoreScenarioDetailsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Box, styled, Typography } from "@mui/material";
import { Box, Skeleton, styled, Typography } from "@mui/material";
import { WindowButtonProps, WindowContentProps } from "@touk/window-manager";
import React, { useMemo } from "react";
import { useTranslation } from "react-i18next";
Expand All @@ -10,6 +10,8 @@ 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";
import LoaderSpinner from "../spinner/Spinner";

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

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

const displayStatus = !scenario.isArchived && !scenario.isFragment;
const displayLabels = scenario.labels.length !== 0;

if (isAllCombinationsLoading) {
return <LoaderSpinner show={true} />;
}

return (
<WindowContent
{...props}
Expand Down Expand Up @@ -73,10 +84,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>
{isCategoryFieldVisible && (
<ItemWrapperStyled>
<ItemLabelStyled>{i18next.t("scenarioDetails.label.category", "Category")}</ItemLabelStyled>
<Typography variant={"caption"}>{scenario.processCategory}</Typography>
</ItemWrapperStyled>
)}
<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,35 +1,17 @@
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";
import { useTranslation } from "react-i18next";
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 { t } = useTranslation();
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
Loading
Loading