Skip to content

Commit

Permalink
LIMS-59: Never write null to DewarTransportHistory.storageLocation (#597
Browse files Browse the repository at this point in the history
)

* 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 <[email protected]>
Co-authored-by: John Holt <[email protected]>
  • Loading branch information
3 people authored Oct 12, 2023
1 parent f03a8d5 commit b8aeda5
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 21 deletions.
10 changes: 7 additions & 3 deletions api/src/Model/Services/AssignData.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace SynchWeb\Model\Services;

use SynchWeb\Utils;

class AssignData
{
private $db;
Expand Down Expand Up @@ -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', '', '');
Expand All @@ -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);
}
Expand Down
31 changes: 17 additions & 14 deletions api/src/Page/Shipment.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1561,16 +1563,16 @@ 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
LEFT OUTER JOIN blsession s ON s.sessionid = d.firstexperimentid
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'])
);
}
Expand Down Expand Up @@ -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'])
);
}
Expand Down Expand Up @@ -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'])
);
}
Expand Down Expand Up @@ -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')));

Expand All @@ -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) {
Expand Down
11 changes: 7 additions & 4 deletions api/tests/Model/Services/AssignDataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,6 +21,7 @@ final class AssignDataTest extends TestCase
private $db;
private $assignData;
private $insertId;
private $stmtStub;

protected function setUp(): void
{
Expand All @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions api/tests/TestUtils.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace Tests;

/**
* Selection of utilities to help testing
*/
class TestUtils
{

/**
* Use the statment mock to return results
* @param stmtStub Statement stub that prepare executes
* @param returnRows array of rows, a row is an array with key/value paits for the return values
* NB keys should probably be upper case
*/
static public function mockDBReturnsResult($stmtStub, $returnRows){

$stmtStub->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;

}
}

0 comments on commit b8aeda5

Please sign in to comment.