ljmotta commented on code in PR #2942:
URL:
https://github.com/apache/incubator-kie-tools/pull/2942#discussion_r1975489364
##########
packages/dmn-editor-envelope/src/NewDmnEditorEnvelopeApi.ts:
##########
@@ -18,11 +18,13 @@
*/
import { KogitoEditorEnvelopeApi } from "@kie-tools-core/editor/dist/api";
+import { NodeEvaluationResults } from "@kie-tools/dmn-editor/dist/DmnEditor";
export interface NewDmnEditorEnvelopeApi extends KogitoEditorEnvelopeApi {
/**
* Open boxed expression editor for given node
* @param nodeId id of the node to open
*/
dmnEditor_openBoxedExpressionEditor(nodeId: string): void;
+ newDmnEditor_showDmnEvaluationResults(evaluationResultsPerNode: Map<string,
NodeEvaluationResults>): void;
Review Comment:
Same as above.
##########
packages/dmn-editor-envelope/src/NewDmnEditorEnvelopeApiFactory.ts:
##########
@@ -43,4 +44,8 @@ export class NewDmnEditorEnvelopeApiImpl
public dmnEditor_openBoxedExpressionEditor(nodeId: string): void {
this.getEditorOrThrowError().openBoxedExpressionEditor(nodeId);
}
+
+ public newDmnEditor_showDmnEvaluationResults(evaluationResultsPerNode:
Map<string, NodeEvaluationResults>): void {
Review Comment:
Same as above.
##########
packages/dmn-editor-envelope/src/DmnEditorRoot.tsx:
##########
@@ -74,6 +74,7 @@ export type DmnEditorRootState = {
keyboardShortcutsRegisterIds: number[];
keyboardShortcutsRegistered: boolean;
error: Error | undefined;
+ evaluationResultsPerNode: Map<string, DmnEditor.NodeEvaluationResults>;
Review Comment:
Please, update to `evaluationResultsByNodeId`, we don't use `Per` for Maps
in the codebase, and I think is important to mention that we are using the
`NodeId`. Also, why not use the same name in the state and prop? I mean,
`evaulationResults`, and with the type you've created:
```tsx
evaulationResultsByNodeId: DmnEditor.EvaluationResults
```
##########
packages/online-editor/src/dmnRunner/DmnRunnerContextProvider.tsx:
##########
@@ -119,6 +126,81 @@ function dmnRunnerResultsReducer(dmnRunnerResults:
DmnRunnerResults, action: Dmn
}
}
+/**
+ * This transformation is needed for these reasons:
+ * ### -1- ###
+ * DMN Runner backend return upper case constants: "SUCCEEDED", "FAILED",
"SKIPPED"
+ * DMN Editor code base uses lover case constants: "succeeded", "failed",
"skipped"
+ *
+ * ### -2- ###
+ * DMN Runner backend return evaluationHitIds as Object:
+ * {
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3715CF: 1,
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3713AD: 2
+ * }
+ * DMN Editor code base uses evaluationHitIds as Map<string, number>
+ * [
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3715CF,
+ * value: 1
+ * },
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3713AD",
+ * value: 2
+ * }
+ * ]
+ *
+ * ### -3- ###
+ * DMN Runner backend return data spilt into junks corresponding to table row
or collection item
+ * DMN Editor want to show aggregated data for everything together
+ *
+ * @param result - DMN Runner backend data
+ * @param evaluationResultsPerNode - transformed data for DMN Editor
+ */
+function transformExtendedServicesDmnResult(
+ result: ExtendedServicesDmnResult,
+ evaluationResultsPerNode: Map<
+ string,
+ { evaluationResult: "succeeded" | "failed" | "skipped";
evaluationHitsCount: Map<string, number> }
+ >
Review Comment:
The same as the first comment. Why not using the `DmnEditor` type? As well,
you could use the same name `evaluationResultsByNodeId`, or taking a look into
your comments, `evaluationResultsByDecisionId`?
##########
packages/online-editor/src/dmnRunner/DmnRunnerContextProvider.tsx:
##########
@@ -119,6 +126,81 @@ function dmnRunnerResultsReducer(dmnRunnerResults:
DmnRunnerResults, action: Dmn
}
}
+/**
+ * This transformation is needed for these reasons:
+ * ### -1- ###
+ * DMN Runner backend return upper case constants: "SUCCEEDED", "FAILED",
"SKIPPED"
+ * DMN Editor code base uses lover case constants: "succeeded", "failed",
"skipped"
+ *
+ * ### -2- ###
+ * DMN Runner backend return evaluationHitIds as Object:
+ * {
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3715CF: 1,
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3713AD: 2
+ * }
+ * DMN Editor code base uses evaluationHitIds as Map<string, number>
+ * [
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3715CF,
+ * value: 1
+ * },
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3713AD",
+ * value: 2
+ * }
+ * ]
+ *
+ * ### -3- ###
+ * DMN Runner backend return data spilt into junks corresponding to table row
or collection item
+ * DMN Editor want to show aggregated data for everything together
+ *
+ * @param result - DMN Runner backend data
+ * @param evaluationResultsPerNode - transformed data for DMN Editor
+ */
+function transformExtendedServicesDmnResult(
+ result: ExtendedServicesDmnResult,
+ evaluationResultsPerNode: Map<
+ string,
+ { evaluationResult: "succeeded" | "failed" | "skipped";
evaluationHitsCount: Map<string, number> }
+ >
+) {
+ result.decisionResults?.forEach((dr) => {
+ const evaluationHitsCount = new Map<string, number>();
+ // ### -2- ###
+ for (const [key, value] of Object.entries(dr.evaluationHitIds)) {
+ evaluationHitsCount.set(`${key}`, value as number);
+ }
Review Comment:
`evaluationHitsCount` -> `evaluationHitsCountByDecisionId`.
I'm not sure if we need this. Based on your comment, the `result` will
already give us the collected number of hits of the in an object, right?
You can use the `object` the same way you could use a map.
const count = dr.evaluationHitIds[id]
##########
packages/online-editor/src/dmnRunner/DmnRunnerContextProvider.tsx:
##########
@@ -119,6 +126,81 @@ function dmnRunnerResultsReducer(dmnRunnerResults:
DmnRunnerResults, action: Dmn
}
}
+/**
+ * This transformation is needed for these reasons:
+ * ### -1- ###
+ * DMN Runner backend return upper case constants: "SUCCEEDED", "FAILED",
"SKIPPED"
+ * DMN Editor code base uses lover case constants: "succeeded", "failed",
"skipped"
+ *
+ * ### -2- ###
+ * DMN Runner backend return evaluationHitIds as Object:
+ * {
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3715CF: 1,
+ * _F0DC8923-5FC7-4200-8BD1-461D5F3713AD: 2
+ * }
+ * DMN Editor code base uses evaluationHitIds as Map<string, number>
+ * [
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3715CF,
+ * value: 1
+ * },
+ * {
+ * key: "_F0DC8923-5FC7-4200-8BD1-461D5F3713AD",
+ * value: 2
+ * }
+ * ]
+ *
+ * ### -3- ###
+ * DMN Runner backend return data spilt into junks corresponding to table row
or collection item
+ * DMN Editor want to show aggregated data for everything together
+ *
+ * @param result - DMN Runner backend data
+ * @param evaluationResultsPerNode - transformed data for DMN Editor
+ */
+function transformExtendedServicesDmnResult(
+ result: ExtendedServicesDmnResult,
+ evaluationResultsPerNode: Map<
+ string,
+ { evaluationResult: "succeeded" | "failed" | "skipped";
evaluationHitsCount: Map<string, number> }
+ >
+) {
+ result.decisionResults?.forEach((dr) => {
+ const evaluationHitsCount = new Map<string, number>();
+ // ### -2- ###
+ for (const [key, value] of Object.entries(dr.evaluationHitIds)) {
+ evaluationHitsCount.set(`${key}`, value as number);
+ }
+ // We want to merge evaluation results that belongs to the same Decision
+ // So we need to check if the Decision wasn't already partially processed
+ if (!evaluationResultsPerNode.has(dr.decisionId)) {
+ evaluationResultsPerNode.set(dr.decisionId, {
+ // ### -1- ###
+ evaluationResult: dr.evaluationStatus.toLowerCase() as "succeeded" |
"failed" | "skipped",
+ evaluationHitsCount: evaluationHitsCount,
+ });
+ } else {
+ const existingEvaluationHitsCount =
evaluationResultsPerNode.get(dr.decisionId)?.evaluationHitsCount;
+ evaluationHitsCount.forEach((value, key) => {
+ // ### -3- ###
+ if (existingEvaluationHitsCount?.has(key)) {
+ existingEvaluationHitsCount.set(key,
(existingEvaluationHitsCount?.get(key) ?? 0) + value);
+ } else {
+ existingEvaluationHitsCount?.set(key, value);
+ }
+ });
+ // TODO - Question
+ // Here is an issue of merging evaluation results that belongs to the
same Decision
+ // For example, what to do if first time evaluation was 'succeeded' and
second time 'skipped'?
+ evaluationResultsPerNode.set(dr.decisionId, {
+ evaluationResult: dr.evaluationStatus.toLowerCase() as "succeeded" |
"failed" | "skipped",
+ evaluationHitsCount: existingEvaluationHitsCount ?? new Map(),
+ });
Review Comment:
That is a good question. I think we would need a new badge for multiple
scenarios? And use the `succeed` or `skipped` only if all evaluations has the
same status. But we could proceed to use the "worst" status, in this case
`skipped`. WDYT @jomarko @tiagobento?
##########
packages/dmn-editor-envelope/src/NewDmnEditorFactory.tsx:
##########
@@ -42,4 +43,8 @@ export class NewDmnEditorInterface extends DmnEditorInterface
{
public openBoxedExpressionEditor(nodeId: string): void {
this.self.openBoxedExpressionEditor(nodeId);
}
+
+ public showDmnEvaluationResults(evaluationResultsPerNode: Map<string,
NodeEvaluationResults>): void {
Review Comment:
Same as above.
##########
packages/online-editor/src/dmnRunner/DmnRunnerContextProvider.tsx:
##########
@@ -119,6 +126,81 @@ function dmnRunnerResultsReducer(dmnRunnerResults:
DmnRunnerResults, action: Dmn
}
}
+/**
+ * This transformation is needed for these reasons:
+ * ### -1- ###
+ * DMN Runner backend return upper case constants: "SUCCEEDED", "FAILED",
"SKIPPED"
+ * DMN Editor code base uses lover case constants: "succeeded", "failed",
"skipped"
Review Comment:
typo `lower`
##########
packages/dmn-editor/src/diagram/nodes/Nodes.tsx:
##########
@@ -395,8 +395,8 @@ export const DecisionNode = React.memo(
const { evaluationResults } = useDmnEditor();
const evaluationResultsClassName = useMemo(
() =>
- isEvaluationHighlightsEnabled && evaluationResults![decision["@_id"]]
!== undefined
- ?
`kie-dmn-editor--decision-node--evaluation-status-${evaluationResults![decision["@_id"]]}`
+ isEvaluationHighlightsEnabled &&
evaluationResults!.get(decision["@_id"])?.evaluationResult !== undefined
+ ?
`kie-dmn-editor--decision-node--evaluation-status-${evaluationResults!.get(decision["@_id"])?.evaluationResult}`
Review Comment:
I'm not sure about using the `evaluationResults!` here. Why not
`evaluationResults?`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
