From b8aeda5fa67bdc4e839b95c946275cf81a9cf96b Mon Sep 17 00:00:00 2001 From: Mark W <24956497+ndg63276@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:00:12 +0100 Subject: [PATCH] LIMS-59: Never write null to DewarTransportHistory.storageLocation (#597) * LIMS-59: Never write null to DewarTransportHistory.storageLocation * LIMS-59: Write a location to DTH when marking a dewar as unprocessing * LIMS-59: Check for nulls before doing inserts * LIMS-59: Ensure tests pass * Update test * tidy --------- Co-authored-by: Mark Williams Co-authored-by: John Holt --- api/src/Model/Services/AssignData.php | 10 +++-- api/src/Page/Shipment.php | 31 ++++++++------ api/tests/Model/Services/AssignDataTest.php | 11 +++-- api/tests/TestUtils.php | 47 +++++++++++++++++++++ 4 files changed, 78 insertions(+), 21 deletions(-) create mode 100644 api/tests/TestUtils.php diff --git a/api/src/Model/Services/AssignData.php b/api/src/Model/Services/AssignData.php index f2d681a41..ed3e4bbaf 100644 --- a/api/src/Model/Services/AssignData.php +++ b/api/src/Model/Services/AssignData.php @@ -2,6 +2,8 @@ namespace SynchWeb\Model\Services; +use SynchWeb\Utils; + class AssignData { private $db; @@ -85,9 +87,10 @@ function updateDewar($dewarId, $status) function deactivateDewar($dewarId) { - $this->updateDewarHistory($dewarId, 'unprocessing'); + $location = $this->db->pq("SELECT storagelocation FROM dewar WHERE dewarid=:1", array($dewarId)); + $this->updateDewarHistory($dewarId, 'unprocessing', $location[0]['STORAGELOCATION']); - $conts = $this->db->pq("SELECT containerid as id FROM container WHERE dewarid=:1", array($dewarId)); + $conts = $this->db->pq("SELECT containerid FROM container WHERE dewarid=:1", array($dewarId)); foreach ($conts as $container) { $this->updateContainerAndHistory($container['CONTAINERID'], 'at facility', '', ''); @@ -99,9 +102,10 @@ function updateDewarHistory($did, $status, $beamline = null, $additionalStatusDe $st = $status; if ($additionalStatusDetail) $st .= ' (' . $additionalStatusDetail . ')'; + $loc = Utils::getValueOrDefault($beamline, ''); $this->db->pq("INSERT INTO dewartransporthistory (dewarid, dewarstatus, storagelocation, arrivaldate) - VALUES (:1, :2, :3, CURRENT_TIMESTAMP)", array($did, $st, $beamline)); + VALUES (:1, :2, :3, CURRENT_TIMESTAMP)", array($did, $st, $loc)); $this->updateDewar($did, $status); } diff --git a/api/src/Page/Shipment.php b/api/src/Page/Shipment.php index 50be8ea2c..8958261a6 100644 --- a/api/src/Page/Shipment.php +++ b/api/src/Page/Shipment.php @@ -858,6 +858,8 @@ function _transfer_dewar() global $transfer_email; if (!$this->has_arg('DEWARID')) $this->_error('No dewar specified'); + if (!$this->has_arg('LOCATION')) + $this->_error('No location specified'); $dew = $this->db->pq("SELECT d.dewarid,s.shippingid FROM dewar d @@ -1046,7 +1048,7 @@ function _dispatch_dewar() $dewar_location = $last_history['STORAGELOCATION']; } else { // Use the current location of the dewar instead if no history - $dewar_location = $dew['STORAGELOCATION']; + $dewar_location = Utils::getValueOrDefault($dew['STORAGELOCATION'], ''); } } // Check if the last history storage location is an EBIC prefix or not @@ -1561,7 +1563,7 @@ function _send_shipment() } $this->db->pq("UPDATE shipping SET shippingstatus='sent to facility' where shippingid=:1", array($ship['SHIPPINGID'])); - $this->db->pq("UPDATE dewar SET dewarstatus='sent to facility' where shippingid=:1", array($ship['SHIPPINGID'])); + $this->db->pq("UPDATE dewar SET dewarstatus='sent to facility', storagelocation='off-site' where shippingid=:1", array($ship['SHIPPINGID'])); $dewars = $this->db->pq("SELECT d.dewarid, s.visit_number as vn, s.beamlinename as bl, TO_CHAR(s.startdate, 'DD-MM-YYYY HH24:MI') as startdate FROM dewar d @@ -1569,8 +1571,8 @@ function _send_shipment() WHERE d.shippingid=:1", array($ship['SHIPPINGID'])); foreach ($dewars as $d) { $this->db->pq( - "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,arrivaldate) - VALUES (s_dewartransporthistory.nextval,:1,'sent to facility',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", + "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,storagelocation,arrivaldate) + VALUES (s_dewartransporthistory.nextval,:1,'sent to facility','off-site',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", array($d['DEWARID']) ); } @@ -2878,11 +2880,11 @@ function _create_awb() continue; $p = $awb['pieces'][$i]; - $this->db->pq("UPDATE dewar SET $tno=:1, deliveryAgent_barcode=:2, dewarstatus='awb created' WHERE dewarid=:3", array($awb['awb'], $p['licenseplate'], $d['DEWARID'])); + $this->db->pq("UPDATE dewar SET $tno=:1, deliveryAgent_barcode=:2, dewarstatus='awb created', storagelocation='off-site' WHERE dewarid=:3", array($awb['awb'], $p['licenseplate'], $d['DEWARID'])); $this->db->pq( - "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,arrivaldate) - VALUES (s_dewartransporthistory.nextval,:1,'awb created',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", + "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,storagelocation,arrivaldate) + VALUES (s_dewartransporthistory.nextval,:1,'awb created','off-site',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", array($d['DEWARID']) ); } @@ -3035,10 +3037,10 @@ function _do_request_pickup($options) WHERE shippingid=:4", array($pickup['confirmationnumber'], $pickup['readybytime'], $pickup['callintime'], $options['shippingid'])); foreach ($options['dewars'] as $i => $d) { - $this->db->pq("UPDATE dewar SET dewarstatus='pickup booked' WHERE dewarid=:1", array($d['DEWARID'])); + $this->db->pq("UPDATE dewar SET dewarstatus='pickup booked', storagelocation='off-site' WHERE dewarid=:1", array($d['DEWARID'])); $this->db->pq( - "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,arrivaldate) - VALUES (s_dewartransporthistory.nextval,:1,'pickup booked',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", + "INSERT INTO dewartransporthistory (dewartransporthistoryid,dewarid,dewarstatus,storagelocation,arrivaldate) + VALUES (s_dewartransporthistory.nextval,:1,'pickup booked','off-site',CURRENT_TIMESTAMP) RETURNING dewartransporthistoryid INTO :id", array($d['DEWARID']) ); } @@ -3170,7 +3172,7 @@ function _cancel_pickup() $this->_error('No such lab contact'); $cont = $cont[0]; - $dewars = $this->db->pq("SELECT d.dewarid + $dewars = $this->db->pq("SELECT d.dewarid, d.storagelocation FROM dewar d WHERE d.shippingid=:1 AND d.deliveryagent_barcode IS NOT NULL", array($this->arg('sid'))); @@ -3191,10 +3193,11 @@ function _cancel_pickup() foreach ($dewars as $i => $d) { $this->db->pq("UPDATE dewar SET dewarstatus='pickup cancelled' WHERE dewarid=:1", array($d['DEWARID'])); + $loc = Utils::getValueOrDefault($d['STORAGELOCATION'], ''); $this->db->pq( - "INSERT INTO dewartransporthistory (dewarid,dewarstatus,arrivaldate) - VALUES (:1,'pickup cancelled',CURRENT_TIMESTAMP)", - array($d['DEWARID']) + "INSERT INTO dewartransporthistory (dewarid,dewarstatus,storagelocation,arrivaldate) + VALUES (:1,'pickup cancelled',:2,CURRENT_TIMESTAMP)", + array($d['DEWARID'], $loc) ); } } catch (\Exception $e) { diff --git a/api/tests/Model/Services/AssignDataTest.php b/api/tests/Model/Services/AssignDataTest.php index 064a69d87..297474e2e 100644 --- a/api/tests/Model/Services/AssignDataTest.php +++ b/api/tests/Model/Services/AssignDataTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\TestCase; use SynchWeb\Model\Services\AssignData; use SynchWeb\Database\Type\MySQL; +use Tests\TestUtils; /** * @runTestsInSeparateProcesses // Needed to allow db mocking @@ -20,6 +21,7 @@ final class AssignDataTest extends TestCase private $db; private $assignData; private $insertId; + private $stmtStub; protected function setUp(): void { @@ -29,7 +31,7 @@ protected function setUp(): void $this->db = new MySQL($connStub); $this->assignData = new AssignData($this->db); - $stmtStub = $this->getMockBuilder(\mysqli_stmt::class) + $this->stmtStub = $this->getMockBuilder(\mysqli_stmt::class) ->disableOriginalConstructor() ->onlyMethods(['bind_param', 'execute', 'get_result', 'close']) ->getMock(); @@ -40,8 +42,8 @@ protected function setUp(): void $this->insertId->expects($this->any())->willReturn(666); } - $stmtStub->method('execute')->willReturn(true); - $connStub->method('prepare')->willReturn($stmtStub); + $this->stmtStub->method('execute')->willReturn(true); + $connStub->method('prepare')->willReturn($this->stmtStub); } public function testGetContainerCreatesCorrectSql(): void @@ -108,8 +110,9 @@ public function testUpdateDewarWithUnprocessingStatusCreatesCorrectSql(): void public function testDeactivateDewarCreatesCorrectSql(): void { + TestUtils::mockDBReturnsResult($this->stmtStub, [['STORAGELOCATION'=> "current location"], ]); $this->assignData->deactivateDewar('testDewarId'); - $this->assertEquals("SELECT containerid as id FROM Container WHERE dewarid='testDewarId'", $this->db->getLastQuery()); + $this->assertEquals("SELECT containerid FROM Container WHERE dewarid='testDewarId'", $this->db->getLastQuery()); } public function testUpdateDewarHistoryCreatesCorrectSql(): void diff --git a/api/tests/TestUtils.php b/api/tests/TestUtils.php new file mode 100644 index 000000000..5e4883891 --- /dev/null +++ b/api/tests/TestUtils.php @@ -0,0 +1,47 @@ +method('get_result')->willReturn(new MockTestResult($returnRows)); + } + +} + +/** + * Class to help with returning mock results from sql + */ +class MockTestResult +{ + public $num_rows; + private $returnRows; + private $row_index = -1; + + function __construct($returnRows){ + $this->returnRows = $returnRows; + $this->num_rows = $returnRows? count($returnRows) : 0; + } + + function fetch_assoc() { + + $this->row_index++; + if ($this->row_index < $this->num_rows) { + return $this->returnRows[$this->row_index]; + } + return null; + + } +}