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

1.18 ports10 #7285

Merged
merged 2 commits into from
Dec 5, 2024
Merged

1.18 ports10 #7285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b580599
[NU-1901] fix deployment status indicator for periodic scenarios (#7283)
Dzuming Dec 4, 2024
281a5d2
Merge branch 'release/1.18' into staging
arkadius Dec 5, 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
7 changes: 4 additions & 3 deletions designer/client/src/components/toolbars/activities/ActivitiesPanelRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export const ActivitiesPanelRow = memo(({ index, style, setRowHeight, handleShow
() => activities.findIndex((activeItem) => activeItem.uiType === "item" && activeItem.type === "SCENARIO_DEPLOYED"),
[activities],
);
const isRunning = firstDeployedIndex === index && scenarioState.status.name === "RUNNING";
const scenarioStatusesToActiveDeploy = ["RUNNING", "SCHEDULED"];
const isDeploymentActive = firstDeployedIndex === index && scenarioStatusesToActiveDeploy.includes(scenarioState.status.name);
Comment on lines +30 to +31
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Extract status constants to a shared location

Consider moving the status array to a constants file to avoid magic strings and enable reuse:

- const scenarioStatusesToActiveDeploy = ["RUNNING", "SCHEDULED"];
+ import { ACTIVE_DEPLOYMENT_STATUSES } from "../../../constants/scenarioStatuses";
+ // In constants file:
+ export const ACTIVE_DEPLOYMENT_STATUSES = ["RUNNING", "SCHEDULED"] as const;

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

const isFirstDateItem = activities.findIndex((activeItem) => activeItem.uiType === "date") === index;

useEffect(() => {
Expand All @@ -41,7 +42,7 @@ export const ActivitiesPanelRow = memo(({ index, style, setRowHeight, handleShow
case "item": {
return (
<ActivityItemProvider>
<ActivityItem activity={activity} ref={rowRef} isRunning={isRunning} searchQuery={searchQuery} />
<ActivityItem activity={activity} ref={rowRef} isDeploymentActive={isDeploymentActive} searchQuery={searchQuery} />
</ActivityItemProvider>
);
}
Expand Down Expand Up @@ -69,7 +70,7 @@ export const ActivitiesPanelRow = memo(({ index, style, setRowHeight, handleShow
return null;
}
}
}, [activity, handleHideRows, handleShowRows, isRunning, isFirstDateItem, searchQuery, t]);
}, [activity, handleHideRows, handleShowRows, isDeploymentActive, isFirstDateItem, searchQuery, t]);

return (
<div style={style} data-testid={`activity-row-${index}`}>
Expand Down
4 changes: 2 additions & 2 deletions designer/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const StyledActivityBody = styled("div")(({ theme }) => ({

export const ActivityItem = forwardRef(
(
{ activity, isRunning, searchQuery }: { activity: ItemActivity; isRunning: boolean; searchQuery: string },
{ activity, isDeploymentActive, searchQuery }: { activity: ItemActivity; isDeploymentActive: boolean; searchQuery: string },
ref: ForwardedRef<HTMLDivElement>,
) => {
const { t } = useTranslation();
Expand All @@ -55,7 +55,7 @@ export const ActivityItem = forwardRef(
>
<StyledActivityContent isActiveFound={activity.isActiveFound} isFound={activity.isFound}>
<ActivityItemHeader
isRunning={isRunning}
isDeploymentActive={isDeploymentActive}
searchQuery={searchQuery}
activity={activity}
isActiveFound={activity.isActiveFound}
Expand Down
12 changes: 6 additions & 6 deletions ...ner/client/src/components/toolbars/activities/ActivityPanelRowItem/ActivityItemHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ const StyledHeaderActionRoot = styled("div")(({ theme }) => ({
gap: theme.spacing(0.5),
}));

const StyledActivityItemHeader = styled("div")<{ isHighlighted: boolean; isRunning: boolean; isActiveFound: boolean }>(
({ theme, isHighlighted, isRunning, isActiveFound }) => ({
const StyledActivityItemHeader = styled("div")<{ isHighlighted: boolean; isDeploymentActive: boolean; isActiveFound: boolean }>(
({ theme, isHighlighted, isDeploymentActive, isActiveFound }) => ({
display: "flex",
alignItems: "center",
padding: theme.spacing(0.5, 0.5, 0.5, 0.75),
borderRadius: theme.spacing(0.5),
...getHeaderColors(theme, isHighlighted, isRunning, isActiveFound),
...getHeaderColors(theme, isHighlighted, isDeploymentActive, isActiveFound),
}),
);

Expand Down Expand Up @@ -167,7 +167,7 @@ const HeaderActivity = ({

interface Props {
activity: ItemActivity;
isRunning: boolean;
isDeploymentActive: boolean;
isActiveFound: boolean;
isFound: boolean;
searchQuery: string;
Expand Down Expand Up @@ -230,7 +230,7 @@ const WithOpenVersion = ({
);
};

const ActivityItemHeader = ({ activity, isRunning, isFound, isActiveFound, searchQuery }: Props) => {
const ActivityItemHeader = ({ activity, isDeploymentActive, isFound, isActiveFound, searchQuery }: Props) => {
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can see that there are no existing test files for the ActivityItemHeader component and its deployment status logic. The code shows that isDeploymentActive is a critical prop that affects the header's styling and behavior through getHeaderColors. Given this finding, I'll generate the final response:

Add tests for ActivityItemHeader's deployment status logic

Please create a new test file ActivityItemHeader.test.tsx to cover:

  1. Visual verification of header styling based on isDeploymentActive prop:
    • When deployment is active and found (isDeploymentActive={true}, isActiveFound={true})
    • When deployment is active but not found (isDeploymentActive={true}, isActiveFound={false})
    • When deployment is inactive (isDeploymentActive={false})
  2. Test header color changes using the getHeaderColors helper
  3. Verify header content rendering with different activity types during deployment
🔗 Analysis chain

Add tests for the new deployment status logic.

Since this change modifies how deployment status is determined, please ensure there are tests covering:

  1. Different combinations of firstDeployedIndex and scenarioState.status.name
  2. Edge cases where deployment status changes
  3. Visual verification of the header styling based on isDeploymentActive

This aligns with the PR objective requiring automated test coverage for changes.

Let's check for existing test coverage:

Also applies to: 282-282

Let me search for existing test files specifically related to ActivityItemHeader and deployment status.

Let me search for any existing test files in the activities directory to see if there's test coverage for related components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for test files related to ActivityItemHeader
fd -e test.tsx -e spec.tsx ActivityItemHeader

# Search for existing test cases related to deployment status
rg -i "test|describe|it.*deployment.*active|running" --type ts

Length of output: 67564

Script:

#!/bin/bash
# Search for test files related to ActivityItemHeader and deployment status
fd -e test.tsx -e spec.tsx ActivityItemHeader

# Search for test cases related to deployment status in ActivityItemHeader
rg -A 5 "isDeploymentActive|deployment.*status" --type ts designer/client/src/components/toolbars/activities

Length of output: 9826

Script:

#!/bin/bash
# Search for test files in the activities directory
fd -e test.tsx -e spec.tsx . designer/client/src/components/toolbars/activities/

# Check for any test coverage of deployment status in existing test files
rg -A 5 "test.*deployment|describe.*deployment" designer/client/test

Length of output: 153

const scenario = useSelector(getScenario);
const { processVersionId } = scenario || {};

Expand Down Expand Up @@ -279,7 +279,7 @@ const ActivityItemHeader = ({ activity, isRunning, isFound, isActiveFound, searc
]);

return (
<StyledActivityItemHeader isHighlighted={isHighlighted} isRunning={isRunning} isActiveFound={isActiveFound}>
<StyledActivityItemHeader isHighlighted={isHighlighted} isDeploymentActive={isDeploymentActive} isActiveFound={isActiveFound}>
<StyledHeaderIcon src={activity.activities.icon} id={activity.uiGeneratedId} />
{getHeaderTitle}
<StyledHeaderActionRoot>
Expand Down
6 changes: 3 additions & 3 deletions designer/client/src/components/toolbars/activities/helpers/activityItemColors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const runningHeaderBackground = (theme: Theme) => blend(theme.palette.background
const activeFoundItemBackground = (theme: Theme) => blend(theme.palette.background.paper, theme.palette.primary.main, 0.2);
const foundItemBackground = (theme: Theme) => blend(theme.palette.background.paper, theme.palette.primary.main, 0.08);

export const getHeaderColors = (theme: Theme, isHighlighted: boolean, isRunning: boolean, isActiveFound: boolean) => {
if (isRunning && isActiveFound) {
export const getHeaderColors = (theme: Theme, isHighlighted: boolean, isDeploymentActive: boolean, isActiveFound: boolean) => {
if (isDeploymentActive && isActiveFound) {
return {
backgroundColor: runningActiveFoundHeaderBackground(theme),
border: activeBorder(theme),
Expand All @@ -27,7 +27,7 @@ export const getHeaderColors = (theme: Theme, isHighlighted: boolean, isRunning:
};
}

if (isRunning) {
if (isDeploymentActive) {
return {
backgroundColor: runningHeaderBackground(theme),
border: defaultBorder(theme),
Expand Down
1 change: 1 addition & 0 deletions docs/Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
* [#7269](https://github.com/TouK/nussknacker/pull/7269) Fixed focus scrolling in expression editor
* [#7270](https://github.com/TouK/nussknacker/pull/7270) Savepoint deserialization fixup - some taken savepoints (e.g. for scenarios with async enrichers) were not deserializable which led to errors during redeployments on Flink
* [#7279](https://github.com/TouK/nussknacker/pull/7279) Fixed Flink TaskManager and Designer containers restarts in installation example
* [#7283](https://github.com/TouK/nussknacker/pull/7283) fix deployment status indicator for periodic scenarios type

### 1.18.0 (22 November 2024)

Expand Down
Loading