Calling Create where a deleted object violates uniqueness

[ShmuelTreiger]

Currently, we have overridden update_or_create() where it will undelete an object if a unique constraint is violated. I would expect similar behavior from a create() override, but this is not currently the case. At the very least, I would love an error informing that the object exists and has been deleted.

Thoughts on implementing either of these options?

[Gagaro]

I think create should not revive an existing instance. We expect a completely new one when using create, this could show an error though (by overwriting the check unique on the model?).

In my case, I try to use partial unique constraints:

UniqueConstraint(fields=['user'], condition=Q(deleted__isnull=True), name='unique_user')

So only active instances are concerned by the unique constraint. Maybe we should document that, it's not the first time I see issue about uniqueness.

[ShmuelTreiger]

So only active instances are concerned by the unique constraint

Hmmm. But then what happens when you try to revive the deleted object? Or you query all_objects and suddenly get 2? Obviously I know the answers, but I'm building tools for people whom I don't expect to be familiar with safedelete, so I'm trying to build in a way which reduces confusion.

Maybe we should document that, it's not the first time I see issue about uniqueness.

This would be great documentation to add. Would have been super helpful to me, and I'm debating going back and adding it in to my code.

this could show an error though

I think this would be great. Something like:

def create(self, *args, **kwargs):
    if model.has_unique_field():
        <query for existing database entry which violates uniqueness and is marked deleted>
            raise Error("Creation of object with <unique arguments> violates Unique constraint with a deleted object; pk=<>")
    super().create(*args, **kwargs)

I think create should not revive an existing instance.

What about making this an option on each model (default to no)?

self.REVIVE_DEAD_OBJECT_ON_CREATE = FALSE

[ShmuelTreiger]

Actually, would be much better to do:

def create(self, *args, **kwargs):
    try:
        super().create(*args, **kwargs)
    except IntegrityError:
      if model.has_unique_field():
          <query for existing database entry which violates uniqueness and is marked deleted>
              raise Error("Creation of object with <unique arguments> violates Unique constraint with a deleted object; pk=<>")
      raise IntegrityError

There's probably a way to wrap it all up in one query, which I think is what you were suggesting about overwriting the model's check unique. I don't know much about it, but I'd be happy to look into it.

[Gagaro]

So only active instances are concerned by the unique constraint

Hmmm. But then what happens when you try to revive the deleted object? Or you query all_objects and suddenly get 2? Obviously I know the answers, but I'm building tools for people whom I don't expect to be familiar with safedelete, so I'm trying to build in a way which reduces confusion.

People should at least know that their deleted objects are not, in fact, deleted. Imho it would be more confusing to create an object, only to find it already have things set (optional fields which come for the revived object and not set during the "creation").

Maybe we should document that, it's not the first time I see issue about uniqueness.

This would be great documentation to add. Would have been super helpful to me, and I'm debating going back and adding it in to my code.

I'll add that somewhere.

this could show an error though (by overwriting the check unique on the model?).

I think this would be great. Something like:

def create(self, *args, **kwargs):
    if model.has_unique_field():
        <query for existing database entry which violates uniqueness and is marked deleted>
            raise Error("Creation of object with <unique arguments> violates Unique constraint with a deleted object; pk=<>")
    super().create(*args, **kwargs)

I think create should not revive an existing instance.

What about making this an option on each model (default to no)?

self.REVIVE_DEAD_OBJECT_ON_CREATE = FALSE

I still don't think this is a good idea, I think this should be done in a custom base model (and I'd actually prefer the update_or_create method to not revive the instance as well, but it has been like that for a long time so I'd rather not touch it now).

[ShmuelTreiger]

Not entirely sure what you mean here. Something like this?

class SafeDeleteModelWarnsOnCreateWhenObjectDeleted(SafeDeleteModel):

I also couldn't really figure out an easier way to raise the proper error other than the method I proposed above

[Gagaro]

Yes something like that with your own create.