diff --git a/src/admin/dbmappings.php b/src/admin/dbmappings.php index 4b57a5c..67db06a 100644 --- a/src/admin/dbmappings.php +++ b/src/admin/dbmappings.php @@ -161,7 +161,13 @@ { // If Columns are send using POST we use them, otherwise we try to use from the view itself, if available if ( isset($_POST['Mappings']) ) + { $AllMappings = $_POST['Mappings']; + // Reset SUBMAPPINGS when rebuilding from POST data so that stale DB-loaded + // string entries (fields removed in a prior delete subop) do not remain and + // cause "Illegal string offset" warnings when iterated as arrays (issue #61). + $content['SUBMAPPINGS'] = array(); + } else if ( isset($content['SUBMAPPINGS']) ) $AllMappings = $content['SUBMAPPINGS']; diff --git a/tests/php/Unit/DbMappingSubMappingsTest.php b/tests/php/Unit/DbMappingSubMappingsTest.php new file mode 100644 index 0000000..047b1d4 --- /dev/null +++ b/tests/php/Unit/DbMappingSubMappingsTest.php @@ -0,0 +1,173 @@ + $dbMappings Mappings as loaded from DB + * (string values, keyed by fieldId) + * @param list $postMappings $_POST['Mappings'] value + * @return array The resulting $content['SUBMAPPINGS'] + */ + private function buildSubMappingsBuggy(array $dbMappings, array $postMappings): array + { + // Step 1: simulate the GET op=edit handler loading DB data as strings. + $submappings = $dbMappings; // values are plain strings here + + // Step 2: simulate the "Additional work" block WITHOUT the fix. + $allMappings = $postMappings; // comes from $_POST['Mappings'] + // (no reset of $submappings) + foreach ($allMappings as $colKey => $fieldName) { + if (!is_numeric($colKey)) { + $submappings[$colKey] = ['MappingFieldID' => $colKey, 'MappingDbFieldName' => $fieldName]; + } else { + $submappings[$fieldName] = ['MappingFieldID' => $fieldName]; + } + } + + return $submappings; + } + + /** + * Simulate the FIXED behaviour: resets SUBMAPPINGS before building from + * $postMappings so that stale DB string entries are discarded. + * + * @param array $dbMappings + * @param list $postMappings + * @return array + */ + private function buildSubMappingsFixed(array $dbMappings, array $postMappings): array + { + // Step 1: simulate the GET op=edit handler loading DB data as strings. + $submappings = $dbMappings; + + // Step 2: simulate the "Additional work" block WITH the fix. + $allMappings = $postMappings; + $submappings = []; // <-- the fix: reset before rebuilding + foreach ($allMappings as $colKey => $fieldName) { + if (!is_numeric($colKey)) { + $submappings[$colKey] = ['MappingFieldID' => $colKey, 'MappingDbFieldName' => $fieldName]; + } else { + $submappings[$fieldName] = ['MappingFieldID' => $fieldName]; + } + } + + return $submappings; + } + + // ----------------------------------------------------------------------- + // Tests + // ----------------------------------------------------------------------- + + /** + * After a first delete the form only sends the remaining fields. + * The ORIGINAL code leaves the removed field as a string in SUBMAPPINGS, + * which would later cause "Illegal string offset" warnings. + */ + public function testBuggyCodeLeavesStaleStringEntryAfterPartialPost(): void + { + $dbMappings = ['field1' => 'dbcol1', 'field2' => 'dbcol2', 'field3' => 'dbcol3']; + // Second delete: field3 was already removed in a prior request so it is + // absent from the form; only field1 and field2 are submitted. + $postMappings = ['field1', 'field2']; + + $result = $this->buildSubMappingsBuggy($dbMappings, $postMappings); + + // field3 is still present as a plain string – the bug. + self::assertArrayHasKey('field3', $result); + self::assertIsString($result['field3'], 'Buggy code leaves field3 as a string'); + + // Trying to set an array offset on that string would trigger the warning. + // Demonstrate the error condition without actually triggering E_WARNING. + self::assertFalse(is_array($result['field3'])); + } + + /** + * The FIXED code resets SUBMAPPINGS before rebuilding from POST, so stale + * DB string entries are never present in the result. + */ + public function testFixedCodeContainsOnlyPostFieldsAsArrays(): void + { + $dbMappings = ['field1' => 'dbcol1', 'field2' => 'dbcol2', 'field3' => 'dbcol3']; + $postMappings = ['field1', 'field2']; + + $result = $this->buildSubMappingsFixed($dbMappings, $postMappings); + + // field3 must be gone. + self::assertArrayNotHasKey('field3', $result, 'Fixed code must not retain stale DB entry for field3'); + + // The remaining entries must be arrays (not strings). + self::assertArrayHasKey('field1', $result); + self::assertIsArray($result['field1']); + self::assertArrayHasKey('field2', $result); + self::assertIsArray($result['field2']); + } + + /** + * When all DB fields are present in the POST (e.g., the very first delete + * operation) both the buggy and fixed paths produce the same correct result: + * all entries are arrays. + */ + public function testAllDbFieldsPresentInPostProducesCorrectResultBothPaths(): void + { + $dbMappings = ['field1' => 'dbcol1', 'field2' => 'dbcol2', 'field3' => 'dbcol3']; + $postMappings = ['field1', 'field2', 'field3']; + + $buggy = $this->buildSubMappingsBuggy($dbMappings, $postMappings); + $fixed = $this->buildSubMappingsFixed($dbMappings, $postMappings); + + foreach (['field1', 'field2', 'field3'] as $field) { + self::assertIsArray($buggy[$field]); + self::assertIsArray($fixed[$field]); + } + } + + /** + * Fixed code: first POST includes all fields → no stale entries. + * Second POST removes one field → SUBMAPPINGS still clean. + */ + public function testSequentialDeletesNeverLeaveStaleEntries(): void + { + $dbMappings = ['field1' => 'dbcol1', 'field2' => 'dbcol2', 'field3' => 'dbcol3']; + + // First delete: field3 removed; form still sends all three before delete. + $after1 = $this->buildSubMappingsFixed($dbMappings, ['field1', 'field2', 'field3']); + unset($after1['field3']); // simulate subop_delete + self::assertCount(2, $after1); + + // Second delete: field2 removed; POST now only sends field1 and field2. + $after2 = $this->buildSubMappingsFixed($dbMappings, ['field1', 'field2']); + unset($after2['field2']); // simulate subop_delete + self::assertCount(1, $after2); + self::assertArrayHasKey('field1', $after2); + self::assertArrayNotHasKey('field2', $after2); + self::assertArrayNotHasKey('field3', $after2); + self::assertIsArray($after2['field1']); + } +}