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

softDelete() provider should not inherit from original params #600

Open rodeyseijkens opened 4 years ago

rodeyseijkens commented 4 years ago

https://github.com/feathersjs-ecosystem/feathers-hooks-common/blob/f91393d839e0f155ccf15f7e7d5f6e1b35977015/lib/services/soft-delete.js#L37

So in the softDelete function the params.provider is inherited from the original method (remove), but it should be an internal call like it was in previous versions (feathers-plus) of softDelete, this was done by setting the provider inside of softDelete to undefined. This is better for permissions etc, cause for example people are allowed to delete (and so call the remove hook) but aren't allowed to patch.

Now the question is how will this be changed, are we adding that back by default like something:

 const result = await service.patch(context.id, data, { ...params, provider: undefined });

Or are we going to add it with a property option like:

softDelete({
  removeParams: async () => {
    return { provider: undefined };
  },
})

And merge it just like how we do with the query.

I think the removeParams is more flexible in my opinion but it is also a bit double cause we have a deletedQuery, what is technically inside the params.query.

Or maybe we should add an removeProvider function, any thoughts?

Depending whats the outcome of this is I can make a PR for either implementation.

Aschmider24 commented 3 months ago

+1 on this featuree if this is not too late.

I think the second implementation leaves the user more flexibility.