Skip to content

add labware updated_at value and return the latest one #5780

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.

Sign up for GitHub

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

Open
wendyyang wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

add labware updated_at value and return the latest one #5780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4619bae
add labware updated_at value and return the latest one
wendyyang May 14, 2026
62216ec
Merge branch 'develop' into Y26-149-labware_location_bug
wendyyang May 14, 2026
4989c67
fix tests
wendyyang May 14, 2026
c9b87f9
Merge branch 'develop' into Y26-149-labware_location_bug
wendyyang May 14, 2026
4b05c16
refactor the test to include human barcode, so the tests will pass
wendyyang May 15, 2026
23afcf4
refactor the lookup_labwhere_location
wendyyang May 15, 2026
79423ce
update date format
wendyyang May 15, 2026
92ac84d
update date format
wendyyang May 15, 2026
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
32 changes: 29 additions & 3 deletions app/models/labware.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,19 @@ def obtain_retention_instructions
end

def lookup_labwhere_location
lookup_labwhere(machine_barcode) || lookup_labwhere(human_barcode)
lookups = [lookup_labwhere(machine_barcode), lookup_labwhere(human_barcode)]

return latest_location(lookups) if lookups.any? { |l| l&.dig(:location).present? }
return 'Not found - There is a problem with Labwhere' if lookups.any? { |l| l&.dig(:error) }

nil
end
Copy link
Copy Markdown
Contributor

@StephenHulme StephenHulme May 15, 2026

Choose a reason for hiding this comment

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

Would it be possible to reduce the complexity of this function (possibly by pulling out some private methods)?

It's difficult to understand exactly what the new code does.

Copy link
Copy Markdown
Contributor Author

@wendyyang wendyyang May 15, 2026

Choose a reason for hiding this comment

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

refactored.


def latest_location(lookups)
lookups.compact
.select { |l| l[:location].present? }
.max_by { |l| l[:updated_at] || Time.zone.at(0) }
.fetch(:location)
end

def lookup_labwhere(barcode)
Expand All @@ -378,9 +390,23 @@ def lookup_labwhere(barcode)
rescue StandardError => e
# rescue LabWhereClient::LabwhereException => e
Rails.logger.error { e }
return 'Not found - There is a problem with Labwhere'
return { error: true, message: e.message }
end
info_from_labwhere.location.location_info if info_from_labwhere.present? && info_from_labwhere.location.present?

return nil unless info_from_labwhere.present? && info_from_labwhere.location.present?

{
location: info_from_labwhere.location.location_info,
updated_at: parse_labwhere_timestamp(info_from_labwhere.updated_at)
}
end

def parse_labwhere_timestamp(timestamp)
return if timestamp.blank?

Time.zone.parse(timestamp)
rescue ArgumentError
nil
end
end
# rubocop:enable Metrics/ClassLength
8 changes: 5 additions & 3 deletions lib/lab_where_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def self.included(base)
class Labware < Endpoint
endpoint_name 'labwares'

attr_reader :barcode, :location
attr_reader :barcode, :location, :updated_at

def self.find_by_barcode(barcode)
return nil if barcode.blank?
Expand All @@ -102,7 +102,8 @@ def self.find_by_barcode(barcode)

def initialize(params)
@barcode = params['barcode']
@location = Location.new(params['location'])
@updated_at = params['updated_at']
@location = Location.new(params['location']) if params['location'].present?
end
end

Expand Down Expand Up @@ -155,7 +156,7 @@ def valid?
class Location < Endpoint
endpoint_name 'locations'

attr_reader :name, :parentage, :barcode
attr_reader :name, :parentage, :barcode, :updated_at

def self.find_by_barcode(barcode)
return nil if barcode.blank?
Expand All @@ -168,6 +169,7 @@ def initialize(params)
@name = params['name']
@parentage = params['parentage']
@barcode = params['barcode']
@updated_at = params['updated_at']
end

def location_info
Expand Down
22 changes: 22 additions & 0 deletions spec/lib/lab_where_client_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,26 @@
end
end
end

describe LabWhereClient::Labware do
let(:params) do
{
'barcode' => '123456',
'updated_at' => '2026-05-06 10:46:00',
'location' => {
'name' => 'f1',
'parentage' => 'Sanger / Ogilvie / AA216',
'barcode' => 'lw-f1-26214',
'updated_at' => '2023-06-06 16:26:00'
}
}
end

it 'captures labware and location updated_at values from the API payload' do
labware = described_class.new(params)

expect(labware.updated_at).to eq('2026-05-06 10:46:00')
expect(labware.location.updated_at).to eq('2023-06-06 16:26:00')
end
end
end
60 changes: 60 additions & 0 deletions spec/models/labware_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,66 @@
plate = create(:plate)
expect(plate.storage_location).to eq('Not found - There is a problem with Labwhere')
end

it 'chooses the newer location when both machine and human barcode lookups return values' do
plate = create(:plate)

allow(LabWhereClient::Labware).to receive(:find_by_barcode).with(plate.machine_barcode).and_return(
LabWhereClient::Labware.new(
{
'barcode' => plate.machine_barcode,
'updated_at' => '2026-05-06 10:46:00',
'location' => {
'name' => 'f1',
'parentage' => 'Sanger / Ogilvie / AA216',
'barcode' => 'lw-f1-26214',
'updated_at' => '2026-05-06 10:46:00'
}
}
)
)
allow(LabWhereClient::Labware).to receive(:find_by_barcode).with(plate.human_barcode).and_return(
LabWhereClient::Labware.new(
{
'barcode' => plate.human_barcode,
'updated_at' => '2026-05-07 10:46:00',
'location' => {
'name' => 'f2',
'parentage' => 'Sanger / Ogilvie / AA216',
'barcode' => 'lw-f2-26214',
'updated_at' => '2026-05-07 10:46:00'
}
}
)
)

expect(plate.labwhere_location).to eq('Sanger / Ogilvie / AA216 - f2')
end

it 'falls back to human barcode location when machine barcode lookup raises an error' do
plate = create(:plate)

allow(LabWhereClient::Labware).to receive(:find_by_barcode).with(plate.machine_barcode).and_raise(
StandardError,
'Timed out reading data from server'
)
allow(LabWhereClient::Labware).to receive(:find_by_barcode).with(plate.human_barcode).and_return(
LabWhereClient::Labware.new(
{
'barcode' => plate.human_barcode,
'updated_at' => '2026-05-07 10:46:00',
'location' => {
'name' => 'f2',
'parentage' => 'Sanger / Ogilvie / AA216',
'barcode' => 'lw-f2-26214',
'updated_at' => '2026-05-07 10:46:00'
}
}
)
)

expect(plate.labwhere_location).to eq('Sanger / Ogilvie / AA216 - f2')
end
end

context 'when retrieving retention instructions' do
Expand Down
72 changes: 65 additions & 7 deletions spec/models/location_report_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,17 @@

before do
[
[plate_1.machine_barcode.to_s, 'Shelf 1', locn_prefix],
[plate_2.machine_barcode.to_s, 'Shelf 2', locn_prefix],
[plate_3.machine_barcode.to_s, 'Shelf 3', locn_prefix],
[tube_1.machine_barcode.to_s, 'Shelf 4', locn_prefix],
[plate_1.machine_barcode.to_s, plate_1.human_barcode.to_s, 'Shelf 1', locn_prefix],
[plate_2.machine_barcode.to_s, plate_2.human_barcode.to_s, 'Shelf 2', locn_prefix],
[plate_3.machine_barcode.to_s, plate_3.human_barcode.to_s, 'Shelf 3', locn_prefix],
[tube_1.machine_barcode.to_s, tube_1.human_barcode.to_s, 'Shelf 4', locn_prefix],
# Tube racks are filtered out so this should not show up
[tube_rack.machine_barcode.to_s, 'Shelf 5', locn_prefix]
].each do |lw_barcode, lw_locn_name, lw_locn_parentage|
stub_lwclient_labware_find_by_bc(lw_barcode:, lw_locn_name:, lw_locn_parentage:)
[tube_rack.machine_barcode.to_s, tube_rack.human_barcode.to_s, 'Shelf 5', locn_prefix]
].each do |machine_bc, human_bc, lw_locn_name, lw_locn_parentage|
stub_lwclient_labware_find_by_bc(lw_barcode: machine_bc, lw_locn_name: lw_locn_name,
lw_locn_parentage: lw_locn_parentage)
stub_lwclient_labware_find_by_bc(lw_barcode: human_bc, lw_locn_name: lw_locn_name,
lw_locn_parentage: lw_locn_parentage)
end

plate_1_set_long_term_storage
Expand Down Expand Up @@ -391,6 +394,11 @@
lw_locn_name: 'Shelf 1',
lw_locn_parentage: locn_prefix
)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_4.human_barcode.to_s,
lw_locn_name: 'Shelf 1',
lw_locn_parentage: locn_prefix
)
end

it_behaves_like 'a successful report'
Expand Down Expand Up @@ -537,6 +545,11 @@
stub_lwclient_locn_children(location_barcode, [])
stub_lwclient_locn_labwares(location_barcode, [p1])
stub_lwclient_labware_find_by_bc(p1)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_1.human_barcode,
lw_locn_name: 'Shelf 1',
lw_locn_parentage: locn_prefix
)

plate_1_set_long_term_storage
end
Expand Down Expand Up @@ -613,6 +626,21 @@
stub_lwclient_labware_find_by_bc(p1)
stub_lwclient_labware_find_by_bc(p2)
stub_lwclient_labware_find_by_bc(t1)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_1.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_2.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)
stub_lwclient_labware_find_by_bc(
lw_barcode: tube_1.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)

plate_1_set_long_term_storage
tube_1_set_long_term_storage
Expand Down Expand Up @@ -670,6 +698,11 @@
stub_lwclient_locn_children(locn_lvl2_b1[:locn_barcode], [])
stub_lwclient_locn_labwares(locn_lvl2_b1[:locn_barcode], [p1])
stub_lwclient_labware_find_by_bc(p1)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_1.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)

# set up Shelf 1 - Box 2 with one labware and no sub-locations
p2 = {
Expand All @@ -681,6 +714,11 @@
stub_lwclient_locn_children(locn_lvl2_b2[:locn_barcode], [])
stub_lwclient_locn_labwares(locn_lvl2_b2[:locn_barcode], [p2])
stub_lwclient_labware_find_by_bc(p2)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_2.human_barcode,
lw_locn_name: 'Box 2',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)

plate_1_set_long_term_storage
end
Expand Down Expand Up @@ -728,6 +766,11 @@
stub_lwclient_locn_children(location_barcode, [locn_lvl2_t1])
stub_lwclient_locn_labwares(location_barcode, [p1])
stub_lwclient_labware_find_by_bc(p1)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_1.human_barcode,
lw_locn_name: 'Shelf 1',
lw_locn_parentage: locn_prefix
)

# set up Shelf 1 - Tray 1 with 1 labware and 1 sub-location
locn_lvl3_b1 = {
Expand All @@ -744,6 +787,11 @@
stub_lwclient_locn_children(locn_lvl2_t1[:locn_barcode], [locn_lvl3_b1])
stub_lwclient_locn_labwares(locn_lvl2_t1[:locn_barcode], [p2])
stub_lwclient_labware_find_by_bc(p2)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_2.human_barcode,
lw_locn_name: 'Tray 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1"
)

# set up Shelf 1 - Tray 1 - Box 1 with 2 labware and no sub-locations
p3 = {
Expand All @@ -761,6 +809,16 @@
stub_lwclient_locn_labwares(locn_lvl3_b1[:locn_barcode], [p3, t1])
stub_lwclient_labware_find_by_bc(p3)
stub_lwclient_labware_find_by_bc(t1)
stub_lwclient_labware_find_by_bc(
lw_barcode: plate_3.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1 - Tray 1"
)
stub_lwclient_labware_find_by_bc(
lw_barcode: tube_1.human_barcode,
lw_locn_name: 'Box 1',
lw_locn_parentage: "#{locn_prefix} - Shelf 1 - Tray 1"
)

plate_1_set_long_term_storage
plate_3_return_after_two_years
Expand Down
Loading