Skip to content

Commit

Permalink
Changed the query in getStockID and updated error handling for duplic…
Browse files Browse the repository at this point in the history
…ate germplasm accessions
  • Loading branch information
carolyncaron committed Dec 15, 2023
1 parent 5e1ef62 commit 19e91ce
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -424,34 +424,58 @@ public function getStockID($germplasm_name, $accession_number, $organism_id) {

$accession_type_id = $this->getCVterm('accession');

// First query the stock table just using the germplasm name and organism ID
// First query the stock table:
// 1. Using a regular condition to ensure the organism_id is a match
// 2. Create an OR condition group to look for records that match germplasm name OR
// the uniquename. Since the unique constraint is organism_id/uniquename/type_id,
// we have to make sure this combo doesn't already exist with a different germplasm
// name.
$query = $this->connection->select('1:stock', 's')
->fields('s', ['stock_id', 'uniquename', 'type_id']);
$query->condition('s.name', $germplasm_name, '=')
->fields('s', ['stock_id', 'name', 'uniquename', 'type_id'])
->condition('s.organism_id', $organism_id, '=');

$orGroup = $query->orConditionGroup()
->condition('s.name', $germplasm_name, '=')
->condition('s.uniquename', $accession_number, '=');

// Now add the OR condition group to the query
$query->condition($orGroup);
$record = $query->execute()->fetchAll();

// We may have retrieved 1+ records that share the germplasm name and/or 1+ records that
// share the accession_number. In this case, throw an error since there's no way to
// enter a new record with a unique organism_id/uniquename/type_id combo in this scenario
if (sizeof($record) >= 2) {
$this->logger->error("Found more than one stock ID for \"@germplasm_name\".", ['@germplasm_name' => $germplasm_name]);
$this->logger->error("Found more than one stock ID for \"@germplasm_name\" and/or \"@accession\".", ['@germplasm_name' => $germplasm_name, '@accession' => $accession_number]);
$this->error_tracker = TRUE;
return false;
}

elseif (sizeof($record) == 1) {
// Handle the situation where a stock record exists
// Check the uniquename matches the accession_number column in the file
// Here we are individually checking that our uniquename, name and type_id all match
// what is in the input file. This is to provide an informative error message if one
// of these don't match. In the future, we may want to handle each case differently.
// For example, some groups may want to allow the same germplasm name but a different
// type_id to be allowed.
// 1. Check the uniquename matches the accession_number column in the file
if ($accession_number != $record[0]->uniquename) {
$this->logger->error("A stock already exists for \"@germplasm_name\" but with an accession of \"@accession\" which does not match the input file.", ['@germplasm_name' => $germplasm_name, '@accession' => $record[0]->uniquename]);
$this->error_tracker = TRUE;
return false;
}
// Check the type_id is of type accession
// 2. Check that our germplasm name matches
if ($germplasm_name != $record[0]->name) {
$this->logger->error("A stock already exists for accession \"@accession\" but with a germplasm name of \"@germplasm_name\" which does not match the input file.", ['@germplasm_name' => $record[0]->name, '@accession' => $accession_number]);
$this->error_tracker = TRUE;
return false;
}
// 3. Check the type_id is of type accession
if ($accession_type_id != $record[0]->type_id) {
$this->logger->error("A stock already exists for \"@germplasm_name\" but with a type ID of \"@type\" which is not of type \"accession\".", ['@germplasm_name' => $germplasm_name, '@type' => $accession_type_id]);
$this->error_tracker = TRUE;
return false;
}

// Confirmed that the selected record matches what's in the upload file, so return the stock_id
return $record[0]->stock_id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public function testGermplasmAccessionImporterGetStockID() {
ob_start();
$grabbed_dup_stock_id = $this->importer->getStockID('stock1', 'TEST:1', $organism_id);
$printed_output = ob_get_clean();
$this->assertTrue($printed_output == 'Found more than one stock ID for "stock1".', "Did not get the expected error message when testing for duplicate stock IDs.");
$this->assertStringContainsString('Found more than one stock ID for "stock1"', $printed_output, "Did not get the expected error message when testing for duplicate stock IDs.");
}

/**
Expand Down

0 comments on commit 19e91ce

Please sign in to comment.