Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in formula parsing when using nested data structures. #5279

Closed
GrononS opened this issue Jul 24, 2018 · 4 comments
Closed

Bug in formula parsing when using nested data structures. #5279

GrononS opened this issue Jul 24, 2018 · 4 comments

Comments

@GrononS
Copy link

GrononS commented Jul 24, 2018

Formulas in a JSON data set that appear after nested data do not parse. Formulas that appear prior to the nested data parse correctly.

http://jsfiddle.net/3uryp9zh/2/

If I issue the following command in the debug window the field will still fail to parse and will instead turn red:
hot.setDataAtCell(2, 4, "=SUM(A1:A2)");

If I issue the following two commands in the debug window the formula will parse:
hot.setDataAtCell(2, 4, "=SUM(A1:A1)");
hot.setDataAtCell(2, 4, "=SUM(A1:A2)");

This applies to the handsontable 5 with the latest jQuery library.

Edit: More Info
If I execute the following code it returns true (this is against the first column of the third row)
hot.validateCell("=SUM(A1:A2)", hot.getCellMeta(2, 0), function(a, b, c){console.debug(a)}, null)

But, if I execute the following command on the fifth cell of the third row it returns false
hot.validateCell("=SUM(A1:A2)", hot.getCellMeta(2, 4), function(a, b, c){console.debug(a)}, null)

The formula is valid for both cells so why does if keep failing until I change the value to something else and then back to the formula that was previously failing?

@AMBudnik
Copy link
Contributor

That is an interesting discovery. Thank you for doing the investigation @GrononS
I have replicated the issue.

We have a very similar case here #4430, however, the data is an array of arrays. I'm extremely eager to find out why this is not working.

@GrononS
Copy link
Author

GrononS commented Jul 26, 2018

I was able to fix part of this problem by altering the countColumns function in handsontable.full.js to recursively count the nested columns in the dataset.
The problem is that the original function was only counting the number of top level columns without taking into consideration the number of nested columns. This caused the number of counted columns to be too small.
This fixed the issue with formulas that were not nested but that followed a nested set of data.

However, nested formulas still do not work which leads me to believe that the formula parser also needs to be changed so it is recursive as well.

For my testing I did the following:

First, I added a new function (outside of handsontable because I was just testing fixes) as follows:

function fCountColumns(pData, pRecursing) {
            function getType(pData) {
                if (Array.isArray(pData)) {
                    return "array";
                } else {
                    return typeof pData;
                }
            }
            var result = 0 + (pRecursing == (false  || typeof pRecurseLvl == "undefined") ? 0 : -1);

            if (getType(pData) == "array") {
                result += pData.length;
                
                for (var i = 0; i < pData.length; i++) {
                    if (getType(pData[i]) == "array" || getType(pData[i]) == "object") {
                        result += fCountColumns(pData[i], true);
                    }
                }
            } else if (getType(pData) == "object") {
                result += Object.keys(pData).length;

                for (var i = 0; i < Object.keys(pData).length; i++) {
                    if (getType(pData[Object.keys(pData)[i]]) == "array" || getType(pData[Object.keys(pData)[i]]) == "object") {
                        result += fCountColumns(pData[Object.keys(pData)[i]], true);
                    }
                }
            }

            return result;
        }

Then I modified the countColumns function in handsontable.full.js to use the new function as follows:

key: 'countColumns',
    value: function countColumns() {
      var result = 0;

        if (Array.isArray(this.data)) {
            if (this.dataType === 'array') {
                result = top.fCountColumns(this.data[0]);
            } else if (this.dataType === 'object') {
                result = top.fCountColumns(this.data[0]);;
            }
        }

      return result;
    }

I would like to use handsontable with formulas in my new project but without formulas working predictably this won't be an option. Do you have any updates on when the formulas module will be reworked and these problems fixed?
I could probably fix the issues myself but that would make upgrading to newer versions of handsontable problematic as I've have to merge my fixes into the new versions.

@AMBudnik
Copy link
Contributor

This issue is closed temporarily. It will be reopened for development as it became a part of New formula plugin task reported at #6466

I will make sure to inform everyone interested in this topic after the official fix.

Please feel welcome to makes any comments on this issue. And if you experience similar behavior feel free to contact me at support@handsontable.com

@AMBudnik
Copy link
Contributor

AMBudnik commented Jun 1, 2021

Hi @GrononS

as previously predicted this issue was related to #4430 and both got officially fixed in v9 released today
Here is an updated demo http://jsfiddle.net/xqn3Lawh/
Then here is a list of changes for the following version https://handsontable.com/docs/9.0.0/tutorial-release-notes.html and https://handsontable.com/docs/9.0.0/tutorial-migration-guide.html a migration guide (from v8 to v9)

@AMBudnik AMBudnik added Status: Released and removed Part of scope This issue is a part of a larger task labels Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants