Additional where param check for a deleted field existence on delete method.

[ivatra]

Description

This pull request addresses an issue in the createDeleteParams function, specifically when attempting to delete a field that has already been marked as deleted. The current implementation sets a new value in the "deletedAt" field (e.g., a new date), which may not be the most intuitive behavior. Instead, it might be more appropriate for Prisma to throw an error. This suggestion may be seen as controversial since Prisma typically returns an error stating the record to delete could not be found, but in our case, it returns an error indicating the record to update was not found. This pull request includes a proposed solution for consideration, aiming to clarify the behavior when repeatedly deleting a record.

Current Behavior and Issue

when the following query is executed:

const res = await prisma.key.delete({
  where: {
    id: 6
  },
  include: {
    game: true,
    manager: true,
  },
});

The actual result is:

{
  "id": 6,
  "managerId": 3,
  "gameId": 10,
  "deletedAt": "2023-12-27T01:57:28.486Z"
}

However, the expected behavior in case of a deletion attempt on an already deleted record should either be null or an error should be thrown, indicating the operation's invalidity.

Proposed Solution

The issue can be resolved by modifying the function to incorporate a check that prevents updating a field if it has already been updated. This can be achieved by spreading ...{ [field]: null } into the where object of the result parameter.

Envirnoment

Prisma-soft-delete-middleware version: 1.1.2
Prisma version: 5.2.0
Node.js version: 18.12.1

[olivierwilkinson]

Hi there,

Thanks for raising this! It's true the behaviour doesn't match the expected behaviour, unfortunately solving the problem requires some more work. It's not possible to pass deleted: null to update queries without Prisma v5 or using the extendedWhereUnique preview feature in Prisma v4.

I'm going to make a separate issue once I have a clearer idea of the problem space and how to support all client versions without having the behaviour change unexpectedly on upgrade. 👍

I'll be sure to reference this PR in the issue and any changes I make 😄

[simon-caignart]

Hey!
Any news regarding this? I was also surprised by this behavior.