[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)
aces · Jun 21, 2023 · c2f41af · c2f41af
1 parent 1cd5f07
commit c2f41af
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 2 deletions.
diff --git 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
@@ -1141,7 +1141,6 @@ function testGetInstanceData()
                 'ID'                          => '1000',
                 'SessionID'                   => '123',
                 'Test_name'                   => 'Test_name1',
-                'CommentID'                   => 'commentID1',
                 'Data_entry'                  => '',
                 'Required_elements_completed' => 'N',
                 'Administration'              => '',