fix(approval): post approved event triggered when not approving

CHANGELOG.md

@@ -1,10 +1,5 @@
 # Changelog
 
-## [v1.8.3](https://github.com/flarum/framework/compare/v1.8.2...v1.8.3)
-### Fixed
-* Console extender does not accept ::class [#3900]
-* Conditional extender instantiation [#3898]
-
 ## [v1.8.2](https://github.com/flarum/framework/compare/v1.8.1...v1.8.2)
 ### Fixed
 * suspended users can abuse avatar upload [#3890]

extensions/approval/src/Listener/ApproveContent.php

@@ -28,18 +28,34 @@ class ApproveContent
         $attributes = $event->data['attributes'];
         $post = $event->post;
 
+        // Nothing to do if it is already approved.
+        if ($post->is_approved) {
+            return;
+        }
+
+        /*
+         * We approve a post in one of two cases:
+         * - The post was unapproved and the allowed action is approving it. We trigger an event.
+         * - The post was unapproved and the allowed actor is hiding or un-hiding it.
+         *   We approve it silently if the action is unhiding.
+         */
+        $approvingSilently = false;
+
         if (isset($attributes['isApproved'])) {
             $event->actor->assertCan('approve', $post);
 
             $isApproved = (bool) $attributes['isApproved'];
-        } elseif (! empty($attributes['isHidden']) && $event->actor->can('approve', $post)) {
+        } elseif (isset($attributes['isHidden']) && $event->actor->can('approve', $post)) {
             $isApproved = true;
+            $approvingSilently = $attributes['isHidden'];
         }
 
         if (! empty($isApproved)) {
             $post->is_approved = true;
 
-            $post->raise(new PostWasApproved($post, $event->actor));
+            if (! $approvingSilently) {
+                $post->raise(new PostWasApproved($post, $event->actor));
+            }
         }
     }
 }

framework/core/src/Extend/Conditional.php

@@ -13,34 +13,17 @@ use Flarum\Extension\Extension;
 use Flarum\Extension\ExtensionManager;
 use Illuminate\Contracts\Container\Container;
 
-/**
- * The Conditional extender allows developers to conditionally apply other extenders
- * based on either boolean values or results from callable functions.
- *
- * This is useful for applying extenders only if certain conditions are met,
- * such as the presence of an enabled extension or a specific configuration setting.
- */
 class Conditional implements ExtenderInterface
 {
     /**
-     * An array of conditions and their associated extenders.
-     *
-     * Each entry should have:
-     * - 'condition': a boolean or callable that should return a boolean.
-     * - 'extenders': an array of extenders, a callable returning an array of extenders, or an invokable class string.
-     *
-     * @var array<array{condition: bool|callable, extenders: ExtenderInterface[]|callable|string}>
+     * @var array<array{condition: bool|callable, extenders: ExtenderInterface[]}>
      */
     protected $conditions = [];
 
     /**
-     * Apply extenders only if a specific extension is enabled.
-     *
-     * @param string $extensionId The ID of the extension.
-     * @param ExtenderInterface[]|callable|string $extenders An array of extenders, a callable returning an array of extenders, or an invokable class string.
-     * @return self
+     * @param ExtenderInterface[] $extenders
      */
-    public function whenExtensionEnabled(string $extensionId, $extenders): self
+    public function whenExtensionEnabled(string $extensionId, array $extenders): self
     {
         return $this->when(function (ExtensionManager $extensions) use ($extensionId) {
             return $extensions->isEnabled($extensionId);
@@ -48,14 +31,10 @@ class Conditional implements ExtenderInterface
     }
 
     /**
-     * Apply extenders based on a condition.
-     *
-     * @param bool|callable $condition A boolean or callable that should return a boolean.
-     *                                 If this evaluates to true, the extenders will be applied.
-     * @param ExtenderInterface[]|callable|string $extenders An array of extenders, a callable returning an array of extenders, or an invokable class string.
-     * @return self
+     * @param bool|callable $condition
+     * @param ExtenderInterface[] $extenders
      */
-    public function when($condition, $extenders): self
+    public function when($condition, array $extenders): self
     {
         $this->conditions[] = [
             'condition' => $condition,
@@ -65,13 +44,6 @@ class Conditional implements ExtenderInterface
         return $this;
     }
 
-    /**
-     * Iterates over the conditions and applies the associated extenders if the conditions are met.
-     *
-     * @param Container $container
-     * @param Extension|null $extension
-     * @return void
-     */
     public function extend(Container $container, Extension $extension = null)
     {
         foreach ($this->conditions as $condition) {
@@ -80,13 +52,7 @@ class Conditional implements ExtenderInterface
             }
 
             if ($condition['condition']) {
-                $extenders = $condition['extenders'];
-
-                if (is_string($extenders) || is_callable($extenders)) {
-                    $extenders = $container->call($extenders);
-                }
-
-                foreach ($extenders as $extender) {
+                foreach ($condition['extenders'] as $extender) {
                     $extender->extend($container, $extension);
                 }
             }

framework/core/src/Extend/Console.php

@@ -10,7 +10,6 @@
 namespace Flarum\Extend;
 
 use Flarum\Extension\Extension;
-use Flarum\Foundation\ContainerUtil;
 use Illuminate\Contracts\Container\Container;
 
 class Console implements ExtenderInterface
@@ -62,11 +61,7 @@ class Console implements ExtenderInterface
             return array_merge($existingCommands, $this->addCommands);
         });
 
-        $container->extend('flarum.console.scheduled', function ($existingScheduled) use ($container) {
-            foreach ($this->scheduled as &$schedule) {
-                $schedule['callback'] = ContainerUtil::wrapCallback($schedule['callback'], $container);
-            }
-
+        $container->extend('flarum.console.scheduled', function ($existingScheduled) {
             return array_merge($existingScheduled, $this->scheduled);
         });
     }

framework/core/src/Foundation/Application.php

@@ -21,7 +21,7 @@ class Application
      *
      * @var string
      */
-    const VERSION = '1.8.3';
+    const VERSION = '1.8.2';
 
     /**
      * The IoC container for the Flarum application.

framework/core/tests/integration/extenders/ConditionalTest.php

@@ -159,140 +159,4 @@ class ConditionalTest extends TestCase
 
         $this->app();
     }
-
-    /** @test */
-    public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callable()
-    {
-        $this->extend(
-            (new Extend\Conditional())
-                ->when(false, TestExtender::class)
-        );
-
-        $this->app();
-
-        $response = $this->send(
-            $this->request('GET', '/api', [
-                'authenticatedAs' => 1,
-            ])
-        );
-
-        $payload = json_decode($response->getBody()->getContents(), true);
-
-        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
-    }
-
-    /** @test */
-    public function conditional_does_instantiate_extender_if_condition_is_true_using_callable()
-    {
-        $this->extend(
-            (new Extend\Conditional())
-                ->when(true, TestExtender::class)
-        );
-
-        $this->app();
-
-        $response = $this->send(
-            $this->request('GET', '/api', [
-                'authenticatedAs' => 1,
-            ])
-        );
-
-        $payload = json_decode($response->getBody()->getContents(), true);
-
-        $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
-    }
-
-    /** @test */
-    public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callback()
-    {
-        $this->extend(
-            (new Extend\Conditional())
-                ->when(false, function (): array {
-                    return [
-                        (new Extend\ApiSerializer(ForumSerializer::class))
-                            ->attributes(function () {
-                                return [
-                                    'customConditionalAttribute' => true
-                                ];
-                            })
-                    ];
-                })
-        );
-
-        $this->app();
-
-        $response = $this->send(
-            $this->request('GET', '/api', [
-                'authenticatedAs' => 1,
-            ])
-        );
-
-        $payload = json_decode($response->getBody()->getContents(), true);
-
-        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
-    }
-
-    /** @test */
-    public function conditional_does_instantiate_extender_if_condition_is_true_using_callback()
-    {
-        $this->extend(
-            (new Extend\Conditional())
-                ->when(true, function (): array {
-                    return [
-                        (new Extend\ApiSerializer(ForumSerializer::class))
-                            ->attributes(function () {
-                                return [
-                                    'customConditionalAttribute' => true
-                                ];
-                            })
-                    ];
-                })
-        );
-
-        $this->app();
-
-        $response = $this->send(
-            $this->request('GET', '/api', [
-                'authenticatedAs' => 1,
-            ])
-        );
-
-        $payload = json_decode($response->getBody()->getContents(), true);
-
-        $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
-    }
-
-    /** @test */
-    public function conditional_does_not_work_if_extension_is_disabled()
-    {
-        $this->extend(
-            (new Extend\Conditional())
-                ->whenExtensionEnabled('dummy-extension-id', TestExtender::class)
-        );
-
-        $response = $this->send(
-            $this->request('GET', '/api', [
-                'authenticatedAs' => 1,
-            ])
-        );
-
-        $payload = json_decode($response->getBody()->getContents(), true);
-
-        $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
-    }
-}
-
-class TestExtender
-{
-    public function __invoke(): array
-    {
-        return [
-            (new Extend\ApiSerializer(ForumSerializer::class))
-                ->attributes(function () {
-                    return [
-                        'customConditionalAttribute' => true
-                    ];
-                })
-        ];
-    }
 }

framework/core/tests/integration/extenders/ConsoleTest.php

@@ -75,23 +75,6 @@ class ConsoleTest extends ConsoleTestCase
 
         $this->assertStringContainsString('cache:clear', $this->runCommand($input));
     }
-
-    /**
-     * @test
-     */
-    public function scheduled_command_exists_when_added_with_class_syntax()
-    {
-        $this->extend(
-            (new Extend\Console())
-                ->schedule('cache:clear', ScheduledCommandCallback::class)
-        );
-
-        $input = [
-            'command' => 'schedule:list'
-        ];
-
-        $this->assertStringContainsString('cache:clear', $this->runCommand($input));
-    }
 }
 
 class CustomCommand extends AbstractCommand
@@ -112,11 +95,3 @@ class CustomCommand extends AbstractCommand
         $this->info('Custom Command.');
     }
 }
-
-class ScheduledCommandCallback
-{
-    public function __invoke(Event $event)
-    {
-        $event->everyMinute();
-    }
-}