ForestAdmin / forest-express-sequelize

🌱 Express/Sequelize agent for Forest Admin to integrate directly to your existing Express/Sequelize backend application.
https://www.forestadmin.com
GNU General Public License v3.0
191 stars 47 forks source link

fix(smart-field): smart-field search use returned query #1058

Open JustinMartinDev opened 1 year ago

JustinMartinDev commented 1 year ago

Definition of Done

General

Actually smart-field search use mutation on query params instead of use returned query value as defined in type definition and in doc.

⚠️ My proposal is to use the returned value, but keep mind it will break the search of people using only mutation.

If you want to keep the mutation, update doc to remove the return query and in type definition change return to void

Security

Scra3 commented 1 year ago

Hello really thanks for your contribution. Indeed there is inconsistency. What was the motivation for your contribution? Did you encounter a bug? 🙏

JustinMartinDev commented 1 year ago

Yes, to avoid any side-effect due to mutation, I created a new object "query" with my additional condition and return it, but my condition was ignored.

After exploring the search-builder, i saw the smart-field search use mutation instead of type definition & doc.

const advancedQuery = {
    ...query,
    where: {
      ...where,
      [Op.and]: [
        {
          ...first,
          [Op.or]: [...first[Op.or], myCondition],
        },
        ...rest,
      ],
    },
  };
Scra3 commented 1 year ago

We have released a v9 3 months ago and a new node-js agent We will update the documentation so for now we're putting your PR aside, it may come in handy. 🙏

We will update the documentation and the TS interface.

Ty for your contribution.