How to Raise DoesNotExist - Django SoftDelete?

Question

I have a Abstract Model SoftDelete like follow.

class SoftDeleteManager(models.Manager):

    def get_queryset(self):
        return super().get_queryset().filter(is_deleted=False)

class SoftDeleteModel(models.Model):
     is_deleted = models.BooleanField(default=0)
     deleted_at = models.DateTimeField(null=True)

     objects = SoftDeleteManager()

     def delete(self):
         self.is_deleted = True
         self.deleted_at = timezone.now()
         self.save()

     class Meta:
        abstract = True

class Employee(SafeDeleteModel):
   pass

Whenever model gets deleted i set is_deleted to True and updating the timestamp deleted_at, and created the custom manager to override initial queryset which return only non deleted fields(is_deleted=False).

employee = Employee.objects.get(pk=1)
employee.delete()
employee.refresh_from_db() // not raising DoesNotExist

But lets say i have a Employee model which uses the SafeDeleteModel for soft delete, after deleting the model like Employee.objects.get(pk=1).delete() when i call employee.refresh_from_db(),its not raising DoesNotExist, but updates the value of is_deleted, deleted_at as expected, what mistake i made here, why its not raising DoesNotExist?

It will raise error only when you completely delete the object but here you are just populating a field is_deleted as true.so when you again going to delete it,it is able to find the object again and not raising the object does not exist. — VIVEK VIKASH, Apr 02 '19 at 12:09
@VIVEKVIKASH (https://github.com/makinacorpus/django-safedelete)[this] library also they are updating a field. called `deleted`, still it raises `DoesNotExist` — Hari, Apr 02 '19 at 12:12
I copy&pasted your code and cannot reproduce your issue. `refresh_from_db()` raises DoesNotExist. (note: I wrote `abstract = True` not `abstarct = True`, assuming this is just a typo on SO, not in your code) — dirkgroten, Apr 02 '19 at 13:37
> test = Deletable.objects.create() > test > test.is_deleted 0 > test.delete() > test.refresh_from_db() user.models.DoesNotExist: Deletable matching query does not exist. — dirkgroten, Apr 02 '19 at 13:42
@dirkgroten i tested in `shell`, still i am getting same issue. let me check one more time — Hari, Apr 02 '19 at 13:43
Wait, according to [this](https://docs.djangoproject.com/en/2.2/_modules/django/db/models/base/#Model.refresh_from_db) `refresh_from_db` is using `_base_manager`, not `_default_manager`, in Django 1.11 it's using `_default_manager`. Seems this was changed from 2.0 to 2.1. — dirkgroten, Apr 02 '19 at 13:50
yes, i don't see any release notes mentioning a change in `refresh_from_db` so i didn't think it would have changed. and I don't understand why it has changed. — dirkgroten, Apr 02 '19 at 13:55
from [https://docs.djangoproject.com/en/2.1/topics/db/managers/#django.db.models.Model._base_manager](this) they mentioned like not to filter any results in base_manager, but in my case it make sense right? — Hari, Apr 02 '19 at 13:55
@dirkgroten so i have to add custom manager as _base_manager right? — Hari, Apr 02 '19 at 13:56
if you do that, it'll work yes. Meaning also that related lookups will filter out soft deleted objects correctly. I can see why refresh_from_db was changed, it make sense to use the base_manager. — dirkgroten, Apr 02 '19 at 13:57

dirkgroten · Accepted Answer · 2019-04-02T14:34:26.957

1

There's been a change in Django 2.1: refresh_from_db() now uses a model's _base_manager, not the _default_manager anymore, like for related queries. This to ensure an object can be refreshed even if it cannot be found by the default manager.

So you should set your SoftDeleteManager as the base manager using base_manager_name. But note this comment:

Base managers aren’t used when querying on related models. For example, if the Question model from the tutorial had a deleted field and a base manager that filters out instances with deleted=True, a queryset like Choice.objects.filter(question__name__startswith='What') would include choices related to deleted questions.

Also I don't know how you would retrieve any object that has been deleted after you make this change, except if you make a special manager to not filter the deleted objects (e.g. deleted_objects).

Note also that I would expect the safedelete package you referred to in your comments to have the same issue, as it's not changing the _base_manager either.

edited Apr 02 '19 at 14:34

answered Apr 02 '19 at 14:00

dirkgroten

20,112
2
29
42

`Base managers aren’t used when querying on related models.`, means it won't use any managers when querying related models? and from the docs `A manager that filters results in get_queryset() is not appropriate for use as a base manager.` so using my custom manager as `base_manager` is bad practice? – Hari Apr 02 '19 at 15:48
1

It's a bit confusing. Assuming `Employee` has a `ForeignKey` to `Company`. The base manager is used when you do this: `company.employees.all()` will be based on the base manager, but `Company.objects.filter(employees__name__icontains="charles")` will not. Because `objects` here is the `Company`'s manager and the query expression doesn't use the related manager. – dirkgroten Apr 02 '19 at 15:56
1

The reason Django docs says it's not appropriate to filter out results in a base manager is that in the above case `company.employees.all()` will not give you the deleted objects, so you can't even do `company.employees.filter(is_deleted=True)`. So there's no way to get the filtered out objects even if you wanted to. – dirkgroten Apr 02 '19 at 15:58
1

Also aggregations like `Company.objects.annotate(num=Count('employees'))` will contain your deleted instances, because they are direct database queries. – dirkgroten Apr 02 '19 at 16:01
Thanks for all the valuable comments., i think though my custom manager filter deleted objects like `company.employees.all()` , still its a desired behaviour(since we are filtering deleted objects only)., so i think my using custom manager as base manager makes sense. – Hari Apr 02 '19 at 16:08
1

Yes, probably, just beware of the related queries, especially in the future, that might give unexpected results if you forget to filter on `is_deleted`. – dirkgroten Apr 02 '19 at 16:11
Yes, I think in such scenario, splitting queries like `ques_pks = Question.objects.values("id").filter(name__startswith='what')` and `Choice.objects.filter(pk__in=set(ques_pks))` will make sense, but comes with cost of performance and multiple queries. – Hari Apr 02 '19 at 16:19

How to Raise DoesNotExist - Django SoftDelete?

1 Answers1