Make setCallback() add to previous callbacks

setCallback() replaces previous callbacks making you have to repeat any logic in the "base case' definition callback if you have a callback on one of the groups. See issue thephpleague#472
jesse-r-s-hines · Jan 5, 2021 · 36cdde2 · 36cdde2
1 parent 62c8c31
commit 36cdde2
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 1 deletion.
diff --git a/src/Definition.php b/src/Definition.php
@@ -145,7 +145,15 @@ public function getMaker()
      */
     public function setCallback(callable $callback)
     {
-        $this->callback = $callback;
+        if ($this->callback) { // "stack" the callbacks so that any previous callbacks will also be called.
+            $oldCallback = $this->callback;
+            $this->callback = function ($model, $saved) use ($oldCallback, $callback) {
+                // If the old callback returns false, the whole thing should return false.
+                return $oldCallback($model, $saved) !== false && $callback($model, $saved) !== false;
+            };
+        } else {
+            $this->callback = $callback;
+        }
 
         return $this;
     }

diff --git a/tests/DefinitionTest.php b/tests/DefinitionTest.php
@@ -88,6 +88,40 @@ public function testCallbackDefinitionFunctions()
         $this->assertNull($definition->getCallback());
     }
 
+    public function testCallbacksStack()
+    {
+        static::$fm->define("CallbacksStackStub")->setCallback(function ($object, $saved) {
+            $object->field1 = true;
+        })->setCallback(function ($object, $saved) {
+            $object->field2 = true;
+        });
+
+        static::$fm->define("group:CallbacksStackStub")->setCallback(function ($object, $saved) {
+            $object->field3 = true;
+        });
+
+        $object = static::$fm->create("group:CallbacksStackStub");
+
+        $this->assertObjectHasAttribute("field1", $object);
+        $this->assertObjectHasAttribute("field2", $object);
+        $this->assertObjectHasAttribute("field3", $object);
+
+        // Test that stacked callback will return false if any of the callbacks return false.
+        $definition = static::$fm->getDefinition("CallbacksStackStub")->clearCallback();
+        $definition
+            ->setCallback(function ($object, $model) { return false; })
+            ->setCallback(function ($object, $model) { return true; })
+            ->setCallback(function ($object, $model) { return true; });
+        $this->assertFalse(call_user_func($definition->getCallback(), null, null));
+
+        $definition = static::$fm->getDefinition("CallbacksStackStub")->clearCallback();
+        $definition
+            ->setCallback(function () {})
+            ->setCallback(function ($object) {})
+            ->setCallback(function ($object, $model) { return true; });
+        $this->assertNotEquals(call_user_func($definition->getCallback(), null, null), false);
+    }
+
     public function testDefineWithReplacementGenerators()
     {
         $user = static::$fm->create('UserModelStub', [
@@ -393,6 +427,18 @@ public function delete()
     }
 }
 
+class CallbacksStackStub
+{
+    public function save()
+    {
+        return true;
+    }
+
+    public function delete()
+    {
+        return true;
+    }
+}
 class UserModelStub
 {
     public function save()