From 8ceb682b3ee3334d0c22fb37babf57a56c232ba2 Mon Sep 17 00:00:00 2001 From: Rene Enriquez Date: Mon, 20 Jul 2020 17:25:19 -0500 Subject: [PATCH 1/9] closes: #196 include customer information for time-entries --- .../time_entries/time_entries_model.py | 9 ++++++++- .../time_entries/time_entries_namespace.py | 14 ++++++++++++++ utils/extend_model.py | 2 ++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index ff04afa8..02f18828 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -17,13 +17,14 @@ from commons.data_access_layer.database import EventContext from time_tracker_api.activities import activities_model +from time_tracker_api.customers import customers_model from utils.extend_model import ( add_project_name_to_time_entries, add_activity_name_to_time_entries, create_in_condition, create_custom_query_from_str, - add_user_email_to_time_entries, + add_user_email_to_time_entries, add_customer_name_to_projects, ) from utils.time import ( datetime_str, @@ -219,10 +220,16 @@ def find_all( time_entries, "activity_id" ) + customer_dao = customers_model.create_dao() + customers = customer_dao.get_all(visible_only=False) + project_dao = projects_model.create_dao() projects = project_dao.get_all( custom_sql_conditions=[custom_conditions], visible_only=False ) + + add_customer_name_to_projects(projects, customers) + add_project_name_to_time_entries(time_entries, projects) activity_dao = activities_model.create_dao() diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index c29b2f2e..0eade8bb 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -138,6 +138,20 @@ description='Email of the user that owns the time-entry', example=faker.email(), ), + 'customer_id': fields.String( + required=False, + title='Customer ID', + max_length=50, + description='Unique ID for the customer the entry belongs to', + example=faker.uuid4(), + ), + 'customer_name': fields.String( + required=False, + title='Customer Name', + max_length=50, + description='Name of the customer the entry belongs to', + example=faker.word(['development', 'QA']), + ), } time_entry_response_fields.update(common_fields) diff --git a/utils/extend_model.py b/utils/extend_model.py index 54d25975..e7065a24 100644 --- a/utils/extend_model.py +++ b/utils/extend_model.py @@ -30,6 +30,8 @@ def add_project_name_to_time_entries(time_entries, projects): if time_entry.project_id == project.id: 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) def add_activity_name_to_time_entries(time_entries, activities): From 2fa0bafc1534a8d80c028468a3d2e0421e9c544e Mon Sep 17 00:00:00 2001 From: roberto Date: Mon, 20 Jul 2020 23:25:24 -0500 Subject: [PATCH 2/9] feat: :ok_hand: #196 --- time_tracker_api/time_entries/time_entries_model.py | 12 +++--------- .../time_entries/time_entries_namespace.py | 2 +- utils/extend_model.py | 4 ++-- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index 02f18828..aa66ff60 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -17,14 +17,13 @@ from commons.data_access_layer.database import EventContext from time_tracker_api.activities import activities_model -from time_tracker_api.customers import customers_model from utils.extend_model import ( - add_project_name_to_time_entries, + add_project_info_to_time_entries, add_activity_name_to_time_entries, create_in_condition, create_custom_query_from_str, - add_user_email_to_time_entries, add_customer_name_to_projects, + add_user_email_to_time_entries, ) from utils.time import ( datetime_str, @@ -220,17 +219,12 @@ def find_all( time_entries, "activity_id" ) - customer_dao = customers_model.create_dao() - customers = customer_dao.get_all(visible_only=False) - project_dao = projects_model.create_dao() projects = project_dao.get_all( custom_sql_conditions=[custom_conditions], visible_only=False ) - add_customer_name_to_projects(projects, customers) - - add_project_name_to_time_entries(time_entries, projects) + add_project_info_to_time_entries(time_entries, projects) activity_dao = activities_model.create_dao() activities = activity_dao.get_all( diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index 0eade8bb..84c48e6a 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -150,7 +150,7 @@ title='Customer Name', max_length=50, description='Name of the customer the entry belongs to', - example=faker.word(['development', 'QA']), + example=faker.company(), ), } time_entry_response_fields.update(common_fields) diff --git a/utils/extend_model.py b/utils/extend_model.py index e7065a24..6eecedcf 100644 --- a/utils/extend_model.py +++ b/utils/extend_model.py @@ -16,9 +16,9 @@ def add_customer_name_to_projects(projects, customers): setattr(project, 'customer_name', customer.name) -def add_project_name_to_time_entries(time_entries, projects): +def add_project_info_to_time_entries(time_entries, projects): """ - Add attribute project_name in time-entry model, based on project_id of the + Add project info in time-entry model, based on project_id of the time_entry :param (list) time_entries: time_entries retrieved from time-entry repository :param (list) projects: projects retrieved from project repository From b5857c63800418b2841dbed11322a86993c19bc4 Mon Sep 17 00:00:00 2001 From: semantic-release Date: Tue, 21 Jul 2020 13:40:41 +0000 Subject: [PATCH 3/9] 0.20.0 Automatically generated by python-semantic-release --- time_tracker_api/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time_tracker_api/version.py b/time_tracker_api/version.py index 482e4a19..2f15b8cd 100644 --- a/time_tracker_api/version.py +++ b/time_tracker_api/version.py @@ -1 +1 @@ -__version__ = '0.19.0' +__version__ = '0.20.0' From 5a851e71bf33d977f3b2b16bf7e3c84238bc1173 Mon Sep 17 00:00:00 2001 From: roberto Date: Wed, 15 Jul 2020 16:46:33 -0500 Subject: [PATCH 4/9] feat: :construction: make paginated endpoint with parameters length and start #192 --- .../time_entries/time_entries_model.py | 41 +++++++++++++++++++ .../time_entries/time_entries_namespace.py | 31 ++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index aa66ff60..fe6e397a 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -211,6 +211,7 @@ def find_all( custom_sql_conditions=custom_sql_conditions, custom_params=custom_params, max_count=kwargs.get("max_count", None), + offset=kwargs.get("offset", 0), ) if time_entries: @@ -430,6 +431,46 @@ def get_all(self, conditions: dict = None, **kwargs) -> list: max_count=limit, ) + def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: + event_ctx = self.create_event_context("read-many") + conditions.update({"owner_id": event_ctx.user_id}) + + # TODO: abstract this method with a function or a class method + custom_query = [] + if "user_id" in conditions: + if event_ctx.is_admin: + conditions.pop("owner_id") + custom_query = ( + [] + if conditions.get("user_id") == "*" + else [ + create_custom_query_from_str( + conditions.get("user_id"), "c.owner_id" + ) + ] + ) + conditions.pop("user_id") + else: + abort( + HTTPStatus.FORBIDDEN, "You don't have enough permissions." + ) + date_range = self.handle_date_filter_args(args=conditions) + + length = conditions.get("length", None) + conditions.pop("length", None) + + start = conditions.get("start", None) + conditions.pop("start", None) + + return self.repository.find_all( + event_ctx, + conditions=conditions, + custom_sql_conditions=custom_query, + date_range=date_range, + max_count=length, + offset=start, + ) + def get(self, id): event_ctx = self.create_event_context("read") diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index 84c48e6a..de4f618c 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -345,3 +345,34 @@ def get(self): """Find the summary of worked time""" conditions = summary_attribs_parser.parse_args() return time_entries_dao.get_worked_time(conditions) + + +paginated_attribs_parser = ns.parser() +paginated_attribs_parser.add_argument( + 'length', + required=True, + type=int, + help="(Filter) The number of rows the endpoint should return.", + location='args', +) + +paginated_attribs_parser.add_argument( + 'start', + required=True, + type=int, + help="(Filter) The number of rows to be removed from the query. (aka offset)", + location='args', +) + + +@ns.route('/paginated') +@ns.response(HTTPStatus.OK, 'Time Entries paginated') +@ns.response(HTTPStatus.NOT_FOUND, 'Time entry not found') +class PaginatedTimeEntry(Resource): + @ns.expect(paginated_attribs_parser) + @ns.doc('list_time_entries_paginated') + @ns.marshal_list_with(time_entry) + def get(self): + """List all time entries paginated""" + conditions = paginated_attribs_parser.parse_args() + return time_entries_dao.get_all_paginated(conditions) From 1d3f63aa2567ed77aa75a0caf73e0abd0c2f8f41 Mon Sep 17 00:00:00 2001 From: roberto Date: Wed, 15 Jul 2020 17:12:36 -0500 Subject: [PATCH 5/9] feat: :construction: add model to marshall response #192 --- .../time_entries/time_entries_model.py | 8 +++++++- .../time_entries/time_entries_namespace.py | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index fe6e397a..12d84e6a 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -462,7 +462,7 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: start = conditions.get("start", None) conditions.pop("start", None) - return self.repository.find_all( + time_entries = self.repository.find_all( event_ctx, conditions=conditions, custom_sql_conditions=custom_query, @@ -471,6 +471,12 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: offset=start, ) + return { + 'records_total': 0, + 'records_filtered': 0, + 'data': time_entries, + } + def get(self, id): event_ctx = self.create_event_context("read") diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index de4f618c..b4c7ddc3 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -347,6 +347,20 @@ def get(self): return time_entries_dao.get_worked_time(conditions) +time_entry_paginated = ns.model( + 'TimeEntryPaginated', + { + 'records_total': fields.Integer( + title='Records total', description='Total number of entries.', + ), + 'records_filtered': fields.Integer( + title='Records filtered', + description='Number of entries returned by the endpoint.', + ), + 'data': fields.List(fields.Nested(time_entry)), + }, +) + paginated_attribs_parser = ns.parser() paginated_attribs_parser.add_argument( 'length', @@ -371,7 +385,7 @@ def get(self): class PaginatedTimeEntry(Resource): @ns.expect(paginated_attribs_parser) @ns.doc('list_time_entries_paginated') - @ns.marshal_list_with(time_entry) + @ns.marshal_list_with(time_entry_paginated) def get(self): """List all time entries paginated""" conditions = paginated_attribs_parser.parse_args() From 9e4e98df4ebc8972406bbc9bc4fee99e1c250822 Mon Sep 17 00:00:00 2001 From: roberto Date: Wed, 15 Jul 2020 23:27:31 -0500 Subject: [PATCH 6/9] feat: :construction: add records_filtered #192 --- time_tracker_api/time_entries/time_entries_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index 12d84e6a..fefc98f3 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -473,7 +473,7 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: return { 'records_total': 0, - 'records_filtered': 0, + 'records_filtered': len(time_entries), 'data': time_entries, } From 676b933d9f585c06f8757cfbcc26d69921c16aa0 Mon Sep 17 00:00:00 2001 From: roberto Date: Fri, 17 Jul 2020 11:58:58 -0500 Subject: [PATCH 7/9] feat: propagate max_count #192 --- time_tracker_api/projects/projects_model.py | 4 +++- time_tracker_api/time_entries/time_entries_model.py | 8 +++++--- time_tracker_api/time_entries/time_entries_namespace.py | 4 ---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/time_tracker_api/projects/projects_model.py b/time_tracker_api/projects/projects_model.py index 62cba129..ff40c7c8 100644 --- a/time_tracker_api/projects/projects_model.py +++ b/time_tracker_api/projects/projects_model.py @@ -77,7 +77,9 @@ def get_all(self, conditions: dict = None, **kwargs) -> list: """ event_ctx = self.create_event_context("read-many") customer_dao = customers_create_dao() - customers = customer_dao.get_all() + customers = customer_dao.get_all( + max_count=kwargs.get('max_count', None) + ) customers_id = [customer.id for customer in customers] conditions = conditions if conditions else {} diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index fefc98f3..9f3f2a41 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -222,7 +222,9 @@ def find_all( project_dao = projects_model.create_dao() projects = project_dao.get_all( - custom_sql_conditions=[custom_conditions], visible_only=False + custom_sql_conditions=[custom_conditions], + visible_only=False, + max_count=kwargs.get("max_count", None), ) add_project_info_to_time_entries(time_entries, projects) @@ -231,6 +233,7 @@ def find_all( activities = activity_dao.get_all( custom_sql_conditions=[custom_conditions_activity], visible_only=False, + max_count=kwargs.get("max_count", None), ) add_activity_name_to_time_entries(time_entries, activities) @@ -472,8 +475,7 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: ) return { - 'records_total': 0, - 'records_filtered': len(time_entries), + 'records_total': len(time_entries), 'data': time_entries, } diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index b4c7ddc3..bbd58c1c 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -353,10 +353,6 @@ def get(self): 'records_total': fields.Integer( title='Records total', description='Total number of entries.', ), - 'records_filtered': fields.Integer( - title='Records filtered', - description='Number of entries returned by the endpoint.', - ), 'data': fields.List(fields.Nested(time_entry)), }, ) From 31a198e120a72dfb87ef04374b273bb4e6293aa6 Mon Sep 17 00:00:00 2001 From: roberto Date: Fri, 17 Jul 2020 13:41:31 -0500 Subject: [PATCH 8/9] fix(time-entries): move custom_query to a function #192 --- .../time_entries/time_entries_model.py | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index 9f3f2a41..7b76b2ab 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -402,12 +402,10 @@ def stop_time_entry_if_was_left_running( ) raise CosmosResourceNotFoundError() - 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}) + def build_custom_query(self, is_admin: bool, conditions: dict = None): custom_query = [] if "user_id" in conditions: - if event_ctx.is_admin: + if is_admin: conditions.pop("owner_id") custom_query = ( [] @@ -423,6 +421,16 @@ def get_all(self, conditions: dict = None, **kwargs) -> list: abort( HTTPStatus.FORBIDDEN, "You don't have enough permissions." ) + return custom_query + + 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}) + + custom_query = self.build_custom_query( + is_admin=event_ctx.is_admin, conditions=conditions, + ) + date_range = self.handle_date_filter_args(args=conditions) limit = conditions.get("limit", None) conditions.pop("limit", None) @@ -438,25 +446,10 @@ def get_all_paginated(self, conditions: dict = None, **kwargs) -> list: event_ctx = self.create_event_context("read-many") conditions.update({"owner_id": event_ctx.user_id}) - # TODO: abstract this method with a function or a class method - custom_query = [] - if "user_id" in conditions: - if event_ctx.is_admin: - conditions.pop("owner_id") - custom_query = ( - [] - if conditions.get("user_id") == "*" - else [ - create_custom_query_from_str( - conditions.get("user_id"), "c.owner_id" - ) - ] - ) - conditions.pop("user_id") - else: - abort( - HTTPStatus.FORBIDDEN, "You don't have enough permissions." - ) + custom_query = self.build_custom_query( + is_admin=event_ctx.is_admin, conditions=conditions, + ) + date_range = self.handle_date_filter_args(args=conditions) length = conditions.get("length", None) From ce0130332f46ad6fc95df6a6e13e2159776d35f4 Mon Sep 17 00:00:00 2001 From: roberto Date: Tue, 21 Jul 2020 11:50:13 -0500 Subject: [PATCH 9/9] test: :white_check_mark: add unit tests #192 --- tests/conftest.py | 9 ++++ .../time_entries_namespace_test.py | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 1f8afe71..5cb5c18d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -181,6 +181,15 @@ def time_entry_repository(app: Flask) -> TimeEntryCosmosDBRepository: return TimeEntryCosmosDBRepository() +@pytest.fixture +def time_entries_dao(): + from time_tracker_api.time_entries.time_entries_namespace import ( + time_entries_dao, + ) + + return time_entries_dao + + @pytest.yield_fixture(scope="module") def running_time_entry( time_entry_repository: TimeEntryCosmosDBRepository, 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 214f130b..cb1417dd 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 @@ -639,3 +639,45 @@ def test_summary_is_called_with_date_range_from_worked_time_module( repository_find_all_mock.assert_called_once_with( ANY, conditions=conditions, date_range=date_range ) + + +def test_paginated_fails_with_no_params( + client: FlaskClient, valid_header: dict, +): + response = client.get('/time-entries/paginated', headers=valid_header) + assert HTTPStatus.BAD_REQUEST == response.status_code + + +def test_paginated_succeeds_with_valid_params( + client: FlaskClient, valid_header: dict, +): + response = client.get( + '/time-entries/paginated?start=10&length=10', headers=valid_header + ) + assert HTTPStatus.OK == response.status_code + + +def test_paginated_response_contains_expected_props( + client: FlaskClient, valid_header: dict, +): + response = client.get( + '/time-entries/paginated?start=10&length=10', headers=valid_header + ) + assert 'data' in json.loads(response.data) + assert 'records_total' in json.loads(response.data) + + +def test_paginated_sends_max_count_and_offset_on_call_to_repository( + client: FlaskClient, valid_header: dict, time_entries_dao +): + time_entries_dao.repository.find_all = Mock(return_value=[]) + + response = client.get( + '/time-entries/paginated?start=10&length=10', headers=valid_header + ) + + time_entries_dao.repository.find_all.assert_called_once() + + args, 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