feathersjs-ecosystem / feathers-hooks-common

Useful hooks for use with FeathersJS services.
https://hooks-common.feathersjs.com
MIT License
193 stars 89 forks source link

refactor(set-field hooks): Handle 'as' field startsWith data and data type array #682

Closed arnoldtkl closed 2 years ago

arnoldtkl commented 2 years ago

To handle the use case where multi:true, create method, the data is in array format. E.g Inside hook

before: {
  all: [],
  find: [],
  get: [],
  create: [setField({ from: "params.user.id", as: "data.user_id" })]
  update:[],
  patch: [],
  remove: []
}

When POST array of data, currently the merge result will become

[ 
  { key: data, key2: data },
  { sample: data, sample2: data },
  user_id: "sample_id"
]

Expected result:

[ 
  { key: data, key2: data, user_id: "sample_id" },
  { sample: data, sample2: data, user_id: "sample_id" }
]
fratzinger commented 2 years ago

Nice! Thanks for the PR! I made a similar hook at feathers-utils called setData a while ago.

I don't know if I would expect setField to throw an error on arrays or that it just works as you intended in your PR.

Reasons for throwing an error:

What do you think @arnoldtkl? Clearly we cannot leave setField to ignore multi:true at all but should we maybe add a new setData that is more declarative than setField?

arnoldtkl commented 2 years ago

Thanks for the quick reply! I agree with you, setData is more straightforward and declarative. Maybe we can make a note at the setField documentation, e.g when dealing in multi:true service , the result may not work as expected, we will recommend check on setData ......

By the way, I will close this pull request. Because my code didn't cover nested arrays and not handling well in the as field format.

fratzinger commented 2 years ago

Very good point to add it to the docs. Would you be up to add a PR for that? Any help is highly appreciated.

arnoldtkl commented 2 years ago

Ok, sure. But, the setData not yet included in this library. I will write a note about handling multi:true may not work as expected.