Skip to content

fix(grid): fixed advancedFilteringExpressionsTree rehydration #15500#15507

Merged
damyanpetev merged 12 commits into19.1.xfrom
mcherkasov/filtering-15500-19.1
Mar 18, 2025
Merged

fix(grid): fixed advancedFilteringExpressionsTree rehydration #15500#15507
damyanpetev merged 12 commits into19.1.xfrom
mcherkasov/filtering-15500-19.1

Conversation

@Otixa
Copy link
Copy Markdown
Contributor

@Otixa Otixa commented Mar 12, 2025

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

@Otixa Otixa changed the title Mcherkasov/filtering 15500 19.1 fix(grid): fixed advancedFilteringExpressionsTree rehydration #15500 Mar 12, 2025
@gedinakova gedinakova added ✅ status: verified Applies to PRs that have passed manual verification 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification ✅ status: verified Applies to PRs that have passed manual verification labels Mar 12, 2025
@gedinakova
Copy link
Copy Markdown
Contributor

gedinakova commented Mar 13, 2025

@Otixa Since 'condition' is not a mandatory property, is there a reason to require having it. Check this out in the focused test. Essentialy, the following is not a valid structure:

this.filterTree = JSON.parse(`{ 
            "filteringOperands":  [
                {
                    "conditionName": "greaterThan",
                    "fieldName": "Downloads",
                    "searchVal": 200
                }
            ],
            "operator": 0
        }`);

While this one is valid:

this.filterTree = JSON.parse(`{ 
             "filteringOperands":  [
                {
                    "conditionName": "greaterThan",
                    "condition":{
                         "name":"greaterThan",
                         "isUnary":false,
                         "iconName":"filter_greater_than"
                     },
                    "fieldName": "Downloads",
                    "searchVal": 200
                }
            ],
            "operator": 0
        }`);

I would expect both to work as we don't get much from having the condition prop. I realize in most cases it will be created via stringifying the actual object, hence this issue will not surface, but I still think we can reconsider.

@gedinakova gedinakova added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Mar 17, 2025
gedinakova
gedinakova previously approved these changes Mar 17, 2025
@gedinakova gedinakova requested a review from damyanpetev March 17, 2025 06:42
gedinakova
gedinakova previously approved these changes Mar 17, 2025
damyanpetev
damyanpetev previously approved these changes Mar 18, 2025
@damyanpetev damyanpetev added the squash-merge Merge PR with "Squash and Merge" option label Mar 18, 2025
@damyanpetev damyanpetev merged commit 8dc0051 into 19.1.x Mar 18, 2025
5 checks passed
@damyanpetev damyanpetev deleted the mcherkasov/filtering-15500-19.1 branch March 18, 2025 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filtering grid: advanced-filtering squash-merge Merge PR with "Squash and Merge" option ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants