From ef45dc4b4f91a48b1e7be17b3feaac278ab7bde5 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 10 May 2024 09:37:04 +0200 Subject: [PATCH 1/6] Catch exception in case of missing ds table --- corehq/apps/userreports/sql/adapter.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/corehq/apps/userreports/sql/adapter.py b/corehq/apps/userreports/sql/adapter.py index c0ed3583881b..86e942381566 100644 --- a/corehq/apps/userreports/sql/adapter.py +++ b/corehq/apps/userreports/sql/adapter.py @@ -211,8 +211,11 @@ def bulk_delete(self, docs, use_shard_col=True): table = self.get_table() doc_ids = [doc['_id'] for doc in docs] delete = table.delete(table.c.doc_id.in_(doc_ids)) - with self.session_context() as session: - session.execute(delete) + try: + with self.session_context() as session: + session.execute(delete) + except ProgrammingError: + return register_data_source_row_change( domain=self.config.domain, From 629697510cd007def6c92790f6fd6db69081e146 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Tue, 21 May 2024 10:47:30 +0200 Subject: [PATCH 2/6] Check for table during error handling --- corehq/apps/userreports/sql/adapter.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/corehq/apps/userreports/sql/adapter.py b/corehq/apps/userreports/sql/adapter.py index 86e942381566..673ad2ec70f1 100644 --- a/corehq/apps/userreports/sql/adapter.py +++ b/corehq/apps/userreports/sql/adapter.py @@ -214,8 +214,11 @@ def bulk_delete(self, docs, use_shard_col=True): try: with self.session_context() as session: session.execute(delete) - except ProgrammingError: - return + except ProgrammingError as e: + if not self.table_exists: + return + else: + raise e register_data_source_row_change( domain=self.config.domain, From 7974e0bc996746bac7f82469a4ea57a249d90f74 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 23 May 2024 09:06:46 +0200 Subject: [PATCH 3/6] Remove redundant else --- corehq/apps/userreports/sql/adapter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/corehq/apps/userreports/sql/adapter.py b/corehq/apps/userreports/sql/adapter.py index 673ad2ec70f1..27e7bad83280 100644 --- a/corehq/apps/userreports/sql/adapter.py +++ b/corehq/apps/userreports/sql/adapter.py @@ -217,8 +217,7 @@ def bulk_delete(self, docs, use_shard_col=True): except ProgrammingError as e: if not self.table_exists: return - else: - raise e + raise e register_data_source_row_change( domain=self.config.domain, From 495c64c2570a4fa53083f7cfe4ed476ac90d0ce3 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 23 May 2024 11:55:52 +0200 Subject: [PATCH 4/6] Add some tests for bulk_delete --- .../apps/userreports/tests/test_adapters.py | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 corehq/apps/userreports/tests/test_adapters.py diff --git a/corehq/apps/userreports/tests/test_adapters.py b/corehq/apps/userreports/tests/test_adapters.py new file mode 100644 index 000000000000..db113cc0c9d9 --- /dev/null +++ b/corehq/apps/userreports/tests/test_adapters.py @@ -0,0 +1,55 @@ +from django.test import SimpleTestCase +from unittest.mock import patch + +from corehq.apps.userreports.sql.adapter import IndicatorSqlAdapter +from corehq.apps.userreports.models import DataSourceConfiguration +from corehq.apps.userreports.app_manager.helpers import clean_table_name + + +class TestIndicatorSqlAdapter(SimpleTestCase): + + def setUp(self): + super().setUp() + config = self._create_data_source_config("domain") + self.adapter = IndicatorSqlAdapter(config) + + def tearDown(self): + self.adapter.drop_table() + super().tearDown() + + @staticmethod + def _create_data_source_config(domain): + indicator = { + "type": "expression", + "expression": { + "type": "property_name", + "property_name": 'name' + }, + "column_id": 'name', + "display_name": 'name', + "datatype": "string" + } + return DataSourceConfiguration( + domain=domain, + display_name='foo', + referenced_doc_type='CommCareCase', + table_id=clean_table_name('domain', 'test-table'), + configured_indicators=[indicator], + ) + + @patch("corehq.apps.userreports.sql.adapter.register_data_source_row_change") + def test_bulk_delete_table_dont_exist(self, register_data_source_row_change_mock): + docs = [{'_id': '1'}] + self.assertFalse(self.adapter.table_exists) + + self.adapter.bulk_delete(docs) + register_data_source_row_change_mock.assert_not_called() + + @patch("corehq.apps.userreports.sql.adapter.register_data_source_row_change") + def test_bulk_delete_table_exists(self, register_data_source_row_change_mock): + docs = [{'_id': '1'}] + self.adapter.build_table() + self.assertTrue(self.adapter.table_exists) + + self.adapter.bulk_delete(docs) + register_data_source_row_change_mock.assert_called() From b0b0ffbaa52cc10132bed02b396d5b3ed02fa294 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 23 May 2024 13:49:08 +0200 Subject: [PATCH 5/6] Try to not use drop_table in tests --- .../apps/userreports/tests/test_adapters.py | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/corehq/apps/userreports/tests/test_adapters.py b/corehq/apps/userreports/tests/test_adapters.py index db113cc0c9d9..d9cb8ea257d7 100644 --- a/corehq/apps/userreports/tests/test_adapters.py +++ b/corehq/apps/userreports/tests/test_adapters.py @@ -8,15 +8,6 @@ class TestIndicatorSqlAdapter(SimpleTestCase): - def setUp(self): - super().setUp() - config = self._create_data_source_config("domain") - self.adapter = IndicatorSqlAdapter(config) - - def tearDown(self): - self.adapter.drop_table() - super().tearDown() - @staticmethod def _create_data_source_config(domain): indicator = { @@ -39,17 +30,23 @@ def _create_data_source_config(domain): @patch("corehq.apps.userreports.sql.adapter.register_data_source_row_change") def test_bulk_delete_table_dont_exist(self, register_data_source_row_change_mock): + config = self._create_data_source_config("test-domain") + adapter = IndicatorSqlAdapter(config) + docs = [{'_id': '1'}] - self.assertFalse(self.adapter.table_exists) + self.assertFalse(adapter.table_exists) - self.adapter.bulk_delete(docs) + adapter.bulk_delete(docs) register_data_source_row_change_mock.assert_not_called() @patch("corehq.apps.userreports.sql.adapter.register_data_source_row_change") def test_bulk_delete_table_exists(self, register_data_source_row_change_mock): + config = self._create_data_source_config("test-domain2") + adapter = IndicatorSqlAdapter(config) + docs = [{'_id': '1'}] - self.adapter.build_table() - self.assertTrue(self.adapter.table_exists) + adapter.build_table() + self.assertTrue(adapter.table_exists) - self.adapter.bulk_delete(docs) + adapter.bulk_delete(docs) register_data_source_row_change_mock.assert_called() From e6884b29b7bab6c457b7edf12c26e271b36f2a4d Mon Sep 17 00:00:00 2001 From: mkangia Date: Tue, 28 May 2024 21:33:18 +0530 Subject: [PATCH 6/6] update test type to allow DB queries --- corehq/apps/userreports/tests/test_adapters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/corehq/apps/userreports/tests/test_adapters.py b/corehq/apps/userreports/tests/test_adapters.py index d9cb8ea257d7..dbb5160394ab 100644 --- a/corehq/apps/userreports/tests/test_adapters.py +++ b/corehq/apps/userreports/tests/test_adapters.py @@ -1,4 +1,4 @@ -from django.test import SimpleTestCase +from django.test import TestCase from unittest.mock import patch from corehq.apps.userreports.sql.adapter import IndicatorSqlAdapter @@ -6,7 +6,7 @@ from corehq.apps.userreports.app_manager.helpers import clean_table_name -class TestIndicatorSqlAdapter(SimpleTestCase): +class TestIndicatorSqlAdapter(TestCase): @staticmethod def _create_data_source_config(domain):