react-querybuilder/react-querybuilder

Incorrect Query format when using formatQuery on a rule group with a singular rule

bratconnex opened this issue · 10 comments

When formatting for JSON logic via formatQuery(query, 'jsonlogic'), if the query object only contains a singular rule, and looks like so:

{
    "combinator": "and",
    "rules": [
        {
            "field": "teams.name",
            "operator": "=",
            "value": "bob",
            "id": ...
        }
    ],
    "id": ...
}

Then the output of the formatQuery function will look like appear in the following (incorrect) format:

{
    "==": [
        {
            "var": "teams.name"
        },
        "bob"
    ]
}

As shown by the object above, the output from formatQuery is missing a layer of nesting, where the key of the array containing the rules objects is the combinator for that rule group level. The expected output is shown below:

{
    "and": [
        {
            "==": [
                {
                    "var": "teams.name"
                },
                "bob"
            ]
        }
    ]
}

It appears that the formatQuery object should return rules objects, within an array, keyed by the combinator selection, regardless of how many rules are selected.

I know I said in the Discord chat that I would fix this, but after thinking about it I'm not sure I should change the current behavior.

The resulting object from formatQuery is a valid JsonLogic rule, and parsing that same rule object with parseJsonLogic results in a valid RQB query object.

This codesandbox demonstrates everything. Code:

import { apply, is_logic } from "json-logic-js";
import { parseJsonLogic } from "react-querybuilder/parseJsonLogic";
import "./styles.css";

const rule = {
  "==": [{ var: "teams.name" }, "bob"],
};

const data = { teams: { name: "bob" } };

document.getElementById("app")!.innerHTML = `
<div>
  <pre>rule === ${JSON.stringify(rule)}</pre>
  <pre>data === ${JSON.stringify(data)}</pre>
  <pre>is_logic(rule) === ${is_logic(rule)}</pre>
  <pre>apply(rule) === ${apply(rule, data)}</pre>
  <pre>query === ${JSON.stringify(parseJsonLogic(rule), null, 2)}</pre>
</div>`;

Result:

rule === {"==":[{"var":"teams.name"},"bob"]}

data === {"teams":{"name":"bob"}}

is_logic(rule) === true

apply(rule) === true

query === {
  "combinator": "and",
  "rules": [
    {
      "field": "teams.name",
      "operator": "=",
      "value": "bob"
    }
  ]
}

If your use case requires the "==" rule be wrapped in an "and" array to be valid, I would suggest that be done in your own code when necessary.

Closing this issue based upon the comment above.

I would like to reopen this issue due to the following:

Query:
image

The resulting jsonLogic is:

{
  "and": [
    {
      "startsWith": [
        {
          "var": "firstName"
        },
        "Stev"
      ]
    },
    {
      "in": [
        {
          "var": "lastName"
        },
        [
          "Vai",
          "Vaughan"
        ]
      ]
    },
    {
      ">": [
        {
          "var": "age"
        },
        "28"
      ]
    },
    {
      "==": [
        {
          "var": "firstName"
        },
        "oi"
      ]
    }
  ]
}

Please correct me if I'm wrong, but I believe it should be:

{
  "and": [
    {
      "startsWith": [
        {
          "var": "firstName"
        },
        "Stev"
      ]
    },
    {
      "in": [
        {
          "var": "lastName"
        },
        ["Vai", "Vaughan"]
      ]
    },
    {
      ">": [
        {
          "var": "age"
        },
        "28"
      ]
    },
    {
      "or": [
        {
          "==": [
            {
              "var": "firstName"
            },
            "oi"
          ]
        }
      ]
    }
  ]
}

@Coooi yeah that's a little different than the other use case, and I can see why you'd want to retain the grouping since the query would end up being different after parsing it back to an RQB object.

What if it maintained the grouping for subgroups with a single rule, but not for the outermost group?

@jakeboone02 That's right, I can't parse it back like that.

Could you maybe give an example of what you mean based on the data I provided?

In fact, the inner grouping (OR) belongs to the outermost one (AND), so I think to reconstruct the whole query we should ideally follow the RQB structure, right?

@Coooi Right, the "and" group (the query itself) has multiple rules so that would stay the same even with the current logic. The "or" group gets collapsed to a single rule with the current logic, but what I'm proposing is that the collapsing only happens when there is only one rule in the entire query (no subgroups).

@jakeboone02 would you maybe share with me an example of what you mean based on the one I posted?

@Coooi Yeah, sorry I was on mobile when I posted my last comment.

Current logic is that this...

{
  "combinator": "and",
  "rules": [
    { "field": "firstName", "operator": "=", "value": "oi" }
  ]
}

...becomes this:

{
  "==": [
    { "var": "firstName" },
    "oi"
  ]
}

The full RQB query is collapsed to a single JsonLogic rule, which (to me) seems ok since parseJsonLogic will reproduce the same query on the way back (unless the combinator is "or"/"xor").

The problem you've spotted is that all groups with a single rule get collapsed, which does not reproduce the same query when parsed.

What I'm proposing is that a query with a single rule will collapse as it does today, but subgroups never collapse, even if they only contain a single rule. So this:

{
  "combinator": "and",
  "rules": [
    { "field": "firstName", "operator": "=", "value": "oi" },
    {
      "combinator": "or",
      "rules": [
        { "field": "lastName", "operator": "=", "value": "io" }
      ]
    }
  ]
}

...would become this:

{
  "and": [
    {
      "==": [
        { "var": "firstName" },
        "oi"
      ]
    },
    {
      "or": [
        {
          "==": [
            { "var": "lastName" },
            "io"
          ]
        }
      ]
    }
  ]
}

If you agree with this I'll create a new issue for it (or you can).

@jakeboone02 Yes! Now it's clear, thank you!
I totally agree, this way the json logic would kinda mirror the query structure.
I'm creating the new issue now, many thanks! 😃

@jakeboone02 There you go: #664