fix(grid): fixed advancedFilteringExpressionsTree rehydration #15500

[Otixa]

Closes #15500

Additional information (check all that apply):

• Bug fix
• New functionality
• Documentation
• Demos
• CI/CD

Checklist:

• All relevant tags have been applied to this PR
• This PR includes unit tests covering all the new code (test guidelines)
• This PR includes API docs for newly added methods/properties (api docs guidelines)
• This PR includes feature/README.MD updates for the feature docs
• This PR includes general feature table updates in the root README.MD
• This PR includes CHANGELOG.MD updates for newly added functionality
• This PR contains breaking changes
• This PR includes ng update migrations for the breaking changes (migrations guidelines)
• This PR includes behavioral changes and the feature specification has been updated with them

[gedinakova]

We need a test for this.

[hanastasov]

Is this PR still in progress? Because it has the verified badge, but commits are made which are not related to some comment requesting changes.

For the last changes, maybe they are OK but I dont see why such change is required:

Obviously it does not bring along any change in how the code works with those interfaces, so it made me question if this is neccesary.

[gedinakova]

Is this PR still in progress? Because it has the verified badge, but commits are made which are not related to some comment requesting changes.

For the last changes, maybe they are OK but I dont see why such change is required:

Obviously it does not bring along any change in how the code works with those interfaces, so it made me question if this is neccesary.

Yes, they are necessary and should cover the scenario where the expression tree is set as follows (this is coming from a sample provided by PwnJS):

advancedFilteringExpressionsTree2: IExpressionTree =	{
    "filteringOperands": [
      {
        "fieldName": "productID",
        "condition": {
          "name": "equals",
          "isUnary": false,
          "iconName": "filter_equal"
        },
        "conditionName": "equals",
        "ignoreCase": true,
        "searchVal": 33,
        "searchTree": null
      }
    ],
    "operator": 0,
    "returnFields": []
  };

I will add a test now, so to stop any further confusion.

[hanastasov]

Thanks Gale for the explanation.

[damyanpetev]

Eh, sure - I suppose the props on the filtering expression/tree technically accept nullish and some odd scenario in an app could evaluate to a null type, but generally shouldn't happen. Don't mind them typed as such regardless.

The change request mostly for the type on return fields, the rest of the comments are merely small improvements.

[damyanpetev]

At some point we might want to dedupe this into a private function we can call in all places :)