Skip to content

Commit

Permalink
Merge pull request #174 from wasade/created_with_kit_id
Browse files Browse the repository at this point in the history
Allow for tracking which kit ID an account was created using
  • Loading branch information
wasade committed Jun 3, 2020
2 parents 57250c4 + ea2c12f commit a5f6e88
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 27 deletions.
6 changes: 4 additions & 2 deletions microsetta_private_api/admin/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ def setup_test_data():
"CA",
12345,
"US"
))
),
"fakekit")
acct_repo.create_account(acc)

acc = Account(ADMIN_ACCT_ID,
Expand All @@ -56,7 +57,8 @@ def setup_test_data():
"CA",
12345,
"US"
))
),
"fakekit")
acct_repo.create_account(acc)
t.commit()

Expand Down
2 changes: 1 addition & 1 deletion microsetta_private_api/api/implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def find_accounts_for_login(token_info):

if acct is None:
return jsonify([]), 200

return jsonify([acct.to_api()]), 200


Expand Down Expand Up @@ -687,7 +688,6 @@ def _validate_account_access(token_info, account_id):
token_associated_account = account_repo.find_linked_account(
token_info['iss'],
token_info['sub'])

account = account_repo.get_account(account_id)
if account is None:
raise NotFound(ACCT_NOT_FOUND_MSG)
Expand Down
15 changes: 5 additions & 10 deletions microsetta_private_api/api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ def run_query_and_content_required_field_test(self, url, action,
else:
raise ValueError(format("unexpect request action: ",
action))

self.assertEqual(400, response.status_code)
resp_obj = json.loads(response.data)
self.assertTrue(curr_expected_msg in resp_obj['detail'])
Expand Down Expand Up @@ -395,12 +394,6 @@ def validate_dummy_acct_response_body(self, response_obj,
# check all input fields/values appear in response body EXCEPT kit_name
# plus additional fields
expected_dict = copy.deepcopy(dummy_acct_dict)
try:
expected_dict.pop(KIT_NAME_KEY)
except KeyError:
# is ok if input did not have a kit name, as this is
# provided on account create but not account update
pass
expected_dict[ACCT_ID_KEY] = real_acct_id_from_body
expected_dict[ACCT_TYPE_KEY] = ACCT_TYPE_VAL
expected_dict[CREATION_TIME_KEY] = real_creation_time
Expand Down Expand Up @@ -641,9 +634,11 @@ def test_account_view_fail_404(self):

# region account update/put tests
@staticmethod
def make_updated_acct_dict():
def make_updated_acct_dict(keep_kit_name=False):
result = copy.deepcopy(DUMMY_ACCT_INFO)
result.pop(KIT_NAME_KEY)

if not keep_kit_name:
result.pop(KIT_NAME_KEY)

result["address"] = {
"city": "Oakland",
Expand All @@ -659,7 +654,7 @@ def test_account_update_success(self):
"""Successfully update existing account"""
dummy_acct_id = create_dummy_acct()

changed_acct_dict = self.make_updated_acct_dict()
changed_acct_dict = self.make_updated_acct_dict(keep_kit_name=True)

# create post input json
input_json = json.dumps(changed_acct_dict)
Expand Down
13 changes: 9 additions & 4 deletions microsetta_private_api/api/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ def setup_test_data():
"CA",
12345,
"US"
))
),
"fakekit")
acct_repo.create_account(acc)

source_repo.create_source(Source(
Expand Down Expand Up @@ -504,7 +505,8 @@ def test_edit_account_info(self):
},
"email": "foo@baz.com",
"first_name": "Dan",
"last_name": "H"
"last_name": "H",
"kit_name": "fakekit"
}

# Hard to guess these two, so let's pop em out
Expand All @@ -514,12 +516,14 @@ def test_edit_account_info(self):

regular_data.pop("account_id")

# Don't fuzz the email--changing the email in the accounts table
# without changing the email in the authorization causes
# Don't fuzz the email or kit_name -- changing the email in the
# accounts table without changing the email in the authorization causes
# authorization errors (as it should)
the_email = regular_data["email"]
kit_name = regular_data['kit_name']
fuzzy_data = fuzz(regular_data)
fuzzy_data['email'] = the_email
fuzzy_data['kit_name'] = kit_name

# submit an invalid account type
fuzzy_data['account_type'] = "Voldemort"
Expand Down Expand Up @@ -563,6 +567,7 @@ def test_edit_account_info(self):
check_response(response)

acc = json.loads(response.data)

acc.pop('creation_time')
acc.pop('update_time')
regular_data['account_type'] = 'standard'
Expand Down
41 changes: 41 additions & 0 deletions microsetta_private_api/db/patches/0063.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
-- create a column that can be used to record what kit was used
-- when the account was created.
ALTER TABLE ag.account ADD COLUMN created_with_kit_id VARCHAR;

-- from existing accounts, recover a kit that is associated with
-- the account. if someone has a single kit, then that kit is used
-- and has to be what was registered with. If they have multiple
-- kits, then we'll just pick one of them as in the old system,
-- users would have had to register with each kit individually.
-- This should be suitable for newly created accounts where
-- people have assigned samples.
UPDATE ag.account a SET created_with_kit_id = ss.kit_id
FROM (SELECT s.account_id, (array_agg(supplied_kit_id))[1] AS kit_id
FROM ag.ag_kit ak
JOIN ag.ag_kit_barcodes akb USING (ag_kit_id)
INNER JOIN ag.source s ON (akb.source_id=s.id)
GROUP BY s.account_id) ss
WHERE a.id = ss.account_id;

-- a large number of kits have samples that were never assigned,
-- which resulted in them not being linked to the "source" table
-- during migration. to handle this, let's use the older links in
-- the database.
UPDATE ag.account SET created_with_kit_id=subquery.kit_id
FROM (SELECT ag_login_id, (array_agg(supplied_kit_id))[1] AS kit_id
FROM ag.account a
JOIN ag.ag_kit ak ON a.id=ak.ag_login_id
JOIN ag.ag_kit_barcodes USING(ag_kit_id)
WHERE created_with_kit_id IS NULL
GROUP BY ag_login_id) AS subquery
WHERE id=subquery.ag_login_id;

-- finally, there are a handful where the created_with_kit_id remains null.
-- on inspection, this appears to happen in the new system when someone
-- has created an account but hasn't yet claimed samples from a kit. We'll
-- set those to the empty string so that we can make created_with_kit_id
-- not null
UPDATE ag.account SET created_with_kit_id=''
WHERE created_with_kit_id IS NULL;

ALTER TABLE ag.account ALTER COLUMN created_with_kit_id SET NOT NULL;
11 changes: 7 additions & 4 deletions microsetta_private_api/model/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ def from_dict(input_dict, auth_iss, auth_sub):
input_dict['address']['city'],
input_dict['address']['state'],
input_dict['address']['post_code'],
input_dict['address']['country_code'],
)
input_dict['address']['country_code']
),
input_dict['kit_name']
)
return result

def __init__(self, account_id, email,
account_type, auth_issuer, auth_sub,
first_name, last_name,
address,
address, created_with_kit_id,
creation_time=None, update_time=None):
self.id = account_id
self.email = email
Expand All @@ -46,6 +47,7 @@ def __init__(self, account_id, email,
self.first_name = first_name
self.last_name = last_name
self.address = address
self.created_with_kit_id = created_with_kit_id
self.creation_time = creation_time
self.update_time = update_time

Expand All @@ -59,7 +61,8 @@ def to_api(self):
"address": self.address.to_api(),
"account_type": self.account_type,
"creation_time": self.creation_time,
"update_time": self.update_time
"update_time": self.update_time,
"kit_name": self.created_with_kit_id
}

def account_matches_auth(self, email, auth_issuer, auth_sub):
Expand Down
15 changes: 9 additions & 6 deletions microsetta_private_api/repo/account_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ def __init__(self, transaction):
"account_type, auth_issuer, auth_sub, " \
"first_name, last_name, " \
"street, city, state, post_code, country_code, " \
"creation_time, update_time"
"created_with_kit_id, creation_time, update_time"

write_cols = "id, email, " \
"account_type, auth_issuer, auth_sub, " \
"first_name, last_name, " \
"street, city, state, post_code, country_code"
"street, city, state, post_code, country_code, " \
"created_with_kit_id"

@staticmethod
def _row_to_addr(r):
Expand All @@ -41,14 +42,16 @@ def _row_to_account(r):
r['account_type'], r['auth_issuer'], r['auth_sub'],
r['first_name'], r['last_name'],
AccountRepo._row_to_addr(r),
r['created_with_kit_id'],
r['creation_time'], r['update_time'])

@staticmethod
def _account_to_row(a):
return (a.id, a.email,
a.account_type, a.auth_issuer, a.auth_sub,
a.first_name, a.last_name) + \
AccountRepo._addr_to_row(a.address)
AccountRepo._addr_to_row(a.address) + \
(a.created_with_kit_id, )

def claim_legacy_account(self, email, auth_iss, auth_sub):
# Returns now-claimed legacy account if an unclaimed legacy account
Expand Down Expand Up @@ -136,7 +139,6 @@ def update_account(self, account):
row_id = row[0:1]
row_email_to_cc = row[1:]
final_row = row_email_to_cc + row_id

try:
cur.execute("UPDATE account "
"SET "
Expand All @@ -150,7 +152,8 @@ def update_account(self, account):
"city = %s, "
"state = %s, "
"post_code = %s, "
"country_code = %s "
"country_code = %s, "
"created_with_kit_id = %s "
"WHERE "
"account.id = %s",
final_row
Expand All @@ -177,7 +180,7 @@ def create_account(self, account):
"%s, %s, "
"%s, %s, %s, "
"%s, %s, "
"%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:
Expand Down

0 comments on commit a5f6e88

Please sign in to comment.