From c2f41af6307ab6990d8cc62358da2ac199cc7726 Mon Sep 17 00:00:00 2001 From: Dave MacFarlane Date: Wed, 21 Jun 2023 09:13:51 -0400 Subject: [PATCH] [instruments] Remove CommentID from getInstanceData. (#8805) The CommentID is not part of the data, it's the foreign key used between the flag table and the instrument table. JSON-based instruments do not have it, and this ensures better consistency between the two so that issues such as #8796 and #8801 will not vary based on instrument type and will be caught sooner. Resolves part of #8804 (Inconsistency #2) --- php/libraries/NDB_BVL_Instrument.class.inc | 7 ++++++- test/unittests/NDB_BVL_Instrument_Test.php | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc index 760ef2c6e4b..27942636f5b 100644 --- a/php/libraries/NDB_BVL_Instrument.class.inc +++ b/php/libraries/NDB_BVL_Instrument.class.inc @@ -2106,12 +2106,17 @@ abstract class NDB_BVL_Instrument extends NDB_Page ['CID' => $this->getCommentID()] ); - $this->instanceData = json_decode($jsondata, true) ?? []; + $this->instanceData = json_decode($jsondata ?? '', true) ?? []; } else { $defaults = $db->pselectRow( "SELECT * FROM $this->table WHERE CommentID=:CID", ['CID' => $this->getCommentID()] ); + // This is only included because it's the primary key. JSON + // data does not include it. Unset so that the two types are + // consistent. Places that need the commentID should be using + // NDB_BVL_Instrument->getCommentID() + unset($defaults['CommentID']); $this->instanceData = $defaults ?? []; } diff --git a/test/unittests/NDB_BVL_Instrument_Test.php b/test/unittests/NDB_BVL_Instrument_Test.php index a0a7a97b9d0..9027cddeabd 100644 --- a/test/unittests/NDB_BVL_Instrument_Test.php +++ b/test/unittests/NDB_BVL_Instrument_Test.php @@ -1141,7 +1141,6 @@ function testGetInstanceData() 'ID' => '1000', 'SessionID' => '123', 'Test_name' => 'Test_name1', - 'CommentID' => 'commentID1', 'Data_entry' => '', 'Required_elements_completed' => 'N', 'Administration' => '',