Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added last_update column to ag_kit_barcodes #558

Merged
merged 6 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions microsetta_private_api/api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@
'sample_edit_locked': False,
'sample_remove_locked': False,
'sample_notes': None,
'sample_last_update': None,
'sample_latest_scan_timestamp': None,
'sample_projects': ['American Gut Project'],
'account_id': None,
'source_id': None,
Expand All @@ -188,6 +190,8 @@
'sample_edit_locked': False,
'sample_remove_locked': False,
'sample_notes': "Oops, I dropped it",
'sample_last_update': "2018-07-21T17:32:28Z",
'sample_latest_scan_timestamp': "2016-07-21T17:32:28Z",
'sample_projects': ['American Gut Project'],
'account_id': 'foobar',
'source_id': 'foobarbaz',
Expand Down Expand Up @@ -588,6 +592,7 @@ def create_dummy_sample_objects(filled=False):
None,
info_dict['source_id'],
info_dict['account_id'],
None,
info_dict["sample_projects"],
None)

Expand Down Expand Up @@ -2652,6 +2657,7 @@ def test_dissociate_sample_from_source_success(self):
# make sure the sample's collection info is wiped
sample_repo = SampleRepo(t)
obs_sample = sample_repo._get_sample_by_id(MOCK_SAMPLE_ID)
obs_sample.last_update = None
self.assertEqual(expected_sample.__dict__, obs_sample.__dict__)

# make sure answered survey no longer associated with any samples
Expand Down
7 changes: 7 additions & 0 deletions microsetta_private_api/db/patches/0135.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- Jan 26, 2024
-- Add sample metadata update timestamp
ALTER TABLE ag.ag_kit_barcodes
ADD COLUMN last_update timestamp with time zone;

COMMENT ON COLUMN ag.ag_kit_barcodes.last_update
IS 'Sample metadata update timestamp';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a field name, last_update is ambiguous. Can the field reflect the type of update it corresponds too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this field name to latest_sample_information_update to clearly state that it's only tracking the info that belongs directly to a sample (e.g. collection date/time and type) and is ignoring other associations like metadata.

Also, we do not need to null the column when a sample is deleted, but it would be beneficial to add a comment to the sample deletion code noting this decision and suggesting it be revisited if we ever use the new column for anything other than its current purpose of UI enhancement.

9 changes: 6 additions & 3 deletions microsetta_private_api/model/sample.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

class Sample(ModelBase):
def __init__(self, sample_id, datetime_collected, site, notes, barcode,
latest_scan_timestamp, source_id, account_id,
latest_scan_timestamp, source_id, account_id, last_update,
sample_projects, latest_scan_status, kit_id=None):
self.id = sample_id
# NB: datetime_collected may be None if sample not yet used
Expand All @@ -17,6 +17,7 @@ def __init__(self, sample_id, datetime_collected, site, notes, barcode,
# NB: _latest_scan_timestamp may be None if not yet returned to lab
self._latest_scan_timestamp = latest_scan_timestamp
self._latest_scan_status = latest_scan_status
self.last_update = last_update
self.sample_projects = sample_projects

self.source_id = source_id
Expand All @@ -42,7 +43,7 @@ def remove_locked(self):

@classmethod
def from_db(cls, sample_id, date_collected, time_collected,
site, notes, barcode, latest_scan_timestamp,
site, notes, barcode, latest_scan_timestamp, last_update,
source_id, account_id, sample_projects, latest_scan_status):
datetime_collected = None
# NB a sample may NOT have date and time collected if it has been sent
Expand All @@ -51,7 +52,7 @@ def from_db(cls, sample_id, date_collected, time_collected,
datetime_collected = datetime.combine(date_collected,
time_collected)
return cls(sample_id, datetime_collected, site, notes, barcode,
latest_scan_timestamp, source_id,
latest_scan_timestamp, last_update, source_id,
account_id, sample_projects, latest_scan_status)

def to_api(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR is changing what the API returns for a sample, can you please update the corresponding component and schema entries in the YAML file?

Expand All @@ -62,7 +63,9 @@ def to_api(self):
"sample_edit_locked": self.edit_locked,
"sample_remove_locked": self.remove_locked,
"sample_datetime": self.datetime_collected,
"sample_latest_scan_timestamp": self._latest_scan_timestamp,
"sample_notes": self.notes,
"sample_last_update": self.last_update,
"source_id": self.source_id,
"account_id": self.account_id,
"sample_projects": list(self.sample_projects),
Expand Down
11 changes: 8 additions & 3 deletions microsetta_private_api/repo/sample_repo.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
import werkzeug
from werkzeug.exceptions import NotFound

Expand All @@ -19,7 +20,8 @@ class SampleRepo(BaseRepo):
ag.ag_kit_barcodes.barcode,
latest_scan.scan_timestamp,
ag.source.id,
ag.source.account_id
ag.source.account_id,
ag.ag_kit_barcodes.last_update
FROM ag.ag_kit_barcodes
LEFT JOIN (
SELECT barcode, max(scan_timestamp) AS scan_timestamp
Expand All @@ -43,7 +45,8 @@ class SampleRepo(BaseRepo):
ag.ag_kit_barcodes.barcode,
latest_scan.scan_timestamp,
ag.source.id,
ag.source.account_id
ag.source.account_id,
ag.ag_kit_barcodes.last_update
FROM ag.ag_kit_barcodes
LEFT JOIN (
SELECT barcode, max(scan_timestamp) AS scan_timestamp
Expand Down Expand Up @@ -302,14 +305,16 @@ def update_info(self, account_id, source_id, sample_info,
"sample_date = %s, "
"sample_time = %s, "
"site_sampled = %s, "
"notes = %s "
"notes = %s, "
"last_update = %s "
"WHERE "
"ag_kit_barcode_id = %s",
(
sample_date,
sample_time,
sample_info.site,
sample_info.notes,
datetime.datetime.utcnow(),
sample_info.id
))

Expand Down
Loading