fix: TT-220 remove custom sql conditions from find all (#292)

* fix: TT-220 refactoring for projects

* fix: TT-238 activities find_all refactoring

* fix: TT-220 Correction in get_all function for projects_model

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

* fix: TT-220 apply refactoring on some code

* fix: TT-220 Bugs corrections

* fix: TT-220 Removes order conditions from activities

* fix: TT-220 make adjustments in projects query for time-entries

* fix: TT-220 adjustments according sonar suggestions

* fix: TT-220 adjustments according sonar suggestions in activities

* fix: TT-220 adjustments according sonar code analysis

* fix: TT-220 adjustments according reviewers suggestions

Co-authored-by: Pablo <Solorzano>
Co-authored-by: Kevin Lopez <[email protected]>

Author: pfsolorzanos

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' in date_range 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,
+        visible_only=True,
         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/activities/activities_namespace_test.py

@@ -71,7 +71,13 @@ def test_list_all_active(
     json_data = json.loads(response.data)
     assert [] == json_data
 
-    repository_find_all_mock.assert_called_once_with(ANY, conditions={})
+    repository_find_all_mock.assert_called_once_with(
+        event_context=ANY,
+        activities_id=ANY,
+        conditions={},
+        visible_only=ANY,
+        max_count=ANY,
+    )
 
 
 def test_list_all_active_activities(
@@ -94,7 +100,11 @@ def test_list_all_active_activities(
     assert [] == json_data
 
     repository_find_all_mock.assert_called_once_with(
-        ANY, conditions={'status': 'active'}
+        event_context=ANY,
+        conditions={'status': 'active'},
+        activities_id=ANY,
+        visible_only=ANY,
+        max_count=ANY,
     )
 
 

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_v2(
+def test_find_all_projects_new_version(
     find_partition_key_value_mock,
     event_context: EventContext,
     project_repository: ProjectCosmosDBRepository,
@@ -28,13 +28,14 @@ def test_find_all_v2(
     project_repository.container = Mock()
     project_repository.container.query_items = query_items_mock
 
-    result = project_repository.find_all_v2(
-        event_context,
-        ['id'],
-        ['customer_id']
+    result = project_repository.find_all(
+        event_context=event_context,
+        conditions={"customer_id": "1"},
+        project_ids=['id'],
+        customer_ids=['customer_id'],
     )
     find_partition_key_value_mock.assert_called_once()
     assert len(result) == 1
     project = result[0]
     assert isinstance(project, ProjectCosmosDBModel)
-    assert project.__dict__ == expected_item
+    assert project.__dict__ == expected_item

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,5 @@ 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

time_tracker_api/activities/activities_model.py

@@ -14,6 +14,7 @@
     convert_list_to_tuple_string,
     create_sql_in_condition,
 )
+from utils.query_builder import CosmosDBQueryBuilder, Order
 
 
 class ActivityDao(CRUDDao):
@@ -85,6 +86,38 @@ def find_all_with_id_in_list(
         function_mapper = self.get_mapper_or_dict(mapper)
         return list(map(function_mapper, result))
 
+    def find_all(
+        self,
+        event_context: EventContext,
+        conditions: dict = None,
+        activities_id: List = None,
+        visible_only=True,
+        max_count=None,
+        offset=0,
+        mapper: Callable = None,
+    ):
+        query_builder = (
+            CosmosDBQueryBuilder()
+            .add_sql_in_condition('id', activities_id)
+            .add_sql_where_equal_condition(conditions)
+            .add_sql_visibility_condition(visible_only)
+            .add_sql_limit_condition(max_count)
+            .add_sql_offset_condition(offset)
+            .build()
+        )
+
+        query_str = query_builder.get_query()
+        tenant_id_value = self.find_partition_key_value(event_context)
+        params = query_builder.get_parameters()
+
+        result = self.container.query_items(
+            query=query_str,
+            parameters=params,
+            partition_key=tenant_id_value,
+        )
+        function_mapper = self.get_mapper_or_dict(mapper)
+        return list(map(function_mapper, result))
+
 
 class ActivityCosmosDBDao(APICosmosDBDao, ActivityDao):
     def __init__(self, repository):
@@ -100,6 +133,25 @@ def get_all_with_id_in_list(
             activity_ids,
         )
 
+    def get_all(
+        self,
+        conditions: dict = None,
+        activities_id: List = None,
+        max_count=None,
+        visible_only=True,
+    ) -> list:
+        event_ctx = self.create_event_context("read-many")
+        max_count = self.repository.get_page_size_or(max_count)
+
+        activities = self.repository.find_all(
+            event_context=event_ctx,
+            conditions=conditions,
+            activities_id=activities_id,
+            visible_only=visible_only,
+            max_count=max_count,
+        )
+        return activities
+
 
 def create_dao() -> ActivityDao:
     repository = ActivityCosmosDBRepository()