From d2f5d5b1d4441ad7f4ed18ca99425d864f81aeca Mon Sep 17 00:00:00 2001 From: kelly Date: Thu, 1 Apr 2021 11:51:49 -0500 Subject: [PATCH 01/10] refactor: TT-201 add fucntion to add update last entry flag in model --- tests/time_tracker_api/api_test.py | 53 ++++++++++++++++++++++++++---- time_tracker_api/api.py | 13 ++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/tests/time_tracker_api/api_test.py b/tests/time_tracker_api/api_test.py index 6f9ad353..a28ffe27 100644 --- a/tests/time_tracker_api/api_test.py +++ b/tests/time_tracker_api/api_test.py @@ -30,16 +30,55 @@ def test_remove_required_constraint(): from flask_restplus import Namespace ns = Namespace('todos', description='Namespace for testing') - sample_model = ns.model('Todo', { - 'id': fields.Integer(readonly=True, description='The task unique identifier'), - 'task': fields.String(required=True, description='The task details'), - 'done': fields.Boolean(required=False, description='Has it being done or not') - }) + sample_model = ns.model( + 'Todo', + { + 'id': fields.Integer( + readonly=True, description='The task unique identifier' + ), + 'task': fields.String( + required=True, description='The task details' + ), + 'done': fields.Boolean( + required=False, description='Has it being done or not' + ), + }, + ) new_model = remove_required_constraint(sample_model) assert new_model is not sample_model for attrib in sample_model: - assert new_model[attrib].required is False, "No attribute should be required" - assert new_model[attrib] is not sample_model[attrib], "No attribute should be required" + assert ( + new_model[attrib].required is False + ), "No attribute should be required" + assert ( + new_model[attrib] is not sample_model[attrib] + ), "No attribute should be required" + + +def test_add_update_last_entry_flag(): + from time_tracker_api.api import add_update_last_entry_flag + from flask_restplus import fields + from flask_restplus import Namespace + + ns = Namespace('todos', description='Namespace for testing') + sample_model = ns.model( + 'Todo', + { + 'id': fields.Integer( + readonly=True, description='The task unique identifier' + ), + 'task': fields.String( + required=True, description='The task details' + ), + }, + ) + + new_model = add_update_last_entry_flag(sample_model) + + assert new_model is not sample_model + + update_last_entry_flag = new_model.get('update_last_entry') + assert update_last_entry_flag is not None diff --git a/time_tracker_api/api.py b/time_tracker_api/api.py index 0cdecb52..4dc34171 100644 --- a/time_tracker_api/api.py +++ b/time_tracker_api/api.py @@ -34,6 +34,19 @@ def remove_required_constraint(model: Model): return result +def add_update_last_entry_flag(time_entry_model: Model): + time_entry_flag = { + 'update_last_entry': fields.Boolean( + title='Update last entry', + required=False, + description='Flag that indicates if the last time entry is updated', + example=True, + ) + } + new_model = time_entry_model.clone('TimeEntryInput', time_entry_flag) + return new_model + + def create_attributes_filter( ns: namespace, model: Model, filter_attrib_names: list ) -> RequestParser: From f528a25c2cf6058727cec34a80d7d9e841d0e0cb Mon Sep 17 00:00:00 2001 From: kelly Date: Thu, 1 Apr 2021 16:42:54 -0500 Subject: [PATCH 02/10] refactor: TT-201 add update_last_entry and get_last_entry in repository, update CosmosDBQueryBuilder with order by condition --- tests/time_tracker_api/api_test.py | 12 +-- tests/utils/query_builder_test.py | 77 ++++++++++++++++--- time_tracker_api/api.py | 6 +- .../time_entries/time_entries_dao.py | 3 + .../time_entries/time_entries_namespace.py | 8 +- .../time_entries/time_entries_repository.py | 40 ++++++++++ utils/query_builder.py | 19 +++++ 7 files changed, 144 insertions(+), 21 deletions(-) diff --git a/tests/time_tracker_api/api_test.py b/tests/time_tracker_api/api_test.py index a28ffe27..a3a17484 100644 --- a/tests/time_tracker_api/api_test.py +++ b/tests/time_tracker_api/api_test.py @@ -58,8 +58,8 @@ def test_remove_required_constraint(): ), "No attribute should be required" -def test_add_update_last_entry_flag(): - from time_tracker_api.api import add_update_last_entry_flag +def test_add_update_last_entry_if_overlap(): + from time_tracker_api.api import add_update_last_entry_if_overlap from flask_restplus import fields from flask_restplus import Namespace @@ -76,9 +76,11 @@ def test_add_update_last_entry_flag(): }, ) - new_model = add_update_last_entry_flag(sample_model) + new_model = add_update_last_entry_if_overlap(sample_model) assert new_model is not sample_model - update_last_entry_flag = new_model.get('update_last_entry') - assert update_last_entry_flag is not None + update_last_entry_if_overlap = new_model.get( + 'update_last_entry_if_overlap' + ) + assert update_last_entry_if_overlap is not None diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index bbe2e2ce..c5b4f492 100644 --- a/tests/utils/query_builder_test.py +++ b/tests/utils/query_builder_test.py @@ -1,5 +1,5 @@ from unittest.mock import patch -from utils.query_builder import CosmosDBQueryBuilder +from utils.query_builder import CosmosDBQueryBuilder, Order from utils.repository import remove_white_spaces import pytest @@ -41,7 +41,9 @@ def test_add_select_conditions_should_update_select_list( ], ) def test_add_sql_in_condition_should_update_where_list( - attribute, ids_list, expected_where_condition_list, + attribute, + ids_list, + expected_where_condition_list, ): query_builder = CosmosDBQueryBuilder().add_sql_in_condition( attribute, ids_list @@ -66,7 +68,9 @@ def test_add_sql_in_condition_should_update_where_list( ], ) def test_add_sql_where_equal_condition_should_update_where_params_list( - data, expected_where_list, expected_params, + data, + expected_where_list, + expected_params, ): query_builder = CosmosDBQueryBuilder().add_sql_where_equal_condition(data) @@ -91,7 +95,8 @@ def test_add_sql_where_equal_condition_with_None_should_not_update_lists(): [(True, ['NOT IS_DEFINED(c.deleted)']), (False, [])], ) def test_add_sql_visibility_condition( - visibility_bool, expected_where_list, + visibility_bool, + expected_where_list, ): query_builder = CosmosDBQueryBuilder().add_sql_visibility_condition( visibility_bool @@ -102,7 +107,12 @@ def test_add_sql_visibility_condition( @pytest.mark.parametrize( - "limit_value,expected_limit", [(1, 1), (10, 10), (None, None),], + "limit_value,expected_limit", + [ + (1, 1), + (10, 10), + (None, None), + ], ) def test_add_sql_limit_condition(limit_value, expected_limit): query_builder = CosmosDBQueryBuilder().add_sql_limit_condition(limit_value) @@ -111,10 +121,16 @@ def test_add_sql_limit_condition(limit_value, expected_limit): @pytest.mark.parametrize( - "offset_value,expected_offset", [(1, 1), (10, 10), (None, None),], + "offset_value,expected_offset", + [ + (1, 1), + (10, 10), + (None, None), + ], ) def test_add_sql_offset_condition( - offset_value, expected_offset, + offset_value, + expected_offset, ): query_builder = CosmosDBQueryBuilder().add_sql_offset_condition( offset_value @@ -125,10 +141,15 @@ def test_add_sql_offset_condition( @pytest.mark.parametrize( "select_conditions,expected_condition", - [([], "*"), (["c.id"], "c.id"), (["c.id", "c.name"], "c.id,c.name"),], + [ + ([], "*"), + (["c.id"], "c.id"), + (["c.id", "c.name"], "c.id,c.name"), + ], ) def test__build_select_return_fields_in_select_list( - select_conditions, expected_condition, + select_conditions, + expected_condition, ): query_builder = CosmosDBQueryBuilder().add_select_conditions( select_conditions @@ -148,7 +169,8 @@ def test__build_select_return_fields_in_select_list( ], ) def test__build_where_should_return_concatenated_conditions( - fields, expected_condition, + fields, + expected_condition, ): query_builder = CosmosDBQueryBuilder().add_sql_where_equal_condition( fields @@ -164,7 +186,9 @@ def test__build_where_should_return_concatenated_conditions( [(1, "OFFSET @offset", [{'name': '@offset', 'value': 1}]), (None, "", [])], ) def test__build_offset( - offset, expected_condition, expected_params, + offset, + expected_condition, + expected_params, ): query_builder = CosmosDBQueryBuilder().add_sql_offset_condition(offset) @@ -179,7 +203,9 @@ def test__build_offset( [(1, "LIMIT @limit", [{'name': '@limit', 'value': 1}]), (None, "", [])], ) def test__build_limit( - limit, expected_condition, expected_params, + limit, + expected_condition, + expected_params, ): query_builder = CosmosDBQueryBuilder().add_sql_limit_condition(limit) @@ -235,3 +261,30 @@ def test_build_with_empty_and_None_attributes_return_query_select_all(): assert query == expected_query assert len(query_builder.get_parameters()) == 0 assert len(query_builder.where_conditions) == 0 + + +def test_order_by_condition(): + query_builder = CosmosDBQueryBuilder().add_sql_order_by_condition( + 'start_date', Order.DESC + ) + + assert len(query_builder.get_parameters()) == 2 + assert query_builder.orderBy == True + + +def test__build_orderBy(): + query_builder = CosmosDBQueryBuilder().add_sql_order_by_condition( + 'start_date', Order.DESC + ) + + orderBy_condition = query_builder._CosmosDBQueryBuilder__build_order_By() + expected_order_string = "ORDER BY c.@attribute @order" + + assert expected_order_string == orderBy_condition + + expected_params = [ + {'name': '@attribute', 'value': 'start_date'}, + {'name': '@order', 'value': 'DESC'}, + ] + + assert expected_params == query_builder.get_parameters() diff --git a/time_tracker_api/api.py b/time_tracker_api/api.py index 4dc34171..b6e5a488 100644 --- a/time_tracker_api/api.py +++ b/time_tracker_api/api.py @@ -34,10 +34,10 @@ def remove_required_constraint(model: Model): return result -def add_update_last_entry_flag(time_entry_model: Model): +def add_update_last_entry_if_overlap(time_entry_model: Model): time_entry_flag = { - 'update_last_entry': fields.Boolean( - title='Update last entry', + 'update_last_entry_if_overlap': fields.Boolean( + title='Update last entry if overlap', required=False, description='Flag that indicates if the last time entry is updated', example=True, diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index 62f4bc78..6cb974f8 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/time_tracker_api/time_entries/time_entries_dao.py @@ -226,6 +226,9 @@ def create(self, data: dict): def update(self, id, data: dict, description=None): event_ctx = self.create_event_context("update", description) + if data.get('update_last_entry_if_overlap'): + self.repository.update_last_entry(data) + time_entry = self.repository.find(id, event_ctx) self.check_whether_current_user_owns_item(time_entry) diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index f2c115fc..6183e18f 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -15,6 +15,7 @@ UUID, NullableString, remove_required_constraint, + update_last_entry_if_overlap, ) from time_tracker_api.time_entries.time_entries_dao import create_dao @@ -267,6 +268,11 @@ def get(self): return time_entries_dao.get_lastest_entries_by_project(conditions={}) +update_entry_input = update_last_entry_if_overlap( + remove_required_constraint(time_entry_input) +) + + @ns.route('/') @ns.response(HTTPStatus.NOT_FOUND, 'This time entry does not exist') @ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, 'The id has an invalid format') @@ -288,7 +294,7 @@ def get(self, id): 'A time entry already exists with this new data or there' ' is a bad reference for the project or activity', ) - @ns.expect(remove_required_constraint(time_entry_input)) + @ns.expect(update_entry_input) @ns.marshal_with(time_entry) def put(self, id): """Update a time entry""" diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 019a79a8..0b6ba2db 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -27,6 +27,8 @@ from time_tracker_api.time_entries.time_entries_query_builder import ( TimeEntryQueryBuilder, ) +from utils.query_builder import CosmosDBQueryBuilder, Order +from utils.time import str_to_datetime class TimeEntryCosmosDBRepository(CosmosDBRepository): @@ -233,6 +235,44 @@ def find_all_v2( function_mapper = self.get_mapper_or_dict(mapper) return list(map(function_mapper, result)) + def get_last_entry(self, owner_id: str, event_context: EventContext): + query_builder = ( + CosmosDBQueryBuilder() + .where_conditions({'owner_id': owner_id}) + .add_sql_order_by_condition('end_date', Order.DESC) + .limit(1) + .offset(1) + .build() + ) + + query_str = query_builder.get_query() + params = query_builder.generate_params() + result = self.container.query_items( + query=query_str, + parameters=params, + partition_key=partition_key_value, + ) + + function_mapper = self.get_mapper_or_dict(mapper) + return list(map(function_mapper, result)) + + def update_last_entry(self, data: dict, event_context: EventContext): + owner_id = data.get('owner_id') + last_entry = self.get_last_entry(owner_id, event_context) + last_entry = last_entry.pop() + + end_date = str_to_datetime(last_entry.end_date) + start_date = data.get('start_date') + start_date = str_to_datetime(start_date) + + if start_date < end_date: + update_date = {'end_date': start_date} + return self.partial_update( + last_entry.id, update_date, event_context + ) + + return False + def on_create(self, new_item_data: dict, event_context: EventContext): CosmosDBRepository.on_create(self, new_item_data, event_context) diff --git a/utils/query_builder.py b/utils/query_builder.py index 9ff4248b..e535cf1a 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -1,5 +1,11 @@ from typing import List from utils.repository import convert_list_to_tuple_string +from enum import Enum + + +class Order(Enum): + DESC = 'DESC' + ASC = 'ASC' class CosmosDBQueryBuilder: @@ -13,6 +19,7 @@ def __init__(self): self.where_conditions = [] self.limit = None self.offset = None + self.orderBy = False def add_select_conditions(self, columns: List[str] = None): columns = columns if columns else ["*"] @@ -50,6 +57,12 @@ def add_sql_offset_condition(self, offset): self.offset = offset return self + def add_sql_order_by_condition(self, attribute: str, order: Order): + self.orderBy = True + self.parameters.append({'name': '@attribute', 'value': attribute}) + self.parameters.append({'name': '@order', 'value': order.name}) + return self + def __build_select(self): if len(self.select_conditions) < 1: self.select_conditions.append("*") @@ -75,15 +88,21 @@ def __build_limit(self): else: return "" + def __build_order_By(self): + if self.orderBy: + return "ORDER BY c.@attribute @order" + def build(self): self.query = """ SELECT {select_conditions} FROM c {where_conditions} + {order_by_condition} {offset_condition} {limit_condition} """.format( select_conditions=self.__build_select(), where_conditions=self.__build_where(), + order_by_condition=self.__build_order_By(), offset_condition=self.__build_offset(), limit_condition=self.__build_limit(), ) From 30a8a85cc74622a870e83e091f7ae2dc53d53288 Mon Sep 17 00:00:00 2001 From: kelly Date: Thu, 1 Apr 2021 16:53:21 -0500 Subject: [PATCH 03/10] test: TT-201 add test method for get_last_entry --- .../time_entries/time_entries_model_test.py | 31 +++++++++++++++++++ .../time_entries/time_entries_repository.py | 2 ++ 2 files changed, 33 insertions(+) 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 ff00a87b..a279016f 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 @@ -310,3 +310,34 @@ def test_find_all_v2( time_entry = result[0] assert isinstance(time_entry, TimeEntryCosmosDBModel) assert time_entry.__dict__ == expected_item + + +@patch( + 'time_tracker_api.time_entries.time_entries_repository.TimeEntryCosmosDBRepository.find_partition_key_value' +) +def test_get_last_entry( + find_partition_key_value_mock, + event_context: EventContext, + time_entry_repository: TimeEntryCosmosDBRepository, +): + expected_item = { + 'id': 'id', + 'start_date': '2021-03-22T10:00:00.000Z', + 'end_date': "2021-03-22T11:00:00.000Z", + 'description': 'do some testing', + 'tenant_id': 'tenant_id', + 'project_id': 'project_id', + 'activity_id': 'activity_id', + 'technologies': ['python'], + } + query_items_mock = Mock(return_value=[expected_item]) + time_entry_repository.container = Mock() + time_entry_repository.container.query_items = query_items_mock + + result = time_entry_repository.get_last_entry('id1', event_context) + find_partition_key_value_mock.assert_called_once() + + assert len(result) == 1 + time_entry = result.pop() + assert isinstance(time_entry, TimeEntryCosmosDBModel) + assert time_entry.__dict__ == expected_item diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 0b6ba2db..7e722d6c 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -247,6 +247,8 @@ def get_last_entry(self, owner_id: str, event_context: EventContext): query_str = query_builder.get_query() params = query_builder.generate_params() + + partition_key_value = self.find_partition_key_value(event_context) result = self.container.query_items( query=query_str, parameters=params, From 4b6beb6645fad8c408528ecc68ba167557be1acb Mon Sep 17 00:00:00 2001 From: kelly Date: Fri, 2 Apr 2021 22:05:17 -0500 Subject: [PATCH 04/10] fix: TT-201 fix import errors and function calls in time_entry_repository --- .../time_entries/time_entries_namespace.py | 4 ++-- .../time_entries/time_entries_repository.py | 16 +++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index 6183e18f..01c4af73 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -15,7 +15,7 @@ UUID, NullableString, remove_required_constraint, - update_last_entry_if_overlap, + add_update_last_entry_if_overlap, ) from time_tracker_api.time_entries.time_entries_dao import create_dao @@ -268,7 +268,7 @@ def get(self): return time_entries_dao.get_lastest_entries_by_project(conditions={}) -update_entry_input = update_last_entry_if_overlap( +update_entry_input = add_update_last_entry_if_overlap( remove_required_constraint(time_entry_input) ) diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 7e722d6c..cac3cf23 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -235,18 +235,24 @@ def find_all_v2( function_mapper = self.get_mapper_or_dict(mapper) return list(map(function_mapper, result)) - def get_last_entry(self, owner_id: str, event_context: EventContext): + def get_last_entry( + self, + owner_id: str, + event_context: EventContext, + visible_only=True, + mapper: Callable = None, + ): query_builder = ( CosmosDBQueryBuilder() - .where_conditions({'owner_id': owner_id}) + .add_sql_where_equal_condition({'owner_id': owner_id}) .add_sql_order_by_condition('end_date', Order.DESC) - .limit(1) - .offset(1) + .add_sql_limit_condition(1) + .add_sql_offset_condition(1) .build() ) query_str = query_builder.get_query() - params = query_builder.generate_params() + params = query_builder.get_parameters() partition_key_value = self.find_partition_key_value(event_context) result = self.container.query_items( From ad2257afe1b6723e3a7dea99099dd26fdabd6954 Mon Sep 17 00:00:00 2001 From: kelly Date: Fri, 2 Apr 2021 22:11:21 -0500 Subject: [PATCH 05/10] fix: TT-201 fix return None in orderBy condition --- utils/query_builder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utils/query_builder.py b/utils/query_builder.py index e535cf1a..d89286f5 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -91,6 +91,8 @@ def __build_limit(self): def __build_order_By(self): if self.orderBy: return "ORDER BY c.@attribute @order" + else: + return "" def build(self): self.query = """ From 8c820edf0b19f77652d658305a8fea295e178b6f Mon Sep 17 00:00:00 2001 From: kelly Date: Sun, 4 Apr 2021 18:26:50 -0500 Subject: [PATCH 06/10] fix: TT-201 missed and bad arguments and order by params not supported by cosmosdb --- time_tracker_api/time_entries/time_entries_dao.py | 2 +- time_tracker_api/time_entries/time_entries_repository.py | 2 +- utils/query_builder.py | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index 6cb974f8..91acf3c2 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/time_tracker_api/time_entries/time_entries_dao.py @@ -227,7 +227,7 @@ def update(self, id, data: dict, description=None): event_ctx = self.create_event_context("update", description) if data.get('update_last_entry_if_overlap'): - self.repository.update_last_entry(data) + self.repository.update_last_entry(data, event_ctx) time_entry = self.repository.find(id, event_ctx) self.check_whether_current_user_owns_item(time_entry) diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index cac3cf23..8d3077d7 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -274,7 +274,7 @@ def update_last_entry(self, data: dict, event_context: EventContext): start_date = str_to_datetime(start_date) if start_date < end_date: - update_date = {'end_date': start_date} + update_date = {'end_date': data.get('start_date')} return self.partial_update( last_entry.id, update_date, event_context ) diff --git a/utils/query_builder.py b/utils/query_builder.py index d89286f5..edc3105c 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -19,7 +19,7 @@ def __init__(self): self.where_conditions = [] self.limit = None self.offset = None - self.orderBy = False + self.order_by = tuple() def add_select_conditions(self, columns: List[str] = None): columns = columns if columns else ["*"] @@ -58,7 +58,7 @@ def add_sql_offset_condition(self, offset): return self def add_sql_order_by_condition(self, attribute: str, order: Order): - self.orderBy = True + self.order_by = (attribute, order.name) self.parameters.append({'name': '@attribute', 'value': attribute}) self.parameters.append({'name': '@order', 'value': order.name}) return self @@ -89,8 +89,9 @@ def __build_limit(self): return "" def __build_order_By(self): - if self.orderBy: - return "ORDER BY c.@attribute @order" + if self.order_by: + attribute, order = self.order_by + return f"ORDER BY c.{attribute} {order}" else: return "" From 8d346679b3484f0b8a6a88df703d4051e7b13551 Mon Sep 17 00:00:00 2001 From: kelly Date: Mon, 5 Apr 2021 12:02:34 -0500 Subject: [PATCH 07/10] refactor: TT-201 delete unnecessary parameters in query_builder and its tests --- tests/utils/query_builder_test.py | 13 +++---------- utils/query_builder.py | 2 -- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index c5b4f492..921d93b0 100644 --- a/tests/utils/query_builder_test.py +++ b/tests/utils/query_builder_test.py @@ -268,8 +268,8 @@ def test_order_by_condition(): 'start_date', Order.DESC ) - assert len(query_builder.get_parameters()) == 2 - assert query_builder.orderBy == True + assert len(query_builder.order_by) == 2 + assert query_builder.order_by == ('start_date', 'DESC') def test__build_orderBy(): @@ -278,13 +278,6 @@ def test__build_orderBy(): ) orderBy_condition = query_builder._CosmosDBQueryBuilder__build_order_By() - expected_order_string = "ORDER BY c.@attribute @order" + expected_order_string = "ORDER BY c.start_date DESC" assert expected_order_string == orderBy_condition - - expected_params = [ - {'name': '@attribute', 'value': 'start_date'}, - {'name': '@order', 'value': 'DESC'}, - ] - - assert expected_params == query_builder.get_parameters() diff --git a/utils/query_builder.py b/utils/query_builder.py index edc3105c..47a6d279 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -59,8 +59,6 @@ def add_sql_offset_condition(self, offset): def add_sql_order_by_condition(self, attribute: str, order: Order): self.order_by = (attribute, order.name) - self.parameters.append({'name': '@attribute', 'value': attribute}) - self.parameters.append({'name': '@order', 'value': order.name}) return self def __build_select(self): From 186a5c09c5a3c12243f700cac9a603592e414d79 Mon Sep 17 00:00:00 2001 From: Vanessa Date: Wed, 7 Apr 2021 16:24:18 -0500 Subject: [PATCH 08/10] refactor: TT-201 Add tests for update last entry and update TimeEntriesDao --- .../time_entries/time_entries_model_test.py | 65 +++++++++++++++++++ .../time_entries_namespace_test.py | 30 +++++++++ 2 files changed, 95 insertions(+) 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 a279016f..aeae805b 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 @@ -341,3 +341,68 @@ def test_get_last_entry( time_entry = result.pop() assert isinstance(time_entry, TimeEntryCosmosDBModel) assert time_entry.__dict__ == expected_item + + +expected_item = { + 'id': 'id', + 'owner_id': '1', + 'start_date': '2021-03-22T10:00:00.000Z', + 'end_date': "2021-03-22T11:00:00.000Z", + 'description': 'do some testing', + 'tenant_id': 'tenant_id', + 'project_id': 'project_id', + 'activity_id': 'activity_id', + 'technologies': ['python'], + +} + +running_item = { + 'id': 'id', + 'owner_id': '1', + 'update_last_entry_if_overlap': True, + 'start_date': '2021-03-22T10:30:00.000Z', + 'end_date': '2021-03-22T11:30:00.000Z', + 'description': 'do some testing', + 'tenant_id': 'tenant_id', + 'project_id': 'project_id', + 'activity_id': 'activity_id', + 'technologies': ['python'], +} + +last_item_update = { + 'id': 'id', + 'owner_id': '1', + 'start_date': '2021-03-22T10:00:00.000Z', + 'end_date': "2021-03-22T10:30:00.000Z", + 'description': 'do some testing', + 'tenant_id': 'tenant_id', + 'project_id': 'project_id', + 'activity_id': 'activity_id', + 'technologies': ['python'], + } + +@pytest.mark.parametrize("expected_item, running_item, last_item_update", + [(expected_item, running_item, last_item_update)]) +def test_update_last_entry( + event_context: EventContext, + time_entry_repository: TimeEntryCosmosDBRepository, + expected_item, + running_item, + last_item_update +): + query_items_mock = Mock(return_value=[expected_item]) + time_entry_repository.container = Mock() + time_entry_repository.container.query_items = query_items_mock + + partial_update_mock = Mock(return_value=[last_item_update]) + time_entry_repository.partial_update = partial_update_mock + + result = time_entry_repository.update_last_entry(running_item, event_context) + + partial_update_mock.assert_called_once() + query_items_mock.assert_called_once() + assert result == [last_item_update] + + + + 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 67063c10..7d286151 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 @@ -881,3 +881,33 @@ def test_paginated_sends_max_count_and_offset_on_call_to_repository( _, kwargs = time_entries_dao.repository.find_all.call_args assert 'max_count' in kwargs and kwargs['max_count'] is not None assert 'offset' in kwargs and kwargs['offset'] is not None + + +def test_update_time_entry_calls_update_last_entry( + client: FlaskClient, + mocker: MockFixture, + valid_header: dict, + valid_id: str, + time_entries_dao, +): + time_entries_dao.repository.partial_update = Mock(return_value={}) + time_entries_dao.repository.find = Mock(return_value={}) + time_entries_dao.check_whether_current_user_owns_item = Mock() + time_entries_dao.repository.update_last_entry = Mock(return_value={}) + + update_time_entry = valid_time_entry_input.copy() + update_time_entry['update_last_entry_if_overlap'] = True + + response = client.put( + f'/time-entries/{valid_id}', + headers=valid_header, + json=update_time_entry, + follow_redirects=True, + ) + + assert HTTPStatus.OK == response.status_code + time_entries_dao.repository.partial_update.assert_called_once() + time_entries_dao.repository.find.assert_called_once() + time_entries_dao.check_whether_current_user_owns_item.assert_called_once() + time_entries_dao.repository.update_last_entry.assert_called_once() + From 7f599d4e5f8a48a8e0d3f17cc259206eaf6c8921 Mon Sep 17 00:00:00 2001 From: kelly Date: Fri, 9 Apr 2021 10:43:15 -0500 Subject: [PATCH 09/10] refactor: TT-201 apply changes --- .../time_entries/time_entries_model_test.py | 5 +-- tests/utils/query_builder_test.py | 37 +++++++++++++++---- .../time_entries/time_entries_dao.py | 9 +++-- .../time_entries/time_entries_repository.py | 19 ++++------ utils/query_builder.py | 6 +-- 5 files changed, 45 insertions(+), 31 deletions(-) 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 a279016f..e7cfb7bf 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 @@ -337,7 +337,4 @@ def test_get_last_entry( result = time_entry_repository.get_last_entry('id1', event_context) find_partition_key_value_mock.assert_called_once() - assert len(result) == 1 - time_entry = result.pop() - assert isinstance(time_entry, TimeEntryCosmosDBModel) - assert time_entry.__dict__ == expected_item + assert result == expected_item diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index 921d93b0..84abc9b9 100644 --- a/tests/utils/query_builder_test.py +++ b/tests/utils/query_builder_test.py @@ -263,21 +263,42 @@ def test_build_with_empty_and_None_attributes_return_query_select_all(): assert len(query_builder.where_conditions) == 0 -def test_order_by_condition(): +@pytest.mark.parametrize( + "attribute,order,expected_order_by", + [ + ('start_date', Order.DESC, ('start_date', 'DESC')), + ('start_date', Order.ASC, ('start_date', 'ASC')), + ], +) +def test_add_sql_order_by_condition( + attribute, + order, + expected_order_by, +): query_builder = CosmosDBQueryBuilder().add_sql_order_by_condition( - 'start_date', Order.DESC + attribute, order ) assert len(query_builder.order_by) == 2 - assert query_builder.order_by == ('start_date', 'DESC') + assert query_builder.order_by == expected_order_by -def test__build_orderBy(): +@pytest.mark.parametrize( + "attribute,order,expected_order_by_condition", + [ + ('start_date', Order.DESC, "ORDER BY c.start_date DESC"), + ('start_date', Order.ASC, "ORDER BY c.start_date ASC"), + ], +) +def test__build_order_by( + attribute, + order, + expected_order_by_condition, +): query_builder = CosmosDBQueryBuilder().add_sql_order_by_condition( - 'start_date', Order.DESC + attribute, order ) - orderBy_condition = query_builder._CosmosDBQueryBuilder__build_order_By() - expected_order_string = "ORDER BY c.start_date DESC" + orderBy_condition = query_builder._CosmosDBQueryBuilder__build_order_by() - assert expected_order_string == orderBy_condition + assert orderBy_condition == expected_order_by_condition diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index 91acf3c2..b04cca0f 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/time_tracker_api/time_entries/time_entries_dao.py @@ -225,13 +225,14 @@ def create(self, data: dict): def update(self, id, data: dict, description=None): event_ctx = self.create_event_context("update", description) - - if data.get('update_last_entry_if_overlap'): - self.repository.update_last_entry(data, event_ctx) - time_entry = self.repository.find(id, event_ctx) self.check_whether_current_user_owns_item(time_entry) + if data.get('update_last_entry_if_overlap'): + self.repository.update_last_entry( + data.get('owner_id'), data.get('start_date'), event_ctx + ) + return self.repository.partial_update( id, data, diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 8d3077d7..28837236 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -261,25 +261,20 @@ def get_last_entry( partition_key=partition_key_value, ) - function_mapper = self.get_mapper_or_dict(mapper) - return list(map(function_mapper, result)) + return result.pop() - def update_last_entry(self, data: dict, event_context: EventContext): - owner_id = data.get('owner_id') + def update_last_entry( + self, owner_id, start_date, event_context: EventContext + ): last_entry = self.get_last_entry(owner_id, event_context) - last_entry = last_entry.pop() + start_date_tmp = start_date end_date = str_to_datetime(last_entry.end_date) - start_date = data.get('start_date') start_date = str_to_datetime(start_date) if start_date < end_date: - update_date = {'end_date': data.get('start_date')} - return self.partial_update( - last_entry.id, update_date, event_context - ) - - return False + update_data = {'end_date': start_date_tmp} + self.partial_update(last_entry.id, update_data, event_context) def on_create(self, new_item_data: dict, event_context: EventContext): CosmosDBRepository.on_create(self, new_item_data, event_context) diff --git a/utils/query_builder.py b/utils/query_builder.py index 47a6d279..9aea4df9 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -19,7 +19,7 @@ def __init__(self): self.where_conditions = [] self.limit = None self.offset = None - self.order_by = tuple() + self.order_by = None def add_select_conditions(self, columns: List[str] = None): columns = columns if columns else ["*"] @@ -86,7 +86,7 @@ def __build_limit(self): else: return "" - def __build_order_By(self): + def __build_order_by(self): if self.order_by: attribute, order = self.order_by return f"ORDER BY c.{attribute} {order}" @@ -103,7 +103,7 @@ def build(self): """.format( select_conditions=self.__build_select(), where_conditions=self.__build_where(), - order_by_condition=self.__build_order_By(), + order_by_condition=self.__build_order_by(), offset_condition=self.__build_offset(), limit_condition=self.__build_limit(), ) From 2a683b1f6dc01a4a14afe521f465f7364f0988de Mon Sep 17 00:00:00 2001 From: kelly Date: Fri, 9 Apr 2021 15:04:29 -0500 Subject: [PATCH 10/10] refactor: TT-201 add type hinting and rename start_date_tmp --- time_tracker_api/time_entries/time_entries_dao.py | 2 +- time_tracker_api/time_entries/time_entries_repository.py | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_dao.py b/time_tracker_api/time_entries/time_entries_dao.py index b04cca0f..5be65759 100644 --- a/time_tracker_api/time_entries/time_entries_dao.py +++ b/time_tracker_api/time_entries/time_entries_dao.py @@ -228,7 +228,7 @@ def update(self, id, data: dict, description=None): time_entry = self.repository.find(id, event_ctx) self.check_whether_current_user_owns_item(time_entry) - if data.get('update_last_entry_if_overlap'): + if data.get('update_last_entry_if_overlap', None): self.repository.update_last_entry( data.get('owner_id'), data.get('start_date'), event_ctx ) diff --git a/time_tracker_api/time_entries/time_entries_repository.py b/time_tracker_api/time_entries/time_entries_repository.py index 91caf6a6..6e7af5c7 100644 --- a/time_tracker_api/time_entries/time_entries_repository.py +++ b/time_tracker_api/time_entries/time_entries_repository.py @@ -265,16 +265,15 @@ def get_last_entry( return function_mapper(next(result)) def update_last_entry( - self, owner_id, start_date, event_context: EventContext + self, owner_id: str, start_date: str, event_context: EventContext ): last_entry = self.get_last_entry(owner_id, event_context) - start_date_tmp = start_date end_date = str_to_datetime(last_entry.end_date) - start_date = str_to_datetime(start_date) + _start_date = str_to_datetime(start_date) - if start_date < end_date: - update_data = {'end_date': start_date_tmp} + if _start_date < end_date: + update_data = {'end_date': start_date} self.partial_update(last_entry.id, update_data, event_context) def on_create(self, new_item_data: dict, event_context: EventContext):