Fix typecasting for TreeView

[mkrecek234]

Solves issue #2021

[mkrecek234]

Fix above works for single value treeItem correctly, however for multiple-value item the typecastLoadField does not support json yet. @mvorisek Can you check please? The sample with multiple-value is not working anymore:

ui/demos/form-control/tree-item-selector.php

Line 44 in c6b9ed7

$control->set([201, 301, 503]);

[mvorisek]

This cannot be right. You cannot mix load/save typecasting direction.

And as posted originally, I would like to have this tested by Behat. Not only to prevent it into the future, but as not tested, it was not fixed yet nor with #1991.

[mkrecek234]

This cannot be right. You cannot mix load/save typecasting direction.

And as posted originally, I would like to have this tested by Behat. Not only to prevent it into the future, but as not tested, it was not fixed yet nor with #1991.

Has to be typecastLoadField and not save which was wrong in the original code. However, the jsonarray data type is not supported, may be you can have a look at it. For integer it is properly working with the initial fix

[mvorisek]

"load" direction is almost surely wrong, as "load" direction implies a value to be loaded from an input, which is probably not the case here

[mkrecek234]

Valid point - the key solution has to be that scalar values shall not be typecasted (see latest commit), as the Vue component cannot handle ids with thousandSeparator. Still typecasting for non-scalars.

Working and tested now in live setup and demos, sorry behat test has to be provided by another contributor

[mvorisek]

Still the fix is wrong, the ID can be internally stored as any type - see #1991 prooving this case - ID wrapped in an object.

[mkrecek234]

Would appreciate to see your improved code proposal - the current implementation in develop branch is definitely wrong, and also not working in a defined setup

[mvorisek]

The main question is why is the typecast an issue. Any fix of getValue method will introduce inconsistency with Form\Control\Input::getValue() method. I am currently out of the time to look into this as the proper fix incl. test might take MD easily.

[mkrecek234]

The main question is why is the typecast an issue. Any fix of getValue method will introduce inconsistency with Form\Control\Input::getValue() method. I am currently out of the time to look into this as the proper fix incl. test might take MD easily.

The Vue TreeItemSelector does not support an Input field value which has thousandSeparator, so ID = 1.750 will not work, even though Atk4 saves and loads it properly. The input field thus has to have a non-typecasted ID inside, otherwise TreeViewItemSelector will not mark the correct item as active.

[mkrecek234]

The main reason for the above issue is simply that treeItems id values are not typecasted, whereas the currently selected value(s) are typecasted. The Vue component simply then cannot match the correct items.
So either we typecast the treeItems node id values or we do not typecast the input current value. In the light of the discussion, as those values are all "hidden" system value, typecasting of integers does not add any value here in my opinion.

[mvorisek]

Please understand the solution you provided cannot work with #1991, ie. the fix is still not correct, there is no reason to handle scalars differently as long as there are not handled differently on reverse typecasting - they are not and never will be.

The correct way to fix this problem is to:
a) provide minimalistic test cases - basic, and if not needed, for edge cases too
b) verify the visible data/integers/datatimes (objects) are typecasted correctly
c) verify all cases/save etc. are fixed properly
d) verify the TreeView coverage has increased (open CodeCov and check uncovered lines, regular scenarios must be covered)

[mvorisek]

Let's start with a failing test.