Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
kbowers-ibm merged PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834 -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
jomarko commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2627173513 @kbowers-ibm yes, I think I will have a look one more time. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1936604615
##
packages/dmn-editor/tests-e2e/checkDecisionTableCellsDataType.spec.ts:
##
@@ -673,7 +665,7 @@ test.describe("Decision Table - Cells Data Type -
Constraint", () => {
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
-await
expect(beePropertiesPanel.decisionTableOutputHeader.getColumnDataType()).toHaveValue(/^\s*rangeType\s$/i);
+await
expect(beePropertiesPanel.decisionTableOutputHeader.getDataType()).toHaveValue(/^\s*rangeType\s$/i);
Review Comment:
> getExpressionDataType())
Hi Kennedy,Thanks for the review .I did the changes.
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
kbowers-ibm commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1935566474
##
packages/dmn-editor/tests-e2e/checkDecisionTableCellsDataType.spec.ts:
##
@@ -673,7 +665,7 @@ test.describe("Decision Table - Cells Data Type -
Constraint", () => {
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
-await
expect(beePropertiesPanel.decisionTableOutputHeader.getColumnDataType()).toHaveValue(/^\s*rangeType\s$/i);
+await
expect(beePropertiesPanel.decisionTableOutputHeader.getDataType()).toHaveValue(/^\s*rangeType\s$/i);
Review Comment:
This line can just be deleted instead of changed from
getColumnDataType to getDataType.
It's preceded by
```
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
```
And due to the nature of the 2 functions it's just checking the same thing
twice in a row.
This happens in multiple test cases, so please update all of them
accordingly! :)
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
kbowers-ibm commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1935566474
##
packages/dmn-editor/tests-e2e/checkDecisionTableCellsDataType.spec.ts:
##
@@ -673,7 +665,7 @@ test.describe("Decision Table - Cells Data Type -
Constraint", () => {
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
-await
expect(beePropertiesPanel.decisionTableOutputHeader.getColumnDataType()).toHaveValue(/^\s*rangeType\s$/i);
+await
expect(beePropertiesPanel.decisionTableOutputHeader.getDataType()).toHaveValue(/^\s*rangeType\s$/i);
Review Comment:
This line can just be deleted instead of changed from
getColumnDataType to getDataType.
It's preceded by
```
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
```
And due to the nature of the 2 functions it's just checking the same thing
twice in a row.
This happens in multiple test cases, so please update all of them
accordingly! :)
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
kbowers-ibm commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1935566474
##
packages/dmn-editor/tests-e2e/checkDecisionTableCellsDataType.spec.ts:
##
@@ -673,7 +665,7 @@ test.describe("Decision Table - Cells Data Type -
Constraint", () => {
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
-await
expect(beePropertiesPanel.decisionTableOutputHeader.getColumnDataType()).toHaveValue(/^\s*rangeType\s$/i);
+await
expect(beePropertiesPanel.decisionTableOutputHeader.getDataType()).toHaveValue(/^\s*rangeType\s$/i);
Review Comment:
This line can just be deleted instead of changed from
getColumnDataType to getDataType.
It's preceded by
```
await
expect(beePropertiesPanel.decisionTableOutputHeader.getExpressionDataType()).toHaveValue(
/^\s*rangeType\s$/i
);
```
And due to the nature of the 2 functions it's just checking the same thing
twice in a row.
This happens in multiple test cases, so please update all of them
accordingly! :)
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2623580142 > Thanks for the changes @Kusuma04-dev , the tests look good! I've been testing it out locally, and noticed that the Column Type is still there (but disabled). Wasn't the final decision to remove it, similar to column name? Hi Kennedy,Thanks for the review.Yes i have removed column type as well. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
kbowers-ibm commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2621572547 Thanks for the changes @Kusuma04-dev , the tests look good! I've been testing it out locally, and noticed that the Column Type is still there (but disabled). Wasn't the final decision to remove it, similar to column name? -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2586759905 > Thank you @danielzhe @Kusuma04-dev for the discussion and updates. I think there is still some small inconsistency that we should address. > > Please follow my steps, when I create single output decision table:  > > then it contains this code snippet: > > ``` > > ``` > > There is no `typeRef` attribute 🟠> > Then I add a output column:  > > then there is code snippet: > > ``` > > > ``` > > that has both `name` and `typeRef` 🟢 > > and when I delete one output column: > >  then there is code snippet: > > ``` > > ``` > > that has `typeRef` 🟠> > the places I mark with 🟠are inconsistent. Both are decision tables with single output column, however once there is `typeRef` and once there is not `typeRef`. hi @jomarko , Thank you for reviewing and noticing the issue , If we have selected typeref when there is single output column then we can see typeref also for it .If we have two output columns with selected typrefs then will have typerefs also and after deleting one column , i did reset for typeref also now so that if we select typeref only we will get otherwise we won't. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2581812872 > Btw, maybe the issue 4 is not related to your PR, @Kusuma04-dev . Hi Daniel,Thanks for the > Thanks for your PR, @Kusuma04-dev ! > > I did some review and found some issues: > > Having only a single output column that is hidden (the default behavior): > > 1. If you add a new output column, in the DMN file, only one column is named. Both should have names in this case. > 2. If you add a new output column, one column comes with the name "Expression name". I think it should be "Output-1" and "Output-2" > 3. I got the issue that @jomarko reported above, but I'm unable to reproduce it again. 😢 > 4. I noticed that I got one output with an invalid typeref (typeRef=""). > > Let me know if you need more clarification. > > Video bellow: https://github.com/user-attachments/assets/3a55cbbe-7f2f-43de-976d-483b43dee0e5 Hi Daniel,Thanks for the review .I have modified the validation now and the above issues are resolved.Can you please confirm once. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2581802684 > @Kusuma04-dev Can you please synchronize this PR with main? Thx Hi Yeser, done. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
danielzhe commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2581738585 Btw, maybe the issue 4 is not related to your PR, @Kusuma04-dev . -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
danielzhe commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2581308817 Thanks for your PR, @Kusuma04-dev ! I did some review and found some issues: Having only a single output column that is hidden (the default behavior): 1. If you add a new output column, in the DMN file, only one column is named. Both should have names in this case. 2. If you add a new output column, one column comes with the name "Expression name". I think it should be "Output-1" and "Output-2" 3. I got the issue that @jomarko reported above, but I'm unable to reproduce it again. 😢 4. I noticed that I got one output with an invalid typeref (typeRef=""). Let me know if you need more clarification. Video bellow: https://github.com/user-attachments/assets/3a55cbbe-7f2f-43de-976d-483b43dee0e5 -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466: A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
yesamer commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2580301132 @Kusuma04-dev Can you please synchronize this PR with main? Thx -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1908644583
##
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##
@@ -777,7 +776,15 @@ export function DecisionTableExpression({
});
}
-const nextOutputColumns = [...(prev.output ?? [])];
+const nextOutputColumns = [
+ ...(prev.output ?? []).map((outputColumn, index) => {
+const outputCopy = { ...outputColumn };
+if (outputCopy["@_name"] == null) {
Review Comment:
Done Jozef.
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1908647768
##
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##
@@ -895,11 +902,19 @@ export function DecisionTableExpression({
case DecisionTableColumnType.OutputClause:
const newOutputs = [...(prev.output ?? [])];
newOutputs.splice(localIndexInsideGroup, 1);
-
+const updatedOutputColumns = [
+ ...(newOutputs ?? []).map((outputColumn) => {
+const outputCopy = { ...outputColumn };
+if (newOutputs.length == 1) {
+ outputCopy["@_name"] = undefined;
+}
+return outputCopy;
+ }),
+];
Review Comment:
Yeah i have updated and added the comment ,regarding ?? [] - it will be only
used to perform map operation when newOutputs is null or undefined and it is to
avoid any runtime errors and it won't impact anything.
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
jomarko commented on code in PR #2834:
URL:
https://github.com/apache/incubator-kie-tools/pull/2834#discussion_r1908347711
##
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##
@@ -895,11 +902,19 @@ export function DecisionTableExpression({
case DecisionTableColumnType.OutputClause:
const newOutputs = [...(prev.output ?? [])];
newOutputs.splice(localIndexInsideGroup, 1);
-
+const updatedOutputColumns = [
+ ...(newOutputs ?? []).map((outputColumn) => {
+const outputCopy = { ...outputColumn };
+if (newOutputs.length == 1) {
+ outputCopy["@_name"] = undefined;
+}
+return outputCopy;
+ }),
+];
Review Comment:
Also, I think we should bring more information. The variable name
`updatedOutputColumns` will be difficult to understand in the future. From the
variable, it is difficult to understand why we do update and what we update.
Could we try to bring more meaningful variable name? Or at least add comment
that we explain update is done because of kie-issues#1466 ?
##
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##
@@ -777,7 +776,15 @@ export function DecisionTableExpression({
});
}
-const nextOutputColumns = [...(prev.output ?? [])];
+const nextOutputColumns = [
+ ...(prev.output ?? []).map((outputColumn, index) => {
+const outputCopy = { ...outputColumn };
+if (outputCopy["@_name"] == null) {
Review Comment:
```suggestion
if (outputCopy["@_name"] === null) {
```
##
packages/boxed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx:
##
@@ -895,11 +902,19 @@ export function DecisionTableExpression({
case DecisionTableColumnType.OutputClause:
const newOutputs = [...(prev.output ?? [])];
newOutputs.splice(localIndexInsideGroup, 1);
-
+const updatedOutputColumns = [
+ ...(newOutputs ?? []).map((outputColumn) => {
+const outputCopy = { ...outputColumn };
+if (newOutputs.length == 1) {
+ outputCopy["@_name"] = undefined;
+}
+return outputCopy;
+ }),
+];
Review Comment:
I am not an expert in the `splice`, however, can it cause `newOutputs` to be
again undefined? Can not we remove one ` ?? []` ? Also I think we should use
`===` instead of `==` in typescript code.
```suggestion
const updatedOutputColumns = [
...(newOutputs).map((outputColumn) => {
const outputCopy = { ...outputColumn };
if (newOutputs.length === 1) {
outputCopy["@_name"] = undefined;
}
return outputCopy;
}),
];
```
--
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: commits-unsubscr...@kie.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org
For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2575249846 Hi, I did few changes in the code, could you please check and let me know? -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
danielzhe commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2573079748 @jomarko Yeah, we're working on it. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] kie-issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
danielzhe commented on PR #2834: URL: https://github.com/apache/incubator-kie-tools/pull/2834#issuecomment-2569261984 As discussed in the private chat, I think this PR is incorrect, but I'll clarify the issue better. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
Re: [PR] Kie issues#1466-A Decision Table with a single output column shouldn't have a name [incubator-kie-tools]
Kusuma04-dev closed pull request #2833: Kie issues#1466-A Decision Table with a single output column shouldn't have a name URL: https://github.com/apache/incubator-kie-tools/pull/2833 -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org
