From 4c0a6035b0edcff3f1a1d7cbff76bb7f185565b3 Mon Sep 17 00:00:00 2001 From: Nancy Date: Mon, 21 Jun 2021 15:44:04 -0400 Subject: [PATCH 01/33] Add queryProgram --- api/queries.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/api/queries.py b/api/queries.py index 8b94cdde..685812ba 100644 --- a/api/queries.py +++ b/api/queries.py @@ -1,6 +1,6 @@ from ariadne import convert_kwargs_to_snake_case, ObjectType from sqlalchemy.sql.expression import func -from database import Dataset, User, Record, Category, CategoryValue, Entry, Team, Role +from database import Dataset, User, Record, Category, CategoryValue, Program, Entry, Team, Role from sqlalchemy.orm.exc import NoResultFound query = ObjectType("Query") @@ -41,6 +41,18 @@ def resolve_users(obj, info): session = info.context['dbsession'] return session.query(User).order_by(User.email.asc()).all() +@query.field("program") +@convert_kwargs_to_snake_case +def resolve_program(obj, info, id): + '''GraphQL query to find a Program based on Program ID. + :param id: Id for the Program to be fetched + :returns: Program dictionary + ''' + session = info.context['dbsession'] + program = session.query(Program).get(id) + + return program + @query.field("dataset") @convert_kwargs_to_snake_case def resolve_dataset(obj, info, id): From 2549f439cc53e216f9d2446eb3aa4eff24051df1 Mon Sep 17 00:00:00 2001 From: Nancy Date: Mon, 21 Jun 2021 17:02:53 -0400 Subject: [PATCH 02/33] Add createProgram + update gql schema --- api/mutations.py | 22 ++++++++++++++++++++++ api/schema.graphql | 12 +++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/api/mutations.py b/api/mutations.py index efe4408b..2f59fcbd 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -293,3 +293,25 @@ def resolve_delete_team(obj, info, id): session.commit() return id +@mutation.field("createProgram") +@convert_kwargs_to_snake_case +def resolve_create_program(obj, info, input): + '''GraphQL mutation to create a Program. + :param input: params for new Program + :returns: Program dictionary + ''' + + session = info.context['dbsession'] + datasets = input.pop('dataset_ids') + targets = input.pop('target_ids') + tags = input.pop('tag_ids') + + program = Program(**input) + program.datasets += [session.merge(Dataset(id=dataset_id)) for dataset_id in datasets] + program.targets += [session.merge(Target(id=target_id)) for target_id in targets] + program.tags += [session.merge(Tag(id=tag_id)) for tag_id in tags] + + session.add(program) + session.commit() + + return program diff --git a/api/schema.graphql b/api/schema.graphql index 3dd3df12..6979eb29 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -72,7 +72,7 @@ type Program @needsPermission(permission: [ADMIN, TEAM_MEMBER]) { team: Team! datasets: [Dataset!]! targets: [Target!]! - tags: [Tag!]! + tags: [Tag!] } type Tag { @@ -219,6 +219,13 @@ input UpdateTeamInput { userIds: [ID!] programIds: [ID!] organizationId: ID +input CreateProgramInput { + name: String! + description: String! + teamId: ID! + datasetIds: [ID!]! + targetIds: [ID!]! + tagIds: [ID!] } input EntryInput { @@ -323,4 +330,7 @@ type Mutation { # Delete a Team deleteTeam(id: ID!): ID! @needsPermission(permission: [ADMIN]) + # Create a Program + createProgram(input: CreateProgramInput!): Program! + @needsPermission(permission: [ADMIN, TEAM_MEMBER]) } From 77a552960db8e219f8c50598f77492f9bc10907f Mon Sep 17 00:00:00 2001 From: Nancy Date: Mon, 21 Jun 2021 18:26:14 -0400 Subject: [PATCH 03/33] [WIP] updateProgram --- api/mutations.py | 26 ++++++++++++++++++++++++++ api/schema.graphql | 14 ++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/api/mutations.py b/api/mutations.py index 2f59fcbd..afab178f 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -315,3 +315,29 @@ def resolve_create_program(obj, info, input): session.commit() return program + +@mutation.field("updateProgram") +@convert_kwargs_to_snake_case +def resolve_update_program(obj, info, input): + '''GraphQL mutation to update a Program. + :param input: params for updated Program + :returns: Updated Program dictionary + ''' + + session = info.context['dbsession'] + datasets = input.pop('dataset_ids', []) + targets = input.pop('target_ids', []) + tags = input.pop('tag_ids', []) + program = session.query(Program).get(input['id']) + if len(datasets) > 0: + program.datasets = [session.merge(Dataset(id=dataset_id)) for dataset_id in datasets] + if len(targets) > 0: + program.targets = [session.merge(Target(id=target_id)) for target_id in targets] + if len(tags) > 0: + program.tags = [session.merge(Tag(id=tag_id)) for tag_id in tags] + for param in input: + setattr(program, param, input[param]) + session.add(program) + session.commit() + + return program \ No newline at end of file diff --git a/api/schema.graphql b/api/schema.graphql index 6979eb29..961c8c80 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -228,6 +228,16 @@ input CreateProgramInput { tagIds: [ID!] } +input UpdateProgramInput { + id: ID! + name: String + description: String + teamId: ID + datasetIds: [ID!] + targetIds: [ID!] + tagIds: [ID!] +} + input EntryInput { id: ID categoryValueId: ID! @@ -333,4 +343,8 @@ type Mutation { # Create a Program createProgram(input: CreateProgramInput!): Program! @needsPermission(permission: [ADMIN, TEAM_MEMBER]) + + # Update a Program + updateProgram(input: UpdateProgramInput!): Program! + @needsPermission(permission: [ADMIN, TEAM_MEMBER]) } From d6e6d6c26f8f04984579718dd58e24bd40240235 Mon Sep 17 00:00:00 2001 From: Nancy Date: Mon, 21 Jun 2021 19:55:56 -0400 Subject: [PATCH 04/33] Add deleteProgram, update gql schema and programQuery --- api/database.py | 19 ++++++++++++++++++- api/mutations.py | 14 +++++++++++++- api/queries.py | 3 +-- api/schema.graphql | 3 +++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/api/database.py b/api/database.py index e0566219..e2a7c30f 100644 --- a/api/database.py +++ b/api/database.py @@ -193,6 +193,13 @@ class Program(Base, PermissionsMixin): def user_is_team_member(self, user): return self.team.user_is_team_member(user) + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) + datasets = session.query(Dataset).filter(Dataset.program_id == self.id).all() + for dataset in datasets: + dataset.soft_delete(session) + session.query(Target).filter(Target.program_id == self.id).update({'deleted':func.now()}, synchronize_session='fetch') class Tag(Base): __tablename__ = 'tag' @@ -331,6 +338,12 @@ def get_not_deleted(cls, session, id_): def user_is_team_member(self, user): return self.program.user_is_team_member(user) + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) + records = session.query(Record).filter(Record.dataset_id == self.id).all() + for record in records: + record.soft_delete(session) class Record(Base, PermissionsMixin): __tablename__ = 'record' @@ -357,7 +370,11 @@ def get_not_deleted(cls, session, id_): def user_is_team_member(self, user): return self.dataset.user_is_team_member(user) - + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) + session.query(Entry).filter(Entry.record_id == self.id).update({'deleted':func.now()}, synchronize_session='fetch') + class Entry(Base, PermissionsMixin): __tablename__ = 'entry' diff --git a/api/mutations.py b/api/mutations.py index afab178f..0c9cefe2 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -340,4 +340,16 @@ def resolve_update_program(obj, info, input): session.add(program) session.commit() - return program \ No newline at end of file + return program + +@mutation.field("deleteProgram") +def resolve_delete_program(obj, info, id): + '''GraphQL mutation to delete a Program. + :param id: UUID of Program to be deleted + :returns: UUID of deleted Program + ''' + session = info.context['dbsession'] + session.query(Program).get(id).soft_delete(session) + session.commit() + + return id \ No newline at end of file diff --git a/api/queries.py b/api/queries.py index 685812ba..9ff1fd1e 100644 --- a/api/queries.py +++ b/api/queries.py @@ -49,8 +49,7 @@ def resolve_program(obj, info, id): :returns: Program dictionary ''' session = info.context['dbsession'] - program = session.query(Program).get(id) - + program = session.query(Program).filter(Program.id == dataset.id, Program.deleted == None).scalar() return program @query.field("dataset") diff --git a/api/schema.graphql b/api/schema.graphql index 961c8c80..654d9d7a 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -347,4 +347,7 @@ type Mutation { # Update a Program updateProgram(input: UpdateProgramInput!): Program! @needsPermission(permission: [ADMIN, TEAM_MEMBER]) + + # Delete a Program + deleteProgram(id: ID!): ID! @needsPermission(permission: [ADMIN, TEAM_MEMBER]) } From b76a90fadeaa9d53764e034178ea0245c62ab32b Mon Sep 17 00:00:00 2001 From: Nancy Date: Mon, 21 Jun 2021 20:06:52 -0400 Subject: [PATCH 05/33] [WIP] Update resolver soft-deletes to use instance methods --- api/mutations.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/api/mutations.py b/api/mutations.py index 0c9cefe2..2bdbc501 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -44,11 +44,13 @@ def resolve_delete_dataset(obj, info, id): :returns: UUID of soft deleted Dataset ''' session = info.context['dbsession'] - session.query(Dataset).filter(Dataset.id == id).update({'deleted':func.now()}, synchronize_session='fetch') - session.query(Record).filter(Record.dataset_id == id).update({'deleted':func.now()}, synchronize_session='fetch') - related_records = session.query(Record).filter(Record.dataset_id == id).all() - for record in related_records: - session.query(Entry).filter(Entry.record_id == record.id).update({'deleted':func.now()}, synchronize_session='fetch') + session.query(Dataset).get(id).soft_delete(session) + + # session.query(Dataset).filter(Dataset.id == id).update({'deleted':func.now()}, synchronize_session='fetch') + # session.query(Record).filter(Record.dataset_id == id).update({'deleted':func.now()}, synchronize_session='fetch') + # related_records = session.query(Record).filter(Record.dataset_id == id).all() + # for record in related_records: + # session.query(Entry).filter(Entry.record_id == record.id).update({'deleted':func.now()}, synchronize_session='fetch') session.commit() return id @@ -135,7 +137,8 @@ def resolve_delete_record(obj, info, id): :returns: UUID of soft deleted Record ''' session = info.context['dbsession'] - session.query(Record).filter(Record.id == id).delete() + session.query(Record).get(id).soft_delete(session) + # session.query(Record).filter(Record.id == id).delete() session.commit() return id From 7094dd40981e675fd8c84a3b1f51f71a36f21222 Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 22 Jun 2021 12:11:34 -0400 Subject: [PATCH 06/33] Cleanup deleteDataset mutation --- api/database.py | 2 +- api/mutations.py | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/api/database.py b/api/database.py index e2a7c30f..4a4028e5 100644 --- a/api/database.py +++ b/api/database.py @@ -333,7 +333,7 @@ class Dataset(Base, PermissionsMixin): @classmethod def get_not_deleted(cls, session, id_): - return session.query(Dataset).filter(Dataset.id == id_, Dataset.deleted == None).first() + return session.query(Dataset).filter(Dataset.id == id_, Dataset.deleted == None).scalar() def user_is_team_member(self, user): return self.program.user_is_team_member(user) diff --git a/api/mutations.py b/api/mutations.py index 2bdbc501..c4a7393c 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -44,13 +44,7 @@ def resolve_delete_dataset(obj, info, id): :returns: UUID of soft deleted Dataset ''' session = info.context['dbsession'] - session.query(Dataset).get(id).soft_delete(session) - - # session.query(Dataset).filter(Dataset.id == id).update({'deleted':func.now()}, synchronize_session='fetch') - # session.query(Record).filter(Record.dataset_id == id).update({'deleted':func.now()}, synchronize_session='fetch') - # related_records = session.query(Record).filter(Record.dataset_id == id).all() - # for record in related_records: - # session.query(Entry).filter(Entry.record_id == record.id).update({'deleted':func.now()}, synchronize_session='fetch') + Dataset.get_not_deleted(session, id).soft_delete(session) session.commit() return id From 78519d0c187895243c405224c50b6612c65375ab Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 22 Jun 2021 12:20:57 -0400 Subject: [PATCH 07/33] Clean up deleteRecord mutation --- api/database.py | 2 +- api/mutations.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/database.py b/api/database.py index 4a4028e5..2bb774aa 100644 --- a/api/database.py +++ b/api/database.py @@ -365,7 +365,7 @@ class Record(Base, PermissionsMixin): @classmethod def get_not_deleted(cls, session, id_): - return session.query(Record).filter(Record.id == id_, Record.deleted == None).first() + return session.query(Record).filter(Record.id == id_, Record.deleted == None).scalar() def user_is_team_member(self, user): return self.dataset.user_is_team_member(user) diff --git a/api/mutations.py b/api/mutations.py index c4a7393c..8cadb79b 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -131,8 +131,7 @@ def resolve_delete_record(obj, info, id): :returns: UUID of soft deleted Record ''' session = info.context['dbsession'] - session.query(Record).get(id).soft_delete(session) - # session.query(Record).filter(Record.id == id).delete() + Record.get_not_deleted(session, id).soft_delete(session) session.commit() return id From e5c9ba198d00355d0d16f5767e4fd272ea5da5ee Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 22 Jun 2021 12:48:40 -0400 Subject: [PATCH 08/33] Making tagids required --- api/schema.graphql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/schema.graphql b/api/schema.graphql index 654d9d7a..df7f9bb2 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -225,7 +225,7 @@ input CreateProgramInput { teamId: ID! datasetIds: [ID!]! targetIds: [ID!]! - tagIds: [ID!] + tagIds: [ID!]! } input UpdateProgramInput { From e98a26195e26c4a73a79661f6cfe5a076b735259 Mon Sep 17 00:00:00 2001 From: Nancy Date: Fri, 25 Jun 2021 17:18:16 -0400 Subject: [PATCH 09/33] Update programQuery to use class method --- api/queries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries.py b/api/queries.py index 9ff1fd1e..a1b71e70 100644 --- a/api/queries.py +++ b/api/queries.py @@ -49,7 +49,7 @@ def resolve_program(obj, info, id): :returns: Program dictionary ''' session = info.context['dbsession'] - program = session.query(Program).filter(Program.id == dataset.id, Program.deleted == None).scalar() + program = Program.get_not_deleted(session, id) return program @query.field("dataset") From 4b2bbd223de3f86916ce8b4d7373f87183a8d8ef Mon Sep 17 00:00:00 2001 From: Nancy Date: Fri, 25 Jun 2021 17:21:25 -0400 Subject: [PATCH 10/33] Update deleteRecord tests to use class method --- api/app_test.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 8c4ef342..92f15ce1 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -1351,9 +1351,9 @@ def test_delete_record(self): user = self.test_users[user_role] # Confirm Record exists, then that it does not. record_id = "742b5971-eeb6-4f7a-8275-6111f2342bb4" - existing_record = self.session.query(Record).filter(Record.id == record_id) - # Count of non-deleted entries should be zero - self.assertEqual(existing_record.count(), 1) + existing_record = Record.get_not_deleted(self.session, record_id) + # Count of non-deleted records should not be None + self.assertNotEqual(existing_record, None) success, result = self.run_graphql_query({ "operationName": "DeleteRecord", "query": """ @@ -1367,12 +1367,12 @@ def test_delete_record(self): }, user=user) self.assertTrue(success) - # Query for Record - record = self.session.query(Record).filter(Record.id == record_id) - # Record count should be zero - self.assertEqual(record.count(), 0) + # Query for Records that were not deleted + record = Record.get_not_deleted(self.session, record_id) + # Record should be None + self.assertEqual(record, None) # Query for all associated Entries - associated_entries = self.session.query(Entry).filter(Entry.record_id == record_id) + associated_entries = self.session.query(Entry).filter(Entry.record_id == record_id, Entry.deleted == None) # Entries count should be zero self.assertEqual(associated_entries.count(), 0) self.assertTrue(self.is_valid_uuid(record_id), "Invalid UUID") @@ -1389,9 +1389,9 @@ def test_delete_record_no_perm(self): user = self.test_users['other'] # Confirm Record exists, then that it does not. record_id = "742b5971-eeb6-4f7a-8275-6111f2342bb4" - existing_record = self.session.query(Record).filter(Record.id == record_id) - # Count of non-deleted entries should be zero - self.assertEqual(existing_record.count(), 1) + existing_record = Record.get_not_deleted(self.session, record_id) + # Count of non-deleted records should not be None + self.assertNotEqual(existing_record, None) success, result = self.run_graphql_query({ "operationName": "DeleteRecord", "query": """ @@ -1407,9 +1407,9 @@ def test_delete_record_no_perm(self): self.assertTrue(success) self.assertResultWasNotAuthed(result) # Query for Record - record = self.session.query(Record).filter(Record.id == record_id) - # Record count should still be one - self.assertEqual(record.count(), 1) + existing_record = Record.get_not_deleted(self.session, record_id) + # Count of non-deleted records should still not be None + self.assertNotEqual(existing_record, None) def test_query_category(self): """Test that anyone can query a category.""" From 9910a0a8243be30c79a1d9eae774c66cb885ba5a Mon Sep 17 00:00:00 2001 From: Nancy Date: Fri, 25 Jun 2021 17:21:42 -0400 Subject: [PATCH 11/33] Add class and instance methods --- api/database.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/api/database.py b/api/database.py index 2bb774aa..30c506ef 100644 --- a/api/database.py +++ b/api/database.py @@ -190,6 +190,10 @@ class Program(Base, PermissionsMixin): server_default=func.now(), onupdate=func.now()) deleted = Column(TIMESTAMP) + @classmethod + def get_not_deleted(cls, session, id_): + return session.query(Program).filter(Program.id == id_, Program.deleted == None).scalar() + def user_is_team_member(self, user): return self.team.user_is_team_member(user) @@ -312,6 +316,17 @@ def get_not_deleted(cls, session, id): return session.query(CategoryValue).\ filter(CategoryValue.id == id, CategoryValue.deleted == None).first() + @classmethod + def get_not_deleted(cls, session, id_): + return session.query(CategoryValue).filter(CategoryValue.id == id_, CategoryValue.deleted == None).scalar() + + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) + entries = session.query(Entry).filter(Entry.category_value_id == self.id).all() + for entry in entries: + entry.soft_delete(session) + session.query(Target).filter(Target.program_id == self.id).update({'deleted':func.now()}, synchronize_session='fetch') class Dataset(Base, PermissionsMixin): __tablename__ = 'dataset' @@ -373,7 +388,9 @@ def user_is_team_member(self, user): def soft_delete(self, session): self.deleted= func.now() session.add(self) - session.query(Entry).filter(Entry.record_id == self.id).update({'deleted':func.now()}, synchronize_session='fetch') + entries = session.query(Entry).filter(Entry.record_id == self.id).all() + for entry in entries: + entry.soft_delete(session) class Entry(Base, PermissionsMixin): __tablename__ = 'entry' @@ -393,9 +410,16 @@ class Entry(Base, PermissionsMixin): server_default=func.now(), onupdate=func.now()) deleted = Column(TIMESTAMP) + @classmethod + def get_not_deleted(cls, session, id_): + return session.query(Entry).filter(Entry.id == id_, Entry.deleted == None).scalar() + def user_is_team_member(self, user): return self.record.user_is_team_member(user) + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) def create_tables(engine, session): print("🍽 Creating tables ...") From 2415811f6f4835269292c3769291785311346f27 Mon Sep 17 00:00:00 2001 From: Nancy Date: Fri, 25 Jun 2021 17:22:41 -0400 Subject: [PATCH 12/33] Update program and categoryVal to use class and instance methods --- api/mutations.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/mutations.py b/api/mutations.py index 8cadb79b..67637741 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -225,9 +225,7 @@ def resolve_delete_category_value(obj, info, id): :returns: UUID of deleted CategoryValue ''' session = info.context['dbsession'] - session.query(CategoryValue).filter(CategoryValue.id == id).update({'deleted':func.now()}, synchronize_session='fetch') - session.query(Entry).filter(Entry.category_value_id == id).update({'deleted':func.now()}, synchronize_session='fetch') - session.query(Target).filter(Target.category_value_id == id).update({'deleted':func.now()}, synchronize_session='fetch') + CategoryValue.get_not_deleted(session, id).soft_delete(session) session.commit() return id @@ -345,7 +343,7 @@ def resolve_delete_program(obj, info, id): :returns: UUID of deleted Program ''' session = info.context['dbsession'] - session.query(Program).get(id).soft_delete(session) + Program.get_not_deleted(session, id).soft_delete(session) session.commit() return id \ No newline at end of file From 8ac6359f567e94c68e8de1ec13b9f77e64160b77 Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 29 Jun 2021 10:39:38 -0500 Subject: [PATCH 13/33] Update categoryValueQuery to handle soft-deletes --- api/queries.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/queries.py b/api/queries.py index a1b71e70..b2055243 100644 --- a/api/queries.py +++ b/api/queries.py @@ -145,10 +145,9 @@ def resolve_category_value(obj, info, id): :returns: CategoryValue dictionary OR None if CategoryValue was deleted ''' session = info.context['dbsession'] - category_value = session.query(CategoryValue).get(id) + category_value = session.query(CategoryValue).filter(CategoryValue.id == id, CategoryValue.deleted == None).first() return category_value - @query.field("team") def resolve_team(obj, info, id): '''GraphQL query to find a Team based on Team ID. From 736a6c5156006cc539921d3dedf1eee92e5c769d Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 29 Jun 2021 17:19:08 -0500 Subject: [PATCH 14/33] Add createProgram test + seed data mod --- api/app_test.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++ api/schema.graphql | 2 ++ 2 files changed, 51 insertions(+) diff --git a/api/app_test.py b/api/app_test.py index 92f15ce1..75f0f269 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2177,7 +2177,56 @@ def test_get_roles(self): else: self.assertResultWasNotAuthed(result) + def test_create_program(self): + success, result = self.run_graphql_query({ + "operationName": "CreateProgram", + "query": """ + mutation CreateProgram($input: CreateProgramInput!) { + createProgram(input: $input) { + id + name + description + team { + name + } + datasets { + name + } + targets { + target + } + } + } + """, + "variables": { + "input": { + "name": "A New Program!", + "description": "A very new program", + "teamId": "472d17da-ff8b-4743-823f-3f01ea21a349", + "datasetIds": ["b3e7d42d-2bb7-4e25-a4e1-b8d30f3f6e89"], + "targetIds": ["b5be10ce-103f-41f2-b4c4-603228724993", "6e6edce5-3d24-4296-b929-5eec26d52afc"], + "tagIds": ["4a2142c0-5416-431d-b62f-0dbfe7574688"] + } + }, + }, user=self.test_users['admin']) + self.assertTrue(success) + print(f'{result}, result here') + self.assertTrue(self.is_valid_uuid(result["data"]["createProgram"]["id"]), "Invalid UUID") + self.assertEqual(result, { + "data": { + "createProgram": { + "id": result["data"]["createProgram"]["id"], + "name": "A New Program!", + "description": "A very new program", + "team": { + "name": "News Team" + }, + "datasets": [{"name": "Breakfast Hour"}], + "targets": [{"target": 0.5}, {"target": 0.5}], + }, + }, + }) if __name__ == '__main__': unittest.main() diff --git a/api/schema.graphql b/api/schema.graphql index df7f9bb2..a6451d50 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -219,6 +219,8 @@ input UpdateTeamInput { userIds: [ID!] programIds: [ID!] organizationId: ID +} + input CreateProgramInput { name: String! description: String! From 1ceec3a0661f9c4726ac52e567e4b676f63db64e Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 29 Jun 2021 17:23:27 -0500 Subject: [PATCH 15/33] Add createProg no perm test --- api/app_test.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/api/app_test.py b/api/app_test.py index 75f0f269..acb653e3 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2227,6 +2227,45 @@ def test_create_program(self): }, }, }) + + def test_create_program_no_perm(self): + """Test that program creation fails for normal (non-admin) users.""" + for user_role in ['other', 'normal']: + user = self.test_users[user_role] + success, result = self.run_graphql_query({ + "operationName": "CreateProgram", + "query": """ + mutation CreateProgram($input: CreateProgramInput!) { + createProgram(input: $input) { + id + name + description + team { + name + } + datasets { + name + } + targets { + target + } + } + } + """, + "variables": { + "input": { + "name": "An (unsuccessfully) updated program!", + "description": "A very (unsuccessfully) updated program", + "teamId": "2c4cfe21-42b1-4eec-b970-5409449d53a5", + "datasetIds": ["b3e7d42d-2bb7-4e25-a4e1-b8d30f3f6e89"], + "targetIds": ["b5be10ce-103f-41f2-b4c4-603228724993", "6e6edce5-3d24-4296-b929-5eec26d52afc"], + "tagIds": ["4a2142c0-5416-431d-b62f-0dbfe7574688"] + }, + }, + }, user=user) + + self.assertTrue(success) + self.assertResultWasNotAuthed(result) if __name__ == '__main__': unittest.main() From a168ccfa0d944530ad39a615c9238bebc9991e36 Mon Sep 17 00:00:00 2001 From: Nancy Date: Tue, 29 Jun 2021 17:41:44 -0500 Subject: [PATCH 16/33] Add deleteProgram test --- api/app_test.py | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/api/app_test.py b/api/app_test.py index acb653e3..2276c55d 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2267,5 +2267,59 @@ def test_create_program_no_perm(self): self.assertTrue(success) self.assertResultWasNotAuthed(result) + def test_delete_program(self): + """Only admins can delete programs.""" + user = self.test_users["admin"] + program_id = "1e73e788-0808-4ee8-9b25-682b6fa3868b" + # Confirm Program exists, then that it does not. + existing_program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) + # Count of existing Program should be one + self.assertEqual(existing_program.count(), 1) + success, result = self.run_graphql_query({ + "operationName": "DeleteProgram", + "query": """ + mutation DeleteProgram($id: ID!) { + deleteProgram(id: $id) + } + """, + "variables": { + "id": program_id, + }, + }, user=user) + self.assertTrue(success) + program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) + self.assertEqual(program.count(), 0) + self.assertTrue(self.is_valid_uuid(program_id), "Invalid UUID") + self.assertEqual(result, { + "data": { + "deleteProgram": program_id + }, + }) + + def test_delete_program_no_perm(self): + """Only admins can delete Programs.""" + for user in ["normal", "other"]: + user = self.test_users[user] + program_id = "1e73e788-0808-4ee8-9b25-682b6fa3868b" + # Confirm Program exists, then that it does not. + existing_program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) + # Count of existing Program should be one + self.assertEqual(existing_program.count(), 1) + success, result = self.run_graphql_query({ + "operationName": "DeleteProgram", + "query": """ + mutation DeleteProgram($id: ID!) { + deleteProgram(id: $id) + } + """, + "variables": { + "id": program_id, + }, + }, user=user) + self.assertTrue(success) + self.assertResultWasNotAuthed(result) + program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) + self.assertEqual(program.count(), 1) + if __name__ == '__main__': unittest.main() From a3054c974f0ab4db51ce5438023c61e7974ca260 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 11:46:07 -0500 Subject: [PATCH 17/33] Update deleteProgram and Dataset tests to use class method Add class method to Category and update tests/cleanup --- api/app_test.py | 45 ++++++++++++++++++++++++--------------------- api/database.py | 26 +++++++++++++++----------- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 2276c55d..5ff4d310 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -958,8 +958,8 @@ def test_update_dataset_no_perm(self): def test_delete_dataset(self): # Confirm Dataset exists, then that it does not. dataset_id = "b3e7d42d-2bb7-4e25-a4e1-b8d30f3f6e89" - existing_dataset= self.session.query(Dataset).filter(Dataset.id == dataset_id, Dataset.deleted == None) - self.assertEqual(existing_dataset.count(), 1) + existing_dataset= Dataset.get_not_deleted(self.session, dataset_id) + self.assertNotEqual(existing_dataset, None) success, result = self.run_graphql_query({ "operationName": "DeleteDataset", "query": """ @@ -998,8 +998,8 @@ def test_delete_dataset_no_perm(self): user = self.test_users[user_role] # Confirm Dataset exists, then that it does not. dataset_id = "b3e7d42d-2bb7-4e25-a4e1-b8d30f3f6e89" - existing_dataset = self.session.query(Dataset).filter(Dataset.id == dataset_id) - self.assertEqual(existing_dataset.count(), 1) + existing_dataset = Dataset.get_not_deleted(self.session, dataset_id) + self.assertNotEqual(existing_dataset, None) success, result = self.run_graphql_query({ "operationName": "DeleteDataset", "query": """ @@ -1015,8 +1015,8 @@ def test_delete_dataset_no_perm(self): self.assertTrue(success) self.assertResultWasNotAuthed(result) # Verify nothing was deleted - existing_dataset = self.session.query(Dataset).filter(Dataset.id == dataset_id) - self.assertEqual(existing_dataset.count(), 1) + existing_dataset = Dataset.get_not_deleted(self.session, dataset_id) + self.assertNotEqual(existing_dataset, None) def test_query_record(self): """Test that dataset records can be queried. @@ -1561,9 +1561,9 @@ def test_delete_category(self): user = self.test_users["admin"] category_id = "51349e29-290e-4398-a401-5bf7d04af75e" # Confirm Category exists, then that it does not. - existing_category = self.session.query(Category).filter(Category.id == category_id, Category.deleted == None) - # Count of existing Category should be one - self.assertEqual(existing_category.count(), 1) + existing_category = Category.get_not_deleted(self.session, category_id) + # Existing Category should not be None + self.assertNotEqual(existing_category, None) success, result = self.run_graphql_query({ "operationName": "DeleteCategory", "query": """ @@ -1576,8 +1576,8 @@ def test_delete_category(self): }, }, user=user) self.assertTrue(success) - category = self.session.query(Category).filter(Category.id == category_id, Category.deleted == None) - self.assertEqual(category.count(), 0) + category = Category.get_not_deleted(self.session, category_id) + self.assertEqual(category, None) self.assertTrue(self.is_valid_uuid(category_id), "Invalid UUID") self.assertEqual(result, { "data": { @@ -2272,9 +2272,9 @@ def test_delete_program(self): user = self.test_users["admin"] program_id = "1e73e788-0808-4ee8-9b25-682b6fa3868b" # Confirm Program exists, then that it does not. - existing_program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) - # Count of existing Program should be one - self.assertEqual(existing_program.count(), 1) + existing_program = Program.get_not_deleted(self.session, program_id) + # Existing Program should not be None + self.assertNotEqual(existing_program, None) success, result = self.run_graphql_query({ "operationName": "DeleteProgram", "query": """ @@ -2287,8 +2287,8 @@ def test_delete_program(self): }, }, user=user) self.assertTrue(success) - program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) - self.assertEqual(program.count(), 0) + program = Program.get_not_deleted(self.session, program_id) + self.assertEqual(program, None) self.assertTrue(self.is_valid_uuid(program_id), "Invalid UUID") self.assertEqual(result, { "data": { @@ -2302,9 +2302,9 @@ def test_delete_program_no_perm(self): user = self.test_users[user] program_id = "1e73e788-0808-4ee8-9b25-682b6fa3868b" # Confirm Program exists, then that it does not. - existing_program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) - # Count of existing Program should be one - self.assertEqual(existing_program.count(), 1) + existing_program = Program.get_not_deleted(self.session, program_id) + # Existing Program should not be None + self.assertNotEqual(existing_program, None) success, result = self.run_graphql_query({ "operationName": "DeleteProgram", "query": """ @@ -2318,8 +2318,11 @@ def test_delete_program_no_perm(self): }, user=user) self.assertTrue(success) self.assertResultWasNotAuthed(result) - program = self.session.query(Program).filter(Program.id == program_id, Program.deleted == None) - self.assertEqual(program.count(), 1) + program = Program.get_not_deleted(self.session, program_id) + self.assertNotEqual(program, None) if __name__ == '__main__': unittest.main() + +# TODO CHECK THAT PROGRAM CHILDREN WERE ALSO SOFT DELETED +# TODO UPDATE TO USE CLASS AND INSTANCE METHODS \ No newline at end of file diff --git a/api/database.py b/api/database.py index 30c506ef..b7106f4d 100644 --- a/api/database.py +++ b/api/database.py @@ -263,6 +263,17 @@ def user_is_team_member(self, user): class Category(Base): __tablename__ = 'category' + id = Column(GUID, primary_key=True, default=uuid.uuid4) + name = Column(String(255), nullable=False) + description = Column(Text, nullable=False) + category_value = relationship('CategoryValue', back_populates='category') + + created = Column(TIMESTAMP, + server_default=func.now(), nullable=False) + updated = Column(TIMESTAMP, + server_default=func.now(), onupdate=func.now()) + deleted = Column(TIMESTAMP) + @classmethod def get_by_name(cls, session, name): return session.query(cls).filter(cls.name == cls.clean_name(name)).first() @@ -271,6 +282,10 @@ def get_by_name(cls, session, name): def clean_name(cls, name): return name.capitalize().strip() + @classmethod + def get_not_deleted(cls, session, id_): + return session.query(Category).filter(Category.id == id_, Category.deleted == None).scalar() + @validates('name') def capitalize_category(self, key, name): # NOTE: `self.__class__` basically just means `Category` here. It's slightly @@ -279,17 +294,6 @@ def capitalize_category(self, key, name): # to override the method. return self.__class__.clean_name(name) - id = Column(GUID, primary_key=True, default=uuid.uuid4) - name = Column(String(255), nullable=False) - description = Column(Text, nullable=False) - category_value = relationship('CategoryValue', back_populates='category') - - created = Column(TIMESTAMP, - server_default=func.now(), nullable=False) - updated = Column(TIMESTAMP, - server_default=func.now(), onupdate=func.now()) - deleted = Column(TIMESTAMP) - class CategoryValue(Base): __tablename__ = 'category_value' From a44e0bcdc6f12b0414041e54bb634dab45bbd104 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 11:53:09 -0500 Subject: [PATCH 18/33] Update deleteCat noperm test to use class methods --- api/app_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 5ff4d310..83cdb7eb 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -1591,9 +1591,9 @@ def test_delete_category_no_perm(self): user = self.test_users[user_role] category_id = "51349e29-290e-4398-a401-5bf7d04af75e" # Confirm Category exists, then that it does not. - existing_category = self.session.query(Category).filter(Category.id == category_id, Category.deleted == None) - # Count of existing Category should be one - self.assertEqual(existing_category.count(), 1) + existing_category = Category.get_not_deleted(self.session, category_id) + # Existing Category should not be None + self.assertNotEqual(existing_category, None) success, result = self.run_graphql_query({ "operationName": "DeleteCategory", "query": """ @@ -1607,8 +1607,8 @@ def test_delete_category_no_perm(self): }, user=user) self.assertTrue(success) self.assertResultWasNotAuthed(result) - category = self.session.query(Category).filter(Category.id == category_id, Category.deleted == None) - self.assertEqual(category.count(), 1) + category = Category.get_not_deleted(self.session, category_id) + self.assertNotEqual(category, None) def test_query_category_value(self): """Test that anyone can query category values""" From 4871083510a04f9673e23f3fc3e7aa957ec07508 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 14:00:58 -0500 Subject: [PATCH 19/33] Update deleteCat test to use class methods --- api/app_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 83cdb7eb..b869bd9b 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -1778,9 +1778,9 @@ def test_delete_category_value(self): user = self.test_users["admin"] category_value_id = "0034d015-0652-497d-ab4a-d42b0bdf08cb" # Confirm Value exists, then that it does not. - existing_category_value = self.session.query(CategoryValue).filter(CategoryValue.id == category_value_id, CategoryValue.deleted == None) - # Count of existing CategoryValue should be one - self.assertEqual(existing_category_value.count(), 1) + existing_category_value = CategoryValue.get_not_deleted(self.session, category_value_id) + # Existing CategoryValue should not be None + self.assertNotEqual(existing_category_value, None) success, result = self.run_graphql_query({ "operationName": "DeleteCategoryValue", "query": """ @@ -1793,8 +1793,8 @@ def test_delete_category_value(self): }, }, user=user) self.assertTrue(success) - category_value = self.session.query(CategoryValue).filter(CategoryValue.id == category_value_id, CategoryValue.deleted == None) - self.assertEqual(category_value.count(), 0) + category_value = CategoryValue.get_not_deleted(self.session, category_value_id) + self.assertEqual(category_value, None) self.assertTrue(self.is_valid_uuid(category_value_id), "Invalid UUID") self.assertEqual(result, { "data": { From 37113d521c08fb8eaf0fc11b36ee4e25b6a2a1a7 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 14:03:01 -0500 Subject: [PATCH 20/33] Update deleteCatValNoPerm test to use class methods --- api/app_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index b869bd9b..c36276ec 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -1808,9 +1808,9 @@ def test_delete_category_value_no_perm(self): user = self.test_users[user] category_value_id = "0034d015-0652-497d-ab4a-d42b0bdf08cb" # Confirm Value exists, then that it does not. - existing_category_value = self.session.query(CategoryValue).filter(CategoryValue.id == category_value_id, CategoryValue.deleted == None) - # Count of existing CategoryValue should be one - self.assertEqual(existing_category_value.count(), 1) + existing_category_value = CategoryValue.get_not_deleted(self.session, category_value_id) + # CExisting CategoryValue should not be None + self.assertNotEqual(existing_category_value, None) success, result = self.run_graphql_query({ "operationName": "DeleteCategoryValue", "query": """ @@ -1824,8 +1824,8 @@ def test_delete_category_value_no_perm(self): }, user=user) self.assertTrue(success) self.assertResultWasNotAuthed(result) - category_value = self.session.query(CategoryValue).filter(CategoryValue.id == category_value_id, CategoryValue.deleted == None) - self.assertEqual(category_value.count(), 1) + category_value = CategoryValue.get_not_deleted(self.session, category_value_id) + self.assertNotEqual(category_value, None) def test_query_team(self): """Test that anyone can query teams""" From a2eeb6910a37de08691b19d0419121e8fc134e57 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 14:56:37 -0500 Subject: [PATCH 21/33] Add queyProgram test --- api/app_test.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/api/app_test.py b/api/app_test.py index c36276ec..a56f7984 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -1837,7 +1837,7 @@ def test_query_team(self): query QueryTeam($id: ID!) { team(id: $id) { id - name + name } } """, @@ -2177,6 +2177,54 @@ def test_get_roles(self): else: self.assertResultWasNotAuthed(result) + def test_query_program(self): + """Test that programs can be queried. + Users on the right team and admins should be able to query programs. + """ + for user_role in ['admin', 'normal']: + user = self.test_users[user_role] + success, result = self.run_graphql_query({ + "operationName": "QueryProgram", + "query": """ + query QueryProgram($id: ID!) { + program(id: $id) { + id + } + } + """, + "variables": { + "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", + }, + }, user=user) + + self.assertTrue(success) + self.assertEqual(result, { + "data": { + "program": { + "id" : "1e73e788-0808-4ee8-9b25-682b6fa3868b" + }, + }, + }) + + def test_query_program_no_perm(self): + """Test that programs can't be queried by users on the wrong team.""" + success, result = self.run_graphql_query({ + "operationName": "QueryProgram", + "query": """ + query QueryProgram($id: ID!) { + program(id: $id) { + id + } + } + """, + "variables": { + "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", + }, + }, user=self.test_users['other']) + + self.assertTrue(success) + self.assertResultWasNotAuthed(result) + def test_create_program(self): success, result = self.run_graphql_query({ "operationName": "CreateProgram", From aec964090e246e861daba35b9d1bb339b4e3919a Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 15:00:08 -0500 Subject: [PATCH 22/33] Add additional test coverage for queryProgram --- api/app_test.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/api/app_test.py b/api/app_test.py index a56f7984..35c6b03f 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2189,6 +2189,11 @@ def test_query_program(self): query QueryProgram($id: ID!) { program(id: $id) { id + description + datasets { + id + name + } } } """, @@ -2201,7 +2206,12 @@ def test_query_program(self): self.assertEqual(result, { "data": { "program": { - "id" : "1e73e788-0808-4ee8-9b25-682b6fa3868b" + "id" : "1e73e788-0808-4ee8-9b25-682b6fa3868b", + "description": "All BBC news programming", + "datasets": [ + {"id": "b3e7d42d-2bb7-4e25-a4e1-b8d30f3f6e89", "name": "Breakfast Hour"}, + {"id": "96336531-9245-405f-bd28-5b4b12ea3798", "name": "12PM - 4PM"} + ] }, }, }) From cda32b9095c9ec9acb1575beff6628715ecc79e5 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 16:49:42 -0500 Subject: [PATCH 23/33] Add updateProgram test --- api/app_test.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/api/app_test.py b/api/app_test.py index 35c6b03f..ccbc09df 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2325,6 +2325,64 @@ def test_create_program_no_perm(self): self.assertTrue(success) self.assertResultWasNotAuthed(result) + def test_update_program(self): + """Test that users on a team / admins can update Programs.""" + for user_role in ['admin', 'normal']: + user = self.test_users[user_role] + success, result = self.run_graphql_query({ + "operationName": "UpdateProgram", + "query": """ + mutation UpdateProgram($input: UpdateProgramInput!) { + updateProgram(input: $input) { + id + name + + } + } + """, + "variables": { + "input": { + "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", + "name": "An updated new Program", + "targetIds": ["40eaeafc-3311-4294-a639-a826eb6495ab", "2d501688-92e3-455e-9685-01141de3dbaf", "4f7897c2-32a1-4b1e-9749-1a8066faca01"] + } + }, + }, user=user) + + self.assertTrue(success) + self.assertTrue(self.is_valid_uuid(result["data"]["updateProgram"]["id"]), "Invalid UUID") + self.assertEqual(result, { + "data": { + "updateProgram": { + "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", + "name": "An updated new Program", + }, + }, + }) + + def test_update_program_no_perm(self): + """Test that users on other teams can't update Programs.""" + user = self.test_users['other'] + success, result = self.run_graphql_query({ + "operationName": "UpdateProgram", + "query": """ + mutation UpdateProgram($input: UpdateProgramInput!) { + updateProgram(input: $input) { + id + } + } + """, + "variables": { + "input": { + "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b" + } + }, + }, user=user) + + self.assertTrue(success) + self.assertResultWasNotAuthed(result) + + def test_delete_program(self): """Only admins can delete programs.""" user = self.test_users["admin"] From cec2b26496d2fd86b8bab63f44cd72119482c002 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 16:52:18 -0500 Subject: [PATCH 24/33] Add updatePrograms to team member permissions --- api/directives.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/directives.py b/api/directives.py index b7aa738d..f8c04e45 100644 --- a/api/directives.py +++ b/api/directives.py @@ -9,6 +9,7 @@ User, Dataset, Record, + Program ) from typing import List, Iterable @@ -99,6 +100,13 @@ def user_has_permission(permissions: Iterable[str], obj, info) -> bool: record = Record.get_not_deleted(session, id_) if record and record.user_is_team_member(current_user): return True + + if info.field_name == 'updateProgram': + checked = True + id_ = info.variable_values['input']['id'] + program = Program.get_not_deleted(session, id_) + if program and program.user_is_team_member(current_user): + return True if not checked: raise InvalidPermissionError(f"Can't enforce TEAM_MEMBER permission on type {type(obj)}, {info.field_name}") From d6f4cdc95f887dff128ad5fcf8371575e00b0ac4 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 16:52:43 -0500 Subject: [PATCH 25/33] Add UUID type coercion for tests --- api/mutations.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/api/mutations.py b/api/mutations.py index 67637741..aa2e7233 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -1,3 +1,5 @@ +import uuid + from ariadne import convert_kwargs_to_snake_case, ObjectType from settings import settings from database import SessionLocal, User, Dataset, Tag, Program, Record, Entry, Category, Target, CategoryValue, Team @@ -324,11 +326,11 @@ def resolve_update_program(obj, info, input): tags = input.pop('tag_ids', []) program = session.query(Program).get(input['id']) if len(datasets) > 0: - program.datasets = [session.merge(Dataset(id=dataset_id)) for dataset_id in datasets] + program.datasets = [session.merge(Dataset(id=uuid.UUID(dataset_id))) for dataset_id in datasets] if len(targets) > 0: - program.targets = [session.merge(Target(id=target_id)) for target_id in targets] + program.targets = [session.merge(Target(id=uuid.UUID(target_id))) for target_id in targets] if len(tags) > 0: - program.tags = [session.merge(Tag(id=tag_id)) for tag_id in tags] + program.tags = [session.merge(Tag(id=uuid.UUID(tag_id))) for tag_id in tags] for param in input: setattr(program, param, input[param]) session.add(program) From 001b7087ca44ea8e88b2ec829097a366136b3279 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 17:09:44 -0500 Subject: [PATCH 26/33] Add test coverage --- api/app_test.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/api/app_test.py b/api/app_test.py index ccbc09df..f947cca9 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2269,7 +2269,6 @@ def test_create_program(self): }, user=self.test_users['admin']) self.assertTrue(success) - print(f'{result}, result here') self.assertTrue(self.is_valid_uuid(result["data"]["createProgram"]["id"]), "Invalid UUID") self.assertEqual(result, { "data": { @@ -2336,6 +2335,12 @@ def test_update_program(self): updateProgram(input: $input) { id name + targets { + id + categoryValue { + name + } + } } } @@ -2356,6 +2361,24 @@ def test_update_program(self): "updateProgram": { "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", "name": "An updated new Program", + "targets": [ + { + "id": "2d501688-92e3-455e-9685-01141de3dbaf", + "categoryValue": { + "name": "Cisgender men"} + }, + { + "id": "4f7897c2-32a1-4b1e-9749-1a8066faca01", + "categoryValue": { + "name": "Trans women"} + }, + { + "id": "40eaeafc-3311-4294-a639-a826eb6495ab", + "categoryValue": { + "name": "Non-binary" + } + } + ] }, }, }) From af8a6b6efdfbff42c19567dc036dca268e863093 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 30 Jun 2021 17:12:34 -0500 Subject: [PATCH 27/33] Add test coverage --- api/app_test.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/app_test.py b/api/app_test.py index f947cca9..127db495 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2341,6 +2341,9 @@ def test_update_program(self): name } } + tags { + name + } } } @@ -2349,7 +2352,8 @@ def test_update_program(self): "input": { "id": "1e73e788-0808-4ee8-9b25-682b6fa3868b", "name": "An updated new Program", - "targetIds": ["40eaeafc-3311-4294-a639-a826eb6495ab", "2d501688-92e3-455e-9685-01141de3dbaf", "4f7897c2-32a1-4b1e-9749-1a8066faca01"] + "targetIds": ["40eaeafc-3311-4294-a639-a826eb6495ab", "2d501688-92e3-455e-9685-01141de3dbaf", "4f7897c2-32a1-4b1e-9749-1a8066faca01"], + "tagIds": ["4a2142c0-5416-431d-b62f-0dbfe7574688"] } }, }, user=user) @@ -2378,6 +2382,9 @@ def test_update_program(self): "name": "Non-binary" } } + ], + "tags": [ + {"name": "News"} ] }, }, From 9484890552ec4605392d8fae4184e751f09931c1 Mon Sep 17 00:00:00 2001 From: Nancy Date: Thu, 1 Jul 2021 11:14:59 -0500 Subject: [PATCH 28/33] Cleaning up queries- simplify to use class methods --- api/queries.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/api/queries.py b/api/queries.py index b2055243..4637eaea 100644 --- a/api/queries.py +++ b/api/queries.py @@ -24,7 +24,6 @@ def resolve_user(obj, info, id): ''' session = info.context['dbsession'] user = session.query(User).filter(User.id == id).first() - return user @user.field("active") @@ -61,7 +60,6 @@ def resolve_dataset(obj, info, id): ''' session = info.context['dbsession'] dataset = Dataset.get_not_deleted(session, id) - return dataset @dataset.field("lastUpdated") @@ -72,7 +70,6 @@ def resolve_dataset_last_updated(dataset, info): :returns: Datetime scalar ''' session = info.context['dbsession'] - return session.query(func.max(Record.updated)).\ filter(Record.dataset_id == dataset.id, Record.deleted == None).\ scalar() @@ -116,7 +113,6 @@ def resolve_sums_category_relationship(count_obj, info): category_value_rel = CategoryValue.get_not_deleted(session, count_obj['category_value_id']) return category_value_rel - @query.field("record") def resolve_record(obj, info, id): '''GraphQL query to find a Record based on Record ID. @@ -124,7 +120,7 @@ def resolve_record(obj, info, id): :returns: Record dictionary OR None if Record was soft-deleted ''' session = info.context['dbsession'] - record = session.query(Record).filter(Record.id == id, Record.deleted == None).first() + record = Record.get_not_deleted(session, id) return record @query.field("category") @@ -134,8 +130,7 @@ def resolve_category(obj, info, id): :returns: Category dictionary OR None if Category was soft-deleted ''' session = info.context['dbsession'] - category = session.query(Category).filter(Category.id == id, Category.deleted == None).first() - + category = Category.get_not_deleted(session, id) return category @query.field("categoryValue") @@ -145,9 +140,9 @@ def resolve_category_value(obj, info, id): :returns: CategoryValue dictionary OR None if CategoryValue was deleted ''' session = info.context['dbsession'] - category_value = session.query(CategoryValue).filter(CategoryValue.id == id, CategoryValue.deleted == None).first() - + category_value = CategoryValue.get_not_deleted(session, id) return category_value + @query.field("team") def resolve_team(obj, info, id): '''GraphQL query to find a Team based on Team ID. @@ -176,3 +171,4 @@ def resolve_roles(obj, info): ''' session = info.context['dbsession'] return session.query(Role).filter(Role.deleted == None).order_by(Role.name.asc()).all() + From 3feb9b28c9d1c6f928f9c2cd05dd3222571cbc51 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 7 Jul 2021 12:05:56 -0400 Subject: [PATCH 29/33] Cleaning up deleteCat --- api/database.py | 4 ++++ api/mutations.py | 5 ++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/database.py b/api/database.py index b7106f4d..f7b45b31 100644 --- a/api/database.py +++ b/api/database.py @@ -294,6 +294,10 @@ def capitalize_category(self, key, name): # to override the method. return self.__class__.clean_name(name) + def soft_delete(self, session): + self.deleted= func.now() + session.add(self) + session.query(CategoryValue).filter(CategoryValue.category_id == self.id).update({'deleted':func.now()}, synchronize_session='fetch') class CategoryValue(Base): __tablename__ = 'category_value' diff --git a/api/mutations.py b/api/mutations.py index aa2e7233..7c71a163 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -182,10 +182,9 @@ def resolve_delete_category(obj, info, id): :returns: UUID of soft deleted Category ''' session = info.context['dbsession'] - session.query(Category).filter(Category.id == id).update({'deleted':func.now()}, synchronize_session='fetch') - session.query(CategoryValue).filter(CategoryValue.category_id == id).update({'deleted':func.now()}, synchronize_session='fetch') + Category.get_not_deleted(session, id).soft_delete(session) session.commit() - + return id @mutation.field("createCategoryValue") From a8a80177836599fc18cc95793cab913b4089f4f3 Mon Sep 17 00:00:00 2001 From: Nancy Date: Wed, 7 Jul 2021 18:10:30 -0400 Subject: [PATCH 30/33] Add test coverage for cascade soft-deletes --- api/app_test.py | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 127db495..40d64b4f 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -13,18 +13,19 @@ from app import schema, app, cookie_authentication from user import user_db from database import ( - Base, - create_tables, - create_dummy_data, - Record, - Entry, - Dataset, - Category, - CategoryValue, - User, - Team, - Program - ) + Base, + create_tables, + create_dummy_data, + Record, + Entry, + Dataset, + Category, + CategoryValue, + User, + Team, + Program, + Target +) from uuid import UUID @@ -1578,6 +1579,11 @@ def test_delete_category(self): self.assertTrue(success) category = Category.get_not_deleted(self.session, category_id) self.assertEqual(category, None) + + # Check that categoryValue was also soft deleted + category_value = self.session.query(CategoryValue).filter(CategoryValue.category_id == category_id, CategoryValue.deleted == None).scalar() + self.assertEqual(category_value, None) + self.assertTrue(self.is_valid_uuid(category_id), "Invalid UUID") self.assertEqual(result, { "data": { @@ -2435,6 +2441,20 @@ def test_delete_program(self): self.assertTrue(success) program = Program.get_not_deleted(self.session, program_id) self.assertEqual(program, None) + + # check that Dataset, Record, Entry, Target were also soft deleted + datasets = self.session.query(Dataset).filter(Dataset.program_id == program_id).all() + for dataset in datasets: + self.assertNotEqual(dataset.deleted, None) + records = self.session.query(Record).filter(Record.dataset_id == dataset.id).all() + for record in records: + self.assertNotEqual(record.deleted, None) + entries = self.session.query(Entry).filter(Entry.record_id == record.id, Entry.deleted == None).first() + self.assertEqual(entries, None) + + targets = self.session.query(Target).filter(Target.program_id == program_id, Target.deleted == None).first() + self.assertEqual(targets, None) + self.assertTrue(self.is_valid_uuid(program_id), "Invalid UUID") self.assertEqual(result, { "data": { @@ -2469,6 +2489,3 @@ def test_delete_program_no_perm(self): if __name__ == '__main__': unittest.main() - -# TODO CHECK THAT PROGRAM CHILDREN WERE ALSO SOFT DELETED -# TODO UPDATE TO USE CLASS AND INSTANCE METHODS \ No newline at end of file From bc58fb571369131cb3411abb774a234ecd94793f Mon Sep 17 00:00:00 2001 From: Nancy Date: Thu, 8 Jul 2021 19:50:19 -0400 Subject: [PATCH 31/33] Add conditional to deletes --- api/mutations.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/api/mutations.py b/api/mutations.py index 7c71a163..09cf07a6 100644 --- a/api/mutations.py +++ b/api/mutations.py @@ -46,7 +46,9 @@ def resolve_delete_dataset(obj, info, id): :returns: UUID of soft deleted Dataset ''' session = info.context['dbsession'] - Dataset.get_not_deleted(session, id).soft_delete(session) + dataset = Dataset.get_not_deleted(session, id) + if dataset is not None: + dataset.soft_delete(session) session.commit() return id @@ -133,7 +135,9 @@ def resolve_delete_record(obj, info, id): :returns: UUID of soft deleted Record ''' session = info.context['dbsession'] - Record.get_not_deleted(session, id).soft_delete(session) + record = Record.get_not_deleted(session, id) + if record is not None: + record.soft_delete(session) session.commit() return id @@ -182,7 +186,9 @@ def resolve_delete_category(obj, info, id): :returns: UUID of soft deleted Category ''' session = info.context['dbsession'] - Category.get_not_deleted(session, id).soft_delete(session) + category = Category.get_not_deleted(session, id) + if category is not None: + category.soft_delete(session) session.commit() return id @@ -226,7 +232,9 @@ def resolve_delete_category_value(obj, info, id): :returns: UUID of deleted CategoryValue ''' session = info.context['dbsession'] - CategoryValue.get_not_deleted(session, id).soft_delete(session) + category_value = CategoryValue.get_not_deleted(session, id) + if category_value is not None: + category_value.soft_delete(session) session.commit() return id @@ -344,7 +352,9 @@ def resolve_delete_program(obj, info, id): :returns: UUID of deleted Program ''' session = info.context['dbsession'] - Program.get_not_deleted(session, id).soft_delete(session) + program = Program.get_not_deleted(session, id) + if program is not None: + program.soft_delete(session) session.commit() return id \ No newline at end of file From b7514ed247c60d54727ecd33c3ac5963f4bb28a6 Mon Sep 17 00:00:00 2001 From: Nancy Date: Thu, 8 Jul 2021 19:55:58 -0400 Subject: [PATCH 32/33] Remove updateProgram team permissions --- api/app_test.py | 4 ++-- api/directives.py | 7 ------- api/schema.graphql | 2 +- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/api/app_test.py b/api/app_test.py index 40d64b4f..cd981464 100644 --- a/api/app_test.py +++ b/api/app_test.py @@ -2331,8 +2331,8 @@ def test_create_program_no_perm(self): self.assertResultWasNotAuthed(result) def test_update_program(self): - """Test that users on a team / admins can update Programs.""" - for user_role in ['admin', 'normal']: + """Test that users on a admins can update Programs.""" + for user_role in ['admin']: user = self.test_users[user_role] success, result = self.run_graphql_query({ "operationName": "UpdateProgram", diff --git a/api/directives.py b/api/directives.py index f8c04e45..e46f38bb 100644 --- a/api/directives.py +++ b/api/directives.py @@ -101,13 +101,6 @@ def user_has_permission(permissions: Iterable[str], obj, info) -> bool: if record and record.user_is_team_member(current_user): return True - if info.field_name == 'updateProgram': - checked = True - id_ = info.variable_values['input']['id'] - program = Program.get_not_deleted(session, id_) - if program and program.user_is_team_member(current_user): - return True - if not checked: raise InvalidPermissionError(f"Can't enforce TEAM_MEMBER permission on type {type(obj)}, {info.field_name}") diff --git a/api/schema.graphql b/api/schema.graphql index a6451d50..40c6bfd8 100644 --- a/api/schema.graphql +++ b/api/schema.graphql @@ -348,7 +348,7 @@ type Mutation { # Update a Program updateProgram(input: UpdateProgramInput!): Program! - @needsPermission(permission: [ADMIN, TEAM_MEMBER]) + @needsPermission(permission: [ADMIN]) # Delete a Program deleteProgram(id: ID!): ID! @needsPermission(permission: [ADMIN, TEAM_MEMBER]) From 57455e2220b84457405b05c70fe54f81932ad44c Mon Sep 17 00:00:00 2001 From: Nancy Date: Thu, 8 Jul 2021 19:57:10 -0400 Subject: [PATCH 33/33] Cleaning up comments --- api/queries.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/queries.py b/api/queries.py index 4637eaea..5230a83e 100644 --- a/api/queries.py +++ b/api/queries.py @@ -20,7 +20,7 @@ def resolve_user(obj, info, id): '''GraphQL query to find a user based on user ID. :param id: Id for the user to be fetched - :returns: User dictionary OR None if User was soft-deleted + :returns: User OR None if User was soft-deleted ''' session = info.context['dbsession'] user = session.query(User).filter(User.id == id).first() @@ -45,7 +45,7 @@ def resolve_users(obj, info): def resolve_program(obj, info, id): '''GraphQL query to find a Program based on Program ID. :param id: Id for the Program to be fetched - :returns: Program dictionary + :returns: Program ''' session = info.context['dbsession'] program = Program.get_not_deleted(session, id) @@ -56,7 +56,7 @@ def resolve_program(obj, info, id): def resolve_dataset(obj, info, id): '''GraphQL query to find a dataset based on dataset ID. :param id: Id for the dataset to be fetched - :returns: Dataset dictionary OR None if Dataset was soft-deleted + :returns: Dataset OR None if Dataset was soft-deleted ''' session = info.context['dbsession'] dataset = Dataset.get_not_deleted(session, id) @@ -117,7 +117,7 @@ def resolve_sums_category_relationship(count_obj, info): def resolve_record(obj, info, id): '''GraphQL query to find a Record based on Record ID. :param id: Id for the Record to be fetched - :returns: Record dictionary OR None if Record was soft-deleted + :returns: Record OR None if Record was soft-deleted ''' session = info.context['dbsession'] record = Record.get_not_deleted(session, id) @@ -127,7 +127,7 @@ def resolve_record(obj, info, id): def resolve_category(obj, info, id): '''GraphQL query to find a Category based on Category ID. :param id: Id for the Category to be fetched - :returns: Category dictionary OR None if Category was soft-deleted + :returns: Category OR None if Category was soft-deleted ''' session = info.context['dbsession'] category = Category.get_not_deleted(session, id) @@ -137,7 +137,7 @@ def resolve_category(obj, info, id): def resolve_category_value(obj, info, id): '''GraphQL query to find a CategoryValue based on CategoryValue ID. :param id: Id for the CategoryValue to be fetched - :returns: CategoryValue dictionary OR None if CategoryValue was deleted + :returns: CategoryValue OR None if CategoryValue was deleted ''' session = info.context['dbsession'] category_value = CategoryValue.get_not_deleted(session, id)