From 391c4620eb98adad989e067a62625a4e3da563a5 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Wed, 29 Jan 2020 11:15:08 -0800 Subject: [PATCH 01/16] Basic framework for testing flask server with one passing unit test. It was honestly hard to find something we could pass :P --- .../api/tests/test_integration.py | 110 ++++++++++++++++++ microsetta_private_api/server.py | 9 +- repo_test_scratch.py | 1 - 3 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 microsetta_private_api/api/tests/test_integration.py diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py new file mode 100644 index 000000000..db471d1f0 --- /dev/null +++ b/microsetta_private_api/api/tests/test_integration.py @@ -0,0 +1,110 @@ +import pytest +import microsetta_private_api.server +from microsetta_private_api.repo.transaction import Transaction +from microsetta_private_api.repo.account_repo import AccountRepo +from microsetta_private_api.model.account import Account +from microsetta_private_api.repo.source_repo import SourceRepo +from microsetta_private_api.model.source import \ + Source, HumanInfo, AnimalInfo, EnvironmentInfo +import datetime +import json + +ACCT_ID = "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff" +HUMAN_ID = "b0b0b0b0-b0b0-b0b0-b0b0-b0b0b0b0b0b0" +DOGGY_ID = "dddddddd-dddd-dddd-dddd-dddddddddddd" +PLANTY_ID = "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee" + + +def json_converter(o): + if isinstance(o, datetime.datetime): + return str(o) + return o.__dict__ + + +def setup_test_data(): + with Transaction() as t: + acct_repo = AccountRepo(t) + acct_repo.delete_account(ACCT_ID) + acc = Account(ACCT_ID, + "foo@baz.com", + "globus", + "Dan", + "H", + '{"a":5, "b":7}', + "USER") + acct_repo.create_account(acc) + + source_repo = SourceRepo(t) + source_repo.delete_source(ACCT_ID, DOGGY_ID) + source_repo.delete_source(ACCT_ID, PLANTY_ID) + source_repo.delete_source(ACCT_ID, HUMAN_ID) + + source_repo.create_source(Source.create_human( + HUMAN_ID, + ACCT_ID, + HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", False, "Mrs Bo", + False, datetime.datetime.utcnow(), "TODO,WutDis?") + )) + source_repo.create_source(Source.create_animal( + DOGGY_ID, + ACCT_ID, + AnimalInfo("Doggy"))) + source_repo.create_source(Source.create_environment( + PLANTY_ID, + ACCT_ID, + EnvironmentInfo("Planty", "The green one"))) + + t.commit() + + +def teardown_test_data(): + with Transaction() as t: + acct_repo = AccountRepo(t) + source_repo = SourceRepo(t) + source_repo.delete_source(ACCT_ID, DOGGY_ID) + source_repo.delete_source(ACCT_ID, PLANTY_ID) + source_repo.delete_source(ACCT_ID, HUMAN_ID) + acct_repo.delete_account(ACCT_ID) + + t.commit() + + +@pytest.fixture +def client(): + setup_test_data() + app = microsetta_private_api.server.build_app() + with app.app.test_client() as client: + yield client + teardown_test_data() + + +def test_get_sources(client): + resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data + sources = json.loads(resp) + assert len([x for x in sources if x['source_name'] == 'Bo']) == 1 + assert len([x for x in sources if x['source_name'] == 'Doggy']) == 1 + assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 + + +# BROKEN +# def test_external_surveys(client): +# # I can't even find a test that we can pass for demo purposes! +# resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data +# sources = json.loads(resp) +# bobo = [x for x in sources if x['source_name'] == 'Bo'][0] +# doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] +# env = [x for x in sources if x['source_name'] == 'Planty'][0] +# +# resp = client.get('/accounts/%s/sources/%s/survey_templates' % +# (ACCT_ID, bobo.id)) +# bobo_surveys = json.loads(resp.data) +# resp = client.get('/accounts/%s/sources/%s/survey_templates' % +# (ACCT_ID, doggy.id)) +# doggy_surveys = json.loads(resp.data) +# resp = client.get('/accounts/%s/sources/%s/survey_templates' % +# (ACCT_ID, env.id)) +# env_surveys = json.loads(resp.data) +# +# assert bobo_surveys == [1, 3, 4, 5] +# assert doggy_surveys == [2] +# assert env_surveys == [3] diff --git a/microsetta_private_api/server.py b/microsetta_private_api/server.py index ef3949db8..170afdf6c 100644 --- a/microsetta_private_api/server.py +++ b/microsetta_private_api/server.py @@ -10,8 +10,8 @@ import connexion from microsetta_private_api.util.util import JsonifyDefaultEncoder -# If we're running in stand alone mode, run the application -if __name__ == '__main__': + +def build_app(): # Create the application instance app = connexion.FlaskApp(__name__) @@ -22,5 +22,10 @@ # Note: app.app is the actual Flask application instance, so any Flask # settings have to be set there. app.app.json_encoder = JsonifyDefaultEncoder + return app + +# If we're running in stand alone mode, run the application +if __name__ == '__main__': + app = build_app() app.run(port=8082, debug=True) diff --git a/repo_test_scratch.py b/repo_test_scratch.py index a28b6b7d3..2d13fbcaa 100644 --- a/repo_test_scratch.py +++ b/repo_test_scratch.py @@ -145,4 +145,3 @@ def json_converter(o): print(survey_model == survey_model2) survey_answers_repo.delete_answered_survey(ACCT_ID, answer_id) - From 0f6b05dfe3d75f4e0ec65fa04c3a0d2cbae59091 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Wed, 29 Jan 2020 13:15:55 -0800 Subject: [PATCH 02/16] Source now returns source_id as specified in the api. test_integration now has two working tests --- .../api/tests/test_integration.py | 74 +++++++++++-------- microsetta_private_api/model/source.py | 3 +- 2 files changed, 46 insertions(+), 31 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index db471d1f0..973d9b1b0 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -25,13 +25,23 @@ def setup_test_data(): with Transaction() as t: acct_repo = AccountRepo(t) acct_repo.delete_account(ACCT_ID) + + creation_time = None + update_time = None + acc = Account(ACCT_ID, - "foo@baz.com", - "globus", - "Dan", - "H", - '{"a":5, "b":7}', - "USER") + "foo@baz.com", + "standard", + "GLOBUS", + "Dan", + "H", + { + "street": "123 Dan Lane", + "city": "Danville", + "state": "CA", + "post_code": 12345, + "country_code": "US" + }) acct_repo.create_account(acc) source_repo = SourceRepo(t) @@ -42,8 +52,9 @@ def setup_test_data(): source_repo.create_source(Source.create_human( HUMAN_ID, ACCT_ID, - HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", False, "Mrs Bo", - False, datetime.datetime.utcnow(), "TODO,WutDis?") + HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", "Mrs Bo", + False, datetime.datetime.utcnow(), None, "Mr. Obtainer", + "18-plus") )) source_repo.create_source(Source.create_animal( DOGGY_ID, @@ -86,25 +97,28 @@ def test_get_sources(client): assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 -# BROKEN -# def test_external_surveys(client): -# # I can't even find a test that we can pass for demo purposes! -# resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data -# sources = json.loads(resp) -# bobo = [x for x in sources if x['source_name'] == 'Bo'][0] -# doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] -# env = [x for x in sources if x['source_name'] == 'Planty'][0] -# -# resp = client.get('/accounts/%s/sources/%s/survey_templates' % -# (ACCT_ID, bobo.id)) -# bobo_surveys = json.loads(resp.data) -# resp = client.get('/accounts/%s/sources/%s/survey_templates' % -# (ACCT_ID, doggy.id)) -# doggy_surveys = json.loads(resp.data) -# resp = client.get('/accounts/%s/sources/%s/survey_templates' % -# (ACCT_ID, env.id)) -# env_surveys = json.loads(resp.data) -# -# assert bobo_surveys == [1, 3, 4, 5] -# assert doggy_surveys == [2] -# assert env_surveys == [3] +def test_surveys(client): + # I can't even find a test that we can pass for demo purposes! + resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data + + sources = json.loads(resp) + bobo = [x for x in sources if x['source_name'] == 'Bo'][0] + doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] + env = [x for x in sources if x['source_name'] == 'Planty'][0] + + resp = client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, bobo['source_id']), ) + bobo_surveys = json.loads(resp.data) + resp = client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, doggy['source_id'])) + doggy_surveys = json.loads(resp.data) + resp = client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, env['source_id'])) + env_surveys = json.loads(resp.data) + + assert bobo_surveys == [1, 3, 4, 5] + assert doggy_surveys == [2] + assert env_surveys == [] diff --git a/microsetta_private_api/model/source.py b/microsetta_private_api/model/source.py index 60891b949..1afc0aaa4 100644 --- a/microsetta_private_api/model/source.py +++ b/microsetta_private_api/model/source.py @@ -71,7 +71,8 @@ def __init__(self, source_id, account_id, source_type, source_data): def to_api(self): result = { "source_type": self.source_type, - "source_name": self.source_data.name + "source_name": self.source_data.name, + "source_id": self.id } if self.source_type == Source.SOURCE_TYPE_HUMAN: From 58aa94620b21878f9d15dcc80e6a717dc3746952 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Wed, 29 Jan 2020 13:17:59 -0800 Subject: [PATCH 03/16] Flake8 --- .../api/tests/test_integration.py | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index 973d9b1b0..a705fb3ee 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -26,22 +26,19 @@ def setup_test_data(): acct_repo = AccountRepo(t) acct_repo.delete_account(ACCT_ID) - creation_time = None - update_time = None - acc = Account(ACCT_ID, - "foo@baz.com", - "standard", - "GLOBUS", - "Dan", - "H", - { - "street": "123 Dan Lane", - "city": "Danville", - "state": "CA", - "post_code": 12345, - "country_code": "US" - }) + "foo@baz.com", + "standard", + "GLOBUS", + "Dan", + "H", + { + "street": "123 Dan Lane", + "city": "Danville", + "state": "CA", + "post_code": 12345, + "country_code": "US" + }) acct_repo.create_account(acc) source_repo = SourceRepo(t) From 408fd135f6ce6e0328904b4605e68ad9f54640f3 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Fri, 31 Jan 2020 14:00:18 -0800 Subject: [PATCH 04/16] Wrapped unit tests in a TestCase subclass to match unittest requirements and other projects in the qiime2 ecosystem --- .../api/tests/test_integration.py | 206 +++++++++--------- 1 file changed, 105 insertions(+), 101 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index a705fb3ee..a0a52b54a 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -8,6 +8,7 @@ Source, HumanInfo, AnimalInfo, EnvironmentInfo import datetime import json +from unittest import TestCase ACCT_ID = "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff" HUMAN_ID = "b0b0b0b0-b0b0-b0b0-b0b0-b0b0b0b0b0b0" @@ -15,107 +16,110 @@ PLANTY_ID = "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee" -def json_converter(o): - if isinstance(o, datetime.datetime): - return str(o) - return o.__dict__ - - -def setup_test_data(): - with Transaction() as t: - acct_repo = AccountRepo(t) - acct_repo.delete_account(ACCT_ID) - - acc = Account(ACCT_ID, - "foo@baz.com", - "standard", - "GLOBUS", - "Dan", - "H", - { - "street": "123 Dan Lane", - "city": "Danville", - "state": "CA", - "post_code": 12345, - "country_code": "US" - }) - acct_repo.create_account(acc) - - source_repo = SourceRepo(t) - source_repo.delete_source(ACCT_ID, DOGGY_ID) - source_repo.delete_source(ACCT_ID, PLANTY_ID) - source_repo.delete_source(ACCT_ID, HUMAN_ID) - - source_repo.create_source(Source.create_human( - HUMAN_ID, - ACCT_ID, - HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", "Mrs Bo", - False, datetime.datetime.utcnow(), None, "Mr. Obtainer", - "18-plus") - )) - source_repo.create_source(Source.create_animal( - DOGGY_ID, - ACCT_ID, - AnimalInfo("Doggy"))) - source_repo.create_source(Source.create_environment( - PLANTY_ID, - ACCT_ID, - EnvironmentInfo("Planty", "The green one"))) - - t.commit() - - -def teardown_test_data(): - with Transaction() as t: - acct_repo = AccountRepo(t) - source_repo = SourceRepo(t) - source_repo.delete_source(ACCT_ID, DOGGY_ID) - source_repo.delete_source(ACCT_ID, PLANTY_ID) - source_repo.delete_source(ACCT_ID, HUMAN_ID) - acct_repo.delete_account(ACCT_ID) - - t.commit() - - -@pytest.fixture -def client(): - setup_test_data() +@pytest.fixture(scope="class") +def client(request): + IntegrationTests.setup_test_data() app = microsetta_private_api.server.build_app() with app.app.test_client() as client: + request.cls.client = client yield client - teardown_test_data() - - -def test_get_sources(client): - resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data - sources = json.loads(resp) - assert len([x for x in sources if x['source_name'] == 'Bo']) == 1 - assert len([x for x in sources if x['source_name'] == 'Doggy']) == 1 - assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 - - -def test_surveys(client): - # I can't even find a test that we can pass for demo purposes! - resp = client.get('/api/accounts/%s/sources' % ACCT_ID).data - - sources = json.loads(resp) - bobo = [x for x in sources if x['source_name'] == 'Bo'][0] - doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] - env = [x for x in sources if x['source_name'] == 'Planty'][0] - - resp = client.get( - '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % - (ACCT_ID, bobo['source_id']), ) - bobo_surveys = json.loads(resp.data) - resp = client.get( - '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % - (ACCT_ID, doggy['source_id'])) - doggy_surveys = json.loads(resp.data) - resp = client.get( - '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % - (ACCT_ID, env['source_id'])) - env_surveys = json.loads(resp.data) - - assert bobo_surveys == [1, 3, 4, 5] - assert doggy_surveys == [2] - assert env_surveys == [] + IntegrationTests.teardown_test_data() + + +@pytest.mark.usefixtures("client") +class IntegrationTests(TestCase): + @staticmethod + def json_converter(o): + if isinstance(o, datetime.datetime): + return str(o) + return o.__dict__ + + @staticmethod + def setup_test_data(): + with Transaction() as t: + acct_repo = AccountRepo(t) + acct_repo.delete_account(ACCT_ID) + + acc = Account(ACCT_ID, + "foo@baz.com", + "standard", + "GLOBUS", + "Dan", + "H", + { + "street": "123 Dan Lane", + "city": "Danville", + "state": "CA", + "post_code": 12345, + "country_code": "US" + }) + acct_repo.create_account(acc) + + source_repo = SourceRepo(t) + source_repo.delete_source(ACCT_ID, DOGGY_ID) + source_repo.delete_source(ACCT_ID, PLANTY_ID) + source_repo.delete_source(ACCT_ID, HUMAN_ID) + + source_repo.create_source(Source.create_human( + HUMAN_ID, + ACCT_ID, + HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", "Mrs Bo", + False, datetime.datetime.utcnow(), None, "Mr. Obtainer", + "18-plus") + )) + source_repo.create_source(Source.create_animal( + DOGGY_ID, + ACCT_ID, + AnimalInfo("Doggy"))) + source_repo.create_source(Source.create_environment( + PLANTY_ID, + ACCT_ID, + EnvironmentInfo("Planty", "The green one"))) + + t.commit() + + @staticmethod + def teardown_test_data(): + with Transaction() as t: + acct_repo = AccountRepo(t) + source_repo = SourceRepo(t) + source_repo.delete_source(ACCT_ID, DOGGY_ID) + source_repo.delete_source(ACCT_ID, PLANTY_ID) + source_repo.delete_source(ACCT_ID, HUMAN_ID) + acct_repo.delete_account(ACCT_ID) + + t.commit() + + @classmethod + def test_get_sources(cls): + resp = cls.client.get('/api/accounts/%s/sources' % ACCT_ID).data + sources = json.loads(resp) + assert len([x for x in sources if x['source_name'] == 'Bo']) == 1 + assert len([x for x in sources if x['source_name'] == 'Doggy']) == 1 + assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 + + @classmethod + def test_surveys(cls): + resp = cls.client.get('/api/accounts/%s/sources' % ACCT_ID).data + + sources = json.loads(resp) + bobo = [x for x in sources if x['source_name'] == 'Bo'][0] + doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] + env = [x for x in sources if x['source_name'] == 'Planty'][0] + + resp = cls.client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, bobo['source_id']), ) + bobo_surveys = json.loads(resp.data) + resp = cls.client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, doggy['source_id'])) + doggy_surveys = json.loads(resp.data) + resp = cls.client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, env['source_id'])) + env_surveys = json.loads(resp.data) + + assert bobo_surveys == [1, 3, 4, 5] + assert doggy_surveys == [2] + assert env_surveys == [] From 8da275fbfe9b1463b22941c973fc05f23718d545 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Fri, 31 Jan 2020 14:25:42 -0800 Subject: [PATCH 05/16] test_integration now runs through both unittest and pytest frameworks --- .../api/tests/test_integration.py | 54 ++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index a0a52b54a..c012da68c 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -28,6 +28,22 @@ def client(request): @pytest.mark.usefixtures("client") class IntegrationTests(TestCase): + def setUp(self): + IntegrationTests.setup_test_data() + app = microsetta_private_api.server.build_app() + self.client = app.app.test_client() + # This isn't perfect, due to possibility of exceptions being thrown + # is there some better pattern I can use to split up what should be + # a 'with' call? + self.client.__enter__() + + def tearDown(self): + # This isn't perfect, due to possibility of exceptions being thrown + # is there some better pattern I can use to split up what should be + # a 'with' call? + self.client.__exit__(None, None, None) + IntegrationTests.teardown_test_data() + @staticmethod def json_converter(o): if isinstance(o, datetime.datetime): @@ -38,8 +54,15 @@ def json_converter(o): def setup_test_data(): with Transaction() as t: acct_repo = AccountRepo(t) + source_repo = SourceRepo(t) + + # Clean up any possible leftovers from failed tests + source_repo.delete_source(ACCT_ID, DOGGY_ID) + source_repo.delete_source(ACCT_ID, PLANTY_ID) + source_repo.delete_source(ACCT_ID, HUMAN_ID) acct_repo.delete_account(ACCT_ID) + # Set up test account with sources acc = Account(ACCT_ID, "foo@baz.com", "standard", @@ -55,11 +78,6 @@ def setup_test_data(): }) acct_repo.create_account(acc) - source_repo = SourceRepo(t) - source_repo.delete_source(ACCT_ID, DOGGY_ID) - source_repo.delete_source(ACCT_ID, PLANTY_ID) - source_repo.delete_source(ACCT_ID, HUMAN_ID) - source_repo.create_source(Source.create_human( HUMAN_ID, ACCT_ID, @@ -90,32 +108,30 @@ def teardown_test_data(): t.commit() - @classmethod - def test_get_sources(cls): - resp = cls.client.get('/api/accounts/%s/sources' % ACCT_ID).data + def test_get_sources(self): + resp = self.client.get('/api/accounts/%s/sources' % ACCT_ID).data sources = json.loads(resp) assert len([x for x in sources if x['source_name'] == 'Bo']) == 1 assert len([x for x in sources if x['source_name'] == 'Doggy']) == 1 assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 - @classmethod - def test_surveys(cls): - resp = cls.client.get('/api/accounts/%s/sources' % ACCT_ID).data + def test_surveys(self): + resp = self.client.get('/api/accounts/%s/sources' % ACCT_ID).data sources = json.loads(resp) bobo = [x for x in sources if x['source_name'] == 'Bo'][0] doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] env = [x for x in sources if x['source_name'] == 'Planty'][0] - resp = cls.client.get( + resp = self.client.get( '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % (ACCT_ID, bobo['source_id']), ) bobo_surveys = json.loads(resp.data) - resp = cls.client.get( + resp = self.client.get( '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % (ACCT_ID, doggy['source_id'])) doggy_surveys = json.loads(resp.data) - resp = cls.client.get( + resp = self.client.get( '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % (ACCT_ID, env['source_id'])) env_surveys = json.loads(resp.data) @@ -123,3 +139,13 @@ def test_surveys(cls): assert bobo_surveys == [1, 3, 4, 5] assert doggy_surveys == [2] assert env_surveys == [] + + # def test_create_new_account(self): + # """ Test: Create a new account using a kit id """ + # response = self.client.post( + # '/api/accounts', + # content_type='application/json', + # data=json.dumps(), + # + # + # cls.client.post() \ No newline at end of file From 99623d033a475ea222a3b996c0e40ec78a54b746 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Fri, 31 Jan 2020 17:16:08 -0800 Subject: [PATCH 06/16] Ability to make mock kits for unit tests, fixed bug in retrieving kits --- microsetta_private_api/repo/kit_repo.py | 37 ++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/microsetta_private_api/repo/kit_repo.py b/microsetta_private_api/repo/kit_repo.py index 95149205f..34eb1a430 100644 --- a/microsetta_private_api/repo/kit_repo.py +++ b/microsetta_private_api/repo/kit_repo.py @@ -19,10 +19,45 @@ def get_kit(self, supplied_kit_id): "ag_kit.ag_kit_id = ag_kit_barcodes.ag_kit_id " "WHERE " "ag_kit.supplied_kit_id = %s", - supplied_kit_id) + (supplied_kit_id,)) rows = cur.fetchall() if len(rows) == 0: return None else: samples = [sample_repo.get_sample(r[1]) for r in rows] return Kit(rows[0][0], samples) + + # NOTE: This should only be used for unit tests! + def create_mock_kit(self, supplied_kit_id): + with self._transaction.cursor() as cur: + kit_id = '77777777-8888-9999-aaaa-bbbbcccccccc' + linker_id = '99999999-aaaa-aaaa-aaaa-bbbbcccccccc' + barcode = '777777777' + cur.execute("INSERT INTO barcode (barcode, status) " + "VALUES(%s, %s)", + (barcode, + 'MOCK SAMPLE FOR UNIT TEST')) + cur.execute("INSERT INTO ag_kit " + "(ag_kit_id, " + "supplied_kit_id, swabs_per_kit) " + "VALUES(%s, %s, %s)", + (kit_id, supplied_kit_id, 1)) + cur.execute("INSERT INTO ag_kit_barcodes " + "(ag_kit_barcode_id, ag_kit_id, barcode) " + "VALUES(%s, %s, %s)", + (linker_id, kit_id, barcode)) + + return kit_id + + def remove_mock_kit(self): + with self._transaction.cursor() as cur: + kit_id = '77777777-8888-9999-aaaa-bbbbcccccccc' + linker_id = '99999999-aaaa-aaaa-aaaa-bbbbcccccccc' + barcode = '777777777' + cur.execute("DELETE FROM ag_kit_barcodes " + "WHERE ag_kit_barcode_id=%s", + (linker_id,)) + cur.execute("DELETE FROM ag_kit WHERE ag_kit_id=%s", + (kit_id,)) + cur.execute("DELETE FROM barcode WHERE barcode = %s", + (barcode,)) From b935afdfc74e1794bea8443c6ae1bfe46759419e Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Fri, 31 Jan 2020 17:17:58 -0800 Subject: [PATCH 07/16] Allowed ag_kit.ag_login_id to be null, as newly created kits need to set this field to null as no account will be associated with them --- microsetta_private_api/db/patches/0051.sql | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/microsetta_private_api/db/patches/0051.sql b/microsetta_private_api/db/patches/0051.sql index 275f5346f..d34669cbe 100644 --- a/microsetta_private_api/db/patches/0051.sql +++ b/microsetta_private_api/db/patches/0051.sql @@ -9,7 +9,6 @@ DROP CONSTRAINT fk_ag_login_surveys; ALTER TABLE ag.source DROP CONSTRAINT fk_source; --- Replaces ag.ag_kit.fk_ag_kit_to_login_id ALTER TABLE ag.ag_kit ADD CONSTRAINT fk_ag_kit_to_login_id FOREIGN KEY (ag_login_id) REFERENCES ag.account (id); @@ -22,4 +21,12 @@ REFERENCES ag.account (id); -- Replaces ag.source.fk_source ALTER TABLE ag.source ADD CONSTRAINT fk_source_account FOREIGN KEY (account_id) -REFERENCES ag.account (id); \ No newline at end of file +REFERENCES ag.account (id); + +-- In the past, each kit could be constructed with an ag_login_id, +-- now, while we have to leave this column around for enabling login +-- by kit_id, we can't really assign kits to accounts, as individual +-- samples are assigned to accounts instead. Thus we have to remove the +-- NOT NULL constraint on this field. +ALTER TABLE ag.ag_kit +ALTER COLUMN ag_login_id DROP NOT NULL; From 6eecbf2c3b2122de396d9d1f096fe69ec4f61588 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Fri, 31 Jan 2020 17:20:22 -0800 Subject: [PATCH 08/16] Building out the testing framework with fuzzing and additional setUp of mock kits/samples. Working through the list of integration tests (yes, they all still fail) --- .../api/tests/test_integration.py | 157 ++++++++++++++++-- 1 file changed, 143 insertions(+), 14 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index c012da68c..308ea5d5d 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -1,5 +1,6 @@ import pytest import microsetta_private_api.server +from microsetta_private_api.repo.kit_repo import KitRepo from microsetta_private_api.repo.transaction import Transaction from microsetta_private_api.repo.account_repo import AccountRepo from microsetta_private_api.model.account import Account @@ -15,17 +16,44 @@ DOGGY_ID = "dddddddd-dddd-dddd-dddd-dddddddddddd" PLANTY_ID = "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee" +SUPPLIED_KIT_ID = "FooFooFoo" + @pytest.fixture(scope="class") def client(request): IntegrationTests.setup_test_data() app = microsetta_private_api.server.build_app() + app.app.testing = True with app.app.test_client() as client: request.cls.client = client yield client IntegrationTests.teardown_test_data() +def check_response(response): + if response.status_code >= 400: + raise Exception("Scary response code: " + str(response.status_code)) + resp_obj = json.loads(response.data) + if isinstance(resp_obj, dict) and resp_obj["message"] is not None: + msg = resp_obj["message"].lower() + if "not" in msg and "implemented" in msg: + raise Exception(response.data) + + +def fuzz(val): + """ A fuzzer for json data """ + if isinstance(val, int): + return val + 7 + if isinstance(val, str): + return "FUZ" + val + "ZY" + if isinstance(val, list): + return ["Q(*.*)Q"] + [fuzz(x) for x in val] + ["P(*.*)p"] + if isinstance(val, dict): + fuzzy = {x : fuzz(y) for x, y in val} + fuzzy['account_type'] = "Voldemort" + return fuzzy + + @pytest.mark.usefixtures("client") class IntegrationTests(TestCase): def setUp(self): @@ -55,12 +83,14 @@ def setup_test_data(): with Transaction() as t: acct_repo = AccountRepo(t) source_repo = SourceRepo(t) + kit_repo = KitRepo(t) # Clean up any possible leftovers from failed tests source_repo.delete_source(ACCT_ID, DOGGY_ID) source_repo.delete_source(ACCT_ID, PLANTY_ID) source_repo.delete_source(ACCT_ID, HUMAN_ID) acct_repo.delete_account(ACCT_ID) + kit_repo.remove_mock_kit() # Set up test account with sources acc = Account(ACCT_ID, @@ -82,7 +112,8 @@ def setup_test_data(): HUMAN_ID, ACCT_ID, HumanInfo("Bo", "bo@bo.com", False, "Mr Bo", "Mrs Bo", - False, datetime.datetime.utcnow(), None, "Mr. Obtainer", + False, datetime.datetime.utcnow(), None, + "Mr. Obtainer", "18-plus") )) source_repo.create_source(Source.create_animal( @@ -94,6 +125,8 @@ def setup_test_data(): ACCT_ID, EnvironmentInfo("Planty", "The green one"))) + kit_repo.create_mock_kit(SUPPLIED_KIT_ID) + t.commit() @staticmethod @@ -101,24 +134,30 @@ def teardown_test_data(): with Transaction() as t: acct_repo = AccountRepo(t) source_repo = SourceRepo(t) + kit_repo = KitRepo(t) source_repo.delete_source(ACCT_ID, DOGGY_ID) source_repo.delete_source(ACCT_ID, PLANTY_ID) source_repo.delete_source(ACCT_ID, HUMAN_ID) acct_repo.delete_account(ACCT_ID) + kit_repo.remove_mock_kit() t.commit() def test_get_sources(self): - resp = self.client.get('/api/accounts/%s/sources' % ACCT_ID).data - sources = json.loads(resp) + resp = self.client.get( + '/api/accounts/%s/sources?language_tag=en_us' % ACCT_ID) + check_response(resp) + sources = json.loads(resp.data) assert len([x for x in sources if x['source_name'] == 'Bo']) == 1 assert len([x for x in sources if x['source_name'] == 'Doggy']) == 1 assert len([x for x in sources if x['source_name'] == 'Planty']) == 1 def test_surveys(self): - resp = self.client.get('/api/accounts/%s/sources' % ACCT_ID).data + resp = self.client.get( + '/api/accounts/%s/sources?language_tag=en_us' % ACCT_ID) + check_response(resp) - sources = json.loads(resp) + sources = json.loads(resp.data) bobo = [x for x in sources if x['source_name'] == 'Bo'][0] doggy = [x for x in sources if x['source_name'] == 'Doggy'][0] env = [x for x in sources if x['source_name'] == 'Planty'][0] @@ -140,12 +179,102 @@ def test_surveys(self): assert doggy_surveys == [2] assert env_surveys == [] - # def test_create_new_account(self): - # """ Test: Create a new account using a kit id """ - # response = self.client.post( - # '/api/accounts', - # content_type='application/json', - # data=json.dumps(), - # - # - # cls.client.post() \ No newline at end of file + def test_create_new_account(self): + """ Test: Create a new account using a kit id """ + response = self.client.post( + '/api/accounts?language_tag=en_us', + content_type='application/json', + data=json.dumps( + { + "address": { + "city": "Springfield", + "country_code": "US", + "post_code": "12345", + "state": "CA", + "street": "123 Main St. E. Apt. 2" + }, + "email": "janedoe@example.com", + "first_name": "Jane", + "last_name": "Doe", + "kit_name": "jb_qhxqe" + } + )) + check_response(response) + + def test_edit_account_info(self): + """ Test: Can we edit account information """ + response = self.client.get( + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,)) + check_response(response) + + acc = json.loads(response.data) + + regular_data = \ + { + "address": { + "street": "123 Dan Lane", + "city": "Danville", + "state": "CA", + "post_code": 12345, + "country_code": "US" + }, + "email": "foo@baz.com", + "first_name": "Dan", + "last_name": "H" + } + + self.assertDictEqual(acc, regular_data, "Check Initial Account Match") + + fuzzy_data = fuzz(regular_data) + + response = self.client.put( + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), + content_type='application/json', + data=json.dumps(fuzzy_data) + ) + check_response(response) + + acc = json.loads(response.data) + # TODO: These actually probably shouldn't match exactly. The api + # should only allow writing of certain fields, so need to check those + # fields were written and others were left out!! + self.assertDictEqual(fuzzy_data, acc, "Check Fuzz Account Match") + + response = self.client.put( + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), + content_type='application/json', + data=json.dumps(regular_data) + ) + check_response(response) + self.assertDictEqual(regular_data, acc, "Check restore to regular") + + def test_add_sample_from_kit(self): + """ Test: Can we add a kit to an existing account? + Note: With the changes in this api, rather than adding a kit + we instead list the samples in that kit, grab an unassociated one, + and then associate that sample with our account + """ + response = self.client.get( + '/api/kits/?language_tag=en_us&kit_name=%s' % SUPPLIED_KIT_ID) + check_response(response) + + unused_barcodes = json.loads(response.data) + barcode = unused_barcodes[0]['sample_barcode'] + + response = self.client.post( + '/api/accounts/%s/sources/%s/samples?language_tag=en_us' % + (ACCT_ID, DOGGY_ID), + content_type='application/json', + data=json.dumps( + { + "sample_id": barcode + }) + ) + check_response(response) + + response = self.client.get( + '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us', + (ACCT_ID, DOGGY_ID, barcode) + ) + check_response(response) + print(response) From 175a42a0334fc915cd6fbc7114da329736f01d35 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Mon, 3 Feb 2020 11:30:27 -0800 Subject: [PATCH 09/16] Added test for account registration. Filled in implementation of account registration that checks for valid kit name then adds the new account --- microsetta_private_api/api/implementation.py | 35 ++++++++- .../api/microsetta_private_api.yaml | 8 ++ .../api/tests/test_integration.py | 76 +++++++++++++------ microsetta_private_api/exceptions.py | 11 +++ microsetta_private_api/repo/account_repo.py | 49 ++++++++---- microsetta_private_api/server.py | 10 +++ 6 files changed, 147 insertions(+), 42 deletions(-) create mode 100644 microsetta_private_api/exceptions.py diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 42899e52c..5246579b8 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -15,6 +15,8 @@ from flask import jsonify, render_template import jwt from base64 import b64decode + +from microsetta_private_api.model.address import Address from microsetta_private_api.repo.transaction import Transaction from microsetta_private_api.repo.account_repo import AccountRepo from microsetta_private_api.repo.source_repo import SourceRepo @@ -23,6 +25,7 @@ from microsetta_private_api.repo.survey_answers_repo import SurveyAnswersRepo from microsetta_private_api.repo.sample_repo import SampleRepo +from microsetta_private_api.model.account import Account from microsetta_private_api.model.source import Source from microsetta_private_api.LEGACY.locale_data import american_gut @@ -68,7 +71,37 @@ def verify_and_decode_token(access_token) -> dict: def register_account(body): - return not_yet_implemented() + # TODO: Do they register with GLOBUS first, then make the account here? + # What should be done with the kit_name? + new_acct_id = str(uuid.uuid4()) + with Transaction() as t: + kit_repo = KitRepo(t) + kit = kit_repo.get_kit(body['kit_name']) + if kit is None: + return jsonify(error=403, text="Incorrect kit_name"), 403 + + acct_repo = AccountRepo(t) + acct_repo.create_account(Account( + new_acct_id, + body['email'], + "standard", + "GLOBUS", # TODO: This is dependent on their login token! + body['first_name'], + body['last_name'], + Address( + body['address']['street'], + body['address']['city'], + body['address']['state'], + body['address']['post_code'], + body['address']['country_code'], + ) + )) + t.commit() + + response = jsonify('') + response.status_code = 201 + response.headers['location'] = '/api/accounts/%s' % new_acct_id + return response def read_account(token_info, account_id): diff --git a/microsetta_private_api/api/microsetta_private_api.yaml b/microsetta_private_api/api/microsetta_private_api.yaml index e8a40b14f..e657067b2 100644 --- a/microsetta_private_api/api/microsetta_private_api.yaml +++ b/microsetta_private_api/api/microsetta_private_api.yaml @@ -45,6 +45,8 @@ paths: $ref: '#/components/schemas/account' '401': $ref: '#/components/responses/401Unauthorized' + '403': + $ref: '#/components/responses/403Forbidden' '422': $ref: '#/components/responses/422UnprocessableEntity' @@ -871,6 +873,12 @@ components: responses: 401Unauthorized: # Can be referenced as '#/components/responses/401Unauthorized' description: Invalid or missing token. + 403Forbidden: # Can be referenced as '#/components/responses/403Forbidden' + description: Incorrect required parameter. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' 404NotFound: # Can be referenced as '#/components/responses/404NotFound' description: The specified resource was not found. 422UnprocessableEntity: diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index 308ea5d5d..c0235bdc6 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -7,6 +7,7 @@ from microsetta_private_api.repo.source_repo import SourceRepo from microsetta_private_api.model.source import \ Source, HumanInfo, AnimalInfo, EnvironmentInfo +from microsetta_private_api.model.address import Address import datetime import json from unittest import TestCase @@ -30,9 +31,12 @@ def client(request): IntegrationTests.teardown_test_data() -def check_response(response): - if response.status_code >= 400: +def check_response(response, expected_status=None): + if expected_status is not None: + assert expected_status == response.status_code + elif response.status_code >= 400: raise Exception("Scary response code: " + str(response.status_code)) + resp_obj = json.loads(response.data) if isinstance(resp_obj, dict) and resp_obj["message"] is not None: msg = resp_obj["message"].lower() @@ -49,7 +53,7 @@ def fuzz(val): if isinstance(val, list): return ["Q(*.*)Q"] + [fuzz(x) for x in val] + ["P(*.*)p"] if isinstance(val, dict): - fuzzy = {x : fuzz(y) for x, y in val} + fuzzy = {x: fuzz(y) for x, y in val} fuzzy['account_type'] = "Voldemort" return fuzzy @@ -99,13 +103,13 @@ def setup_test_data(): "GLOBUS", "Dan", "H", - { - "street": "123 Dan Lane", - "city": "Danville", - "state": "CA", - "post_code": 12345, - "country_code": "US" - }) + Address( + "123 Dan Lane", + "Danville", + "CA", + 12345, + "US" + )) acct_repo.create_account(acc) source_repo.create_source(Source.create_human( @@ -180,27 +184,49 @@ def test_surveys(self): assert env_surveys == [] def test_create_new_account(self): + + # Clean up before the test in case we already have a janedoe + with Transaction() as t: + AccountRepo(t).delete_account_by_email("janedoe@example.com") + t.commit() + """ Test: Create a new account using a kit id """ + acct_json = json.dumps( + { + "address": { + "city": "Springfield", + "country_code": "US", + "post_code": "12345", + "state": "CA", + "street": "123 Main St. E. Apt. 2" + }, + "email": "janedoe@example.com", + "first_name": "Jane", + "last_name": "Doe", + "kit_name": "jb_qhxqe" + }) + + # First register should succeed response = self.client.post( '/api/accounts?language_tag=en_us', content_type='application/json', - data=json.dumps( - { - "address": { - "city": "Springfield", - "country_code": "US", - "post_code": "12345", - "state": "CA", - "street": "123 Main St. E. Apt. 2" - }, - "email": "janedoe@example.com", - "first_name": "Jane", - "last_name": "Doe", - "kit_name": "jb_qhxqe" - } - )) + data=acct_json + ) check_response(response) + # Second should fail with duplicate email 422 + response = self.client.post( + '/api/accounts?language_tag=en_us', + content_type='application/json', + data=acct_json + ) + check_response(response, 422) + + # Clean up after this test so we don't leave the account around + with Transaction() as t: + AccountRepo(t).delete_account_by_email("janedoe@example.com") + t.commit() + def test_edit_account_info(self): """ Test: Can we edit account information """ response = self.client.get( diff --git a/microsetta_private_api/exceptions.py b/microsetta_private_api/exceptions.py new file mode 100644 index 000000000..928944322 --- /dev/null +++ b/microsetta_private_api/exceptions.py @@ -0,0 +1,11 @@ +# Note: werkzeug has error handlers for most http codes, but it doesn't have +# 422. Are we sure this is a standard error code? Anyway, we need a custom +# exception to register for 422 and RepoException will work for it. + + +class RepoException(Exception): + """ + Converts psycopg2 exceptions into messages + that can be displayed to the user + """ + pass diff --git a/microsetta_private_api/repo/account_repo.py b/microsetta_private_api/repo/account_repo.py index bc0a383b9..806b1a839 100644 --- a/microsetta_private_api/repo/account_repo.py +++ b/microsetta_private_api/repo/account_repo.py @@ -1,5 +1,8 @@ +import psycopg2 + from microsetta_private_api.repo.base_repo import BaseRepo from microsetta_private_api.model.account import Account +from microsetta_private_api.exceptions import RepoException class AccountRepo(BaseRepo): @@ -29,11 +32,11 @@ def _row_to_addr(r): @staticmethod def _addr_to_row(addr): - return (addr["street"], - addr["city"], - addr["state"], - addr["post_code"], - addr["country_code"]) + return (addr.street, + addr.city, + addr.state, + addr.post_code, + addr.country_code) @staticmethod def _row_to_account(r): @@ -91,20 +94,34 @@ def update_account(self, account): return cur.rowcount == 1 def create_account(self, account): - with self._transaction.cursor() as cur: - cur.execute("INSERT INTO account (" + - AccountRepo.write_cols + - ") " - "VALUES(" - "%s, %s, " - "%s, %s, " - "%s, %s, " - "%s, %s, %s, %s, %s)", - AccountRepo._account_to_row(account)) - return cur.rowcount == 1 + try: + with self._transaction.cursor() as cur: + cur.execute("INSERT INTO account (" + + AccountRepo.write_cols + + ") " + "VALUES(" + "%s, %s, " + "%s, %s, " + "%s, %s, " + "%s, %s, %s, %s, %s)", + AccountRepo._account_to_row(account)) + return cur.rowcount == 1 + except psycopg2.errors.UniqueViolation as e: + if e.diag.constraint_name == 'idx_account_email': + # TODO: Ugh. Localization of error messages is needed someday. + raise RepoException("Email %s is not available" + % account.email) from e + # Unknown exception, re raise it. + raise e def delete_account(self, account_id): with self._transaction.cursor() as cur: cur.execute("DELETE FROM account WHERE account.id = %s", (account_id,)) return cur.rowcount == 1 + + def delete_account_by_email(self, email): + with self._transaction.cursor() as cur: + cur.execute("DELETE FROM account WHERE account.email = %s", + (email,)) + return cur.rowcount == 1 diff --git a/microsetta_private_api/server.py b/microsetta_private_api/server.py index 170afdf6c..5d80b664f 100644 --- a/microsetta_private_api/server.py +++ b/microsetta_private_api/server.py @@ -1,4 +1,7 @@ #!/usr/bin/env python3 +from flask import jsonify + +from microsetta_private_api.exceptions import RepoException """ Basic flask/connexion-based web application @@ -11,6 +14,10 @@ from microsetta_private_api.util.util import JsonifyDefaultEncoder +def handle_422(repo_exc): + return jsonify(code=422, message=str(repo_exc)), 422 + + def build_app(): # Create the application instance app = connexion.FlaskApp(__name__) @@ -22,6 +29,9 @@ def build_app(): # Note: app.app is the actual Flask application instance, so any Flask # settings have to be set there. app.app.json_encoder = JsonifyDefaultEncoder + + # Set mapping from exception type to response code + app.app.register_error_handler(RepoException, handle_422) return app From 9036605389ff388a679d788fa3c7873845d5e173 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 13:21:57 -0800 Subject: [PATCH 10/16] Implemented read_sample_associations, associate_sample, read_sample_association and integration test: add_sample_from_kit --- microsetta_private_api/api/implementation.py | 21 ++- .../api/tests/test_integration.py | 18 +-- microsetta_private_api/model/sample.py | 1 + microsetta_private_api/repo/kit_repo.py | 2 +- microsetta_private_api/repo/sample_repo.py | 127 +++++++++++++++--- 5 files changed, 137 insertions(+), 32 deletions(-) diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 5246579b8..170a9153d 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -271,11 +271,26 @@ def submit_answered_survey(account_id, source_id, language_tag, body): def read_sample_associations(account_id, source_id): - return not_yet_implemented() + with Transaction() as t: + sample_repo = SampleRepo(t) + samples = sample_repo.get_samples_by_source(account_id, source_id) + + api_samples = [x.to_api() for x in samples] + return jsonify(api_samples), 200 def associate_sample(account_id, source_id, body): - return not_yet_implemented() + with Transaction() as t: + sample_repo = SampleRepo(t) + sample_repo.associate_sample(account_id, + source_id, + body['sample_id']) + t.commit() + response = jsonify('') + response.status_code = 201 + response.headers['location'] = '/api/accounts/%s/sources/%s/samples/%s' % \ + (account_id, source_id, ['sample_id']) + return response def read_sample_association(account_id, source_id, sample_id): @@ -285,7 +300,7 @@ def read_sample_association(account_id, source_id, sample_id): if sample is None: return jsonify(error=404, text="Sample not found"), 404 - return jsonify(sample), 200 + return jsonify(sample.to_api()), 200 def update_sample_association(account_id, source_id, sample_id, body): diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index c0235bdc6..1c7d397fb 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -38,7 +38,7 @@ def check_response(response, expected_status=None): raise Exception("Scary response code: " + str(response.status_code)) resp_obj = json.loads(response.data) - if isinstance(resp_obj, dict) and resp_obj["message"] is not None: + if isinstance(resp_obj, dict) and "message" in resp_obj: msg = resp_obj["message"].lower() if "not" in msg and "implemented" in msg: raise Exception(response.data) @@ -90,11 +90,11 @@ def setup_test_data(): kit_repo = KitRepo(t) # Clean up any possible leftovers from failed tests + kit_repo.remove_mock_kit() source_repo.delete_source(ACCT_ID, DOGGY_ID) source_repo.delete_source(ACCT_ID, PLANTY_ID) source_repo.delete_source(ACCT_ID, HUMAN_ID) acct_repo.delete_account(ACCT_ID) - kit_repo.remove_mock_kit() # Set up test account with sources acc = Account(ACCT_ID, @@ -139,11 +139,11 @@ def teardown_test_data(): acct_repo = AccountRepo(t) source_repo = SourceRepo(t) kit_repo = KitRepo(t) + kit_repo.remove_mock_kit() source_repo.delete_source(ACCT_ID, DOGGY_ID) source_repo.delete_source(ACCT_ID, PLANTY_ID) source_repo.delete_source(ACCT_ID, HUMAN_ID) acct_repo.delete_account(ACCT_ID) - kit_repo.remove_mock_kit() t.commit() @@ -284,8 +284,8 @@ def test_add_sample_from_kit(self): '/api/kits/?language_tag=en_us&kit_name=%s' % SUPPLIED_KIT_ID) check_response(response) - unused_barcodes = json.loads(response.data) - barcode = unused_barcodes[0]['sample_barcode'] + unused_samples = json.loads(response.data) + sample_id = unused_samples[0]['sample_id'] response = self.client.post( '/api/accounts/%s/sources/%s/samples?language_tag=en_us' % @@ -293,14 +293,14 @@ def test_add_sample_from_kit(self): content_type='application/json', data=json.dumps( { - "sample_id": barcode + "sample_id": sample_id }) ) check_response(response) response = self.client.get( - '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us', - (ACCT_ID, DOGGY_ID, barcode) + '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us' % + (ACCT_ID, DOGGY_ID, sample_id) ) - check_response(response) print(response) + check_response(response) diff --git a/microsetta_private_api/model/sample.py b/microsetta_private_api/model/sample.py index 9d471c2d3..dfa2d11fe 100644 --- a/microsetta_private_api/model/sample.py +++ b/microsetta_private_api/model/sample.py @@ -37,6 +37,7 @@ def from_db(cls, sample_id, date_collected, time_collected, def to_api(self): return { + "sample_id": self.id, "sample_barcode": self.barcode, "sample_site": self.site, "sample_locked": self.is_locked, diff --git a/microsetta_private_api/repo/kit_repo.py b/microsetta_private_api/repo/kit_repo.py index 34eb1a430..eaaa439a1 100644 --- a/microsetta_private_api/repo/kit_repo.py +++ b/microsetta_private_api/repo/kit_repo.py @@ -47,7 +47,7 @@ def create_mock_kit(self, supplied_kit_id): "VALUES(%s, %s, %s)", (linker_id, kit_id, barcode)) - return kit_id + return self.get_kit(supplied_kit_id) def remove_mock_kit(self): with self._transaction.cursor() as cur: diff --git a/microsetta_private_api/repo/sample_repo.py b/microsetta_private_api/repo/sample_repo.py index e51961493..71ca61f72 100644 --- a/microsetta_private_api/repo/sample_repo.py +++ b/microsetta_private_api/repo/sample_repo.py @@ -1,34 +1,48 @@ +import werkzeug + from microsetta_private_api.repo.base_repo import BaseRepo from microsetta_private_api.model.sample import Sample +from microsetta_private_api.exceptions import RepoException + class SampleRepo(BaseRepo): def __init__(self, transaction): super().__init__(transaction) - def get_sample(self, sample_id): - with self._transaction.cursor() as cur: + def get_samples_by_source(self, account_id, source_id): + with self._transaction.cursor as cur: cur.execute( "SELECT " - "ag.ag_kit_barcodes.sample_date, " - "ag.ag_kit_barcodes.sample_time, " - "ag.ag_kit_barcodes.site_sampled, " - "ag.ag_kit_barcodes.notes, " - "barcodes.barcode.barcode, " - "barcodes.barcode.scan_date " - "FROM " - "ag.ag_kit_barcodes " - "LEFT JOIN barcodes.barcode ON " - "ag.ag_kit_barcodes.barcode = barcodes.barcode.barcode " - "WHERE ag_kit_barcodes.ag_kit_barcode_id = %s", - (sample_id,)) + "ag_kit_barcodes.ag_kit_barcode_id, " + "ag_kit_barcodes.sample_date, " + "ag_kit_barcodes.sample_time, " + "ag_kit_barcodes.site_sampled, " + "ag_kit_barcodes.notes, " + "ag_kit_barcodes.barcode, " + "barcode.scan_date " + "FROM ag_kit_barcodes " + "LEFT JOIN barcode " + "USING (barcode) " + "LEFT JOIN source " + "ON ag_kit_barcodes.source_id = source.id " + "WHERE " + "source.account_id = %s AND " + "source.id = %s", + (source_id, account_id) + ) - sample_row = cur.fetchone() - if sample_row is None: - return None - - sample_barcode = sample_row[4] + samples = [] + for sample_row in cur.fetchall(): + barcode = sample_row[5] + sample_projects = self._retrieve_projects(barcode) + s = Sample.from_db(*sample_row, + sample_projects) + samples.append(s) + return samples + def _retrieve_projects(self, sample_barcode): + with self._transaction.cursor() as cur: # If there is a sample, we can look for the projects associated # with it. We do this as a secondary query: cur.execute("SELECT barcodes.project.project FROM " @@ -48,7 +62,82 @@ def get_sample(self, sample_id): project_rows = cur.fetchall() sample_projects = [project[0] for project in project_rows] + return sample_projects + + def get_sample(self, sample_id): + with self._transaction.cursor() as cur: + cur.execute( + "SELECT " + "ag.ag_kit_barcodes.sample_date, " + "ag.ag_kit_barcodes.sample_time, " + "ag.ag_kit_barcodes.site_sampled, " + "ag.ag_kit_barcodes.notes, " + "barcodes.barcode.barcode, " + "barcodes.barcode.scan_date " + "FROM " + "ag.ag_kit_barcodes " + "LEFT JOIN barcodes.barcode ON " + "ag.ag_kit_barcodes.barcode = barcodes.barcode.barcode " + "WHERE ag_kit_barcodes.ag_kit_barcode_id = %s", + (sample_id,)) + + sample_row = cur.fetchone() + if sample_row is None: + return None + + sample_barcode = sample_row[4] + sample_projects = self._retrieve_projects(sample_barcode) return Sample.from_db(sample_id, *sample_row, sample_projects) + + # TODO: Should this throw if the sample is already associated with + # another source in the same account? Technically they could disassociate + # the sample first... + def associate_sample(self, account_id, source_id, sample_id): + with self._transaction.cursor() as cur: + cur.execute("SELECT " + "ag_kit_barcode_id, " + "source.account_id, " + "source.id " + "FROM " + "ag_kit_barcodes " + "LEFT OUTER JOIN source " + "ON ag_kit_barcodes.source_id = source.id " + "WHERE " + "ag_kit_barcode_id = %s", + (sample_id,)) + row = cur.fetchone() + if row is None: + raise werkzeug.exceptions.NotFound("No sample ID: %s" % + sample_id) + if row[2] is not None: + if row[1] != account_id: + # This is the case where the sample is already assigned in + # another account + raise RepoException("Sample is already assigned") + else: + # This is the case where the sample is already assigned in + # the same account + self._update_sample_association(sample_id, source_id) + else: + # This is the case where the sample is not yet assigned + self._update_sample_association(sample_id, source_id) + + # TODO: I'm still not entirely happy with the linking between samples and + # sources. The new source_id is direct (and required for environmental + # samples, which have no surveys) but samples can also link to + # surveys which then may link to sources. + # Having multiple pathways to link in the db is a recipe for badness. + # Should something be done with the source_barcodes_surveys table? (which + # itself is required for linking samples to surveys!) + def _update_sample_association(self, sample_id, source_id): + with self._transaction.cursor() as cur: + cur.execute("UPDATE " + "ag_kit_barcodes " + "SET " + "source_id = %s " + "WHERE " + "ag_kit_barcode_id = %s", + (source_id, sample_id)) From 6b3bb1b50b7324c6be0e74d10fbb16a74572d285 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 14:50:14 -0800 Subject: [PATCH 11/16] New test, bobo takes a survey, survey_template_id is now an int in the api, matching the database layer. --- microsetta_private_api/api/implementation.py | 18 ++-- .../api/microsetta_private_api.yaml | 8 +- .../api/tests/test_integration.py | 90 ++++++++++++++++++- microsetta_private_api/repo/sample_repo.py | 2 + .../repo/survey_answers_repo.py | 1 + 5 files changed, 108 insertions(+), 11 deletions(-) diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 170a9153d..0cf8c9080 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -256,18 +256,24 @@ def submit_answered_survey(account_id, source_id, language_tag, body): # TODO: Rename survey_text to survey_model/model to match Vue's naming? with Transaction() as t: survey_answers_repo = SurveyAnswersRepo(t) - success = survey_answers_repo.submit_answered_survey( + survey_answers_id = survey_answers_repo.submit_answered_survey( account_id, source_id, language_tag, body["survey_template_id"], body["survey_text"] ) - if success: - t.commit() - return jsonify(''), 201 - return jsonify(code=422, - message="Could not submit answered survey"), 422 + t.commit() + + response = jsonify('') + response.status_code = 201 + response.headers['location'] = '/api/accounts/%s' \ + '/sources/%s' \ + '/surveys/%s' % \ + (account_id, + source_id, + survey_answers_id) + return response def read_sample_associations(account_id, source_id): diff --git a/microsetta_private_api/api/microsetta_private_api.yaml b/microsetta_private_api/api/microsetta_private_api.yaml index e657067b2..7980ebeb3 100644 --- a/microsetta_private_api/api/microsetta_private_api.yaml +++ b/microsetta_private_api/api/microsetta_private_api.yaml @@ -824,7 +824,7 @@ components: in: path description: Unique id specifying a sample associated with a source schema: - $ref: '#/components/schemas/survey_id' + $ref: '#/components/schemas/sample_id' required: true survey_id: name: survey_id @@ -1115,8 +1115,8 @@ components: # survey template section survey_template_id: - type: string - example: "e928f910-bf11-4cda-93b5-c34c8914b97f" + type: integer + example: 3 survey_template_title: type: string example: "Personal Information" @@ -1213,7 +1213,7 @@ components: example: "69f697cb-8e52-4a4f-8db2-efffcfa186a5" survey_text: # The contents of this string ARE structured, but their structure is not specified in THIS api. - type: string + type: object example: '{ | "1": "Omnivore", | "2": "No", | diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index 1c7d397fb..39e80f5f1 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -1,10 +1,13 @@ import pytest +import werkzeug + import microsetta_private_api.server from microsetta_private_api.repo.kit_repo import KitRepo from microsetta_private_api.repo.transaction import Transaction from microsetta_private_api.repo.account_repo import AccountRepo from microsetta_private_api.model.account import Account from microsetta_private_api.repo.source_repo import SourceRepo +from microsetta_private_api.repo.survey_answers_repo import SurveyAnswersRepo from microsetta_private_api.model.source import \ Source, HumanInfo, AnimalInfo, EnvironmentInfo from microsetta_private_api.model.address import Address @@ -33,7 +36,7 @@ def client(request): def check_response(response, expected_status=None): if expected_status is not None: - assert expected_status == response.status_code + assert response.status_code == expected_status elif response.status_code >= 400: raise Exception("Scary response code: " + str(response.status_code)) @@ -58,6 +61,29 @@ def fuzz(val): return fuzzy +def fuzz_field(field, model): + if field['type'] == "input" or field['type'] == "textArea": + model[field['id']] = 'bo' + if field['type'] == "select": + model[field['id']] = field['values'][0] + if field['type'] == 'checklist': + model[field['id']] = [field['values'][0]] + + +def fuzz_form(form): + """ Fills in a vue form with junk data """ + model = {} + if form['fields'] is not None: + for field in form['fields']: + fuzz_field(field, model) + if form['groups'] is not None: + for group in form['groups']: + if group['fields'] is not None: + for field in group['fields']: + fuzz_field(field, model) + return model + + @pytest.mark.usefixtures("client") class IntegrationTests(TestCase): def setUp(self): @@ -88,9 +114,15 @@ def setup_test_data(): acct_repo = AccountRepo(t) source_repo = SourceRepo(t) kit_repo = KitRepo(t) + survey_answers_repo = SurveyAnswersRepo(t) # Clean up any possible leftovers from failed tests kit_repo.remove_mock_kit() + + answers = survey_answers_repo.list_answered_surveys(ACCT_ID, + HUMAN_ID) + for survey_id in answers: + survey_answers_repo.delete_answered_survey(ACCT_ID, survey_id) source_repo.delete_source(ACCT_ID, DOGGY_ID) source_repo.delete_source(ACCT_ID, PLANTY_ID) source_repo.delete_source(ACCT_ID, HUMAN_ID) @@ -183,6 +215,62 @@ def test_surveys(self): assert doggy_surveys == [2] assert env_surveys == [] + def test_bobo_takes_a_survey(self): + """ + Check that a user can login to an account, + list sources, + pick a source, + list surveys for that source, + pick a survey, + retrieve that survey + submit answers to that survey + """ + resp = self.client.get( + '/api/accounts/%s/sources?language_tag=en_us' % ACCT_ID) + check_response(resp) + sources = json.loads(resp.data) + bobo = [x for x in sources if x['source_name'] == 'Bo'][0] + resp = self.client.get( + '/api/accounts/%s/sources/%s/survey_templates?language_tag=en_us' % + (ACCT_ID, bobo['source_id'])) + bobo_surveys = json.loads(resp.data) + chosen_survey = bobo_surveys[0] + resp = self.client.get( + '/api/accounts/%s/sources/%s/survey_templates/%s' + '?language_tag=en_us' % + (ACCT_ID, bobo['source_id'], chosen_survey)) + check_response(resp) + + model = fuzz_form(json.loads(resp.data)) + resp = self.client.post( + '/api/accounts/%s/sources/%s/surveys?language_tag=en_us' + % (ACCT_ID, bobo['source_id']), + content_type='application/json', + data=json.dumps( + { + 'survey_template_id': chosen_survey, + 'survey_text': model + }) + ) + check_response(resp, 201) + loc = resp.headers.get("Location") + url = werkzeug.urls.url_parse(loc) + survey_id = url.path.split('/')[-1] + + # TODO: Need a sanity check, is returned Location supposed to specify + # query parameters? + resp = self.client.get(loc + "?language_tag=en_us") + check_response(resp) + retrieved_survey = json.loads(resp.data) + self.assertDictEqual(retrieved_survey, model) + + # Clean up after the new survey + with Transaction() as t: + repo = SurveyAnswersRepo(t) + found = repo.delete_answered_survey(ACCT_ID, survey_id) + assert found + t.commit() + def test_create_new_account(self): # Clean up before the test in case we already have a janedoe diff --git a/microsetta_private_api/repo/sample_repo.py b/microsetta_private_api/repo/sample_repo.py index 71ca61f72..4bce02e2f 100644 --- a/microsetta_private_api/repo/sample_repo.py +++ b/microsetta_private_api/repo/sample_repo.py @@ -95,6 +95,8 @@ def get_sample(self, sample_id): # TODO: Should this throw if the sample is already associated with # another source in the same account? Technically they could disassociate # the sample first... + # TODO: Should this throw if the sample is "locked"? + # ie: If barcodes.barcode.scan_date is not null? def associate_sample(self, account_id, source_id, sample_id): with self._transaction.cursor() as cur: cur.execute("SELECT " diff --git a/microsetta_private_api/repo/survey_answers_repo.py b/microsetta_private_api/repo/survey_answers_repo.py index 6178a48f0..603532453 100644 --- a/microsetta_private_api/repo/survey_answers_repo.py +++ b/microsetta_private_api/repo/survey_answers_repo.py @@ -161,6 +161,7 @@ def delete_answered_survey(self, acct_id, survey_id): cur.execute("DELETE FROM ag_login_surveys WHERE " "ag_login_id = %s AND survey_id = %s", (acct_id, survey_id)) + return True # True if this account owns this survey_answer_id, else False def _acct_owns_survey(self, acct_id, survey_id): From 36bf559458fe259af5b67116cfce69915f615c2d Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 16:32:44 -0800 Subject: [PATCH 12/16] Newly working test for update account info --- microsetta_private_api/api/implementation.py | 14 +++++-- .../api/tests/test_integration.py | 38 +++++++++++++++---- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 0cf8c9080..5fdf3ab85 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -125,10 +125,16 @@ def update_account(account_id, body): # TODO: add 422 handling - acc.first_name = body["first_name"] - acc.last_name = body["last_name"] - acc.email = body["email"] - acc.address = body["address"] + acc.first_name = body['first_name'] + acc.last_name = body['last_name'] + acc.email = body['email'] + acc.address = Address( + body['address']['street'], + body['address']['city'], + body['address']['state'], + body['address']['post_code'], + body['address']['country_code'] + ) acct_repo.update_account(acc) t.commit() diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index 39e80f5f1..5bd0992f8 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -56,8 +56,7 @@ def fuzz(val): if isinstance(val, list): return ["Q(*.*)Q"] + [fuzz(x) for x in val] + ["P(*.*)p"] if isinstance(val, dict): - fuzzy = {x: fuzz(y) for x, y in val} - fuzzy['account_type'] = "Voldemort" + fuzzy = {x: fuzz(y) for x, y in val.items()} return fuzzy @@ -318,18 +317,20 @@ def test_create_new_account(self): def test_edit_account_info(self): """ Test: Can we edit account information """ response = self.client.get( - '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,)) + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), + headers={'Authorization': 'Bearer PutMySecureOauthTokenHere'}) check_response(response) acc = json.loads(response.data) regular_data = \ { + "account_type": "standard", "address": { "street": "123 Dan Lane", "city": "Danville", "state": "CA", - "post_code": 12345, + "post_code": "12345", "country_code": "US" }, "email": "foo@baz.com", @@ -337,29 +338,52 @@ def test_edit_account_info(self): "last_name": "H" } + # Hard to guess these two, so let's pop em out + acc.pop("creation_time") + acc.pop("update_time") self.assertDictEqual(acc, regular_data, "Check Initial Account Match") fuzzy_data = fuzz(regular_data) + fuzzy_data['account_type'] = "Voldemort" + print("---\nYou should see a validation error in unittest:") response = self.client.put( '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), content_type='application/json', data=json.dumps(fuzzy_data) ) + print("---") + # Check that malicious user can't write any field they want + check_response(response, 400) + + # Check that data can be written once request is not malformed + fuzzy_data.pop('account_type') + response = self.client.put( + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), + content_type='application/json', + data=json.dumps(fuzzy_data) + ) + check_response(response) acc = json.loads(response.data) - # TODO: These actually probably shouldn't match exactly. The api - # should only allow writing of certain fields, so need to check those - # fields were written and others were left out!! + fuzzy_data['account_type'] = 'standard' + acc.pop('creation_time') + acc.pop('update_time') self.assertDictEqual(fuzzy_data, acc, "Check Fuzz Account Match") + regular_data.pop('account_type') response = self.client.put( '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), content_type='application/json', data=json.dumps(regular_data) ) check_response(response) + + acc = json.loads(response.data) + acc.pop('creation_time') + acc.pop('update_time') + regular_data['account_type'] = 'standard' self.assertDictEqual(regular_data, acc, "Check restore to regular") def test_add_sample_from_kit(self): From 6c560ac004b7cc27afce59be7da3fa9e873cc44b Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 18:54:21 -0800 Subject: [PATCH 13/16] Added source_name to non human sources. Test and implementation for creation of sources --- microsetta_private_api/api/implementation.py | 66 ++++++++-- .../api/microsetta_private_api.yaml | 2 + .../api/tests/test_integration.py | 116 ++++++++++++++++-- microsetta_private_api/model/account.py | 1 + microsetta_private_api/model/source.py | 43 +++++-- microsetta_private_api/repo/source_repo.py | 4 +- 6 files changed, 199 insertions(+), 33 deletions(-) diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 5fdf3ab85..32ae10643 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -27,12 +27,14 @@ from microsetta_private_api.model.account import Account from microsetta_private_api.model.source import Source +from microsetta_private_api.model.source import human_info_from_api from microsetta_private_api.LEGACY.locale_data import american_gut from microsetta_private_api.util import vue_adapter import uuid import json +from datetime import date TOKEN_KEY = "QvMWMnlOqBbNsM88AMxpzcJMbBUu/w8U9joIaNYjuEbwEYhLIB5FqEoFWnfLN3JZN4SD0LAtZOwFNqyMLmNruBLqEvbpjQzM6AY+BfXGxDVFL65c9Xw8ocd6t1nF6YvTpHGB4NJhUwngjIQmFx+6TCa5wArtEqUeoIc1ukVTYbioRkxzi5ju8cc9/PoInB0c7wugMz5ihAPWohpDc4kCotYv7C2K/e9J9CPdwbiLJKYKxO4zSQAqk+Sj4wRcn7bJqIOIT6BlvvnzRGXYG33qXAxGylM4UySj7ltwSGOIY0/JUvKEej3fX17C8wWtJvrjbFQacNhoglqfWq2GeOdRSA== " # noqa: E501 TEMP_ACCESS_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJodHRwOi8vbXlhcHAuY29tLyIsInN1YiI6InVzZXJzL3VzZXIxMjM0Iiwic2NvcGUiOiJzZWxmLCBhZG1pbnMiLCJqdGkiOiJkMzBkMzA5ZS1iZTQ5LTRjOWEtYjdhYi1hMGU3MTIwYmFlZDMiLCJpYXQiOjE1NzIzNzY4OTUsImV4cCI6MTU3MjM4MDQ5NX0.EMooERuy2Z4tC_TsXJe6Vx8yCgzTzI_qh84a5DsKPRw" # noqa: E501 @@ -96,11 +98,12 @@ def register_account(body): body['address']['country_code'], ) )) + new_acct = acct_repo.get_account(new_acct_id) t.commit() - response = jsonify('') + response = jsonify(new_acct.to_api()) response.status_code = 201 - response.headers['location'] = '/api/accounts/%s' % new_acct_id + response.headers['Location'] = '/api/accounts/%s' % new_acct_id return response @@ -154,14 +157,32 @@ def create_source(account_id, body): with Transaction() as t: source_repo = SourceRepo(t) source_id = str(uuid.uuid4()) - source_info = None - new_source = Source.from_json(source_id, account_id, source_info) + + if body['source_type'] == 'human': + # TODO: Unfortunately, humans require a lot of special handling, + # and we started mixing Source calls used for transforming to/ + # from the database with source calls to/from the api. + # Would be nice to split this out better. + new_source = Source(source_id, + account_id, + 'human', + human_info_from_api(body, + consent_date=date.today(), + date_revoked=None)) + else: + new_source = Source.build_source(source_id, account_id, body) + source_repo.create_source(new_source) + # Must pull from db to get creation_time, update_time s = source_repo.get_source(account_id, new_source.id) t.commit() - # TODO: What about 404 and 422 errors? - return jsonify(s.to_api()), 200 + + response = jsonify(s.to_api()) + response.status_code = 201 + response.headers['Location'] = '/api/accounts/%s/sources/%s' % \ + (account_id, source_id) + return response def read_source(account_id, source_id): @@ -273,7 +294,7 @@ def submit_answered_survey(account_id, source_id, language_tag, body): response = jsonify('') response.status_code = 201 - response.headers['location'] = '/api/accounts/%s' \ + response.headers['Location'] = '/api/accounts/%s' \ '/sources/%s' \ '/surveys/%s' % \ (account_id, @@ -300,7 +321,7 @@ def associate_sample(account_id, source_id, body): t.commit() response = jsonify('') response.status_code = 201 - response.headers['location'] = '/api/accounts/%s/sources/%s/samples/%s' % \ + response.headers['Location'] = '/api/accounts/%s/sources/%s/samples/%s' % \ (account_id, source_id, ['sample_id']) return response @@ -361,12 +382,33 @@ def render_consent_doc(account_id): def create_human_source_from_consent(account_id, body): - # A human source's participant name becomes the source name. Note pop - # removes the existing `participant_name` key if exists, else errors. + # Must convert consent form body into object processable by create_source. + # Not adding any error handling here because if 'participant_name' isn't # here, we SHOULD be getting an error. - body['source_name'] = body.pop('participant_name') + source = { + 'source_type': "human", + 'source_name': body['participant_name'], + 'consent': { + 'participant_email': body['participant_email'], + 'age_range': body['age_range'] + } + } + + child_keys = ['parent_1_name', 'parent_2_name', + 'deceased_parent', 'obtainer_name'] + + any_in = False + for key in child_keys: + if key in body: + any_in = True + + if any_in: + source['consent']['child_info'] = {} + for key in child_keys: + if key in body: + source['consent']['child_info'][key] = body[key] # NB: Don't expect to handle errors 404, 422 in this function; expect to # farm out to `create_source` - return create_source(account_id, body) + return create_source(account_id, source) diff --git a/microsetta_private_api/api/microsetta_private_api.yaml b/microsetta_private_api/api/microsetta_private_api.yaml index 7980ebeb3..4bd75d686 100644 --- a/microsetta_private_api/api/microsetta_private_api.yaml +++ b/microsetta_private_api/api/microsetta_private_api.yaml @@ -1073,6 +1073,8 @@ components: source_type: type: string enum: [animal, environmental] + source_name: + type: string source_description: type: string nullable: true diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index 5bd0992f8..fecddddc2 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -40,11 +40,12 @@ def check_response(response, expected_status=None): elif response.status_code >= 400: raise Exception("Scary response code: " + str(response.status_code)) - resp_obj = json.loads(response.data) - if isinstance(resp_obj, dict) and "message" in resp_obj: - msg = resp_obj["message"].lower() - if "not" in msg and "implemented" in msg: - raise Exception(response.data) + if response.headers.get("Content-Type") == "application/json": + resp_obj = json.loads(response.data) + if isinstance(resp_obj, dict) and "message" in resp_obj: + msg = resp_obj["message"].lower() + if "not" in msg and "implemented" in msg: + raise Exception(response.data) def fuzz(val): @@ -122,9 +123,8 @@ def setup_test_data(): HUMAN_ID) for survey_id in answers: survey_answers_repo.delete_answered_survey(ACCT_ID, survey_id) - source_repo.delete_source(ACCT_ID, DOGGY_ID) - source_repo.delete_source(ACCT_ID, PLANTY_ID) - source_repo.delete_source(ACCT_ID, HUMAN_ID) + for source in source_repo.get_sources_in_account(ACCT_ID): + source_repo.delete_source(ACCT_ID, source.id) acct_repo.delete_account(ACCT_ID) # Set up test account with sources @@ -154,7 +154,7 @@ def setup_test_data(): source_repo.create_source(Source.create_animal( DOGGY_ID, ACCT_ID, - AnimalInfo("Doggy"))) + AnimalInfo("Doggy", "Doggy The Dog"))) source_repo.create_source(Source.create_environment( PLANTY_ID, ACCT_ID, @@ -301,7 +301,18 @@ def test_create_new_account(self): ) check_response(response) - # Second should fail with duplicate email 422 + # TODO: Is it weird that we return the new object AND its location? + + # And should give us the account with ID and the location of it + loc = response.headers.get("Location") + url = werkzeug.urls.url_parse(loc) + acct_id_from_loc = url.path.split('/')[-1] + new_acct = json.loads(response.data) + acct_id_from_obj = new_acct['account_id'] + assert acct_id_from_loc is not None + assert acct_id_from_loc == acct_id_from_obj + + # Second register should fail with duplicate email 422 response = self.client.post( '/api/accounts?language_tag=en_us', content_type='application/json', @@ -325,6 +336,7 @@ def test_edit_account_info(self): regular_data = \ { + "account_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff", "account_type": "standard", "address": { "street": "123 Dan Lane", @@ -343,6 +355,7 @@ def test_edit_account_info(self): acc.pop("update_time") self.assertDictEqual(acc, regular_data, "Check Initial Account Match") + regular_data.pop("account_id") fuzzy_data = fuzz(regular_data) fuzzy_data['account_type'] = "Voldemort" @@ -368,6 +381,7 @@ def test_edit_account_info(self): acc = json.loads(response.data) fuzzy_data['account_type'] = 'standard' + fuzzy_data["account_id"] = "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff" acc.pop('creation_time') acc.pop('update_time') self.assertDictEqual(fuzzy_data, acc, "Check Fuzz Account Match") @@ -384,6 +398,8 @@ def test_edit_account_info(self): acc.pop('creation_time') acc.pop('update_time') regular_data['account_type'] = 'standard' + regular_data["account_id"] = "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff" + self.assertDictEqual(regular_data, acc, "Check restore to regular") def test_add_sample_from_kit(self): @@ -414,5 +430,83 @@ def test_add_sample_from_kit(self): '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us' % (ACCT_ID, DOGGY_ID, sample_id) ) - print(response) check_response(response) + + def test_create_non_human_sources(self): + # TODO: Looks like the 201 for sources are specified to + # both return a Location header and the newly created object. This + # seems inconsistent maybe? Consistent with Account, inconsistent + # with survey_answers and maybe source+sample assocations? What's + # right? + + kitty = { + "source_type": "animal", + "source_name": "Fluffy", + "source_description": "FLUFFERNUTTER!!!" + } + desky = { + "source_type": "environmental", + "source_name": "The Desk", + "source_description": "It's a desk." + } + + for new_source in [kitty, desky]: + resp = self.client.post( + '/api/accounts/%s/sources?language_tag=en_us' % (ACCT_ID,), + content_type='application/json', + data=json.dumps(new_source) + ) + + check_response(resp) + loc = resp.headers.get("Location") + url = werkzeug.urls.url_parse(loc) + source_id_from_loc = url.path.split('/')[-1] + new_source = json.loads(resp.data) + source_id_from_obj = new_source['source_id'] + assert source_id_from_loc is not None + assert source_id_from_obj == source_id_from_obj + + # TODO: It would be standard to make a test database and delete it + # or keep it entirely in memory. But the change scripts add in + # too much data to a default database to make this feasible for + # quickly running tests during development. Can we do better than + # this pattern of adding and immediately deleting data during + # testing? The cleanup is not particularly robust + + # Clean Up by deleting the new sources + # TODO: Do I -really- need to specify a language_tag to delete??? + self.client.delete(loc + "?language_tag=en_us") + + def test_create_human_source(self): + """To add a human source, we need to get consent""" + resp = self.client.get('/api/accounts/%s/consent?language_tag=en_us' % + (ACCT_ID,)) + check_response(resp) + + # TODO: This should probably fail as it doesn't perfectly match one of + # the four variants of consent that can be passed in. Split it up? + resp = self.client.post( + '/api/accounts/%s/consent?language_tag=en_us' % + (ACCT_ID,), + content_type='application/x-www-form-urlencoded', + data="age_range=18-plus&" + "participant_name=Joe%20Schmoe&" + "participant_email=joe%40schmoe%2Ecom&" + "parent_1_name=Mr%2E%20Schmoe&" + "parent_2_name=Mrs%2E%20Schmoe&" + "deceased_parent=false&" + "obtainer_name=MojoJojo" + ) + check_response(resp, 201) + + check_response(resp) + loc = resp.headers.get("Location") + url = werkzeug.urls.url_parse(loc) + source_id_from_loc = url.path.split('/')[-1] + new_source = json.loads(resp.data) + source_id_from_obj = new_source['source_id'] + assert source_id_from_loc is not None + assert source_id_from_obj == source_id_from_obj + + self.client.delete(loc + "?language_tag=en_us") + diff --git a/microsetta_private_api/model/account.py b/microsetta_private_api/model/account.py index 556971df4..c088cd6e6 100644 --- a/microsetta_private_api/model/account.py +++ b/microsetta_private_api/model/account.py @@ -21,6 +21,7 @@ def to_api(self): # api users are not given the account id # or the auth_provider return { + "account_id": self.id, "first_name": self.first_name, "last_name": self.last_name, "email": self.email, diff --git a/microsetta_private_api/model/source.py b/microsetta_private_api/model/source.py index 1afc0aaa4..8435a31f6 100644 --- a/microsetta_private_api/model/source.py +++ b/microsetta_private_api/model/source.py @@ -1,11 +1,35 @@ import json + +from microsetta_private_api.exceptions import RepoException from microsetta_private_api.model.model_base import ModelBase +def human_info_from_api(human_source, consent_date, date_revoked): + consent = human_source["consent"] + age_range = consent['age_range'] + is_juvenile = age_range in ['0-6', '7-12', '13-17'] + if is_juvenile: + child_info = consent.get("child_info") + else: + child_info = {} + + return HumanInfo( + human_source["source_name"], + consent["participant_email"], + is_juvenile, + child_info.get("parent_1_name"), + child_info.get("parent_2_name"), + child_info.get("deceased_parent"), + consent_date, + date_revoked, + child_info.get("obtainer_name"), + age_range) + + def human_decoder(obj): if isinstance(obj, dict): return HumanInfo( - obj["name"], + obj["source_name"], obj["email"], obj["is_juvenile"], obj["parent1_name"], @@ -20,13 +44,13 @@ def human_decoder(obj): def animal_decoder(obj): if isinstance(obj, dict): - return AnimalInfo(obj["name"]) + return AnimalInfo(obj["source_name"], obj["source_description"]) return obj def environment_decoder(obj): if isinstance(obj, dict): - return EnvironmentInfo(obj["name"], obj["description"]) + return EnvironmentInfo(obj["source_name"], obj["source_description"]) return obj @@ -47,8 +71,9 @@ def __init__(self, name, email, is_juvenile, class AnimalInfo: - def __init__(self, name): + def __init__(self, name, description): self.name = name + self.description = description class EnvironmentInfo: @@ -122,10 +147,12 @@ def create_environment(cls, source_id, account_id, env_info): env_info) @classmethod - def from_json(cls, source_id, account_id, typed_json_data): - decoder_hook = DECODER_HOOKS[typed_json_data["source_type"]] - return cls(source_id, account_id, typed_json_data["source_type"], - json.loads(typed_json_data, object_hook=decoder_hook)) + def build_source(cls, source_id, account_id, source_info_dict): + decoder_hook = DECODER_HOOKS[source_info_dict["source_type"]] + return cls(source_id, + account_id, + source_info_dict["source_type"], + decoder_hook(source_info_dict)) DECODER_HOOKS = { diff --git a/microsetta_private_api/repo/source_repo.py b/microsetta_private_api/repo/source_repo.py index 664925d1f..cc313d06a 100644 --- a/microsetta_private_api/repo/source_repo.py +++ b/microsetta_private_api/repo/source_repo.py @@ -27,7 +27,7 @@ def __init__(self, transaction): def _row_to_source(r): hook = DECODER_HOOKS[r['source_type']] source_data = { - 'name': r['source_name'], + 'source_name': r['source_name'], 'email': r['participant_email'], 'is_juvenile': r['is_juvenile'], 'parent1_name': r['parent_1_name'], @@ -37,7 +37,7 @@ def _row_to_source(r): 'date_revoked': r['date_revoked'], 'assent_obtainer': r['assent_obtainer'], 'age_range': r['age_range'], - 'description': r['description'] + 'source_description': r['description'] } return Source(r[0], r[1], r[2], hook(source_data)) From 7210ff4b0a56d9c0e95715a63325012a0b7cbef4 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 18:57:14 -0800 Subject: [PATCH 14/16] flake8. Grrrr. --- microsetta_private_api/api/tests/test_integration.py | 1 - microsetta_private_api/model/source.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index fecddddc2..f8438f6be 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -509,4 +509,3 @@ def test_create_human_source(self): assert source_id_from_obj == source_id_from_obj self.client.delete(loc + "?language_tag=en_us") - diff --git a/microsetta_private_api/model/source.py b/microsetta_private_api/model/source.py index 8435a31f6..d66c26c24 100644 --- a/microsetta_private_api/model/source.py +++ b/microsetta_private_api/model/source.py @@ -1,6 +1,3 @@ -import json - -from microsetta_private_api.exceptions import RepoException from microsetta_private_api.model.model_base import ModelBase From d3ed1fde409d9e938dd928c9b53ee83d606c4eed Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Tue, 4 Feb 2020 19:13:38 -0800 Subject: [PATCH 15/16] Sample retrieval now validates the account and source at the repo layer --- microsetta_private_api/api/implementation.py | 2 +- .../api/tests/test_integration.py | 15 ++++++ microsetta_private_api/repo/kit_repo.py | 2 +- microsetta_private_api/repo/sample_repo.py | 50 +++++++++++++++++-- 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 32ae10643..e7d355825 100644 --- a/microsetta_private_api/api/implementation.py +++ b/microsetta_private_api/api/implementation.py @@ -329,7 +329,7 @@ def associate_sample(account_id, source_id, body): def read_sample_association(account_id, source_id, sample_id): with Transaction() as t: sample_repo = SampleRepo(t) - sample = sample_repo.get_sample(sample_id) + sample = sample_repo.get_sample(account_id, source_id, sample_id) if sample is None: return jsonify(error=404, text="Sample not found"), 404 diff --git a/microsetta_private_api/api/tests/test_integration.py b/microsetta_private_api/api/tests/test_integration.py index f8438f6be..02235dd58 100644 --- a/microsetta_private_api/api/tests/test_integration.py +++ b/microsetta_private_api/api/tests/test_integration.py @@ -426,12 +426,27 @@ def test_add_sample_from_kit(self): ) check_response(response) + # Check that we can now see this sample response = self.client.get( '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us' % (ACCT_ID, DOGGY_ID, sample_id) ) check_response(response) + # Check that we can't see this sample from outside the account/source + NOT_ACCT_ID = "12341234-1234-1234-1234-123412341234" + response = self.client.get( + '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us' % + (NOT_ACCT_ID, DOGGY_ID, sample_id) + ) + check_response(response, 404) + + response = self.client.get( + '/api/accounts/%s/sources/%s/samples/%s?language_tag=en_us' % + (ACCT_ID, HUMAN_ID, sample_id) + ) + check_response(response, 404) + def test_create_non_human_sources(self): # TODO: Looks like the 201 for sources are specified to # both return a Location header and the newly created object. This diff --git a/microsetta_private_api/repo/kit_repo.py b/microsetta_private_api/repo/kit_repo.py index eaaa439a1..409a810c8 100644 --- a/microsetta_private_api/repo/kit_repo.py +++ b/microsetta_private_api/repo/kit_repo.py @@ -24,7 +24,7 @@ def get_kit(self, supplied_kit_id): if len(rows) == 0: return None else: - samples = [sample_repo.get_sample(r[1]) for r in rows] + samples = [sample_repo._get_sample_by_id(r[1]) for r in rows] return Kit(rows[0][0], samples) # NOTE: This should only be used for unit tests! diff --git a/microsetta_private_api/repo/sample_repo.py b/microsetta_private_api/repo/sample_repo.py index 4bce02e2f..ae23c5461 100644 --- a/microsetta_private_api/repo/sample_repo.py +++ b/microsetta_private_api/repo/sample_repo.py @@ -64,7 +64,9 @@ def _retrieve_projects(self, sample_barcode): sample_projects = [project[0] for project in project_rows] return sample_projects - def get_sample(self, sample_id): + + def _get_sample_by_id(self, sample_id): + """ Do not use from api layer, you must validate account and source.""" with self._transaction.cursor() as cur: cur.execute( "SELECT " @@ -75,10 +77,15 @@ def get_sample(self, sample_id): "barcodes.barcode.barcode, " "barcodes.barcode.scan_date " "FROM " - "ag.ag_kit_barcodes " - "LEFT JOIN barcodes.barcode ON " + "ag.ag_kit_barcodes " + "LEFT JOIN barcodes.barcode " + "ON " "ag.ag_kit_barcodes.barcode = barcodes.barcode.barcode " - "WHERE ag_kit_barcodes.ag_kit_barcode_id = %s", + "LEFT JOIN source " + "ON " + "ag.ag_kit_barcodes.source_id = source.id " + "WHERE " + "ag_kit_barcodes.ag_kit_barcode_id = %s", (sample_id,)) sample_row = cur.fetchone() @@ -92,6 +99,41 @@ def get_sample(self, sample_id): *sample_row, sample_projects) + def get_sample(self, account_id, source_id, sample_id): + with self._transaction.cursor() as cur: + cur.execute( + "SELECT " + "ag.ag_kit_barcodes.sample_date, " + "ag.ag_kit_barcodes.sample_time, " + "ag.ag_kit_barcodes.site_sampled, " + "ag.ag_kit_barcodes.notes, " + "barcodes.barcode.barcode, " + "barcodes.barcode.scan_date " + "FROM " + "ag.ag_kit_barcodes " + "LEFT JOIN barcodes.barcode " + "ON " + "ag.ag_kit_barcodes.barcode = barcodes.barcode.barcode " + "LEFT JOIN source " + "ON " + "ag.ag_kit_barcodes.source_id = source.id " + "WHERE " + "ag_kit_barcodes.ag_kit_barcode_id = %s AND " + "source.id = %s AND " + "source.account_id = %s", + (sample_id, source_id, account_id)) + + sample_row = cur.fetchone() + if sample_row is None: + return None + + sample_barcode = sample_row[4] + sample_projects = self._retrieve_projects(sample_barcode) + + return Sample.from_db(sample_id, + *sample_row, + sample_projects) + # TODO: Should this throw if the sample is already associated with # another source in the same account? Technically they could disassociate # the sample first... From c7aefa286c344746660ade1e4a0fa6fa273ad6c0 Mon Sep 17 00:00:00 2001 From: dhakim87 Date: Thu, 6 Feb 2020 11:32:15 -0800 Subject: [PATCH 16/16] flake8 --- microsetta_private_api/repo/sample_repo.py | 1 - 1 file changed, 1 deletion(-) diff --git a/microsetta_private_api/repo/sample_repo.py b/microsetta_private_api/repo/sample_repo.py index ae23c5461..3046c913c 100644 --- a/microsetta_private_api/repo/sample_repo.py +++ b/microsetta_private_api/repo/sample_repo.py @@ -64,7 +64,6 @@ def _retrieve_projects(self, sample_barcode): sample_projects = [project[0] for project in project_rows] return sample_projects - def _get_sample_by_id(self, sample_id): """ Do not use from api layer, you must validate account and source.""" with self._transaction.cursor() as cur: