diff --git a/microsetta_private_api/api/implementation.py b/microsetta_private_api/api/implementation.py index 42899e52c..e7d355825 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,13 +25,16 @@ 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.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 @@ -68,7 +73,38 @@ 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'], + ) + )) + new_acct = acct_repo.get_account(new_acct_id) + t.commit() + + response = jsonify(new_acct.to_api()) + response.status_code = 201 + response.headers['Location'] = '/api/accounts/%s' % new_acct_id + return response def read_account(token_info, account_id): @@ -92,10 +128,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() @@ -115,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): @@ -223,36 +283,57 @@ 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): - 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): 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 - return jsonify(sample), 200 + return jsonify(sample.to_api()), 200 def update_sample_association(account_id, source_id, sample_id, body): @@ -301,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 e8a40b14f..4bd75d686 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' @@ -822,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 @@ -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: @@ -1065,6 +1073,8 @@ components: source_type: type: string enum: [animal, environmental] + source_name: + type: string source_description: type: string nullable: true @@ -1107,8 +1117,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" @@ -1205,7 +1215,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 new file mode 100644 index 000000000..02235dd58 --- /dev/null +++ b/microsetta_private_api/api/tests/test_integration.py @@ -0,0 +1,526 @@ +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 +import datetime +import json +from unittest import TestCase + +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" + +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, expected_status=None): + if expected_status is not None: + assert response.status_code == expected_status + elif response.status_code >= 400: + raise Exception("Scary response code: " + str(response.status_code)) + + 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): + """ 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.items()} + 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): + 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): + return str(o) + return o.__dict__ + + @staticmethod + def setup_test_data(): + with Transaction() as t: + 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) + 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 + acc = Account(ACCT_ID, + "foo@baz.com", + "standard", + "GLOBUS", + "Dan", + "H", + Address( + "123 Dan Lane", + "Danville", + "CA", + 12345, + "US" + )) + acct_repo.create_account(acc) + + 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", "Doggy The Dog"))) + source_repo.create_source(Source.create_environment( + PLANTY_ID, + ACCT_ID, + EnvironmentInfo("Planty", "The green one"))) + + kit_repo.create_mock_kit(SUPPLIED_KIT_ID) + + t.commit() + + @staticmethod + def teardown_test_data(): + with Transaction() as t: + 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) + + t.commit() + + def test_get_sources(self): + 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?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] + 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 = 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 = 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 = 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) + + assert bobo_surveys == [1, 3, 4, 5] + 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 + 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=acct_json + ) + check_response(response) + + # 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', + 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( + '/api/accounts/%s?language_tag=en_us' % (ACCT_ID,), + headers={'Authorization': 'Bearer PutMySecureOauthTokenHere'}) + check_response(response) + + acc = json.loads(response.data) + + regular_data = \ + { + "account_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeffffffff", + "account_type": "standard", + "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" + } + + # 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") + + regular_data.pop("account_id") + 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) + 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") + + 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' + 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): + """ 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_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' % + (ACCT_ID, DOGGY_ID), + content_type='application/json', + data=json.dumps( + { + "sample_id": sample_id + }) + ) + 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 + # 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/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; 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/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/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/model/source.py b/microsetta_private_api/model/source.py index 60891b949..d66c26c24 100644 --- a/microsetta_private_api/model/source.py +++ b/microsetta_private_api/model/source.py @@ -1,11 +1,32 @@ -import json 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 +41,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 +68,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: @@ -71,7 +93,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: @@ -121,10 +144,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/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/repo/kit_repo.py b/microsetta_private_api/repo/kit_repo.py index 95149205f..409a810c8 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] + 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! + 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 self.get_kit(supplied_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,)) diff --git a/microsetta_private_api/repo/sample_repo.py b/microsetta_private_api/repo/sample_repo.py index e51961493..3046c913c 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,)) - - sample_row = cur.fetchone() - if sample_row is None: - return None + "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_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,125 @@ 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_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 " + "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", + (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) + + 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... + # 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 " + "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)) 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)) 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): diff --git a/microsetta_private_api/server.py b/microsetta_private_api/server.py index ef3949db8..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 @@ -10,8 +13,12 @@ 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 handle_422(repo_exc): + return jsonify(code=422, message=str(repo_exc)), 422 + + +def build_app(): # Create the application instance app = connexion.FlaskApp(__name__) @@ -23,4 +30,12 @@ # 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 + + +# If we're running in stand alone mode, run the application +if __name__ == '__main__': + app = build_app() app.run(port=8082, debug=True)