Skip to content

Commit

Permalink
[NU-1977] [NU-1949] Window components fixes (#7508)
Browse files Browse the repository at this point in the history
* 31+ days in windowLength are not lost, small fe fix

* Added default values to time window components, so there will be less red alerts
Also changed default in session window for `endSessionCondition` from true to false

---------

Co-authored-by: Szymon Bogusz <[email protected]>
  • Loading branch information
ForrestFairy and Szymon Bogusz authored Jan 30, 2025
1 parent 9090c86 commit bb92ddb
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type Props = {
const SPEL_DURATION_SWITCHABLE_TO_REGEX =
/^T\(java\.time\.Duration\)\.parse\('(-)?P([0-9]{1,}D)?(T((-)?[0-9]{1,}H)?((-)?[0-9]{1,}M)?((-)?[0-9]{1,}S)?)?'\)$/;
const NONE_DURATION = {
days: () => null,
asDays: () => null,
hours: () => null,
minutes: () => null,
seconds: () => null,
Expand Down Expand Up @@ -60,14 +60,7 @@ export const DurationEditor: ExtendedEditor<Props> = (props: Props) => {
(expression: string): Duration => {
const decodeExecResult = durationFormatter.decode(expression);

const duration =
decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult);
return {
days: duration.days(),
hours: duration.hours(),
minutes: duration.minutes(),
seconds: duration.seconds(),
};
return duration(decodeExecResult);
},
[durationFormatter],
);
Expand Down Expand Up @@ -97,3 +90,13 @@ DurationEditor.notSwitchableToHint = () =>
"editors.duration.noSwitchableToHint",
"Expression must match pattern T(java.time.Duration).parse('P(n)DT(n)H(n)M') to switch to basic mode",
);

export function duration(decodeExecResult) {
const duration = decodeExecResult == null || typeof decodeExecResult !== "string" ? NONE_DURATION : moment.duration(decodeExecResult);
return {
days: Math.floor(duration.asDays()),
hours: duration.hours(),
minutes: duration.minutes(),
seconds: duration.seconds(),
};
}
31 changes: 29 additions & 2 deletions designer/client/test/Editors/DurationEditor-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import * as React from "react";

import { render, screen } from "@testing-library/react";
import { DualEditorMode, EditorType } from "../../src/components/graph/node-modal/editors/expression/Editor";
import { DurationEditor } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor";
import { DurationEditor, duration } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor";
import { TimeRange } from "../../src/components/graph/node-modal/editors/expression/Duration/TimeRangeComponent";
import { mockFormatter, mockFieldErrors, mockValueChange } from "./helpers";
import { NuThemeProvider } from "../../src/containers/theme/nuThemeProvider";
import { nodeInputWithError } from "../../src/components/graph/node-modal/NodeDetailsContent/NodeTableStyled";
import { FormatterType, typeFormatters } from "../../src/components/graph/node-modal/editors/expression/Formatter";
import type { Duration } from "../../src/components/graph/node-modal/editors/expression/Duration/DurationEditor";

describe(DurationEditor.name, () => {
it("should display validation error when the field is required", () => {
Expand All @@ -32,3 +33,29 @@ describe(DurationEditor.name, () => {
expect(screen.getByText("validation error")).toBeInTheDocument();
});
});

describe(`${duration.name} function`, () => {
it("should parse duration without loosing days if more than 31", () => {
const emptyDuration: Duration = { days: 0, hours: 0, minutes: 0, seconds: 0 };
const formatter = typeFormatters[FormatterType.Duration];
const oneMinute = "PT1M";
const oneHour = "PT1H";
const almostOneDay = "PT23H";
const oneDayOneHour = "PT25H";
const oneDay = "P1D";
const fortyDays = "P40D";
const mix = "P1DT1H1M";

const times = [oneMinute, oneHour, almostOneDay, oneDayOneHour, oneDay, fortyDays, mix];
const results = [
{ ...emptyDuration, minutes: 1 },
{ ...emptyDuration, hours: 1 },
{ ...emptyDuration, hours: 23 },
{ ...emptyDuration, days: 1, hours: 1 },
{ ...emptyDuration, days: 1 },
{ ...emptyDuration, days: 40 },
{ days: 1, hours: 1, minutes: 1, seconds: 0 },
];
expect(times.map(formatter.decode).map(duration)).toStrictEqual(results);
});
});
5 changes: 5 additions & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@
cannot overlap, if they do, an exception is thrown.
* [#7504](https://github.com/TouK/nussknacker/pull/7504) Return scenario validation error when an incompatible change was introduced in a fragment or component parameter definition.
* [#7468](https://github.com/TouK/nussknacker/pull/7468) Configurable namespace separator (was fixed to `_`), added namespace tag to Lite engine metrics and fixed namespacing of Kafka consumer groups.
* [#7508](https://github.com/TouK/nussknacker/pull/7508) Fixes for window components:
* Can now pass more than 31 days as `windowLength` and it won't be reduced to remainder of 31
* Introduced some default values:
* For all - default `windowLength` is 1 hour
* For `aggregate-session` - default `endSessionCondition` is now false

## 1.18

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object sampleTransformers {
defaultMode = DualEditorMode.SIMPLE
) aggregator: Aggregator,
@ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef],
@ParamName("windowLength") length: java.time.Duration,
@ParamName("windowLength") @DefaultValue("T(java.time.Duration).parse('PT1H')") length: java.time.Duration,
@ParamName("emitWhenEventLeft") @DefaultValue("false") emitWhenEventLeft: Boolean,
@OutputVariableName variableName: String
)(implicit nodeId: NodeId): ContextTransformation = {
Expand Down Expand Up @@ -112,7 +112,7 @@ object sampleTransformers {
defaultMode = DualEditorMode.SIMPLE
) aggregator: Aggregator,
@ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef],
@ParamName("windowLength") length: java.time.Duration,
@ParamName("windowLength") @DefaultValue("T(java.time.Duration).parse('PT1H')") length: java.time.Duration,
@ParamName("emitWhen") trigger: TumblingWindowTrigger,
@OutputVariableName variableName: String
)(implicit nodeId: NodeId): ContextTransformation = {
Expand Down Expand Up @@ -174,8 +174,10 @@ object sampleTransformers {
defaultMode = DualEditorMode.SIMPLE
) aggregator: Aggregator,
@ParamName("aggregateBy") aggregateBy: LazyParameter[AnyRef],
@ParamName("endSessionCondition") endSessionCondition: LazyParameter[java.lang.Boolean],
@ParamName("sessionTimeout") sessionTimeout: java.time.Duration,
@ParamName("endSessionCondition") @DefaultValue("false") endSessionCondition: LazyParameter[java.lang.Boolean],
@ParamName("sessionTimeout") @DefaultValue(
"T(java.time.Duration).parse('PT1H')"
) sessionTimeout: java.time.Duration,
@ParamName("emitWhen") trigger: SessionWindowTrigger,
@OutputVariableName variableName: String
)(implicit nodeId: NodeId): ContextTransformation = {
Expand Down

0 comments on commit bb92ddb

Please sign in to comment.