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

Add Support for Case Deletion #33831

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
3570ec2
Add support for deleting individual cases (dry run only)
minhaminha Oct 25, 2023
580b5a1
Modify UI based on feedback (list structure, extra deletion confirmat…
minhaminha Nov 3, 2023
556d299
More UI changes based on feedback (list reorganization, using list-gr…
minhaminha Nov 17, 2023
5f1c4c4
Reconstruct form name if xmlns_to_name returns the xmlns
minhaminha Nov 21, 2023
707da7d
Add support for deleting cases and forms (no more dry run) + add tests
minhaminha Dec 1, 2023
8847642
Add line in task to delete old enough soft deleted cases + change wai…
minhaminha Dec 4, 2023
52b9443
Merge branch 'master' into ml/case-deletion
minhaminha Dec 4, 2023
870f8ca
add es_test decorator
minhaminha Dec 4, 2023
f78631c
Make small changes based on PR feedback
minhaminha Dec 5, 2023
d96c8a9
Refactor get_cases_and_forms_for_deletion in 3 separate functions + a…
minhaminha Dec 6, 2023
4e36269
Use attrs classes instead of complex dictionaries to pass around disp…
minhaminha Dec 8, 2023
3481e4f
Further break down get_case_and_display_data
minhaminha Dec 13, 2023
588b377
Disable hard deletion of eligible data
minhaminha Dec 13, 2023
e91c974
Undo delete task changes
minhaminha Dec 14, 2023
53852ac
Integrate case deletion into form deletion if the form had created an…
minhaminha Jan 25, 2024
f40be4b
Add more tests for form deletion, new case deletion util functions
minhaminha Jan 25, 2024
3c00173
Various refactors based on PR feedback
minhaminha Feb 8, 2024
fe480b3
Refactor tests + add more tests
minhaminha Feb 9, 2024
0f5e737
Refactor main case walk function into two smaller class methods, upda…
minhaminha Feb 13, 2024
2ee8572
Remove case hard deletion method (in favor of eventual tombstoning me…
minhaminha Feb 13, 2024
52c42bb
Merge branch 'master' into ml/case-deletion
minhaminha Feb 15, 2024
98dd353
small nit changes
minhaminha Feb 23, 2024
86f5deb
Make walk_through_case_forms always returns a dict
minhaminha Feb 23, 2024
18fd6ba
Add in memory caching for forms and case blocks
minhaminha Feb 26, 2024
9c65a10
Make sure forms are actually cached in TempFormCache, remove domain a…
minhaminha Mar 4, 2024
83872d4
Text changes + fix escaping issue
minhaminha Mar 5, 2024
76c6760
Merge branch 'master' into ml/case-deletion
minhaminha Mar 7, 2024
e5b25f8
Make small changes (remove manual escaping, add more tests about the …
minhaminha Mar 11, 2024
09ad4ff
Add TempCaseCache and tests, add cleanup to tests that create forms a…
minhaminha Mar 12, 2024
b93a8cb
Simplify test to save only once
minhaminha Mar 12, 2024
cf79b54
Merge branch 'master' into ml/case-deletion
minhaminha Mar 22, 2024
d3197eb
Fix bug caused by returned form list if there is no create form
minhaminha Mar 22, 2024
53f3877
Fix bug that raises a 404 when getting apps without an app_id
minhaminha Mar 25, 2024
1e49ef8
Minor text and code change suggestions
minhaminha Mar 25, 2024
e1fc391
Fix 404 raising bug when trying to fetch deleted forms + small refact…
minhaminha Mar 26, 2024
e5a2f9b
Fix bug that prevents bulk imports from able to delete
minhaminha Mar 26, 2024
9538140
Fix form order for large deletions
minhaminha Mar 28, 2024
55d63a3
Make form and case soft deletion all or nothing + prevent same forms …
minhaminha Mar 29, 2024
37d3f06
Minor fix
minhaminha Mar 29, 2024
23f27df
Small typo fix
minhaminha Mar 29, 2024
2565d07
Small typo fix part 2
minhaminha Apr 1, 2024
3a9d827
Reverse sorted list so latest form is archived first
minhaminha Apr 1, 2024
04bdb7b
Add test covering bulk form archive error handling
minhaminha Apr 2, 2024
1b1b86d
Typo fix in success message
minhaminha Apr 3, 2024
1b4b297
Make soft_delete_cases_and_forms a celery task (needs more work)
minhaminha Apr 3, 2024
c9b879e
Merge branch 'master' into ml/case-deletion
minhaminha Apr 3, 2024
b062be8
Merge branch 'master' into ml/case-deletion
gherceg May 28, 2024
bfa16b7
Merge branch 'master' into ml/case-deletion
minhaminha Jun 25, 2024
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
6 changes: 5 additions & 1 deletion corehq/apps/cleanup/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from corehq.apps.cleanup.utils import get_cutoff_date_for_data_deletion
from corehq.apps.domain.models import Domain
from corehq.apps.hqwebapp.tasks import mail_admins_async
from corehq.form_processor.models import XFormInstance
from corehq.form_processor.models import CommCareCase, XFormInstance


UNDEFINED_XMLNS_LOG_DIR = settings.LOG_HOME

Expand All @@ -34,10 +35,13 @@ def permanently_delete_eligible_data(dry_run=False):
dry_run_tag = '[DRY RUN] ' if dry_run else ''
cutoff_date = get_cutoff_date_for_data_deletion()
form_counts = XFormInstance.objects.hard_delete_forms_before_cutoff(cutoff_date, dry_run=dry_run)
case_counts = CommCareCase.objects.hard_delete_cases_before_cutoff(cutoff_date, dry_run=dry_run)

logger.info(f"{dry_run_tag}'permanently_delete_eligible_data' ran with the following results:\n")
for table, count in form_counts.items():
logger.info(f"{dry_run_tag}{count} {table} objects were deleted.")
for table, count in case_counts.items():
logger.info(f"{dry_run_tag}{count} {table} objects were deleted.")


@periodic_task(run_every=crontab(minute=0, hour=0), queue=getattr(settings, 'CELERY_PERIODIC_QUEUE', 'celery'))
Expand Down
4 changes: 4 additions & 0 deletions corehq/apps/reports/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ class TableauAPIError(Exception):
def __init__(self, message, code=None):
self.code = int(code) if code else None
super().__init__(message)


class TooManyCases(ValueError):
pass
259 changes: 257 additions & 2 deletions corehq/apps/reports/standard/cases/case_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import csv
import io
import re
from attrs import define, field
from collections import defaultdict
from datetime import datetime

Expand All @@ -15,7 +16,7 @@
JsonResponse,
)
from django.utils.decorators import method_decorator
from django.utils.html import format_html
from django.utils.html import escape, format_html
from django.utils.translation import get_language
from django.utils.translation import gettext as _
from django.utils.translation import gettext_lazy
Expand All @@ -24,6 +25,7 @@
from django_prbac.utils import has_privilege
from memoized import memoized

from casexml.apps.case import const
from casexml.apps.case.cleanup import close_case, rebuild_case_from_forms
from casexml.apps.case.mock import CaseBlock
from casexml.apps.case.templatetags.case_tags import case_inline_display
Expand All @@ -42,14 +44,15 @@
from corehq.apps.accounting.utils import domain_has_privilege
from corehq.apps.analytics.tasks import track_workflow
from corehq.apps.app_manager.const import USERCASE_TYPE
from corehq.apps.app_manager.dbaccessors import get_latest_app_ids_and_versions
from corehq.apps.app_manager.dbaccessors import get_latest_app_ids_and_versions, get_app
from corehq.apps.data_dictionary.models import CaseProperty
from corehq.apps.data_dictionary.util import is_case_type_deprecated
from corehq.apps.domain.decorators import login_and_domain_required
from corehq.apps.export.const import KNOWN_CASE_PROPERTIES
from corehq.apps.export.models import CaseExportDataSchema
from corehq.apps.export.utils import is_occurrence_deleted
from corehq.apps.hqcase.utils import (
CASEBLOCK_CHUNKSIZE,
EDIT_FORM_XMLNS,
resave_case,
submit_case_blocks,
Expand All @@ -66,8 +69,10 @@
)
from corehq.apps.products.models import SQLProduct
from corehq.apps.reports.display import xmlns_to_name
from corehq.apps.reports.exceptions import TooManyCases
from corehq.apps.reports.view_helpers import case_hierarchy_context
from corehq.apps.reports.views import (
archive_form,
DATE_FORMAT,
BaseProjectReportSectionView,
get_data_cleaning_updates,
Expand All @@ -76,6 +81,7 @@
from corehq.apps.users.models import HqPermissions
from corehq.form_processor.exceptions import CaseNotFound
from corehq.form_processor.interfaces.dbaccessors import LedgerAccessors
from corehq.form_processor.interfaces.processor import FormProcessorInterface
from corehq.form_processor.models import (
CommCareCase,
UserRequestedRebuild,
Expand Down Expand Up @@ -555,6 +561,255 @@ def close_case_view(request, domain, case_id):
return HttpResponseRedirect(reverse('case_data', args=[domain, case_id]))


MAX_CASE_COUNT = CASEBLOCK_CHUNKSIZE
MAX_SUBCASE_DEPTH = 3


def get_case_and_display_data(case_obj, domain):
"""
Given a case object, recursively checks the case's related submission forms and for each form, the related
cases it has affected, and so on, until it goes through all related cases and their related forms.
This recursion is capped at the MAX_CASE_COUNT and MAX_SUBCASE_DEPTH values defined above.

:param case_obj: The main case object targeted for deletion.
:return: Returns 2 dictionaries: one with the complete list of cases and forms to reference for soft deletion
once the user confirms the action and another with organized dictionaries of case data to be parsed by the
html template. The latter will be further modified in format_case_data_for_display.
"""
@define
class DeleteCase:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could these dataclasses and related utility functions live in a separate utils file outside of this report so that the dataclasses (which are looking great!) and the other related functions don't have to be nested under a larger wrapper function? It's still a bit difficult to digest get_case_and_display_data from top to bottom due to all the nesting. It would be great if walk_case_relations was further broken down too...

Copy link
Member

@biyeun biyeun Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps the location for this proposed file can be corehq.apps.hqcase.case_deletion_utils?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like there was also a comment earlier about adding unit tests for these extracted functions...seconding this as well! hqcase seems like a more appropriate place for these tests rather that reports, so having the utils file there will also make the organization much more digestible for future understanding

name = field()
url = field()
is_primary = field(default=False)
delete_forms = field(factory=list)

@define
class DeleteForm:
name = field()
url = field()
affected_cases = field(factory=list)

@define
class FormAffectedCases:
case_name = field(default=None)
is_current_case = field(default=False)
actions = field(factory=list)

@define
class AffectedCase:
id = field()
name = field()
url = field()
affected_forms = field(factory=list)

def get_affected_case(case_id):
for affected_case in affected_cases_display:
if affected_case.id == case_id:
return affected_case
affected_case = AffectedCase(id=case_id, name=None, url=None)
affected_cases_display.append(affected_case)
return affected_case

@define
class AffectedForm:
name = field()
url = field()
actions = field()

@define
class ReopenedCase:
name = field()
url = field()
closing_form = field()

delete_cases = [] # list of cases to be soft deleted
delete_forms = [] # list of forms to be soft deleted

# For formatting the list of cases/submission forms
form_names = {}

delete_cases_display = []
reopened_cases_display = []
affected_cases_display = []

update_actions = [const.CASE_ACTION_INDEX,
const.CASE_ACTION_UPDATE,
const.CASE_ACTION_ATTACHMENT,
const.CASE_ACTION_COMMTRACK,
const.CASE_ACTION_REBUILD]

def walk_case_relations(case, subcase_count):
if case.case_id not in delete_cases:
delete_cases.append(case.case_id)
if len(delete_cases) > MAX_CASE_COUNT or subcase_count >= MAX_SUBCASE_DEPTH:
raise TooManyCases("Too many cases to delete")
current_case = DeleteCase(name=case.name, url=reverse('case_data', args=[domain, case.case_id]))
if len(delete_cases) == 1: # only add primary label to the main case
current_case.is_primary = True
delete_cases_display.append(current_case)
xforms = case.xform_ids
case_xforms = []
for xform in xforms:
if xform not in case_xforms:
case_xforms.append(xform)

# iterating through all non-archived forms related to the case
for form_id in case_xforms:
if form_id not in delete_forms:
delete_forms.insert(0, form_id)
form_object = XFormInstance.objects.get_form(form_id, domain)
if form_id not in form_names:
form_names[form_id] = escape(xmlns_to_name(domain, form_object.xmlns, form_object.app_id))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an example guidance to further break down this function...
could all the places here where form_names[form_id] is being set be converted into a utility function that looks like...

def get_form_name(domain, form_object):
    form_name = escape(xmlns_to_name(domain, form_object.xmlns, form_object.app_id))
    if form_name == form_object.xmlns:
        inferred_name = [
            get_app(domain, form_object.app_id).name or "[Unknown App]",
            "[Unknown Module]",
             form_object.name or "[Unknown Form]"
         ]
         form_name = escape(' > '.join(inferred_name))
    return form_name

then right before you set current_form, you can do...

form_names[form_id] = get_form_name(domain, form_object)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up puling this and the process that gets the case's xforms into their own functions but no matter what I tried, I couldn't break down walk_case_relations further, partly because everything I tried ended up being very redundant and required passing around a lot of paramaters/context when it'd be easier to just access them in the main function. Maybe I'm thinking about this in the wrong way?

I think even just pulling out all the dataclass definitions and those 2 functions make get_case_and_display_data as a whole, way more digestable. Do you have any more thoughts on this?

if form_names[form_id] == form_object.xmlns:
form_name = [
get_app(domain, form_object.app_id).name or "[Unknown App]",
"[Unknown Module]",
form_object.name or "[Unknown Form]"
]
form_names[form_id] = escape(' > '.join(form_name))
current_form = DeleteForm(name=form_names[form_id],
url=reverse('render_form_data', args=[domain, form_id]))
current_case.delete_forms.append(current_form)
case_db = FormProcessorInterface(domain).casedb_cache(
domain=domain,
load_src="get_case_and_display_data",
)
touched_cases = FormProcessorInterface(domain).get_cases_from_forms(case_db, [form_object])
case_actions = []

# iterating through all cases affected by the current form
for touched_id in touched_cases:
case_object = touched_cases[touched_id].case
actions = list(touched_cases[touched_id].actions)
if touched_id == case.case_id:
case_actions.append(FormAffectedCases(is_current_case=True, actions=', '.join(actions)))
elif touched_id not in delete_cases:
if touched_id not in case_actions:
case_actions.append(FormAffectedCases(case_name=case_object.name,
actions=', '.join(actions)))
if const.CASE_ACTION_CREATE in actions and touched_id != case.case_id:
walk_case_relations(case_object, subcase_count + 1)
if const.CASE_ACTION_CLOSE in actions:
reopened_cases_display.append(
ReopenedCase(name=case_object.name,
url=reverse('case_data', args=[domain, touched_id]),
closing_form=reverse('render_form_data', args=[domain, form_id])))
if any(action in actions for action in update_actions):
affected = get_affected_case(touched_id)
if not affected.name:
affected.name = case_object.name
affected.affected_forms.append(
AffectedForm(name=form_names[form_id],
url=reverse('render_form_data', args=[domain, form_id]),
actions=', '.join(actions)))
current_form.affected_cases = case_actions

walk_case_relations(case_obj, subcase_count=0)
affected_cases_display = [case for case in affected_cases_display if case.id not in delete_cases]

return {
'case_delete_list': delete_cases,
'form_delete_list': delete_forms,
'delete_cases': delete_cases_display,
'reopened_cases': reopened_cases_display,
'affected_cases': affected_cases_display,
}


@location_safe
def get_cases_and_forms_for_deletion(request, domain, case_id):
case_instance = safely_get_case(request, domain, case_id)
try:
case_data = get_case_and_display_data(case_instance, domain)
except TooManyCases:
messages.error(request, _("Deleting this case would delete too many related cases. "
"Please delete some of this cases' subcases before attempting"
"to delete this case."))
return {'redirect': True}

case_data.update({
'main_case_name': case_instance.name,
'redirect': False
})
return case_data


@location_safe
class DeleteCaseView(BaseProjectReportSectionView):
urlname = 'soft_delete_case_view'
page_title = gettext_lazy('Delete Case and Related Forms')
template_name = 'reports/reportdata/case_delete.html'
delete_dict = {}

@method_decorator(require_case_view_permission)
def dispatch(self, request, *args, **kwargs):
self.delete_dict = get_cases_and_forms_for_deletion(request, self.domain, self.case_id)
if self.delete_dict['redirect']:
return HttpResponseRedirect(reverse('case_data', args=[self.domain, self.case_id]))
return super(DeleteCaseView, self).dispatch(request, *args, **kwargs)

@property
def case_id(self):
return self.kwargs['case_id']

@property
def domain(self):
return self.kwargs['domain']

@property
def page_url(self):
return reverse(self.urlname, args=(self.domain, self.case_id))

@property
def page_context(self):
context = {
"main_case_id": self.case_id,
}
context.update(self.delete_dict)
return context

def post(self, request, *args, **kwargs):
if request.POST.get('input') != self.delete_dict['main_case_name']:
messages.error(request, "Incorrect name. Please enter the case name as shown into the textbox.")
millerdev marked this conversation as resolved.
Show resolved Hide resolved
return HttpResponseRedirect(self.page_url)
msg, error = soft_delete_cases_and_forms(request, self.domain, self.delete_dict['case_delete_list'],
self.delete_dict['form_delete_list'])
if error:
messages.error(request, msg, extra_tags='html')
return HttpResponseRedirect(reverse('case_data', args=[self.domain, self.case_id]))
else:
msg = self.delete_dict['main_case_name'] + msg
messages.success(request, msg)
return HttpResponseRedirect(reverse('project_report_dispatcher',
args=(self.domain, 'submit_history')))


@location_safe
@require_permission(HqPermissions.edit_data)
def soft_delete_cases_and_forms(request, domain, case_delete_list, form_delete_list):
"""
Archiving the form that created the case will automatically "unmake" the case, but won't soft-delete
the case. After soft deletion, hard deletion will happen 90 days from the deletion date by an
automated deletion task.
"""
error = False
msg = ", its related subcases and submission forms were deleted successfully."
millerdev marked this conversation as resolved.
Show resolved Hide resolved
for form in form_delete_list:
if archive_form(request, domain, form, is_case_delete=True):
form_instance = XFormInstance.objects.get_form(form, domain)
form_instance.soft_delete()
else:
# I'm fairly certain this will never enter here but this is just in case something does go wrong
error = True
msg = "The form {} could not be deleted. Please try manually archiving, then deleting the form," \
"before trying to delete this case again.".format(form)
millerdev marked this conversation as resolved.
Show resolved Hide resolved
break
if not error:
CommCareCase.objects.soft_delete_cases(domain, list(case_delete_list))

return msg, error


@location_safe
@require_case_view_permission
@require_permission(HqPermissions.edit_data)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,12 @@ <h1>{% blocktrans %}Section: {{ section_id }}{% endblocktrans %}</h1>
</button>
</form>
{% endif %}
{% if not is_usercase %}
<a class="btn btn-danger pull-left" href="{% url 'soft_delete_case_view' domain case_id %}">
<i class="fa fa-trash"></i>
{% trans 'Delete Case' %}
</a>
{% endif %}
</div>
</div>
{% endif %}
Expand Down
Loading
Loading