From 58de09b85d71fefbfd53ddabf424da580ddbe38e Mon Sep 17 00:00:00 2001 From: Pablo Date: Mon, 10 May 2021 12:30:23 -0500 Subject: [PATCH 01/12] fix: TT-220 refactoring for projects --- .../projects/projects_model_test.py | 10 +++---- time_tracker_api/projects/projects_model.py | 30 +++++++++---------- .../time_entries/time_entries_repository.py | 6 ++-- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/tests/time_tracker_api/projects/projects_model_test.py b/tests/time_tracker_api/projects/projects_model_test.py index 7338fdfe..7475299a 100644 --- a/tests/time_tracker_api/projects/projects_model_test.py +++ b/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_new_version( find_partition_key_value_mock, event_context: EventContext, project_repository: ProjectCosmosDBRepository, @@ -28,13 +28,11 @@ 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, ['id'], ['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 \ No newline at end of file + assert project.__dict__ == expected_item diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index f72ff09d..a5d72fce 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -68,10 +68,10 @@ def __init__(self): mapper=ProjectCosmosDBModel, ) - def find_all_v2( + def find_all( self, event_context: EventContext, - project_ids: List[str], + project_ids: List[str] = None, customer_ids: List[str] = None, visible_only=True, mapper: Callable = None, @@ -97,7 +97,9 @@ class ProjectCosmosDBDao(APICosmosDBDao, ProjectDao): def __init__(self, repository): CosmosDBDao.__init__(self, repository) - def get_all(self, conditions: dict = None, **kwargs) -> list: + def get_all( + self, conditions: dict = None, project_ids: List = None, **kwargs + ) -> list: """ Get all the projects an active client has :param (dict) conditions: Conditions for querying the database @@ -110,29 +112,27 @@ def get_all(self, conditions: dict = None, **kwargs) -> list: max_count=kwargs.get('max_count', None) ) + # TODO: evaluate another approach in order that memory filtering will be make in Database instead customers_id = [ customer.id for customer in customers if customer.status == 'active' ] conditions = conditions if conditions else {} - custom_condition = "c.customer_id IN {}".format( - str(tuple(customers_id)) + customers_ids = [v for k, v in conditions.items()] + customers_ids = ( + customers_ids + customers_id if project_ids else customers_ids + ) + + projects = self.repository.find_all( + event_context=event_ctx, + project_ids=project_ids, + customer_ids=customers_ids, ) - # TODO this must be refactored to be used from the utils module ↑ - if "custom_sql_conditions" in kwargs: - kwargs["custom_sql_conditions"].append(custom_condition) - else: - kwargs["custom_sql_conditions"] = [custom_condition] - projects = self.repository.find_all(event_ctx, conditions, **kwargs) add_customer_name_to_projects(projects, customers) return projects - def get_all_with_id_in_list(self, id_list): - event_ctx = self.create_event_context("read-many") - return self.repository.find_all_v2(event_ctx, id_list) - def create_dao() -> ProjectDao: repository = ProjectCosmosDBRepository() diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 7128ee8e..d95bb433 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -146,14 +146,16 @@ def add_complementary_info( self, time_entries=None, max_count=None, exist_conditions=False ): if time_entries: - custom_conditions = create_in_condition(time_entries, "project_id") + project_ids_set = set([x.project_id for x in time_entries]) + project_ids = list(project_ids_set) + custom_conditions_activity = create_in_condition( time_entries, "activity_id" ) project_dao = projects_model.create_dao() projects = project_dao.get_all( - custom_sql_conditions=[custom_conditions], + project_ids=project_ids, visible_only=False, max_count=max_count, ) From 6399ba61477650a95d51acd2b4f4b95cfe10ba7c Mon Sep 17 00:00:00 2001 From: Pablo Date: Tue, 11 May 2021 16:33:55 -0500 Subject: [PATCH 02/12] fix: TT-238 activities find_all refactoring --- .../activities/activities_namespace_test.py | 14 ++++- .../activities/activities_model.py | 53 +++++++++++++++++++ .../time_entries/time_entries_repository.py | 8 +-- utils/query_builder.py | 11 ++-- 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/tests/time_tracker_api/activities/activities_namespace_test.py b/tests/time_tracker_api/activities/activities_namespace_test.py index d397117b..13958d8a 100644 --- a/tests/time_tracker_api/activities/activities_namespace_test.py +++ b/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, ) diff --git a/time_tracker_api/activities/activities_model.py b/time_tracker_api/activities/activities_model.py index a80bc384..24287bf5 100644 --- a/time_tracker_api/activities/activities_model.py +++ b/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,39 @@ 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=0, + 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_order_by_condition('id', Order.ASC) + .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 +134,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() diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index d95bb433..2cc1cbb8 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -149,9 +149,8 @@ def add_complementary_info( project_ids_set = set([x.project_id for x in time_entries]) project_ids = list(project_ids_set) - custom_conditions_activity = create_in_condition( - time_entries, "activity_id" - ) + activity_ids_set = set([x.activity_id for x in time_entries]) + activity_ids = list(activity_ids_set) project_dao = projects_model.create_dao() projects = project_dao.get_all( @@ -164,10 +163,11 @@ def add_complementary_info( activity_dao = activities_model.create_dao() activities = activity_dao.get_all( - custom_sql_conditions=[custom_conditions_activity], + activities_id=activity_ids, visible_only=False, max_count=max_count, ) + add_activity_name_to_time_entries(time_entries, activities) users = AzureConnection().users() diff --git a/utils/query_builder.py b/utils/query_builder.py index 1fd0e5c9..2899aab4 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -53,7 +53,7 @@ def add_sql_limit_condition(self, limit): return self def add_sql_offset_condition(self, offset): - if offset: + if offset != None: self.offset = offset return self @@ -61,8 +61,9 @@ def add_sql_order_by_condition(self, attribute: str, order: Order): self.order_by = (attribute, order.name) return self - - def add_sql_not_in_condition(self, attribute: str = None, ids_list: List[str] = None): + def add_sql_not_in_condition( + self, attribute: str = None, ids_list: List[str] = None + ): if ids_list and attribute and len(ids_list) > 0: ids_values = convert_list_to_tuple_string(ids_list) self.where_conditions.append(f"c.{attribute} NOT IN {ids_values}") @@ -80,14 +81,14 @@ def __build_where(self): return "" def __build_offset(self): - if self.offset: + if self.offset != None: self.parameters.append({'name': '@offset', 'value': self.offset}) return "OFFSET @offset" else: return "" def __build_limit(self): - if self.limit: + if self.limit != None: self.parameters.append({'name': '@limit', 'value': self.limit}) return "LIMIT @limit" else: From 7400b90de7588fa68ea39b25f10911bbe1380b85 Mon Sep 17 00:00:00 2001 From: Kevin Lopez Date: Wed, 12 May 2021 15:22:02 -0500 Subject: [PATCH 03/12] fix: TT-220 Correction in get_all function for projects_model --- time_tracker_api/projects/projects_model.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index a5d72fce..1e0c7d0f 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -118,16 +118,20 @@ def get_all( for customer in customers if customer.status == 'active' ] + conditions = conditions if conditions else {} - customers_ids = [v for k, v in conditions.items()] - customers_ids = ( - customers_ids + customers_id if project_ids else customers_ids + customers_ids_conditions = [v for k, v in conditions.items()] + + customers_ids_conditions = ( + customers_ids_conditions + customers_id + if customers_id + else customers_id ) projects = self.repository.find_all( event_context=event_ctx, project_ids=project_ids, - customer_ids=customers_ids, + customer_ids=customers_ids_conditions, ) add_customer_name_to_projects(projects, customers) From 9e9fd0c939ef18f75169b1fdff8f45b826c9987a Mon Sep 17 00:00:00 2001 From: Pablo Date: Thu, 13 May 2021 18:10:11 -0500 Subject: [PATCH 04/12] fix: TT-220 Remove custom_sql_params from methods and replace for new arguments and function calls --- commons/data_access_layer/cosmos_db.py | 45 ++++--- .../projects/projects_model_test.py | 2 +- .../time_entries/time_entries_model_test.py | 21 +-- .../time_entries_namespace_test.py | 4 +- tests/utils/query_builder_test.py | 38 +++++- .../time_entries/time_entries_dao.py | 39 ++---- .../time_entries/time_entries_repository.py | 122 ++++++------------ utils/extend_model.py | 16 ++- utils/query_builder.py | 15 +++ 9 files changed, 139 insertions(+), 163 deletions(-) diff --git a/commons/data_access_layer/cosmos_db.py b/commons/data_access_layer/cosmos_db.py index 6c1ed54d..5843e0fa 100644 --- a/commons/data_access_layer/cosmos_db.py +++ b/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, diff --git a/tests/time_tracker_api/projects/projects_model_test.py b/tests/time_tracker_api/projects/projects_model_test.py index 7475299a..2ac693cd 100644 --- a/tests/time_tracker_api/projects/projects_model_test.py +++ b/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, diff --git a/tests/time_tracker_api/time_entries/time_entries_model_test.py b/tests/time_tracker_api/time_entries/time_entries_model_test.py index 0bf801b5..c4d8b354 100644 --- a/tests/time_tracker_api/time_entries/time_entries_model_test.py +++ b/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' diff --git a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py b/tests/time_tracker_api/time_entries/time_entries_namespace_test.py index d01cdd6e..0954bd7c 100644 --- a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py +++ b/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( diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index ab1b7204..8d5776f4 100644 --- a/tests/utils/query_builder_test.py +++ b/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 \ No newline at end of file + 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 diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index b46260ae..7a4f3a11 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/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,7 +72,7 @@ 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: @@ -79,11 +80,7 @@ def build_custom_query(self, is_admin: bool, conditions: dict = None): 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,7 +165,7 @@ 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, ) @@ -188,11 +173,11 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: 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, diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 2cc1cbb8..791dda92 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -49,41 +49,21 @@ def create_sql_ignore_id_condition(id: str): else: return "AND c.id!=@ignore_id" - @staticmethod - def create_sql_date_range_filter(date_range: dict) -> str: - if 'start_date' and 'end_date' in date_range: - return """ - ((c.start_date BETWEEN @start_date AND @end_date) OR - (c.end_date BETWEEN @start_date AND @end_date)) - """ - else: - return '' - def find_all_entries( self, event_context: EventContext, conditions: dict = None, - custom_sql_conditions: List[str] = None, date_range: dict = None, **kwargs, ): conditions = conditions if conditions else {} - custom_sql_conditions = ( - custom_sql_conditions if custom_sql_conditions else [] - ) date_range = date_range if date_range else {} - custom_sql_conditions.append( - self.create_sql_date_range_filter(date_range) - ) - - custom_params = self.generate_params(date_range) time_entries = CosmosDBRepository.find_all( self, event_context=event_context, conditions=conditions, - custom_sql_conditions=custom_sql_conditions, - custom_params=custom_params, + date_range=date_range, max_count=kwargs.get("max_count", None), offset=kwargs.get("offset", 0), ) @@ -93,51 +73,35 @@ def count( self, event_context: EventContext, conditions: dict = None, - custom_sql_conditions: List[str] = None, + owner_ids: list = None, date_range: dict = None, visible_only=True, **kwargs, ): conditions = conditions if conditions else {} - custom_sql_conditions = ( - custom_sql_conditions if custom_sql_conditions else [] - ) date_range = date_range if date_range else {} - custom_sql_conditions.append( - self.create_sql_date_range_filter(date_range) - ) - - custom_params = self.generate_params(date_range) + date_range_params = self.generate_params(date_range) partition_key_value = self.find_partition_key_value(event_context) - params = [ - {"name": "@partition_key_value", "value": partition_key_value}, - ] - params.extend(self.generate_params(conditions)) - params.extend(custom_params) + params = self.generate_params(conditions) + params.extend(date_range_params) - query_str = """ - SELECT VALUE COUNT(1) FROM c - WHERE c.{partition_key_attribute}=@partition_key_value - {conditions_clause} - {visibility_condition} - {custom_sql_conditions_clause} - """.format( - partition_key_attribute=self.partition_key_attribute, - visibility_condition=self.create_sql_condition_for_visibility( - visible_only - ), - conditions_clause=self.create_sql_where_conditions(conditions), - custom_sql_conditions_clause=self.create_custom_sql_conditions( - custom_sql_conditions - ), + query_builder = ( + CosmosDBQueryBuilder() + .add_select_conditions(["VALUE COUNT(1)"]) + .add_sql_in_condition('owner_id', owner_ids) + .add_sql_where_equal_condition(conditions) + .add_sql_visibility_condition(visible_only) + .add_date_range(date_range) + .build() ) - flask.current_app.logger.debug(query_str) + query_str = query_builder.get_query() + tenant_id_value = self.find_partition_key_value(event_context) result = self.container.query_items( query=query_str, parameters=params, - partition_key=partition_key_value, + partition_key=tenant_id_value, ) return result.next() @@ -179,57 +143,49 @@ def add_complementary_info( def find_all( self, conditions, - custom_sql_conditions, event_context: EventContext, date_range: dict = None, + owner_ids: list = None, test_user_ids=None, offset=0, max_count=None, visible_only=True, - custom_params: dict = None, mapper: Callable = None, ): - partition_key_value = self.find_partition_key_value(event_context) max_count = self.get_page_size_or(max_count) params = [ - {"name": "@partition_key_value", "value": partition_key_value}, {"name": "@offset", "value": offset}, - {"name": "@max_count", "value": max_count}, + {"name": "@limit", "value": max_count}, ] - params.extend(self.generate_params(conditions)) - params.extend(custom_params) + date_range = date_range if date_range else {} + date_range_params = ( + self.generate_params(date_range) if date_range else [] + ) - query_str = """ - SELECT * FROM c - WHERE c.{partition_key_attribute}=@partition_key_value - {conditions_clause} - {visibility_condition} - {test_users_exclusion_condition} - {custom_sql_conditions_clause} - {order_clause} - OFFSET @offset LIMIT @max_count - """.format( - partition_key_attribute=self.partition_key_attribute, - visibility_condition=self.create_sql_condition_for_visibility( - visible_only - ), - conditions_clause=self.create_sql_where_conditions(conditions), - test_users_exclusion_condition=self.create_sql_test_users_exclusion_condition( - test_user_ids - ), - custom_sql_conditions_clause=self.create_custom_sql_conditions( - custom_sql_conditions - ), - order_clause=self.create_sql_order_clause(), + params.extend(self.generate_params(conditions) if conditions else []) + params.extend(date_range_params) + + query_builder = ( + CosmosDBQueryBuilder() + .add_sql_in_condition('owner_id', owner_ids) + .add_sql_where_equal_condition(conditions) + .add_sql_visibility_condition(visible_only) + .add_date_range(date_range) + .add_sql_not_in_condition(test_user_ids) + .add_sql_order_by_condition('start_date', Order.DESC) + .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) result = self.container.query_items( query=query_str, parameters=params, - partition_key=partition_key_value, - max_item_count=max_count, + partition_key=tenant_id_value, ) function_mapper = self.get_mapper_or_dict(mapper) diff --git a/utils/extend_model.py b/utils/extend_model.py index 6eecedcf..6ce1593d 100644 --- a/utils/extend_model.py +++ b/utils/extend_model.py @@ -28,7 +28,11 @@ def add_project_info_to_time_entries(time_entries, projects): for time_entry in time_entries: for project in projects: if time_entry.project_id == project.id: - name = project.name + " (archived)" if project.is_deleted() else project.name + name = ( + project.name + " (archived)" + if project.is_deleted() + else project.name + ) setattr(time_entry, 'project_name', name) setattr(time_entry, 'customer_id', project.customer_id) setattr(time_entry, 'customer_name', project.customer_name) @@ -38,7 +42,11 @@ def add_activity_name_to_time_entries(time_entries, activities): for time_entry in time_entries: for activity in activities: if time_entry.activity_id == activity.id: - name = activity.name + " (archived)" if activity.is_deleted() else activity.name + name = ( + activity.name + " (archived)" + if activity.is_deleted() + else activity.name + ) setattr(time_entry, 'activity_name', name) @@ -89,3 +97,7 @@ def create_custom_query_from_str( else: query_str = "{} = '{}'".format(first_attr, data[0]) return query_str + + +def create_list_from_str(data: str, delimiter: str = ",") -> str: + return [id for id in data.split(delimiter)] if data else [] diff --git a/utils/query_builder.py b/utils/query_builder.py index 2899aab4..68740d46 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -20,6 +20,7 @@ def __init__(self): self.limit = None self.offset = None self.order_by = None + self.date_range = None def add_select_conditions(self, columns: List[str] = None): columns = columns if columns else ["*"] @@ -69,6 +70,11 @@ def add_sql_not_in_condition( self.where_conditions.append(f"c.{attribute} NOT IN {ids_values}") return self + def add_date_range(self, date_range: dict = None): + if date_range: + self.date_range = date_range + return self + def __build_select(self): if len(self.select_conditions) < 1: self.select_conditions.append("*") @@ -101,16 +107,25 @@ def __build_order_by(self): else: return "" + def __build_date_range(self): + if self.date_range: + and_keyword = "AND " if len(self.where_conditions) > 0 else "" + return f"{and_keyword}((c.start_date BETWEEN @start_date AND @end_date) OR (c.end_date BETWEEN @start_date AND @end_date))" + else: + return "" + def build(self): self.query = """ SELECT {select_conditions} FROM c {where_conditions} + {date_range_condition} {order_by_condition} {offset_condition} {limit_condition} """.format( select_conditions=self.__build_select(), where_conditions=self.__build_where(), + date_range_condition=self.__build_date_range(), order_by_condition=self.__build_order_by(), offset_condition=self.__build_offset(), limit_condition=self.__build_limit(), From c95fc27cf7792c4e2e4a20dbbca6dbf68925edf4 Mon Sep 17 00:00:00 2001 From: Pablo Date: Fri, 14 May 2021 10:45:11 -0500 Subject: [PATCH 05/12] fix: TT-220 apply refactoring on some code --- time_tracker_api/projects/projects_model.py | 2 +- time_tracker_api/time_entries/time_entries_dao.py | 6 ++---- time_tracker_api/time_entries/time_entries_repository.py | 7 ------- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index 1e0c7d0f..9cb7c39c 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -125,7 +125,7 @@ def get_all( customers_ids_conditions = ( customers_ids_conditions + customers_id if customers_id - else customers_id + else customers_ids_conditions ) projects = self.repository.find_all( diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index 7a4f3a11..98fb64b4 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/time_tracker_api/time_entries/time_entries_dao.py @@ -98,8 +98,7 @@ def get_all(self, conditions: dict = None, **kwargs) -> list: conditions=conditions, ) date_range = self.handle_date_filter_args(args=conditions) - limit = conditions.get("limit", None) - conditions.pop("limit", None) + limit = conditions.pop("limit", None) azure_connection = AzureConnection() current_user_is_tester = azure_connection.is_test_user( event_ctx.user_id @@ -143,12 +142,11 @@ def get_lastest_entries_by_project( for id_project in projects_ids: conditions.update({"project_id": id_project}) - limit = 1 latest = self.repository.find_all_entries( event_ctx, conditions=conditions, date_range=date_range, - max_count=limit, + max_count=1, ) if len(latest) > 0: diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 791dda92..3ab18cab 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -196,13 +196,6 @@ def find_all( time_entries, max_count, exist_conditions ) - def create_sql_test_users_exclusion_condition(self, test_user_ids): - if test_user_ids != None: - tuple_string = convert_list_to_tuple_string(test_user_ids) - return "AND c.owner_id NOT IN {list}".format(list=tuple_string) - - return "" - def get_last_entry( self, owner_id: str, From a77a4205d9bd75916a343c7c59978514622cdb7d Mon Sep 17 00:00:00 2001 From: Kevin Lopez Date: Fri, 14 May 2021 15:50:22 -0500 Subject: [PATCH 06/12] fix: TT-220 Bugs corrections --- time_tracker_api/time_entries/time_entries_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 3ab18cab..6ee08b37 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -173,7 +173,7 @@ def find_all( .add_sql_where_equal_condition(conditions) .add_sql_visibility_condition(visible_only) .add_date_range(date_range) - .add_sql_not_in_condition(test_user_ids) + .add_sql_not_in_condition('owner_id', test_user_ids) .add_sql_order_by_condition('start_date', Order.DESC) .add_sql_limit_condition(max_count) .add_sql_offset_condition(offset) From d43d88b64aac2c1bd8905a2f0715df33832402d6 Mon Sep 17 00:00:00 2001 From: Kevin Lopez Date: Fri, 14 May 2021 16:19:03 -0500 Subject: [PATCH 07/12] fix: TT-220 Removes order conditions from activities --- time_tracker_api/activities/activities_model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/time_tracker_api/activities/activities_model.py b/time_tracker_api/activities/activities_model.py index 24287bf5..10d4aeb8 100644 --- a/time_tracker_api/activities/activities_model.py +++ b/time_tracker_api/activities/activities_model.py @@ -101,7 +101,6 @@ def find_all( .add_sql_in_condition('id', activities_id) .add_sql_where_equal_condition(conditions) .add_sql_visibility_condition(visible_only) - .add_sql_order_by_condition('id', Order.ASC) .add_sql_limit_condition(max_count) .add_sql_offset_condition(offset) .build() From 37df4290592a93bdf9a0916609cf6eef86fed479 Mon Sep 17 00:00:00 2001 From: Pablo Date: Fri, 14 May 2021 18:15:18 -0500 Subject: [PATCH 08/12] fix: TT-220 make adjustments in projects query for time-entries --- .../projects/projects_model_test.py | 2 +- time_tracker_api/projects/projects_model.py | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/time_tracker_api/projects/projects_model_test.py b/tests/time_tracker_api/projects/projects_model_test.py index 2ac693cd..43cba41b 100644 --- a/tests/time_tracker_api/projects/projects_model_test.py +++ b/tests/time_tracker_api/projects/projects_model_test.py @@ -29,7 +29,7 @@ def test_find_all_projects_new_version( project_repository.container.query_items = query_items_mock result = project_repository.find_all( - event_context, ['id'], ['customer_id'] + event_context, {"customer_id": "1"}, ['id'], ['customer_id'] ) find_partition_key_value_mock.assert_called_once() assert len(result) == 1 diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index 9cb7c39c..32ea5929 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -71,13 +71,17 @@ def __init__(self): def find_all( self, event_context: EventContext, + conditions: dict = None, project_ids: List[str] = None, customer_ids: List[str] = None, visible_only=True, mapper: Callable = None, ): + params = self.generate_params(conditions) if conditions else [] + query_builder = ( CosmosDBQueryBuilder() + .add_sql_where_equal_condition(conditions) .add_sql_in_condition("id", project_ids) .add_sql_in_condition("customer_id", customer_ids) .add_sql_visibility_condition(visible_only) @@ -87,6 +91,7 @@ def find_all( tenant_id_value = self.find_partition_key_value(event_context) result = self.container.query_items( query=query_str, + parameters=params, partition_key=tenant_id_value, ) function_mapper = self.get_mapper_or_dict(mapper) @@ -120,18 +125,12 @@ def get_all( ] conditions = conditions if conditions else {} - customers_ids_conditions = [v for k, v in conditions.items()] - - customers_ids_conditions = ( - customers_ids_conditions + customers_id - if customers_id - else customers_ids_conditions - ) projects = self.repository.find_all( event_context=event_ctx, + conditions=conditions, project_ids=project_ids, - customer_ids=customers_ids_conditions, + customer_ids=customers_id, ) add_customer_name_to_projects(projects, customers) From 9eb0e9678edb0f25759f7042487b95ab3d1a43de Mon Sep 17 00:00:00 2001 From: Pablo Date: Wed, 19 May 2021 08:51:57 -0500 Subject: [PATCH 09/12] fix: TT-220 adjustments according sonar suggestions --- commons/data_access_layer/cosmos_db.py | 4 ++-- .../projects/projects_model_test.py | 5 ++++- time_tracker_api/activities/activities_model.py | 4 ++-- time_tracker_api/projects/projects_model.py | 6 ++---- .../time_entries/time_entries_repository.py | 14 +------------- utils/extend_model.py | 2 +- utils/query_builder.py | 6 ++++++ 7 files changed, 18 insertions(+), 23 deletions(-) diff --git a/commons/data_access_layer/cosmos_db.py b/commons/data_access_layer/cosmos_db.py index 5843e0fa..9cdf7f1c 100644 --- a/commons/data_access_layer/cosmos_db.py +++ b/commons/data_access_layer/cosmos_db.py @@ -208,7 +208,7 @@ def attach_context(data: dict, event_context: EventContext): @staticmethod def create_sql_date_range_filter(date_range: dict) -> str: - if 'start_date' and 'end_date' in date_range: + 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)) @@ -251,9 +251,9 @@ def find_all( event_context: EventContext, conditions: 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 {} diff --git a/tests/time_tracker_api/projects/projects_model_test.py b/tests/time_tracker_api/projects/projects_model_test.py index 43cba41b..af7c76f6 100644 --- a/tests/time_tracker_api/projects/projects_model_test.py +++ b/tests/time_tracker_api/projects/projects_model_test.py @@ -29,7 +29,10 @@ def test_find_all_projects_new_version( project_repository.container.query_items = query_items_mock result = project_repository.find_all( - event_context, {"customer_id": "1"}, ['id'], ['customer_id'] + 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 diff --git a/time_tracker_api/activities/activities_model.py b/time_tracker_api/activities/activities_model.py index 10d4aeb8..8c77cba9 100644 --- a/time_tracker_api/activities/activities_model.py +++ b/time_tracker_api/activities/activities_model.py @@ -89,10 +89,10 @@ def find_all_with_id_in_list( def find_all( self, event_context: EventContext, - conditions: dict = None, + conditions, activities_id: List = None, visible_only=True, - max_count=0, + max_count=None, offset=0, mapper: Callable = None, ): diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index 32ea5929..fcbd13a7 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -71,14 +71,12 @@ def __init__(self): def find_all( self, event_context: EventContext, - conditions: dict = None, + conditions, project_ids: List[str] = None, customer_ids: List[str] = None, visible_only=True, mapper: Callable = None, ): - params = self.generate_params(conditions) if conditions else [] - query_builder = ( CosmosDBQueryBuilder() .add_sql_where_equal_condition(conditions) @@ -89,6 +87,7 @@ def find_all( ) 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, @@ -117,7 +116,6 @@ def get_all( max_count=kwargs.get('max_count', None) ) - # TODO: evaluate another approach in order that memory filtering will be make in Database instead customers_id = [ customer.id for customer in customers diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 6ee08b37..ef6a9f9a 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -82,7 +82,6 @@ def count( date_range = date_range if date_range else {} date_range_params = self.generate_params(date_range) - partition_key_value = self.find_partition_key_value(event_context) params = self.generate_params(conditions) params.extend(date_range_params) @@ -153,19 +152,7 @@ def find_all( mapper: Callable = None, ): max_count = self.get_page_size_or(max_count) - - params = [ - {"name": "@offset", "value": offset}, - {"name": "@limit", "value": max_count}, - ] - 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) if conditions else []) - params.extend(date_range_params) query_builder = ( CosmosDBQueryBuilder() @@ -181,6 +168,7 @@ def find_all( ) query_str = query_builder.get_query() + params = query_builder.get_parameters() tenant_id_value = self.find_partition_key_value(event_context) result = self.container.query_items( query=query_str, diff --git a/utils/extend_model.py b/utils/extend_model.py index 6ce1593d..b13faa44 100644 --- a/utils/extend_model.py +++ b/utils/extend_model.py @@ -99,5 +99,5 @@ def create_custom_query_from_str( return query_str -def create_list_from_str(data: str, delimiter: str = ",") -> str: +def create_list_from_str(data: str, delimiter: str = ",") -> list: return [id for id in data.split(delimiter)] if data else [] diff --git a/utils/query_builder.py b/utils/query_builder.py index 68740d46..f62ae7cd 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -109,6 +109,12 @@ def __build_order_by(self): def __build_date_range(self): if self.date_range: + self.parameters.append( + {'name': '@start_date', 'value': self.date_range['start_date']} + ) + self.parameters.append( + {'name': '@end_date', 'value': self.date_range['end_date']} + ) and_keyword = "AND " if len(self.where_conditions) > 0 else "" return f"{and_keyword}((c.start_date BETWEEN @start_date AND @end_date) OR (c.end_date BETWEEN @start_date AND @end_date))" else: From d13c19d2a9123d830463d8f224b145a6f579145b Mon Sep 17 00:00:00 2001 From: Pablo Date: Wed, 19 May 2021 09:09:35 -0500 Subject: [PATCH 10/12] fix: TT-220 adjustments according sonar suggestions in activities --- time_tracker_api/activities/activities_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time_tracker_api/activities/activities_model.py b/time_tracker_api/activities/activities_model.py index 8c77cba9..47d012f7 100644 --- a/time_tracker_api/activities/activities_model.py +++ b/time_tracker_api/activities/activities_model.py @@ -89,7 +89,7 @@ def find_all_with_id_in_list( def find_all( self, event_context: EventContext, - conditions, + conditions: dict = None, activities_id: List = None, visible_only=True, max_count=None, From 8a2682a6e31ada3b20ac527932710ccc50934278 Mon Sep 17 00:00:00 2001 From: Pablo Date: Wed, 19 May 2021 14:06:21 -0500 Subject: [PATCH 11/12] fix: TT-220 adjustments according sonar code analysis --- time_tracker_api/projects/projects_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index fcbd13a7..4b3b77ab 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -71,7 +71,7 @@ def __init__(self): def find_all( self, event_context: EventContext, - conditions, + conditions: dict = None, project_ids: List[str] = None, customer_ids: List[str] = None, visible_only=True, From ede5c4cc1af20849103642cdaa75c16afa976edd Mon Sep 17 00:00:00 2001 From: Pablo Date: Thu, 20 May 2021 14:44:49 -0500 Subject: [PATCH 12/12] fix: TT-220 adjustments according reviewers suggestions --- tests/utils/query_builder_test.py | 24 ------------------- .../time_entries/time_entries_repository.py | 23 ++++++------------ utils/query_builder.py | 21 ---------------- 3 files changed, 7 insertions(+), 61 deletions(-) diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index 8d5776f4..742730db 100644 --- a/tests/utils/query_builder_test.py +++ b/tests/utils/query_builder_test.py @@ -331,27 +331,3 @@ def test_add_sql_not_in_condition( ) 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 diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index ef6a9f9a..c4bc7f02 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -78,24 +78,18 @@ def count( visible_only=True, **kwargs, ): - conditions = conditions if conditions else {} - date_range = date_range if date_range else {} - - date_range_params = self.generate_params(date_range) - params = self.generate_params(conditions) - params.extend(date_range_params) - query_builder = ( - CosmosDBQueryBuilder() + TimeEntryQueryBuilder() .add_select_conditions(["VALUE COUNT(1)"]) .add_sql_in_condition('owner_id', owner_ids) .add_sql_where_equal_condition(conditions) .add_sql_visibility_condition(visible_only) - .add_date_range(date_range) + .add_sql_date_range_condition(date_range) .build() ) query_str = query_builder.get_query() + params = query_builder.get_parameters() tenant_id_value = self.find_partition_key_value(event_context) result = self.container.query_items( query=query_str, @@ -109,11 +103,8 @@ def add_complementary_info( self, time_entries=None, max_count=None, exist_conditions=False ): if time_entries: - project_ids_set = set([x.project_id for x in time_entries]) - project_ids = list(project_ids_set) - - activity_ids_set = set([x.activity_id for x in time_entries]) - activity_ids = list(activity_ids_set) + project_ids = list(set([x.project_id for x in time_entries])) + activity_ids = list(set([x.activity_id for x in time_entries])) project_dao = projects_model.create_dao() projects = project_dao.get_all( @@ -155,11 +146,11 @@ def find_all( date_range = date_range if date_range else {} query_builder = ( - CosmosDBQueryBuilder() + TimeEntryQueryBuilder() .add_sql_in_condition('owner_id', owner_ids) .add_sql_where_equal_condition(conditions) .add_sql_visibility_condition(visible_only) - .add_date_range(date_range) + .add_sql_date_range_condition(date_range) .add_sql_not_in_condition('owner_id', test_user_ids) .add_sql_order_by_condition('start_date', Order.DESC) .add_sql_limit_condition(max_count) diff --git a/utils/query_builder.py b/utils/query_builder.py index f62ae7cd..2899aab4 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -20,7 +20,6 @@ def __init__(self): self.limit = None self.offset = None self.order_by = None - self.date_range = None def add_select_conditions(self, columns: List[str] = None): columns = columns if columns else ["*"] @@ -70,11 +69,6 @@ def add_sql_not_in_condition( self.where_conditions.append(f"c.{attribute} NOT IN {ids_values}") return self - def add_date_range(self, date_range: dict = None): - if date_range: - self.date_range = date_range - return self - def __build_select(self): if len(self.select_conditions) < 1: self.select_conditions.append("*") @@ -107,31 +101,16 @@ def __build_order_by(self): else: return "" - def __build_date_range(self): - if self.date_range: - self.parameters.append( - {'name': '@start_date', 'value': self.date_range['start_date']} - ) - self.parameters.append( - {'name': '@end_date', 'value': self.date_range['end_date']} - ) - and_keyword = "AND " if len(self.where_conditions) > 0 else "" - return f"{and_keyword}((c.start_date BETWEEN @start_date AND @end_date) OR (c.end_date BETWEEN @start_date AND @end_date))" - else: - return "" - def build(self): self.query = """ SELECT {select_conditions} FROM c {where_conditions} - {date_range_condition} {order_by_condition} {offset_condition} {limit_condition} """.format( select_conditions=self.__build_select(), where_conditions=self.__build_where(), - date_range_condition=self.__build_date_range(), order_by_condition=self.__build_order_by(), offset_condition=self.__build_offset(), limit_condition=self.__build_limit(),