kie-issues#1466: A Decision Table with a single output column shouldn't have a name #2834

Kusuma04-dev · 2025-01-03T13:55:00Z

closes apache/incubator-kie-issues#1466 - A Decision Table with a single output column shouldn't have a name

Before fix:

After fix:

danielzhe · 2025-01-03T13:59:54Z

As discussed in the private chat, I think this PR is incorrect, but I'll clarify the issue better.

jomarko

@Kusuma04-dev thank you for the PR. Currently, the failing tests are with high probability related to code changes you have introduced. But we need to wait for @danielzhe feedback before we fix the tests. It seems that @danielzhe will propose different solution and that may mean different or no tests failing.

danielzhe · 2025-01-06T13:09:26Z

@jomarko Yeah, we're working on it.

Kusuma04-dev · 2025-01-07T13:03:38Z

Hi,
I did few changes in the code, could you please check and let me know?

jomarko

Thank you for changes @Kusuma04-dev

UI

I think we are fine on the UI side. There I see this behavior

Single output column

Is represented with single level column header. This column header informs about the decision ndoe name and the decision node data type.

Multiple output columns

Are represented by two level column header

Backend

There I think we have still the issue that name attribute is present in the output tag. See:

  <dmn:decision name="bbb" id="_055202A8-C02C-4A75-AE67-CFA23080D2B3">
    <dmn:variable name="bbb" id="_E77AD5FF-91D7-4E03-84D8-87D82A8E002C" typeRef="boolean" />
    <dmn:decisionTable id="_31E8BF8C-BE7E-456C-B9D4-0F42D9B11572" typeRef="boolean" hitPolicy="UNIQUE" label="bbb">
      <dmn:input id="_A0628B74-A903-4833-B4FC-CD0DA4688E48">
        <dmn:inputExpression id="_2678C2CE-521B-4032-83F2-1760A14E20CB" typeRef="Any">
          <dmn:text>aaa</dmn:text>
        </dmn:inputExpression>
      </dmn:input>
      <dmn:output id="_2FB820B3-99DE-4709-B2CB-F88B196C5063" name="Output-2" />
      <dmn:annotation name="Annotations" />
      <dmn:rule id="_BF8CA92E-8BDA-43AB-AAD6-8CE7510F5D08">
        <dmn:inputEntry id="_92915712-1449-4D81-8354-43EC22F18B7A">
          <dmn:text>-</dmn:text>
        </dmn:inputEntry>
        <dmn:outputEntry id="_96A3DC02-EBD9-49A4-B9D9-43C2AD22666F">
          <dmn:text>false</dmn:text>
        </dmn:outputEntry>
        <dmn:annotationEntry>
          <dmn:text>// Your annotations here</dmn:text>
        </dmn:annotationEntry>
      </dmn:rule>
    </dmn:decisionTable>
  </dmn:decision>

That is source of:

and my understanding is name="Output-2" should not be produced in the source XML.

jomarko

Thank you for updates, some comments to code from my side. I postponed a manual check as there is a chance the code will change after posting my review.

...xed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx

yesamer · 2025-01-09T14:08:55Z

@Kusuma04-dev Can you please synchronize this PR with main? Thx

danielzhe · 2025-01-09T21:49:04Z

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):

If you add a new output column, in the DMN file, only one column is named. Both should have names in this case.
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"
I got the issue that @jomarko reported above, but I'm unable to reproduce it again. 😢
I noticed that I got one output with an invalid typeref (typeRef="<Undefined>").

Let me know if you need more clarification.

Video bellow:
https://github.com/user-attachments/assets/3a55cbbe-7f2f-43de-976d-483b43dee0e5

danielzhe · 2025-01-10T04:47:45Z

Btw, maybe the issue 4 is not related to your PR, @Kusuma04-dev .

Kusuma04-dev · 2025-01-10T05:22:45Z

@Kusuma04-dev Can you please synchronize this PR with main? Thx

Hi Yeser, done.

Kusuma04-dev · 2025-01-10T05:32:16Z

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):

If you add a new output column, in the DMN file, only one column is named. Both should have names in this case.

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"

I got the issue that @jomarko reported above, but I'm unable to reproduce it again. 😢

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.

jomarko

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:

<output id="_781CCB4D-6421-4D4B-B15F-43FF9AE7CA3E" />

There is no typeRef attribute 🟠

Then I add a output column:

then there is code snippet:

      <output id="_781CCB4D-6421-4D4B-B15F-43FF9AE7CA3E" name="Output-1" typeRef="boolean" />
      <output id="_80267764-71E9-4444-A786-904ADBA52E04" name="Output-2" typeRef="number" />

that has both name and typeRef 🟢

and when I delete one output column:

then there is code snippet:

      <output id="_80267764-71E9-4444-A786-904ADBA52E04" typeRef="number" />

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.

… deleting 2nd output column

Kusuma04-dev · 2025-01-13T10:43:49Z

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:
<output id="_781CCB4D-6421-4D4B-B15F-43FF9AE7CA3E" />
There is no typeRef attribute 🟠

Then I add a output column:

then there is code snippet:
      <output id="_781CCB4D-6421-4D4B-B15F-43FF9AE7CA3E" name="Output-1" typeRef="boolean" />
      <output id="_80267764-71E9-4444-A786-904ADBA52E04" name="Output-2" typeRef="number" />
that has both name and typeRef 🟢

and when I delete one output column:

then there is code snippet:
      <output id="_80267764-71E9-4444-A786-904ADBA52E04" typeRef="number" />
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.

jomarko

Thank you for the updates.

…ss deleted then there should be only one output column ,as we have removed column name for single output type is also ignored

kbowers-ibm · 2025-01-29T12:59:25Z

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?

Kusuma04-dev · 2025-01-30T05:55:33Z

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.

kbowers-ibm

Hi @Kusuma04-dev,
Tested and everything is looking great! Just one minor nitpick regarding the tests, and then it will be ready to merge.

kbowers-ibm · 2025-01-30T12:55:52Z

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);


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! :)

getExpressionDataType())

Hi Kennedy,Thanks for the review .I did the changes.

kbowers-ibm

Thanks Kusuma, LGTM! @jomarko given all the changes would you like to rereview before merge?

A Decision Table with a single output column shouldn't have a name

4311a3b

Kusuma04-dev requested a review from tiagobento as a code owner January 3, 2025 13:55

jomarko requested review from danielzhe and jomarko January 6, 2025 09:12

jomarko reviewed Jan 6, 2025

View reviewed changes

kbowers-ibm self-requested a review January 6, 2025 09:29

danielzhe added pr: DO NOT MERGE Draft PR, not ready for merging area:dmn pr: wip PR is still under development labels Jan 6, 2025

Kusuma04-dev and others added 3 commits January 7, 2025 17:59

Merge branch 'apache:main' into kie-issues#apache#1466

de546f3

decisio table with single ouput shouldn't have name

020e6cd

removing unused variable decalaration

71f6fd7

Kusuma04-dev requested a review from jomarko January 8, 2025 04:11

jomarko reviewed Jan 8, 2025

View reviewed changes

fixing name issue while deleting output column

2b7eec4

danielzhe removed pr: DO NOT MERGE Draft PR, not ready for merging pr: wip PR is still under development labels Jan 8, 2025

Kusuma04-dev requested a review from jomarko January 9, 2025 04:47

jomarko reviewed Jan 9, 2025

View reviewed changes

...xed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx Outdated Show resolved Hide resolved

...xed-expression-component/src/expressions/DecisionTableExpression/DecisionTableExpression.tsx Outdated Show resolved Hide resolved

modified equality operator

09a6022

Kusuma04-dev requested a review from jomarko January 9, 2025 11:52

tiagobento changed the title ~~kie-issues#1466-A Decision Table with a single output column shouldn't have a name~~ kie-issues#1466: A Decision Table with a single output column shouldn't have a name Jan 9, 2025

Merge branch 'apache:main' into kie-issues#apache#1466

ccc8312

adding correct validation

f81b3f5

jomarko mentioned this pull request Jan 13, 2025

[NO-ISSUE] Adding additional DMN examples to the editor #2844

Closed

jomarko reviewed Jan 13, 2025

View reviewed changes

making typeref also undefined when there is singleoutput column after…

fde162c

… deleting 2nd output column

Kusuma04-dev requested a review from jomarko January 13, 2025 10:44

jomarko approved these changes Jan 13, 2025

View reviewed changes

Kusuma04-dev mentioned this pull request Jan 15, 2025

A Decision Table with a single output column shouldn't have a name apache/incubator-kie-issues#1466

Open

chinnamatli kusumalatha and others added 5 commits January 16, 2025 12:31

fixing test case when there are 2 output columns with types and one i…

e2a5d18

…ss deleted then there should be only one output column ,as we have removed column name for single output type is also ignored

Merge branch 'apache:main' into kie-issues#apache#1466

52fc101

Merge branch 'apache:main' into kie-issues#apache#1466

09ea44e

Merge branch 'apache:main' into kie-issues#apache#1466

32eb74e

fixing test cases

8779d19

removing column type

25b5a3f

kbowers-ibm reviewed Jan 30, 2025

View reviewed changes

test case issues

290df2c

kbowers-ibm approved these changes Jan 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kie-issues#1466: A Decision Table with a single output column shouldn't have a name #2834

kie-issues#1466: A Decision Table with a single output column shouldn't have a name #2834

Kusuma04-dev commented Jan 3, 2025

danielzhe commented Jan 3, 2025

jomarko left a comment

danielzhe commented Jan 6, 2025

Kusuma04-dev commented Jan 7, 2025

jomarko left a comment

jomarko left a comment

yesamer commented Jan 9, 2025

danielzhe commented Jan 9, 2025

danielzhe commented Jan 10, 2025

Kusuma04-dev commented Jan 10, 2025

Kusuma04-dev commented Jan 10, 2025

jomarko left a comment

Kusuma04-dev commented Jan 13, 2025

jomarko left a comment

kbowers-ibm commented Jan 29, 2025

Kusuma04-dev commented Jan 30, 2025

kbowers-ibm left a comment

kbowers-ibm Jan 30, 2025 •

edited

Loading

Kusuma04-dev Jan 31, 2025

kbowers-ibm left a comment

kie-issues#1466: A Decision Table with a single output column shouldn't have a name #2834

Are you sure you want to change the base?

kie-issues#1466: A Decision Table with a single output column shouldn't have a name #2834

Conversation

Kusuma04-dev commented Jan 3, 2025

danielzhe commented Jan 3, 2025

jomarko left a comment

Choose a reason for hiding this comment

danielzhe commented Jan 6, 2025

Kusuma04-dev commented Jan 7, 2025

jomarko left a comment

Choose a reason for hiding this comment

UI

Single output column

Multiple output columns

Backend

jomarko left a comment

Choose a reason for hiding this comment

yesamer commented Jan 9, 2025

danielzhe commented Jan 9, 2025

danielzhe commented Jan 10, 2025

Kusuma04-dev commented Jan 10, 2025

Kusuma04-dev commented Jan 10, 2025

jomarko left a comment

Choose a reason for hiding this comment

Kusuma04-dev commented Jan 13, 2025

jomarko left a comment

Choose a reason for hiding this comment

kbowers-ibm commented Jan 29, 2025

Kusuma04-dev commented Jan 30, 2025

kbowers-ibm left a comment

Choose a reason for hiding this comment

kbowers-ibm Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Kusuma04-dev Jan 31, 2025

Choose a reason for hiding this comment

kbowers-ibm left a comment

Choose a reason for hiding this comment

kbowers-ibm Jan 30, 2025 •

edited

Loading