Merge pull request #5041 from broadinstitute/delete-search-functionality-cleanup

Delete search functionality cleanup

Author: hanars

seqr/management/commands/deactivate_project_search.py

@@ -1,8 +1,7 @@
 from django.core.management.base import BaseCommand, CommandError
 
-from clickhouse_search.search import delete_clickhouse_project
 from seqr.models import Project, Sample
-from seqr.utils.search.utils import backend_specific_call
+from seqr.utils.search.utils import es_only
 
 import logging
 logger = logging.getLogger(__name__)
@@ -12,6 +11,7 @@ class Command(BaseCommand):
     def add_arguments(self, parser):
         parser.add_argument('project')
 
+    @es_only
     def handle(self, *args, **options):
         project = Project.objects.get(guid=options['project'])
 
@@ -21,15 +21,3 @@ def handle(self, *args, **options):
         updated = Sample.bulk_update(user=None, update_json={'is_active': False}, individual__family__project=project)
 
         logger.info(f'Deactivated {len(updated)} samples')
-
-        if updated:
-            dataset_types = Sample.objects.filter(guid__in=updated).values_list('dataset_type', 'sample_type').order_by('dataset_type').distinct()
-            for dataset_type, sample_type in dataset_types:
-                backend_specific_call(
-                    lambda *args, **kwargs: True, self._delete_clickhouse_project,
-                )(project, dataset_type, sample_type)
-
-    @staticmethod
-    def _delete_clickhouse_project(project, dataset_type, sample_type):
-        info = delete_clickhouse_project(project, dataset_type, sample_type)
-        logger.info(info)

seqr/management/tests/deactivate_project_search_tests.py

@@ -15,9 +15,8 @@
 VARIANT_ID = '21-3343353-GAGA-G'
 
 
-class DeactivateProjectSearchTest(object):
-
-    DELETE_SUCCESS_LOGS = []
+class ElasticsearchDeactivateProjectSearchTest(AuthenticationTestCase):
+    fixtures = ['users', '1kg_project']
 
     @responses.activate
     @mock.patch('seqr.management.commands.deactivate_project_search.input')
@@ -47,54 +46,21 @@ def test_command(self, mock_input):
                 'updateType': 'bulk_update'},
             }),
             ('Deactivated 11 samples', None),
-        ] + self.DELETE_SUCCESS_LOGS)
+        ])
 
         active_samples = Sample.objects.filter(individual__family__project__guid=PROJECT_GUID, is_active=True)
         self.assertEqual(active_samples.count(), 0)
-        self._assert_expected_delete()
 
         # Re-running has no effect
         self.reset_logs()
         call_command('deactivate_project_search', PROJECT_GUID)
         self.assert_json_logs(user=None, expected=[('Deactivated 0 samples', None)])
 
-    def _assert_expected_delete(self):
-        pass
-
-class ElasticsearchDeactivateProjectSearchTest(AuthenticationTestCase, DeactivateProjectSearchTest):
-    fixtures = ['users', '1kg_project']
-
 
-class ClickhouseDeactivateProjectSearchTest(AnvilAuthenticationTestCase, DeactivateProjectSearchTest):
+class ClickhouseDeactivateProjectSearchTest(AnvilAuthenticationTestCase):
     fixtures = ['users', '1kg_project', 'reference_data', 'clickhouse_search']
 
-    DELETE_SUCCESS_LOGS = [
-        ('Deleted all MITO search data for project 1kg project nåme with uniçøde', None),
-        ('Deleted all SNV_INDEL search data for project 1kg project nåme with uniçøde', None),
-        ('Deleted all GCNV search data for project 1kg project nåme with uniçøde', None),
-    ]
-
-    def _assert_expected_delete(self):
-        self.assertEqual(EntriesSnvIndel.objects.filter(project_guid=PROJECT_GUID).count(), 0)
-        self.assertEqual(ProjectGtStatsSnvIndel.objects.filter(project_guid=PROJECT_GUID).count(), 0)
-        self.assertEqual(EntriesMito.objects.filter(project_guid=PROJECT_GUID).count(), 0)
-        self.assertEqual(ProjectGtStatsMito.objects.filter(project_guid=PROJECT_GUID).count(), 0)
-        self.assertEqual(EntriesGcnv.objects.filter(project_guid=PROJECT_GUID).count(), 0)
-
-        updated_seqr_pops_by_key = dict(AnnotationsSnvIndel.objects.all().join_seqr_pop().values_list('key', 'seqrPop'))
-        self.assertDictEqual(updated_seqr_pops_by_key, {
-            1: (2, 2, 1, 1),
-            2: (1, 1, 0, 0),
-            3: (0, 0, 0, 0),
-            4: (0, 0, 0, 0),
-            5: (1, 1, 0, 0),
-            6: (0, 0, 0, 0),
-            22: (0, 3, 0, 1),
-        })
-
-        updated_mito_pops_by_key = dict(AnnotationsMito.objects.all().join_seqr_pop().values_list('key', 'seqrPop'))
-        self.assertDictEqual(updated_mito_pops_by_key, {
-            6: (0, 0, 0, 0),
-            7: (0, 0, 0, 0),
-            8: (0, 0, 0, 0),
-        })
+    def test_command(self):
+        with self.assertRaises(ValueError) as e:
+            call_command('deactivate_project_search', 'foo')
+        self.assertEqual(str(e.exception), 'handle is disabled without the elasticsearch backend')

seqr/views/apis/data_manager_api.py

@@ -425,7 +425,8 @@ def trigger_delete_project(request):
     request_json = json.loads(request.body)
     project_guid = request_json.pop('project')
     project = Project.objects.get(guid=project_guid)
-    return _trigger_data_update(delete_clickhouse_project, request_json, project)
+    samples = Sample.objects.filter(individual__family__project=project)
+    return _trigger_data_update(delete_clickhouse_project, request.user, samples, request_json, project)
 
 
 @data_manager_required
@@ -434,15 +435,18 @@ def trigger_delete_family(request):
     request_json = json.loads(request.body)
     family_guid = request_json.pop('family')
     project = Project.objects.get(family__guid=family_guid)
-    return _trigger_data_update(delete_clickhouse_family, request_json, project, family_guid)
+    samples = Sample.objects.filter(individual__family__guid=family_guid)
+    return _trigger_data_update(delete_clickhouse_family, request.user, samples, request_json, project, family_guid)
 
 
-def _trigger_data_update(clickhouse_func, request_json, project, *args):
+def _trigger_data_update(clickhouse_func, user, samples, request_json, project, *args):
     dataset_type = request_json.get('datasetType')
-    sample_types = Sample.objects.filter(
-        individual__family__project=project, dataset_type=dataset_type, is_active=True,
-    ).values_list('sample_type', flat=True).distinct() if dataset_type == Sample.DATASET_TYPE_SV_CALLS else [None]
-    info = []
+    samples = samples.filter(dataset_type=dataset_type, is_active=True)
+    sample_types = list(
+        samples.values_list('sample_type', flat=True).distinct()
+    ) if dataset_type == Sample.DATASET_TYPE_SV_CALLS else [None]
+    updated = Sample.bulk_update(user=user, update_json={'is_active': False}, queryset=samples)
+    info = [f'Deactivated search for {len(updated)} individuals']
     for sample_type in sample_types:
         info.append(clickhouse_func(project, *args, dataset_type=dataset_type, sample_type=sample_type))
     return create_json_response({'info': info})

seqr/views/apis/data_manager_api_tests.py

@@ -14,7 +14,7 @@
 from seqr.views.utils.orm_to_json_utils import _get_json_for_models
 from seqr.views.utils.test_utils import AuthenticationTestCase, AnvilAuthenticationTestCase, AirtableTest
 from seqr.utils.search.elasticsearch.es_utils_tests import urllib3_responses
-from seqr.models import Individual, RnaSeqOutlier, RnaSeqTpm, RnaSeqSpliceOutlier, RnaSample, Project, PhenotypePrioritization
+from seqr.models import Individual, Sample, RnaSeqOutlier, RnaSeqTpm, RnaSeqSpliceOutlier, RnaSample, Project, PhenotypePrioritization
 from settings import SEQR_SLACK_LOADING_NOTIFICATION_CHANNEL
 
 PROJECT_GUID = 'R0001_1kg'
@@ -2053,7 +2053,10 @@ def _test_validate_dataset_type(self, url):
     def _assert_expected_delete_project(self, response):
         self.assertEqual(response.status_code, 200)
         self.assertDictEqual(response.json(), {
-            'info': ['Deleted all SNV_INDEL search data for project 1kg project n\xe5me with uni\xe7\xf8de'],
+            'info': [
+                'Deactivated search for 7 individuals',
+                'Deleted all SNV_INDEL search data for project 1kg project n\xe5me with uni\xe7\xf8de',
+            ],
         })
         self.assertEqual(EntriesSnvIndel.objects.filter(project_guid=PROJECT_GUID).count(), 0)
         self.assertEqual(ProjectGtStatsSnvIndel.objects.filter(project_guid=PROJECT_GUID).count(), 0)
@@ -2069,12 +2072,23 @@ def _assert_expected_delete_project(self, response):
             22: (0, 3, 0, 1),
         })
 
+        project_samples = Sample.objects.filter(individual__family__project__guid=PROJECT_GUID, is_active=True)
+        self.assertEqual(project_samples.filter(dataset_type='SNV_INDEL').count(), 0)
+        self.assertEqual(project_samples.count(), 4)
+
     def _assert_expected_delete_family(self, response):
         self.assertEqual(response.status_code, 200)
         self.assertDictEqual(response.json(), {
-            'info': ['Clickhouse does not support deleting individual families from project. Manually delete GCNV data for F000002_2 in project R0001_1kg'],
+            'info': [
+                'Deactivated search for 3 individuals',
+                'Clickhouse does not support deleting individual families from project. Manually delete GCNV data for F000002_2 in project R0001_1kg',
+            ],
         })
 
+        family_samples = Sample.objects.filter(individual__family_id=2, is_active=True)
+        self.assertEqual(family_samples.filter(dataset_type='SV').count(), 0)
+        self.assertEqual(family_samples.count(),4)
+
     def _assert_expected_airtable_errors(self, url):
         responses.replace(
             responses.GET, 'https://api.airtable.com/v0/app3Y97xtbbaOopVR/Samples',

ui/pages/DataManagement/components/TriggerSearchDataUpdatePages.jsx

@@ -46,25 +46,25 @@ const FAMILY_FIELDS = [
   DATASET_TYPE_FIELD,
 ]
 
-const TriggerSearchDataUpdateForm = ({ path, fields }) => (
+const TriggerSearchDataUpdateForm = ({ entity, fields }) => (
   <SubmitFormPage
-    header={`Trigger ${snakecaseToTitlecase(path)}`}
-    url={`/api/data_management/trigger_${path}`}
+    header={`Trigger Delete ${snakecaseToTitlecase(entity)} Search`}
+    url={`/api/data_management/trigger_delete_${entity}`}
     fields={fields}
   />
 )
 
 TriggerSearchDataUpdateForm.propTypes = {
-  path: PropTypes.string,
+  entity: PropTypes.string,
   fields: PropTypes.arrayOf(PropTypes.object),
 }
 
 const TriggerDeleteProjects = () => (
-  <TriggerSearchDataUpdateForm path="delete_project" fields={PROJECT_FIELDS} />
+  <TriggerSearchDataUpdateForm entity="project" fields={PROJECT_FIELDS} />
 )
 
 const TriggerDeleteFamilies = () => (
-  <TriggerSearchDataUpdateForm path="delete_family" fields={FAMILY_FIELDS} />
+  <TriggerSearchDataUpdateForm entity="family" fields={FAMILY_FIELDS} />
 )
 
 export default [