Merge pull request #8655 from ProcessMaker/bugfix/FOUR-28469

FOUR-28469: Error with Subrocesses - Missing data between from child request to parent request

Author: nolanpro

ProcessMaker/Models/CallActivity.php

@@ -97,22 +97,10 @@ protected function completeSubprocess(TokenInterface $token, ExecutionInstanceIn
         $store = $closedInstance->getDataStore();
         $allData = $store->getData();
 
-        // Determine which data should be merged back from the subprocess.
-        $updatedKeys = method_exists($store, 'getUpdated')
-            ? $store->getUpdated()
-            : null;
-
-        if ($updatedKeys === null) {
-            // Legacy behavior or no tracking available: copy all data.
-            $data = $allData;
-        } elseif ($updatedKeys === []) {
-            // Nothing was updated in the subprocess: do not merge anything.
-            $data = [];
-        } else {
-            // Merge only the explicitly updated keys.
-            $updatedKeys = array_values((array) $updatedKeys);
-            $data = array_intersect_key($allData, array_flip($updatedKeys));
-        }
+        $data = $this->resolveUpdatedData($store, $allData);
+        $parentData = $token->getInstance()->getDataStore()->getData();
+        $data = $this->mergeNewKeys($data, $allData, $parentData);
+        $data = $this->mergeChangedKeys($data, $allData, $parentData);
 
         $dataManager = new DataManager();
         $dataManager->updateData($token, $data);
@@ -125,6 +113,62 @@ protected function completeSubprocess(TokenInterface $token, ExecutionInstanceIn
         return $this;
     }
 
+    /**
+     * Decide which data from the subprocess should be merged based on updated keys.
+     */
+    protected function resolveUpdatedData($store, array $allData): array
+    {
+        $updatedKeys = method_exists($store, 'getUpdated')
+            ? $store->getUpdated()
+            : null;
+
+        if ($updatedKeys === null) {
+            return $allData;
+        }
+
+        if ($updatedKeys === []) {
+            return [];
+        }
+
+        $updatedKeys = array_values((array) $updatedKeys);
+
+        return array_intersect_key($allData, array_flip($updatedKeys));
+    }
+
+    /**
+     * Merge keys that exist only in the subprocess data.
+     */
+    protected function mergeNewKeys(array $data, array $allData, array $parentData): array
+    {
+        $newKeys = array_diff(array_keys($allData), array_keys($parentData));
+        if (empty($newKeys)) {
+            return $data;
+        }
+
+        return $data + array_intersect_key($allData, array_flip($newKeys));
+    }
+
+    /**
+     * Merge keys that changed in the subprocess but may not have been tracked.
+     */
+    protected function mergeChangedKeys(array $data, array $allData, array $parentData): array
+    {
+        $changedKeys = [];
+        foreach ($allData as $key => $value) {
+            if (array_key_exists($key, $parentData) && $parentData[$key] !== $value) {
+                $changedKeys[] = $key;
+            }
+        }
+
+        if (empty($changedKeys)) {
+            return $data;
+        }
+
+        $pendingKeys = array_diff($changedKeys, array_keys($data));
+
+        return $data + array_intersect_key($allData, array_flip($pendingKeys));
+    }
+
     /**
      * Catch a subprocess error
      *

tests/unit/ProcessMaker/Models/CallActivityMergeTest.php

@@ -0,0 +1,104 @@
+<?php
+
+namespace Tests\Unit\ProcessMaker\Models;
+
+use PHPUnit\Framework\TestCase;
+use ProcessMaker\Models\CallActivity;
+
+class CallActivityMergeTest extends TestCase
+{
+    private function callActivity(): CallActivity
+    {
+        return new class extends CallActivity {
+            public function callResolveUpdatedData($store, array $allData): array
+            {
+                return $this->resolveUpdatedData($store, $allData);
+            }
+
+            public function callMergeNewKeys(array $data, array $allData, array $parentData): array
+            {
+                return $this->mergeNewKeys($data, $allData, $parentData);
+            }
+
+            public function callMergeChangedKeys(array $data, array $allData, array $parentData): array
+            {
+                return $this->mergeChangedKeys($data, $allData, $parentData);
+            }
+        };
+    }
+
+    public function testResolveUpdatedDataReturnsAllWhenUpdatedIsNull(): void
+    {
+        $store = new class {
+            public function getUpdated()
+            {
+                return null;
+            }
+        };
+
+        $callActivity = $this->callActivity();
+        $all = ['a' => 1, 'b' => 2];
+
+        $result = $callActivity->callResolveUpdatedData($store, $all);
+
+        $this->assertSame($all, $result);
+    }
+
+    public function testResolveUpdatedDataReturnsEmptyWhenUpdatedIsEmptyArray(): void
+    {
+        $store = new class {
+            public function getUpdated(): array
+            {
+                return [];
+            }
+        };
+
+        $callActivity = $this->callActivity();
+        $all = ['a' => 1, 'b' => 2];
+
+        $result = $callActivity->callResolveUpdatedData($store, $all);
+
+        $this->assertSame([], $result);
+    }
+
+    public function testResolveUpdatedDataReturnsOnlyUpdatedKeys(): void
+    {
+        $store = new class {
+            public function getUpdated(): array
+            {
+                return ['b'];
+            }
+        };
+
+        $callActivity = $this->callActivity();
+        $all = ['a' => 1, 'b' => 2, 'c' => 3];
+
+        $result = $callActivity->callResolveUpdatedData($store, $all);
+
+        $this->assertSame(['b' => 2], $result);
+    }
+
+    public function testMergeNewKeysAddsKeysAbsentInParent(): void
+    {
+        $callActivity = $this->callActivity();
+        $data = ['a' => 1];
+        $all = ['a' => 1, 'b' => 2, 'c' => 3];
+        $parent = ['a' => 1];
+
+        $result = $callActivity->callMergeNewKeys($data, $all, $parent);
+
+        $this->assertSame(['a' => 1, 'b' => 2, 'c' => 3], $result);
+    }
+
+    public function testMergeChangedKeysAddsDifferencesNotTracked(): void
+    {
+        $callActivity = $this->callActivity();
+        $data = ['a' => 1]; // already tracked
+        $all = ['a' => 1, 'b' => 99, 'c' => 'new'];
+        $parent = ['a' => 1, 'b' => 2, 'c' => 'old'];
+
+        $result = $callActivity->callMergeChangedKeys($data, $all, $parent);
+
+        $this->assertSame(['a' => 1, 'b' => 99, 'c' => 'new'], $result);
+    }
+}