refactor: remove preloading/listing of posts in the show discussion e…

…ndpoint

extensions/flags/extend.php

@@ -52,11 +52,6 @@
  (new Extend\ApiResource(Resource\ForumResource::class))
  ->fields(ForumResourceFields::class),
 
- (new Extend\ApiResource(Resource\DiscussionResource::class))
- ->endpoint(Endpoint\Show::class, function (Endpoint\Show $endpoint) {
- return $endpoint->addDefaultInclude(['posts.flags', 'posts.flags.user']);
- }),
-
  (new Extend\ApiResource(Resource\PostResource::class))
  ->endpoint([Endpoint\Index::class, Endpoint\Show::class], function (Endpoint\Index|Endpoint\Show $endpoint) {
  return $endpoint->addDefaultInclude(['flags', 'flags.user']);

extensions/likes/extend.php

@@ -50,11 +50,6 @@ function (Endpoint\Index|Endpoint\Show|Endpoint\Create|Endpoint\Update $endpoint
  }
  ),
 
- (new Extend\ApiResource(Resource\DiscussionResource::class))
- ->endpoint(Endpoint\Show::class, function (Endpoint\Show $endpoint): Endpoint\Endpoint {
- return $endpoint->addDefaultInclude(['posts.likes']);
- }),
-
  (new Extend\Event())
  ->listen(PostWasLiked::class, Listener\SendNotificationWhenPostIsLiked::class)
  ->listen(PostWasUnliked::class, Listener\SendNotificationWhenPostIsUnliked::class)

extensions/likes/tests/integration/api/ListPostsTest.php

@@ -173,9 +173,10 @@ public function likes_relation_returns_limited_results_and_shows_only_visible_po
  {
  // Show discussion endpoint
  $response = $this->send(
- $this->request('GET', '/api/discussions/100', [
+ $this->request('GET', '/api/posts', [
  'authenticatedAs' => 2,
  ])->withQueryParams([
+ 'filter' => ['discussion' => 100],
  'include' => $include,
  ])
  );
@@ -184,7 +185,7 @@ public function likes_relation_returns_limited_results_and_shows_only_visible_po
 
  $this->assertEquals(200, $response->getStatusCode(), $body);
 
- $included = json_decode($body, true)['included'] ?? [];
+ $included = json_decode($body, true)['data'] ?? [];
 
  $likes = collect($included)
  ->where('type', 'posts')
@@ -206,8 +207,7 @@ public function likes_relation_returns_limited_results_and_shows_only_visible_po
  public static function likesIncludeProvider(): array
  {
  return [
- ['posts,posts.likes'],
- ['posts.likes'],
+ ['likes'],
  [null],
  ];
  }

extensions/mentions/extend.php

@@ -81,15 +81,6 @@
  'lastPost.mentionsPosts.user', 'lastPost.mentionsPosts.discussion', 'lastPost.mentionsGroups',
  ],
  ]);
- })
- ->endpoint(Endpoint\Show::class, function (Endpoint\Show $endpoint): Endpoint\Show {
- return $endpoint->addDefaultInclude(['posts.mentionedBy', 'posts.mentionedBy.user', 'posts.mentionedBy.discussion'])
- ->eagerLoadWhenIncluded([
- 'posts' => [
- 'posts.mentionsUsers', 'posts.mentionsPosts', 'posts.mentionsPosts.user',
- 'posts.mentionsPosts.discussion', 'posts.mentionsGroups'
- ],
- ]);
  }),
 
  (new Extend\ApiResource(Resource\UserResource::class))
@@ -128,9 +119,6 @@
  ]),
 
  (new Extend\ApiResource(Resource\DiscussionResource::class))
- ->endpoint(Endpoint\Show::class, function (Endpoint\Show $endpoint): Endpoint\Show {
- return $endpoint->eagerLoadWhenIncluded(['posts' => ['posts.mentionsTags']]);
- })
  ->endpoint(Endpoint\Index::class, function (Endpoint\Index $endpoint): Endpoint\Index {
  return $endpoint->eagerLoadWhenIncluded(['firstPost' => ['firstPost.mentionsTags'], 'lastPost' => ['lastPost.mentionsTags']]);
  }),

extensions/mentions/tests/integration/api/ListPostsTest.php

@@ -207,14 +207,15 @@ public function mentioned_by_relation_returns_limited_results_and_shows_only_vis
 
  // Show discussion endpoint
  $response = $this->send(
- $this->request('GET', '/api/discussions/100', [
+ $this->request('GET', '/api/posts', [
  'authenticatedAs' => 2,
  ])->withQueryParams([
+ 'filter' => ['discussion' => 100],
  'include' => $include,
  ])
  );
 
- $included = json_decode($body = $response->getBody()->getContents(), true)['included'] ?? [];
+ $included = json_decode($body = $response->getBody()->getContents(), true)['data'] ?? [];
 
  $this->assertEquals(200, $response->getStatusCode(), $body);
 
@@ -233,8 +234,7 @@ public function mentioned_by_relation_returns_limited_results_and_shows_only_vis
  public static function mentionedByIncludeProvider(): array
  {
  return [
- ['posts,posts.mentionedBy'],
- ['posts.mentionedBy'],
+ ['mentionedBy'],
  [null],
  ];
  }

extensions/tags/extend.php

@@ -178,10 +178,4 @@ function (Endpoint\Index|Endpoint\Show|Endpoint\Create $endpoint) {
  return $endpoint
  ->addDefaultInclude(['eventPostMentionsTags']);
  }),
-
- (new Extend\ApiResource(Resource\DiscussionResource::class))
- ->endpoint(Endpoint\Show::class, function (Endpoint\Show $endpoint) {
- return $endpoint
- ->addDefaultInclude(['posts.eventPostMentionsTags']);
- }),
 ];

framework/core/src/Api/Resource/DiscussionResource.php

@@ -75,7 +75,6 @@ public function endpoints(): array
  ->authenticated()
  ->can('startDiscussion')
  ->defaultInclude([
- 'posts',
  'user',
  'lastPostedUser',
  'firstPost',
@@ -89,12 +88,14 @@ public function endpoints(): array
  Endpoint\Show::make()
  ->defaultInclude([
  'user',
- 'posts',
- 'posts.discussion',
- 'posts.user',
- 'posts.user.groups',
- 'posts.editedUser',
- 'posts.hiddenUser'
+ 'lastPostedUser',
+ 'firstPost',
+ 'firstPost.discussion',
+ 'firstPost.user',
+ 'firstPost.user.groups',
+ 'firstPost.editedUser',
+ 'firstPost.hiddenUser',
+ 'lastPost'
  ]),
  Endpoint\Index::make()
  ->defaultInclude([
@@ -206,54 +207,14 @@ public function fields(): array
  ->type('posts'),
  Schema\Relationship\ToMany::make('posts')
  ->withLinkage(function (Context $context) {
- return $context->showing(self::class);
+ return $context->showing(self::class)
+ || $context->creating(self::class)
+ || $context->creating(PostResource::class);
  })
- ->includable()
- // @todo: remove this, and send a second request from the frontend to /posts instead. Revert Serializer::addIncluded while you're at it.
  ->get(function (Discussion $discussion, Context $context) {
- $showingDiscussion = $context->showing(self::class);
-
- if (! $showingDiscussion) {
- return fn () => $discussion->posts->all();
- }
-
- /** @var Endpoint\Show $endpoint */
- $endpoint = $context->endpoint;
-
- $actor = $context->getActor();
-
- $limit = PostResource::$defaultLimit;
-
- if (($near = Arr::get($context->request->getQueryParams(), 'page.near')) > 1) {
- $offset = $this->posts->getIndexForNumber($discussion->id, $near, $actor);
- $offset = max(0, $offset - $limit / 2);
- } else {
- $offset = $endpoint->extractOffsetValue($context, $endpoint->defaultExtracts($context));
- }
-
- /** @var Endpoint\Endpoint $endpoint */
- $endpoint = $context->endpoint;
-
- $posts = $discussion->posts()
- ->whereVisibleTo($actor)
- ->with($endpoint->getEagerLoadsFor('posts', $context))
- ->with($endpoint->getWhereEagerLoadsFor('posts', $context))
- ->orderBy('number')
- ->skip($offset)
- ->take($limit)
- ->get();
-
- /** @var Post $post */
- foreach ($posts as $post) {
- $post->setRelation('discussion', $discussion);
- }
-
- $allPosts = $discussion->posts()->whereVisibleTo($actor)->orderBy('number')->pluck('id')->all();
- $loadedPosts = $posts->all();
-
- array_splice($allPosts, $offset, $limit, $loadedPosts);
-
- return $allPosts;
+ // @todo: is it possible to refactor the frontend to not need all post IDs?
+ // some kind of percentage-based stream scrubber?
+ return $discussion->posts()->whereVisibleTo($context->getActor())->select('id')->get()->all();
  }),
  Schema\Relationship\ToOne::make('mostRelevantPost')
  ->visible(fn (Discussion $model, Context $context) => $context->listing())

framework/core/src/Api/Resource/PostResource.php

@@ -101,7 +101,6 @@ public function endpoints(): array
  ->defaultInclude([
  'user',
  'discussion',
- 'discussion.posts',
  'discussion.lastPostedUser'
  ]),
  Endpoint\Update::make()

framework/core/src/Api/Serializer.php

@@ -146,24 +146,17 @@ private function whenResolved($value, $callback, bool $prepend = false): void
  */
  public function addIncluded(Relationship $field, $model, ?array $include): array
  {
- if (is_object($model)) {
- $relatedResource = $this->resourceForModel($field, $model);
-
- if ($include === null) {
- return [
- 'type' => $relatedResource->type(),
- 'id' => $relatedResource->getId($model, $this->context),
- ];
- }
+ $relatedResource = $this->resourceForModel($field, $model);
 
- $data = $this->addToMap($relatedResource, $model, $include);
- } else {
- $data = [
- 'type' => $field->collections[0],
- 'id' => (string) $model,
+ if ($include === null) {
+ return [
+ 'type' => $relatedResource->type(),
+ 'id' => $relatedResource->getId($model, $this->context),
  ];
  }
 
+ $data = $this->addToMap($relatedResource, $model, $include);
+
  return [
  'type' => $data['type'],
  'id' => $data['id'],
@@ -181,7 +174,7 @@ private function resourceForModel(Relationship $field, $model): Resource
  }
 
  throw new RuntimeException(
- 'No resource type defined to represent model '.get_class($model),
+ 'No resource type defined to represent model ' . get_class($model),
  );
  }
 

framework/core/src/Forum/Content/Discussion.php

@@ -33,15 +33,7 @@ public function __invoke(Document $document, Request $request): Document
  $near = intval(Arr::get($queryParams, 'near'));
  $page = max(1, intval(Arr::get($queryParams, 'page')), 1 + intdiv($near, 20));
 
- $params = [
- 'page' => [
- 'near' => $near,
- 'offset' => ($page - 1) * 20,
- 'limit' => 20
- ]
- ];
-
- $apiDocument = $this->getApiDocument($request, $id, $params);
+ $apiDocument = $this->getApiDocument($request, $id);
 
  $getResource = function ($link) use ($apiDocument) {
  return Arr::first($apiDocument->included, function ($value) use ($link) {
@@ -64,9 +56,21 @@ public function __invoke(Document $document, Request $request): Document
  ($queryString ? '?'.$queryString : '');
  };
 
+ $params = [
+ 'filter' => [
+ 'discussion' => intval($id),
+ ],
+ 'page' => [
+ 'near' => $near,
+ 'offset' => ($page - 1) * 20,
+ 'limit' => 20,
+ ],
+ ];
+
+ $postsApiDocument = $this->getPostsApiDocument($request, $params);
  $posts = [];
 
- foreach ($apiDocument->included as $resource) {
+ foreach ($postsApiDocument->data as $resource) {
  if ($resource->type === 'posts' && isset($resource->relationships->discussion) && isset($resource->attributes->contentHtml)) {
  $posts[] = $resource;
  }
@@ -77,6 +81,15 @@ public function __invoke(Document $document, Request $request): Document
 
  $document->title = $apiDocument->data->attributes->title;
  $document->content = $this->view->make('flarum.forum::frontend.content.discussion', compact('apiDocument', 'page', 'hasPrevPage', 'hasNextPage', 'getResource', 'posts', 'url'));
+
+ $apiDocument->included = array_values(array_filter($apiDocument->included, function ($value) {
+ return $value->type !== 'posts';
+ }));
+ $apiDocument->included = array_merge($apiDocument->included, $postsApiDocument->data, $postsApiDocument->included);
+ $apiDocument->included = array_values(array_filter($apiDocument->included, function ($value) use ($apiDocument) {
+ return $value->type !== 'discussions' || $value->id !== $apiDocument->data->id;
+ }));
+
  $document->payload['apiDocument'] = $apiDocument;
 
  $document->canonicalUrl = $url([]);
@@ -91,7 +104,7 @@ public function __invoke(Document $document, Request $request): Document
  *
  * @throws RouteNotFoundException
  */
- protected function getApiDocument(Request $request, string $id, array $params): object
+ protected function getApiDocument(Request $request, string $id, array $params = []): object
  {
  $params['bySlug'] = true;
 
@@ -104,4 +117,21 @@ protected function getApiDocument(Request $request, string $id, array $params):
  ->getBody()
  );
  }
+
+ /**
+ * Get the result of an API request to list the posts of a discussion.
+ *
+ * @throws RouteNotFoundException
+ */
+ protected function getPostsApiDocument(Request $request, array $params): object
+ {
+ return json_decode(
+ $this->api
+ ->withoutErrorHandling()
+ ->withParentRequest($request)
+ ->withQueryParams($params)
+ ->get("/posts")
+ ->getBody()
+ );
+ }
 }

framework/core/tests/integration/api/discussions/ShowTest.php

@@ -88,26 +88,31 @@ public function guest_cannot_see_empty_discussion()
  public function guest_cannot_see_hidden_posts()
  {
  $response = $this->send(
- $this->request('GET', '/api/discussions/4')
+ $this->request('GET', '/api/posts')
+ ->withQueryParams([
+ 'filter' => ['discussion' => 4]
+ ])
  );
 
  $json = json_decode($response->getBody()->getContents(), true);
 
- $this->assertEmpty(Arr::get($json, 'data.relationships.posts.data'));
+ $this->assertEmpty(Arr::get($json, 'data'));
  }
 
  #[Test]
  public function author_can_see_hidden_posts()
  {
  $response = $this->send(
- $this->request('GET', '/api/discussions/4', [
+ $this->request('GET', '/api/posts', [
  'authenticatedAs' => 2,
+ ])->withQueryParams([
+ 'filter' => ['discussion' => 4]
  ])
  );
 
  $json = json_decode($response->getBody()->getContents(), true);
 
- $this->assertEquals(2, Arr::get($json, 'data.relationships.posts.data.0.id'), $response->getBody()->getContents());
+ $this->assertEquals(2, Arr::get($json, 'data.0.id'), $response->getBody()->getContents());
  }
 
  #[Test]

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

@@ -59,12 +59,15 @@ public function when_allowed_guests_can_see_hidden_posts()
  );
 
  $response = $this->send(
- $this->request('GET', '/api/discussions/2')
+ $this->request('GET', '/api/posts')
+ ->withQueryParams([
+ 'filter' => ['discussion' => 2]
+ ])
  );
 
  $json = json_decode($response->getBody()->getContents(), true);
 
- $this->assertEquals(1, Arr::get($json, 'data.relationships.posts.data.0.id'));
+ $this->assertEquals(1, Arr::get($json, 'data.0.id'));
  }
 
  #[Test]