github-mirror/framework
2
0
Fork 0
mirror of https://github.com/flarum/framework.git synced 2024-11-25 09:41:49 +08:00
Code Issues Actions 46 Packages Projects Releases Wiki Activity

refactor: remove listing of posts in the show discussion endpoint (#4067)

Browse Source
This commit is contained in:
Sami Mazouz 2024-10-16 18:02:46 +01:00 committed by GitHub
parent 40996de39a
commit b0e8f5ca36
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 84 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
extensions/flags/extend.php
View File

@ -52,11 +52,6 @@ return [
(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']);

5
extensions/likes/extend.php
View File

@ -50,11 +50,6 @@ return [
}
),
(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)

8
extensions/likes/tests/integration/api/ListPostsTest.php
View File

@ -173,9 +173,10 @@ class ListPostsTest extends TestCase
{
// 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 @@ class ListPostsTest extends TestCase
$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 @@ class ListPostsTest extends TestCase
public static function likesIncludeProvider(): array
{
return [
['posts,posts.likes'],
['posts.likes'],
['likes'],
[null],
];
}

12
extensions/mentions/extend.php
View File

@ -81,15 +81,6 @@ return [
'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 @@ return [
]),
(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']]);
}),

8
extensions/mentions/tests/integration/api/ListPostsTest.php
View File

@ -207,14 +207,15 @@ class ListPostsTest extends TestCase
// 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 @@ class ListPostsTest extends TestCase
public static function mentionedByIncludeProvider(): array
{
return [
['posts,posts.mentionedBy'],
['posts.mentionedBy'],
['mentionedBy'],
[null],
];
}

6
extensions/tags/extend.php
View File

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

67
framework/core/src/Api/Resource/DiscussionResource.php
View File

@ -75,7 +75,6 @@ class DiscussionResource extends AbstractDatabaseResource
->authenticated()
->can('startDiscussion')
->defaultInclude([
'posts',
'user',
'lastPostedUser',
'firstPost',
@ -89,12 +88,14 @@ class DiscussionResource extends AbstractDatabaseResource
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 @@ class DiscussionResource extends AbstractDatabaseResource
->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())

1
framework/core/src/Api/Resource/PostResource.php
View File

@ -101,7 +101,6 @@ class PostResource extends AbstractDatabaseResource
->defaultInclude([
'user',
'discussion',
'discussion.posts',
'discussion.lastPostedUser'
]),
Endpoint\Update::make()

21
framework/core/src/Api/Serializer.php
View File

@ -146,24 +146,17 @@ class Serializer extends \Tobyz\JsonApiServer\Serializer
*/
public function addIncluded(Relationship $field, $model, ?array $include): array
{
if (is_object($model)) {
$relatedResource = $this->resourceForModel($field, $model);
$relatedResource = $this->resourceForModel($field, $model);
if ($include === null) {
return [
'type' => $relatedResource->type(),
'id' => $relatedResource->getId($model, $this->context),
];
}
$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'],

52
framework/core/src/Forum/Content/Discussion.php
View File

@ -33,15 +33,7 @@ class Discussion
$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 @@ class Discussion
($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 @@ class Discussion
$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 @@ class Discussion
*
* @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 @@ class Discussion
->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()
);
}
}

13
framework/core/tests/integration/api/discussions/ShowTest.php
View File

@ -88,26 +88,31 @@ class ShowTest extends TestCase
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]

7
framework/core/tests/integration/extenders/ModelVisibilityTest.php
View File

@ -59,12 +59,15 @@ class ModelVisibilityTest extends TestCase
);
$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]