diff --git a/onadata/apps/api/tests/models/test_project.py b/onadata/apps/api/tests/models/test_project.py index 283476e89d..690ae913a0 100644 --- a/onadata/apps/api/tests/models/test_project.py +++ b/onadata/apps/api/tests/models/test_project.py @@ -1,19 +1,15 @@ import os from onadata.apps.api import tools -from onadata.apps.api.tests.models.test_abstract_models import\ - TestAbstractModels - -from onadata.apps.logger.models.xform import XForm +from onadata.apps.api.tests.models.test_abstract_models import TestAbstractModels from onadata.apps.logger.models.instance import Instance from onadata.apps.logger.models.project import Project +from onadata.apps.logger.models.xform import XForm from onadata.apps.logger.xform_instance_parser import XLSFormError - from onadata.apps.main.tests.test_base import TestBase class TestProject(TestAbstractModels, TestBase): - def test_create_organization_project(self): organization = self._create_organization("modilabs", self.user) project_name = "demo" @@ -21,9 +17,10 @@ def test_create_organization_project(self): self.assertIsInstance(project, Project) self.assertEqual(project.name, project_name) - user_deno = self._create_user('deno', 'deno') + user_deno = self._create_user("deno", "deno") project = tools.create_organization_project( - organization, project_name, user_deno) + organization, project_name, user_deno + ) self.assertIsNone(project) def test_project_soft_delete_works_when_no_exception_is_raised(self): @@ -36,19 +33,17 @@ def test_project_soft_delete_works_when_no_exception_is_raised(self): organization = self._create_organization("modilabs", self.user) project_name = "demo" project = self._create_project(organization, project_name, self.user) - sample_xml = """Phonefuctionalbroken""" # noqa + sample_xml = """Phonefuctionalbroken""" # noqa XForm.objects.create( user=self.user, xml=sample_xml, - id_string='domestic_animals', - title='domestic_animals_in_kenyan_homes', - project=project + id_string="domestic_animals", + title="domestic_animals_in_kenyan_homes", + project=project, ) project.soft_delete() - self.assertEqual( - 1, Project.objects.filter(deleted_at__isnull=False).count()) - self.assertEqual( - 1, XForm.objects.filter(deleted_at__isnull=False).count()) + self.assertEqual(0, Project.objects.count()) + self.assertEqual(0, XForm.objects.count()) def test_project_detetion_reverts_when_an_exception_raised(self): """ @@ -63,26 +58,33 @@ def test_project_detetion_reverts_when_an_exception_raised(self): organization = self._create_organization("modilabs", self.user) project_name = "demo" project = self._create_project(organization, project_name, self.user) - sample_json = '{"default_language": "default", ' \ - '"id_string": "Water_2011_03_17", "children": [], ' \ - '"name": "Water_2011_03_17", ' \ - '"title": "Water_2011_03_17", "type": "survey"}' - f = open(os.path.join( - os.path.dirname(os.path.abspath(__file__)), - '../../../', - 'logger/tests/', - "Water_Translated_2011_03_10.xml" - )) + sample_json = ( + '{"default_language": "default", ' + '"id_string": "Water_2011_03_17", "children": [], ' + '"name": "Water_2011_03_17", ' + '"title": "Water_2011_03_17", "type": "survey"}' + ) + f = open( + os.path.join( + os.path.dirname(os.path.abspath(__file__)), + "../../../", + "logger/tests/", + "Water_Translated_2011_03_10.xml", + ) + ) xml = f.read() f.close() xform = XForm.objects.create( - xml=xml, user=self.user, json=sample_json, project=project) + xml=xml, user=self.user, json=sample_json, project=project + ) - f = open(os.path.join(os.path.dirname( - os.path.abspath(__file__)), - '../../../', - 'logger/tests/', - 'Water_Translated_2011_03_10_2011-03-10_14-38-28.xml') + f = open( + os.path.join( + os.path.dirname(os.path.abspath(__file__)), + "../../../", + "logger/tests/", + "Water_Translated_2011_03_10_2011-03-10_14-38-28.xml", + ) ) xml = f.read() f.close() @@ -92,24 +94,27 @@ def test_project_detetion_reverts_when_an_exception_raised(self): try: XForm.objects.raw( "UPDATE logger_xform SET id_string='a New ID String' \ - WHERE id={};".format(xform.id))[0] + WHERE id={};".format( + xform.id + ) + )[0] except TypeError: pass xform_refetch = XForm.objects.all()[0] - self.assertEqual('a New ID String', xform_refetch.id_string) + self.assertEqual("a New ID String", xform_refetch.id_string) with self.assertRaises(XLSFormError): project.soft_delete() - self.assertEqual(1, Project.objects.filter( - deleted_at__isnull=True).count()) + self.assertEqual(1, Project.objects.filter(deleted_at__isnull=True).count()) self.assertIsNone(project.deleted_at) - self.assertEqual(1, XForm.objects.filter( - project=project, deleted_at__isnull=True).count()) + self.assertEqual( + 1, + XForm.objects.filter(project=project, deleted_at__isnull=True).count(), + ) # Try deleting the Xform; it should also roll back due to the exception with self.assertRaises(XLSFormError): XForm.objects.all()[0].soft_delete() - self.assertEqual(1, XForm.objects.filter( - deleted_at__isnull=True).count()) + self.assertEqual(1, XForm.objects.filter(deleted_at__isnull=True).count()) self.assertIsNone(XForm.objects.all()[0].deleted_at) diff --git a/onadata/apps/api/tests/viewsets/test_attachment_viewset.py b/onadata/apps/api/tests/viewsets/test_attachment_viewset.py index 95b0948d1e..e5ec3b0eb4 100644 --- a/onadata/apps/api/tests/viewsets/test_attachment_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_attachment_viewset.py @@ -2,6 +2,7 @@ """ Test Attachment viewsets. """ + import os from django.utils import timezone @@ -211,14 +212,12 @@ def test_data_list_with_xform_in_delete_async(self): self.assertNotEqual(response.get("Cache-Control"), None) self.assertEqual(response.status_code, 200) self.assertTrue(isinstance(response.data, list)) - initial_count = len(response.data) self.xform.deleted_at = timezone.now() self.xform.save() request = self.factory.get("/", data={"xform": self.xform.pk}, **self.extra) response = self.list_view(request) - self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.data), initial_count - 1) + self.assertEqual(response.status_code, 404) def test_list_view_filter_by_xform(self): self._submit_transport_instance_w_attachment() diff --git a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py index 1b91199eba..b254e76ba8 100644 --- a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py @@ -2,6 +2,7 @@ """ Test BriefcaseViewset """ + import codecs import os import shutil @@ -792,9 +793,10 @@ def test_query_optimization_fence(self): self.assertEqual(instances.count(), optimized_instances.count()) op_sql_query = ( 'SELECT "logger_instance"."id", "logger_instance"."uuid" FROM "logger_instance"' - f' WHERE "logger_instance"."id" IN ({optimized_instances[0].get("pk")},' + f' WHERE ("logger_instance"."deleted_at" IS NULL AND "logger_instance"."id"' + f' IN ({optimized_instances[0].get("pk")},' f' {optimized_instances[1].get("pk")}, {optimized_instances[2].get("pk")},' - f' {optimized_instances[3].get("pk")})' + f' {optimized_instances[3].get("pk")}))' ) self.assertEqual(str(optimized_instances.query), op_sql_query) diff --git a/onadata/apps/api/tests/viewsets/test_metadata_viewset.py b/onadata/apps/api/tests/viewsets/test_metadata_viewset.py index 2640f6fd5c..8b6ca8a261 100644 --- a/onadata/apps/api/tests/viewsets/test_metadata_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_metadata_viewset.py @@ -2,6 +2,7 @@ """ Tests the MetaDataViewSet. """ + # pylint: disable=too-many-lines import os from builtins import open @@ -242,9 +243,7 @@ def test_delete_xform_deletes_media_metadata(self): self.xform.soft_delete() # Confirm that all metadata was deleted response2 = self.view(request) - self.assertEqual(response2.status_code, 200) - self.assertEqual(len(response2.data), 0) - self.assertEqual(response2.data, []) + self.assertEqual(response2.status_code, 404) def test_windows_csv_file_upload_to_metadata(self): data_value = "transportation.csv" diff --git a/onadata/apps/api/tests/viewsets/test_xform_viewset.py b/onadata/apps/api/tests/viewsets/test_xform_viewset.py index ab4e6e3be6..5245f15814 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_xform_viewset.py @@ -2,6 +2,7 @@ """ Tests the XForm viewset. """ + from __future__ import unicode_literals import codecs @@ -3629,7 +3630,6 @@ def test_delete_xform_async(self, mock_get_status): with HTTMock(enketo_mock): mock_get_status.return_value = {"job_status": "PENDING"} self._publish_xls_form_to_project() - count = XForm.objects.count() view = XFormViewSet.as_view( { "delete": "delete_async", @@ -3642,7 +3642,7 @@ def test_delete_xform_async(self, mock_get_status): self.assertEqual(response.status_code, 202) self.assertTrue("job_uuid" in response.data) self.assertTrue("time_async_triggered" in response.data) - self.assertEqual(count, XForm.objects.count()) + self.assertEqual(0, XForm.objects.count()) view = XFormViewSet.as_view({"get": "delete_async"}) @@ -3654,7 +3654,7 @@ def test_delete_xform_async(self, mock_get_status): self.assertEqual(response.status_code, 202) self.assertEqual(response.data, {"job_status": "PENDING"}) - xform = XForm.objects.get(pk=formid) + xform = XForm.objects.all_with_deleted().get(pk=formid) self.assertIsNotNone(xform.deleted_at) self.assertTrue("deleted-at" in xform.id_string) diff --git a/onadata/apps/logger/models/attachment.py b/onadata/apps/logger/models/attachment.py index 96f55b6061..290b9ccb6e 100644 --- a/onadata/apps/logger/models/attachment.py +++ b/onadata/apps/logger/models/attachment.py @@ -2,6 +2,7 @@ """ Attachment model. """ + import hashlib import mimetypes import os @@ -9,6 +10,8 @@ from django.contrib.auth import get_user_model from django.db import models +from onadata.libs.models import SoftDeleteManager + def get_original_filename(filename): """Returns the filename removing the hashed random string added to it when we have @@ -83,6 +86,8 @@ class Attachment(models.Model): on_delete=models.SET_NULL, ) + objects = SoftDeleteManager() + class Meta: app_label = "logger" diff --git a/onadata/apps/logger/models/data_view.py b/onadata/apps/logger/models/data_view.py index 51278f4f25..6de02b0d58 100644 --- a/onadata/apps/logger/models/data_view.py +++ b/onadata/apps/logger/models/data_view.py @@ -2,6 +2,7 @@ """ DataView model class """ + import datetime import json @@ -9,11 +10,12 @@ from django.contrib.gis.db import models from django.db import connection from django.db.models.signals import post_delete, post_save +from django.db.utils import DataError from django.utils import timezone from django.utils.translation import gettext as _ -from django.db.utils import DataError from onadata.apps.viewer.parsed_instance_tools import get_where_clause +from onadata.libs.models import SoftDeleteManager from onadata.libs.models.sorting import ( # noqa pylint: disable=unused-import json_order_by, json_order_by_params, @@ -22,8 +24,8 @@ from onadata.libs.utils.cache_tools import ( # noqa pylint: disable=unused-import DATAVIEW_COUNT, DATAVIEW_LAST_SUBMISSION_TIME, - XFORM_LINKED_DATAVIEWS, PROJ_OWNER_CACHE, + XFORM_LINKED_DATAVIEWS, safe_delete, ) from onadata.libs.utils.common_tags import ( @@ -123,6 +125,8 @@ class DataView(models.Model): blank=True, ) + objects = SoftDeleteManager() + class Meta: app_label = "logger" verbose_name = _("Data View") diff --git a/onadata/apps/logger/models/entity.py b/onadata/apps/logger/models/entity.py index 14bfb039cd..516400b0fe 100644 --- a/onadata/apps/logger/models/entity.py +++ b/onadata/apps/logger/models/entity.py @@ -2,8 +2,8 @@ Entity model """ -import uuid import importlib +import uuid from django.contrib.auth import get_user_model from django.db import models, transaction @@ -12,7 +12,7 @@ from onadata.apps.logger.models.entity_list import EntityList from onadata.apps.logger.models.instance import Instance from onadata.apps.logger.models.registration_form import RegistrationForm -from onadata.libs.models import BaseModel +from onadata.libs.models import BaseModel, SoftDeleteManager User = get_user_model() @@ -30,6 +30,8 @@ class Entity(BaseModel): deleted_at = models.DateTimeField(null=True, blank=True) deleted_by = models.ForeignKey(User, null=True, on_delete=models.SET_NULL) + objects = SoftDeleteManager() + def __str__(self) -> str: return f"{self.pk}|{self.entity_list}" diff --git a/onadata/apps/logger/models/entity_list.py b/onadata/apps/logger/models/entity_list.py index 669614a4d9..cb094f25dc 100644 --- a/onadata/apps/logger/models/entity_list.py +++ b/onadata/apps/logger/models/entity_list.py @@ -6,17 +6,16 @@ from django.contrib.auth.models import Group, Permission from django.contrib.contenttypes.fields import GenericRelation from django.db import models, transaction -from django.utils.translation import gettext_lazy as _ from django.utils import timezone +from django.utils.translation import gettext_lazy as _ - -from guardian.models import UserObjectPermissionBase, GroupObjectPermissionBase from guardian.compat import user_model_label +from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase from onadata.apps.logger.models.project import Project from onadata.apps.logger.models.xform import clear_project_cache from onadata.apps.main.models.meta_data import MetaData -from onadata.libs.models import BaseModel +from onadata.libs.models import BaseModel, SoftDeleteManager from onadata.libs.utils.model_tools import queryset_iterator User = get_user_model() @@ -43,6 +42,8 @@ class EntityList(BaseModel): deleted_at = models.DateTimeField(null=True, blank=True) deleted_by = models.ForeignKey(User, null=True, on_delete=models.SET_NULL) + objects = SoftDeleteManager() + def __str__(self): return f"{self.name}|{self.project}" diff --git a/onadata/apps/logger/models/instance.py b/onadata/apps/logger/models/instance.py index b487cd123d..ef095837ce 100644 --- a/onadata/apps/logger/models/instance.py +++ b/onadata/apps/logger/models/instance.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- +# pylint: disable=too-many-lines """ Instance model class """ + import math import sys from datetime import datetime @@ -18,15 +20,19 @@ from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext as _ -from multidb.pinning import use_master from celery import current_task from deprecated import deprecated +from multidb.pinning import use_master from taggit.managers import TaggableManager from onadata.apps.logger.models.submission_review import SubmissionReview from onadata.apps.logger.models.survey_type import SurveyType -from onadata.apps.logger.models.xform import XFORM_TITLE_LENGTH, XForm +from onadata.apps.logger.models.xform import ( + XFORM_TITLE_LENGTH, + SoftDeleteManager, + XForm, +) from onadata.apps.logger.xform_instance_parser import ( XFormInstanceParser, clean_and_parse_xml, @@ -35,11 +41,11 @@ from onadata.celeryapp import app from onadata.libs.data.query import get_numeric_fields from onadata.libs.utils.cache_tools import ( - PROJECT_DATE_MODIFIED_CACHE, DATAVIEW_COUNT, IS_ORG, PROJ_NUM_DATASET_CACHE, PROJ_SUB_DATE_CACHE, + PROJECT_DATE_MODIFIED_CACHE, XFORM_COUNT, XFORM_DATA_VERSIONS, XFORM_SUBMISSION_COUNT_FOR_DAY, @@ -386,7 +392,8 @@ def update_project_date_modified(instance_id, _): # the etag value of the projects endpoint try: instance = ( - Instance.objects.select_related("xform__project") + Instance.objects.all_with_deleted() + .select_related("xform__project") .only("xform__project__date_modified") .get(pk=instance_id) ) @@ -693,6 +700,7 @@ class Instance(models.Model, InstanceBaseClass): has_a_review = models.BooleanField(_("has_a_review"), default=False) tags = TaggableManager() + objects = SoftDeleteManager() class Meta: app_label = "logger" diff --git a/onadata/apps/logger/models/project.py b/onadata/apps/logger/models/project.py index fdef798b0e..91dfb4f44c 100644 --- a/onadata/apps/logger/models/project.py +++ b/onadata/apps/logger/models/project.py @@ -2,6 +2,7 @@ """ Project model class """ + from django.apps import apps from django.conf import settings from django.contrib.auth import get_user_model @@ -15,7 +16,7 @@ from guardian.shortcuts import assign_perm, get_perms_for_model from taggit.managers import TaggableManager -from onadata.libs.models.base_model import BaseModel +from onadata.libs.models.base_model import BaseModel, SoftDeleteManager from onadata.libs.utils.common_tags import OWNER_TEAM_NAME # pylint: disable=invalid-name @@ -118,7 +119,7 @@ class Project(BaseModel): on_delete=models.SET_NULL, ) - objects = models.Manager() + objects = SoftDeleteManager() tags = TaggableManager(related_name="project_tags") prefetched = PrefetchManager() diff --git a/onadata/apps/logger/models/xform.py b/onadata/apps/logger/models/xform.py index af4add620a..918877c495 100644 --- a/onadata/apps/logger/models/xform.py +++ b/onadata/apps/logger/models/xform.py @@ -2,6 +2,7 @@ """ The XForm model """ + # pylint: disable=too-many-lines import hashlib import json @@ -33,7 +34,7 @@ from taggit.managers import TaggableManager from onadata.apps.logger.xform_instance_parser import XLSFormError, clean_and_parse_xml -from onadata.libs.models.base_model import BaseModel +from onadata.libs.models.base_model import BaseModel, SoftDeleteManager from onadata.libs.utils.cache_tools import ( PROJ_BASE_FORMS_CACHE, PROJ_FORMS_CACHE, @@ -894,6 +895,7 @@ class XForm(XFormMixin, BaseModel): is_merged_dataset = models.BooleanField(default=False) is_instance_json_regenerated = models.BooleanField(default=False) tags = TaggableManager() + objects = SoftDeleteManager() class Meta: app_label = "logger" diff --git a/onadata/apps/main/models/meta_data.py b/onadata/apps/main/models/meta_data.py index ed9ab47875..a445e9ce24 100644 --- a/onadata/apps/main/models/meta_data.py +++ b/onadata/apps/main/models/meta_data.py @@ -2,13 +2,14 @@ """ MetaData model """ + from __future__ import unicode_literals +import hashlib import logging import mimetypes import os from contextlib import closing -import hashlib from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey @@ -23,9 +24,10 @@ import requests +from onadata.libs.models import SoftDeleteManager from onadata.libs.utils.cache_tools import ( - XFORM_METADATA_CACHE, XFORM_MANIFEST_CACHE, + XFORM_METADATA_CACHE, safe_delete, ) from onadata.libs.utils.common_tags import ( @@ -203,7 +205,7 @@ class MetaData(models.Model): object_id = models.PositiveIntegerField(null=True, blank=True) content_object = GenericForeignKey("content_type", "object_id") - objects = models.Manager() + objects = SoftDeleteManager() class Meta: app_label = "main" diff --git a/onadata/libs/models/__init__.py b/onadata/libs/models/__init__.py index 1de471a73a..d6163cf9e1 100644 --- a/onadata/libs/models/__init__.py +++ b/onadata/libs/models/__init__.py @@ -2,4 +2,6 @@ """ Model utility classes and functions. """ + from .base_model import BaseModel # noqa +from .base_model import SoftDeleteManager # noqa diff --git a/onadata/libs/models/base_model.py b/onadata/libs/models/base_model.py index 074aa9fa8d..37f20d7df0 100644 --- a/onadata/libs/models/base_model.py +++ b/onadata/libs/models/base_model.py @@ -2,6 +2,7 @@ """ BaseModel abstract class - sets date_created/date_modified fields. """ + from django.db import models @@ -15,3 +16,27 @@ class BaseModel(models.Model): class Meta: abstract = True + + +class SoftDeleteQuerySet(models.QuerySet): + """Custom queryset that only returns objects that have not been deleted""" + + def active(self): + """Return only objects that have not been deleted""" + return self.filter(deleted_at__isnull=True) + + def all_with_deleted(self): + """Return all objects, including those that have been deleted""" + return self + + +class SoftDeleteManager(models.Manager): + """Custom manager that only returns objects that have not been deleted""" + + def get_queryset(self): + """Return queryset that only returns objects that have not been deleted""" + return SoftDeleteQuerySet(self.model, using=self._db).active() + + def all_with_deleted(self): + """Return all objects, including those that have been deleted""" + return SoftDeleteQuerySet(self.model, using=self._db)