-
Notifications
You must be signed in to change notification settings - Fork 317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing Manually cascade delete M2M models when related model is deleted #2620
base: develop
Are you sure you want to change the base?
Changes from 6 commits
28d3d5b
a743a29
ad3749d
5b7985a
f6334b8
3892c2b
e159212
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,10 @@ def save(self, *args, **kwargs) -> None: | |
return super().save(*args, **kwargs) | ||
|
||
def delete(self, *args, **kwargs) -> None: | ||
if ConsultationBed.objects.filter(bed=self).exists(): | ||
error = f"Cannot delete Bed {self} because they are referenced as `bed` in ConsultationBed records." | ||
raise ValidationError(error) | ||
|
||
DraKen0009 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we actually delete this ? ie call the super delete method ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, didnt see the old bits ! |
||
AssetBed.objects.filter(bed=self).update(deleted=True) | ||
super().delete(*args, **kwargs) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from django.conf import settings | ||
from django.contrib.auth import get_user_model | ||
from django.contrib.postgres.fields import ArrayField | ||
from django.core.exceptions import ValidationError | ||
from django.core.validators import MinValueValidator | ||
from django.db import models, transaction | ||
from django.db.models import IntegerChoices | ||
|
@@ -302,14 +303,26 @@ def save(self, *args, **kwargs) -> None: | |
|
||
@transaction.atomic | ||
def delete(self, *args): | ||
from care.facility.models.asset import Asset, AssetLocation | ||
from care.facility.models.asset import ( | ||
Asset, | ||
AssetLocation, | ||
FacilityDefaultAssetLocation, | ||
) | ||
from care.facility.models.patient_sample import PatientSample | ||
DraKen0009 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if FacilityDefaultAssetLocation.objects.filter(facility=self).exists(): | ||
error = f"Cannot delete Facility {self} because they are referenced as `facility` in FacilityDefaultAssetLocation records." | ||
raise ValidationError(error) | ||
AssetLocation.objects.filter(facility_id=self.id).update(deleted=True) | ||
Asset.objects.filter( | ||
current_location_id__in=AssetLocation._base_manager.filter( # noqa: SLF001 | ||
facility_id=self.id | ||
).values_list("id", flat=True) | ||
).update(deleted=True) | ||
FacilityUser.objects.filter(facility=self).delete() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shifting might also be affected |
||
PatientSample.objects.filter(testing_facility=self).update( | ||
testing_facility=None | ||
) | ||
return super().delete(*args) | ||
|
||
@property | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make more sense to create a static errors and make it translatable as well, also keep the errors very human readable like