Skip to content
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

Refactor to use SoftDeleteManager #2725

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 45 additions & 40 deletions onadata/apps/api/tests/models/test_project.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
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"
project = self._create_project(organization, project_name, self.user)
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 = """<?xml version="1.0" ?><h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:xsd="http://www.w3.org/2001/XMLSchema"><h:head><h:title>Phone</h:title><model><instance><phone id="Phone_2011-02-04_00-09-18"><device_id/><start/><end/><visible_id/><phone_number/><status/><note/></phone></instance><bind jr:preload="property" jr:preloadParams="deviceid" nodeset="/phone/device_id" required="true()" type="string"/><bind jr:preload="timestamp" jr:preloadParams="start" nodeset="/phone/start" required="true()" type="dateTime"/><bind jr:preload="timestamp" jr:preloadParams="end" nodeset="/phone/end" required="true()" type="dateTime"/><bind constraint="regex(., '^\d{3}$')" jr:constraintMsg="Please enter the three digit string from the back of the phone." nodeset="/phone/visible_id" required="true()" type="string"/><bind nodeset="/phone/phone_number" required="true()" type="string"/><bind nodeset="/phone/status" required="true()" type="select1"/><bind nodeset="/phone/note" required="true()" type="string"/></model></h:head><h:body><input ref="/phone/visible_id"><label>What is the three digit label on the back of this phone?</label></input><input ref="/phone/phone_number"><label>What is the current phone number to call this phone?</label></input><select1 ref="/phone/status"><label>What is the status of this phone?</label><item><label>Functional</label><value>fuctional</value></item><item><label>Broken</label><value>broken</value></item></select1><input ref="/phone/note"><label>Please enter any comments about this phone.</label></input></h:body></h:html>""" # noqa
sample_xml = """<?xml version="1.0" ?><h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:xsd="http://www.w3.org/2001/XMLSchema"><h:head><h:title>Phone</h:title><model><instance><phone id="Phone_2011-02-04_00-09-18"><device_id/><start/><end/><visible_id/><phone_number/><status/><note/></phone></instance><bind jr:preload="property" jr:preloadParams="deviceid" nodeset="/phone/device_id" required="true()" type="string"/><bind jr:preload="timestamp" jr:preloadParams="start" nodeset="/phone/start" required="true()" type="dateTime"/><bind jr:preload="timestamp" jr:preloadParams="end" nodeset="/phone/end" required="true()" type="dateTime"/><bind constraint="regex(., '^\d{3}$')" jr:constraintMsg="Please enter the three digit string from the back of the phone." nodeset="/phone/visible_id" required="true()" type="string"/><bind nodeset="/phone/phone_number" required="true()" type="string"/><bind nodeset="/phone/status" required="true()" type="select1"/><bind nodeset="/phone/note" required="true()" type="string"/></model></h:head><h:body><input ref="/phone/visible_id"><label>What is the three digit label on the back of this phone?</label></input><input ref="/phone/phone_number"><label>What is the current phone number to call this phone?</label></input><select1 ref="/phone/status"><label>What is the status of this phone?</label><item><label>Functional</label><value>fuctional</value></item><item><label>Broken</label><value>broken</value></item></select1><input ref="/phone/note"><label>Please enter any comments about this phone.</label></input></h:body></h:html>""" # 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)
5 changes: 2 additions & 3 deletions onadata/apps/api/tests/viewsets/test_attachment_viewset.py
Original file line number Diff line number Diff line change
@@ -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()
6 changes: 4 additions & 2 deletions onadata/apps/api/tests/viewsets/test_briefcase_viewset.py
Original file line number Diff line number Diff line change
@@ -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)

5 changes: 2 additions & 3 deletions onadata/apps/api/tests/viewsets/test_metadata_viewset.py
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 3 additions & 3 deletions onadata/apps/api/tests/viewsets/test_xform_viewset.py
Original file line number Diff line number Diff line change
@@ -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)
5 changes: 5 additions & 0 deletions onadata/apps/logger/models/attachment.py
Original file line number Diff line number Diff line change
@@ -2,13 +2,16 @@
"""
Attachment model.
"""

import hashlib
import mimetypes
import os

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"

8 changes: 6 additions & 2 deletions onadata/apps/logger/models/data_view.py
Original file line number Diff line number Diff line change
@@ -2,18 +2,20 @@
"""
DataView model class
"""

import datetime
import json

from django.conf import settings
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")
6 changes: 4 additions & 2 deletions onadata/apps/logger/models/entity.py
Original file line number Diff line number Diff line change
@@ -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}"

9 changes: 5 additions & 4 deletions onadata/apps/logger/models/entity_list.py
Original file line number Diff line number Diff line change
@@ -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}"

16 changes: 12 additions & 4 deletions onadata/apps/logger/models/instance.py
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 3 additions & 2 deletions onadata/apps/logger/models/project.py
Original file line number Diff line number Diff line change
@@ -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()

4 changes: 3 additions & 1 deletion onadata/apps/logger/models/xform.py
Original file line number Diff line number Diff line change
@@ -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"
8 changes: 5 additions & 3 deletions onadata/apps/main/models/meta_data.py
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions onadata/libs/models/__init__.py
Original file line number Diff line number Diff line change
@@ -2,4 +2,6 @@
"""
Model utility classes and functions.
"""

from .base_model import BaseModel # noqa
from .base_model import SoftDeleteManager # noqa
25 changes: 25 additions & 0 deletions onadata/libs/models/base_model.py
Original file line number Diff line number Diff line change
@@ -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)