Repository navigation
Better handle paranoid models #405
Description
We should be cautious of any methods that do get/find AFTER modification. This can cause issues when using paranoid models. Sequelize's paranoid option automatically adds WHERE deleatedAt IS NULL to queries. This means if the user patch/updates deletedAt: new Date(), the the adapter's uses of _getOrFind after the Model.update will fail.
For example service.patch(1, { deletedAt: new Date() }) will update the record, but will then throw NotFound due to this line:
feathers-sequelize/src/adapter.ts
Line 346 in 46af5bd
I am not sure if this library can accomplish this. The developer should not be using patch/update to remove. Note error will still happen if using the underscore or dollar methods too. We could probably inspect the Model to determine the paranoid setting and deleted key, but thats super complicated.
I don't know if this should be fixed or documented, but I wanted to capture it here.
Activity
Servaker commentedon Dec 1, 2022
There are situations when the master data is maintained in a third-party system and transferred automatically to the Featners App. The same record from the master system can come in the beginning with the status
deletedand then be restored. If Sequelize is used with theparanoidoption enabled, then the only way to restore the record, i.e. setdeleted_at=nullfield, this is to execute theservice.patch(1, { deleted_at: null })method withparanoid=falseparameter set in the request. As a result the correct line with all fields anddeletedAt: nullis produced. I don't see the errorNoteFoundyou are reporting.If the
service.patch(1, { deletedAt: null })method is executed without theparanoid=falseparameter set, then the record will not be updated, the timestamp in thedeleted_atfield will be preserved, and aNotFoundmessage will be returned in response.Executing the
service.update(1, { deleted_at: null })method, even with theparanoid=falseparameter, does not remove the deletion timestamp and does not restore the record.feathers-sequelize 6.3.6.DaddyWarbucks commentedon Dec 2, 2022
@Servaker This only happens when "setting"
deletedAtto a date, not when "unsetting" it with null. Anything that does{ deletedAt: null }works as expected. But when trying to update/patch with{ deletedAt: new Date() }it fails as described.The developer really shouldn't be using patch/update to set
{ deletedAt: new Date() }and should just use the remove method. But it's still a potential error.DaddyWarbucks commentedon Jun 19, 2024
This should be fixed with V8. We no longer fetch data after mutative methods. So the data/query should no longer clash.