preventChanges hook should support dot notation

[claustres]

trafficstars

Steps to reproduce

Setup the hook like this on a service:

before: {
    patch: [preventChanges(true, ['quotas'])]
  }

Expected behavior

If quotas is actually a nested object and I perform a patch like this using the mongo adapter service.patch(id, { 'quotas.members': 200 }), I expect the hook to raise an error.

Actual behavior

The hook does not filter data properties targetting a prevented field when using dot notation, neither raises an error.

It is not necessarily a bug as one could probably avoid this by registering all available nested properties like this:

before: {
    patch: [preventChanges(true, ['quotas.members'])]
  }

However, in some cases, it is not possible to know all properties upfront. Moreover, it seems to me a convenient way (or shortcut) to discard all properties of a nested object.

We implemented it on our side for now if it can help and seems relevent for this module as well. If so we could try to make a PR but welcome any feedback first.

[fratzinger]

Nice addition. I don't use dot-notation and I don't use mongo myself but I see the use case. But you maybe want to prevent changes for preventChanges(true, ['quotas']) but you want to pass changes for preventChanges(true, ['quotas.members']).

Also maybe you have a column with dot-notation. In postgres e.g. it's possible to define a column as 'quotas.members'. So you could have a column 'quotas' as integer and 'quotas.members' as integer in the same table. It's does not sound like a good design but it's possible.

So for maximum flexibility you maybe should define it like this:

before: {
    patch: [preventChanges(true, [{ name: 'quotas', nested: true }])]
}

where nested: true is your described behavior and nested: false would pass for service.patch(id, { 'quotas.members': 200 })

What do you think?

[claustres]

For sure we could manage it this way but I am not able to get your point and if it's necessary:

1. quotas is an atomic property and preventChanges(true, ['quotas']) will do the work.
2. quotas is a nested object and you'd like to prevent access to the whole subobject so that preventChanges(true, ['quotas']) should do the work as well.
3. quotas is a nested object and you'd like to prevent access to some of the subobject properties so that preventChanges(true, ['quotas.xxx', 'quotas.yyy']) should do the work.

As 1 and 2 are mutually exclusive I don't see the need to distinguish with a nestedflag but I might be wrong, let me know .

Another option is to be able to provide field names as a regex like this preventChanges(true, [/^quotas/]). and rely on test() to find matching data payload keys in this case (we could check if the field name is a regex object or a simple string I guess).

Yet, I wonder if we also need to support the following construct in Feathers for some adapters, which can make things harder as we need to 'dotify' the payload first: service.patch(id, { quotas: { members: 200 } })