[2.x] fix: distinguish a missing announcement excerpt from an empty one (#4902)

The announcements fetcher read each excerpt from the `firstPost` include and
ran it through `makeExcerpt()` regardless of whether that post arrived. With no
post, `Arr::get(null, ...)` yields `''` and the excerpt became an empty string —
the same value a post with no text would produce.

That is how the admin announcements widget came to render every card blank
without anyone noticing: the response looked well-formed and said, in effect,
"these announcements have no content". The actual fault was upstream, where
fof/gamification had narrowed the `firstPost` eager load and stopped the include
being serialized at all.

The excerpt is now null where the post never arrived, and an empty string only
where the post is genuinely empty. The widget already guards on a falsy excerpt,
so nothing renders differently — but a future recurrence is legible instead of
silent.

Tests: that the request asks for the relationships the excerpt and author are
read from — nothing asserted that before, so a lost include would not have been
noticed; that a missing include yields null; and that an empty post still yields
an empty string, so the distinction holds in both directions.

Author: imorland

framework/core/js/src/admin/components/AnnouncementItem.tsx

@@ -13,7 +13,8 @@ export interface AnnouncementData {
   createdAt: string;
   isSticky: boolean;
   url: string;
-  excerpt: string;
+  /** Null where the announcement's first post did not arrive, as against an empty string for a post with no text. */
+  excerpt: string | null;
   authorName: string | null;
   avatarUrl: string | null;
 }

framework/core/src/Announcements/AnnouncementsFetcher.php

@@ -95,7 +95,14 @@ public function fetch(): array
                 'createdAt' => $createdAt,
                 'isSticky' => (bool) Arr::get($discussion, 'attributes.isSticky', false),
                 'url' => 'https://discuss.flarum.org/d/'.$slug,
-                'excerpt' => $this->makeExcerpt(Arr::get($firstPost, 'attributes.contentHtml', '')),
+                // Null where the post never arrived, as against an empty string
+                // for a post that genuinely has no text. Collapsing the two hid
+                // a real fault: discuss.flarum.org stopped serializing the
+                // `firstPost` include, and every forum's announcements widget
+                // rendered blank cards that read as "these posts are empty".
+                'excerpt' => $firstPost === null
+                    ? null
+                    : $this->makeExcerpt(Arr::get($firstPost, 'attributes.contentHtml', '')),
                 'authorName' => Arr::get($user, 'attributes.displayName'),
                 'avatarUrl' => Arr::get($user, 'attributes.avatarUrl'),
             ];

framework/core/tests/unit/Announcements/AnnouncementsFetcherTest.php

@@ -16,6 +16,7 @@
 use GuzzleHttp\Exception\ConnectException;
 use GuzzleHttp\Handler\MockHandler;
 use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Middleware;
 use GuzzleHttp\Psr7\Request;
 use GuzzleHttp\Psr7\Response;
 use Mockery as m;
@@ -32,16 +33,22 @@ protected function setUp(): void
         $this->appInfo->shouldReceive('identifyDatabaseVersion')->andReturn('8.0.32');
     }
 
-    private function makeFetcher(array $responses): AnnouncementsFetcher
+    /**
+     * @param array $history Populated with the requests actually sent, so a test
+     *                       can assert what was asked for and not only what came
+     *                       back.
+     */
+    private function makeFetcher(array $responses, array &$history = []): AnnouncementsFetcher
     {
-        $mock = new MockHandler($responses);
-        $client = new Client(['handler' => HandlerStack::create($mock)]);
+        $stack = HandlerStack::create(new MockHandler($responses));
+        $stack->push(Middleware::history($history));
+
+        $client = new Client(['handler' => $stack]);
 
         $fetcher = new AnnouncementsFetcher($this->appInfo);
 
         // Inject the mock client via reflection
         $ref = new \ReflectionProperty($fetcher, 'client');
-        $ref->setAccessible(true);
         $ref->setValue($fetcher, $client);
 
         return $fetcher;
@@ -189,6 +196,86 @@ public function test_skips_discussions_missing_required_fields(): void
         $this->assertEquals('Valid', $result[0]['title']);
     }
 
+    /**
+     * The excerpt and author are read out of `included`, which only arrives if
+     * the request asks for those relationships. Every other test here hands the
+     * transform a well-formed `included` array, so none of them would notice the
+     * request losing its `include`.
+     */
+    public function test_requests_the_relationships_the_excerpt_and_author_need(): void
+    {
+        $history = [];
+        $fetcher = $this->makeFetcher([$this->makeApiResponse([$this->makeDiscussion()])], $history);
+
+        $fetcher->fetch();
+
+        $this->assertCount(1, $history);
+
+        $query = [];
+        parse_str($history[0]['request']->getUri()->getQuery(), $query);
+
+        $this->assertArrayHasKey('include', $query, 'The request did not ask for any relationships.');
+
+        $includes = array_map('trim', explode(',', $query['include']));
+
+        $this->assertContains('firstPost', $includes, 'Without firstPost there is no content to excerpt.');
+        $this->assertContains('user', $includes, 'Without user there is no author name or avatar.');
+    }
+
+    /**
+     * A well-formed response whose `included` is empty, because the relationship
+     * was not serialized — which is what discuss.flarum.org returned while
+     * fof/gamification narrowed the `firstPost` eager load, leaving every
+     * announcement card in every forum's admin panel blank.
+     *
+     * "The excerpt never arrived" and "the post has no text" are different
+     * things, and collapsing both to an empty string is why that went unnoticed.
+     * Only the absence of an excerpt is reportable, so absence is what it says.
+     */
+    public function test_excerpt_is_null_when_the_first_post_was_not_included(): void
+    {
+        $fetcher = $this->makeFetcher([
+            $this->makeApiResponse(
+                [$this->makeDiscussion([], [
+                    'firstPost' => ['data' => ['type' => 'posts', 'id' => '10']],
+                ])],
+                // Declared on the discussion, absent from `included`.
+                []
+            ),
+        ]);
+
+        $result = $fetcher->fetch();
+
+        $this->assertNull(
+            $result[0]['excerpt'],
+            'A missing include produced an empty-string excerpt, indistinguishable from a post with no content.'
+        );
+    }
+
+    /**
+     * A post that genuinely has no text still reports an empty excerpt rather
+     * than null, so the two cases stay distinguishable in both directions.
+     */
+    public function test_excerpt_is_empty_when_the_first_post_has_no_content(): void
+    {
+        $fetcher = $this->makeFetcher([
+            $this->makeApiResponse(
+                [$this->makeDiscussion([], [
+                    'firstPost' => ['data' => ['type' => 'posts', 'id' => '10']],
+                ])],
+                [[
+                    'type' => 'posts',
+                    'id' => '10',
+                    'attributes' => ['contentHtml' => ''],
+                ]]
+            ),
+        ]);
+
+        $result = $fetcher->fetch();
+
+        $this->assertSame('', $result[0]['excerpt']);
+    }
+
     public function test_throws_on_network_failure(): void
     {
         $fetcher = $this->makeFetcher([