fix: TT-220 Remove custom_sql_params from methods and replace for new arguments and function calls

Author: Pablo

commons/data_access_layer/cosmos_db.py

@@ -173,17 +173,6 @@ def create_sql_where_conditions(
         else:
             return ""
 
-    @staticmethod
-    def create_custom_sql_conditions(custom_sql_conditions: List[str]) -> str:
-        if len(custom_sql_conditions) > 0:
-            return "AND {custom_sql_conditions_clause}".format(
-                custom_sql_conditions_clause=" AND ".join(
-                    custom_sql_conditions
-                )
-            )
-        else:
-            return ''
-
     @staticmethod
     def generate_params(conditions: dict) -> list:
         result = []
@@ -217,6 +206,16 @@ def attach_context(data: dict, event_context: EventContext):
             "session_id": event_context.session_id,
         }
 
+    @staticmethod
+    def create_sql_date_range_filter(date_range: dict) -> str:
+        if 'start_date' and 'end_date' in date_range:
+            return """
+                AND ((c.start_date BETWEEN @start_date AND @end_date) OR
+                 (c.end_date BETWEEN @start_date AND @end_date))
+                """
+        else:
+            return ''
+
     def create(
         self, data: dict, event_context: EventContext, mapper: Callable = None
     ):
@@ -251,19 +250,13 @@ def find_all(
         self,
         event_context: EventContext,
         conditions: dict = None,
-        custom_sql_conditions: List[str] = None,
-        custom_params: dict = None,
+        date_range: dict = None,
         max_count=None,
         offset=0,
         visible_only=True,
         mapper: Callable = None,
     ):
         conditions = conditions if conditions else {}
-        custom_sql_conditions = (
-            custom_sql_conditions if custom_sql_conditions else []
-        )
-        custom_params = custom_params if custom_params else {}
-
         partition_key_value = self.find_partition_key_value(event_context)
         max_count = self.get_page_size_or(max_count)
         params = [
@@ -277,15 +270,20 @@ def find_all(
             status_value = conditions.get('status')
             conditions.pop('status')
 
+        date_range = date_range if date_range else {}
+        date_range_params = (
+            self.generate_params(date_range) if date_range else []
+        )
         params.extend(self.generate_params(conditions))
-        params.extend(custom_params)
+        params.extend(date_range_params)
+
         query_str = """
             SELECT * FROM c
             WHERE c.{partition_key_attribute}=@partition_key_value
             {conditions_clause}
-            {visibility_condition}
             {active_condition}
-            {custom_sql_conditions_clause}
+            {date_range_sql_condition}
+            {visibility_condition}
             {order_clause}
             OFFSET @offset LIMIT @max_count
             """.format(
@@ -295,11 +293,12 @@ def find_all(
             ),
             active_condition=self.create_sql_active_condition(status_value),
             conditions_clause=self.create_sql_where_conditions(conditions),
-            custom_sql_conditions_clause=self.create_custom_sql_conditions(
-                custom_sql_conditions
+            date_range_sql_condition=self.create_sql_date_range_filter(
+                date_range
             ),
             order_clause=self.create_sql_order_clause(),
         )
+
         result = self.container.query_items(
             query=query_str,
             parameters=params,

tests/time_tracker_api/projects/projects_model_test.py

@@ -11,7 +11,7 @@
 @patch(
     'time_tracker_api.projects.projects_model.ProjectCosmosDBRepository.find_partition_key_value'
 )
-def test_find_all_new_version(
+def test_find_all_projects_new_version(
     find_partition_key_value_mock,
     event_context: EventContext,
     project_repository: ProjectCosmosDBRepository,

tests/time_tracker_api/time_entries/time_entries_model_test.py

@@ -278,23 +278,11 @@ def test_updated_item_without_deleted_key_should_call_validate_data(
 @patch(
     'commons.data_access_layer.cosmos_db.CosmosDBRepository.generate_params'
 )
-@patch(
-    'commons.data_access_layer.cosmos_db.CosmosDBRepository.create_sql_condition_for_visibility'
-)
-@patch(
-    'commons.data_access_layer.cosmos_db.CosmosDBRepository.create_sql_where_conditions'
-)
-@patch(
-    'commons.data_access_layer.cosmos_db.CosmosDBRepository.create_custom_sql_conditions'
-)
 @patch(
     'time_tracker_api.time_entries.time_entries_repository.TimeEntryCosmosDBRepository.add_complementary_info'
 )
-def test_find_all_v2(
+def test_find_all_time_entries_new_version(
     add_complementary_info_mock,
-    create_custom_sql_conditions_mock,
-    create_sql_where_conditions_mock,
-    create_sql_condition_for_visibility_mock,
     generate_params_mock,
     get_page_size_or_mock,
     find_partition_key_value_mock,
@@ -319,25 +307,20 @@ def test_find_all_v2(
 
     result = time_entry_repository.find_all(
         conditions={"user_id": "*"},
-        custom_sql_conditions=[],
         event_context=event_context,
         date_range={
             'start_date': "2021-03-22T10:00:00.000Z",
             'end_date': "2021-03-22T11:00:00.000Z",
         },
-        custom_params={},
     )
 
     find_partition_key_value_mock.assert_called_once()
     get_page_size_or_mock.assert_called_once()
+
     assert len(result) == 1
     time_entry = result[0]
     assert time_entry == expected_item
 
-    create_sql_condition_for_visibility_mock.assert_called_once()
-    create_sql_where_conditions_mock.assert_called_once()
-    create_custom_sql_conditions_mock.assert_called_once()
-
 
 @patch(
     'time_tracker_api.time_entries.time_entries_repository.TimeEntryCosmosDBRepository.find_partition_key_value'

tests/time_tracker_api/time_entries/time_entries_namespace_test.py

@@ -197,7 +197,7 @@ def test_get_time_entry_should_succeed_with_valid_id(
     Mock(),
 )
 @patch(
-    'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.build_custom_query',
+    'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.get_owner_ids',
     Mock(),
 )
 @patch(
@@ -278,7 +278,7 @@ def test_get_time_entries_by_type_of_user_when_is_user_tester(
     Mock(),
 )
 @patch(
-    'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.build_custom_query',
+    'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.get_owner_ids',
     Mock(),
 )
 @patch(

tests/utils/query_builder_test.py

@@ -303,6 +303,7 @@ def test__build_order_by(
 
     assert orderBy_condition == expected_order_by_condition
 
+
 @pytest.mark.parametrize(
     "attribute,ids_list,expected_not_in_list",
     [
@@ -313,8 +314,11 @@ def test__build_order_by(
         ("id", ["id"], ["c.id NOT IN ('id')"]),
         ("id", ["id1", "id2"], ["c.id NOT IN ('id1', 'id2')"]),
         ("owner_id", ["id1", "id2"], ["c.owner_id NOT IN ('id1', 'id2')"]),
-        ("customer_id", ["id1", "id2"], [
-         "c.customer_id NOT IN ('id1', 'id2')"]),
+        (
+            "customer_id",
+            ["id1", "id2"],
+            ["c.customer_id NOT IN ('id1', 'id2')"],
+        ),
     ],
 )
 def test_add_sql_not_in_condition(
@@ -325,7 +329,29 @@ def test_add_sql_not_in_condition(
     query_builder = CosmosDBQueryBuilder().add_sql_not_in_condition(
         attribute, ids_list
     )
-    assert len(query_builder.where_conditions) == len(
-        expected_not_in_list
-    )
-    assert query_builder.where_conditions == expected_not_in_list
+    assert len(query_builder.where_conditions) == len(expected_not_in_list)
+    assert query_builder.where_conditions == expected_not_in_list
+
+
+@pytest.mark.parametrize(
+    "date_range,expected_value,expected_expression",
+    [
+        (
+            {
+                'start_date': '2021-05-09T00:00:00-05:00',
+                'end_date': '2021-05-15T23:59:59-05:00',
+            },
+            {
+                'start_date': '2021-05-09T00:00:00-05:00',
+                'end_date': '2021-05-15T23:59:59-05:00',
+            },
+            "((c.start_date BETWEEN @start_date AND @end_date) OR (c.end_date BETWEEN @start_date AND @end_date))",
+        )
+    ],
+)
+def test_add_date_range(date_range, expected_value, expected_expression):
+    query_builder = CosmosDBQueryBuilder().add_date_range(date_range)
+    result = query_builder._CosmosDBQueryBuilder__build_date_range()
+
+    assert query_builder.date_range == expected_value
+    assert result == expected_expression

time_tracker_api/time_entries/time_entries_dao.py

@@ -8,6 +8,7 @@
     add_project_info_to_time_entries,
     add_activity_name_to_time_entries,
     create_custom_query_from_str,
+    create_list_from_str,
 )
 from utils.time import (
     datetime_str,
@@ -71,19 +72,15 @@ def check_time_entry_is_not_started(self, data):
                 "The specified time entry is already running",
             )
 
-    def build_custom_query(self, is_admin: bool, conditions: dict = None):
+    def get_owner_ids(self, is_admin: bool, conditions: dict = None):
         custom_query = []
         if "user_id" in conditions:
             if is_admin:
                 conditions.pop("owner_id")
                 custom_query = (
                     []
                     if conditions.get("user_id") == "*"
-                    else [
-                        create_custom_query_from_str(
-                            conditions.get("user_id"), "c.owner_id"
-                        )
-                    ]
+                    else create_list_from_str(conditions.get("user_id"))
                 )
                 conditions.pop("user_id")
             else:
@@ -96,7 +93,7 @@ def get_all(self, conditions: dict = None, **kwargs) -> list:
         event_ctx = self.create_event_context("read-many")
         conditions.update({"owner_id": event_ctx.user_id})
         is_complete_query = conditions.get("user_id") == '*'
-        custom_query = self.build_custom_query(
+        owner_ids = self.get_owner_ids(
             is_admin=event_ctx.is_admin,
             conditions=conditions,
         )
@@ -108,13 +105,6 @@ def get_all(self, conditions: dict = None, **kwargs) -> list:
             event_ctx.user_id
         )
 
-        custom_query.append(
-            TimeEntryCosmosDBRepository.create_sql_date_range_filter(
-                date_range
-            )
-        )
-        custom_params = CosmosDBRepository.generate_params(date_range)
-
         test_user_ids = (
             azure_connection.get_test_user_ids()
             if not current_user_is_tester and is_complete_query
@@ -123,11 +113,10 @@ def get_all(self, conditions: dict = None, **kwargs) -> list:
 
         time_entries_list = self.repository.find_all(
             conditions=conditions,
-            custom_sql_conditions=custom_query,
             test_user_ids=test_user_ids,
+            owner_ids=owner_ids,
             date_range=date_range,
             max_count=limit,
-            custom_params=custom_params,
             event_context=event_ctx,
         )
 
@@ -138,10 +127,7 @@ def get_lastest_entries_by_project(
     ) -> list:
         event_ctx = self.create_event_context("read-many")
         conditions.update({"owner_id": event_ctx.user_id})
-        custom_query = self.build_custom_query(
-            is_admin=event_ctx.is_admin,
-            conditions=conditions,
-        )
+
         date_range = self.handle_date_filter_args(args=conditions)
 
         project_dao = projects_model.create_dao()
@@ -161,13 +147,12 @@ def get_lastest_entries_by_project(
             latest = self.repository.find_all_entries(
                 event_ctx,
                 conditions=conditions,
-                custom_sql_conditions=custom_query,
                 date_range=date_range,
                 max_count=limit,
             )
 
             if len(latest) > 0:
-                result.append(latest[0])
+                self.append = result.append(latest[0])
 
         add_activity_name_to_time_entries(result, activities)
         add_project_info_to_time_entries(result, projects)
@@ -180,19 +165,19 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list:
         get_all_conditions.pop("start")
         event_ctx = self.create_event_context("read-many")
         get_all_conditions.update({"owner_id": event_ctx.user_id})
-        custom_query = self.build_custom_query(
+        owner_ids = self.get_owner_ids(
             is_admin=event_ctx.is_admin,
             conditions=get_all_conditions,
         )
         date_range = self.handle_date_filter_args(args=get_all_conditions)
         records_total = self.repository.count(
             event_ctx,
             conditions=get_all_conditions,
-            custom_sql_conditions=custom_query,
+            owner_ids=owner_ids,
             date_range=date_range,
         )
         conditions.update({"owner_id": event_ctx.user_id})
-        custom_query = self.build_custom_query(
+        owner_ids = self.get_owner_ids(
             is_admin=event_ctx.is_admin,
             conditions=conditions,
         )
@@ -203,9 +188,9 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list:
         conditions.pop("start", None)
 
         time_entries = self.repository.find_all(
-            event_ctx,
+            event_context=event_ctx,
             conditions=conditions,
-            custom_sql_conditions=custom_query,
+            owner_ids=owner_ids,
             date_range=date_range,
             max_count=length,
             offset=start,