Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: paginated list limit hard to change #3918

Merged
merged 5 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default class QueueState {

return app.store.find<Task[]>('package-manager-tasks', params || {}).then((data) => {
this.tasks = data;
this.total = data.payload.meta?.total;
this.total = data.payload.meta?.total!;

m.redraw();

Expand Down
6 changes: 5 additions & 1 deletion framework/core/js/src/common/Store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ export interface ApiPayloadPlural {
next?: string;
prev?: string;
};
meta?: MetaInformation;
meta?: MetaInformation & {
total?: number;
page?: number;
perPage?: number;
};
}

export type ApiPayload = ApiPayloadSingle | ApiPayloadPlural;
Expand Down
26 changes: 22 additions & 4 deletions framework/core/js/src/common/states/PaginatedListState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,15 @@ export interface PaginatedListRequestParams extends Omit<ApiQueryParamsPlural, '
}

export default abstract class PaginatedListState<T extends Model, P extends PaginatedListParams = PaginatedListParams> {
/**
* This value should not be relied upon when preloading an API document.
* In those cases the pageSize should be taken from the meta information of the preloaded
* document. Checkout `DiscussionListState.loadPage` for an example.
*/
public static DEFAULT_PAGE_SIZE = 20;

protected location!: PaginationLocation;
protected pageSize: number;
protected pageSize: number | null;

protected pages: Page<T>[] = [];
protected params: P = {} as P;
Expand All @@ -35,7 +42,7 @@ export default abstract class PaginatedListState<T extends Model, P extends Pagi
protected loadingPrev: boolean = false;
protected loadingNext: boolean = false;

protected constructor(params: P = {} as P, page: number = 1, pageSize: number = 20) {
protected constructor(params: P = {} as P, page: number = 1, pageSize: number | null = null) {
this.params = params;

this.location = { page };
Expand Down Expand Up @@ -108,12 +115,23 @@ export default abstract class PaginatedListState<T extends Model, P extends Pagi
...reqParams,
page: {
...reqParams.page,
offset: this.pageSize * (page - 1),
offset: (this.pageSize && this.pageSize * (page - 1)) || 0,
limit: this.pageSize || undefined,
},
include,
};

return app.store.find<T[]>(this.type, params);
return app.store.find<T[]>(this.type, params).then((results) => {
/*
* If this state does not rely on a preloaded API document to know the page size,
* then there is no initial list, and therefore the page size can be taken from subsequent requests.
*/
if (!this.pageSize) {
this.pageSize = results.payload?.meta?.perPage || PaginatedListState.DEFAULT_PAGE_SIZE;
}

return results;
});
}

/**
Expand Down
3 changes: 2 additions & 1 deletion framework/core/js/src/forum/states/DiscussionListState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default class DiscussionListState<P extends DiscussionListParams = Discus
protected eventEmitter: EventEmitter;

constructor(params: P, page: number = 1) {
super(params, page, 20);
super(params, page, null);

this.eventEmitter = globalEventEmitter.on('discussion.deleted', this.deleteDiscussion.bind(this));
}
Expand Down Expand Up @@ -44,6 +44,7 @@ export default class DiscussionListState<P extends DiscussionListParams = Discus

if (preloadedDiscussions) {
this.initialLoading = false;
this.pageSize = preloadedDiscussions.payload.meta?.perPage || DiscussionListState.DEFAULT_PAGE_SIZE;

return Promise.resolve(preloadedDiscussions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Notification from '../../common/models/Notification';

export default class NotificationListState extends PaginatedListState<Notification> {
constructor() {
super({}, 1, 20);
super({}, 1, null);
}

get type(): string {
Expand Down
20 changes: 20 additions & 0 deletions framework/core/src/Api/Controller/AbstractListController.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,24 @@ protected function createElement(mixed $data, SerializerInterface $serializer):
}

abstract protected function data(ServerRequestInterface $request, Document $document): iterable;

protected function addPaginationData(Document $document, ServerRequestInterface $request, string $url, ?int $total): void
{
$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);

$document->addPaginationLinks(
$url,
$request->getQueryParams(),
$offset,
$limit,
$total,
);

$document->setMeta([
'total' => $total,
'perPage' => $limit,
'page' => $offset / $limit + 1,
]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ protected function data(ServerRequestInterface $request, Document $document): it
$results = $this->filterer->filter($criteria, $limit, $offset);
}

$document->addPaginationLinks(
$this->addPaginationData(
$document,
$request,
$this->url->to('api')->route('discussions.index'),
$request->getQueryParams(),
$offset,
$limit,
$results->areMoreResults() ? null : 0
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,10 @@ protected function data(ServerRequestInterface $request, Document $document): it
$areMoreResults = true;
}

$document->addPaginationLinks(
$this->addPaginationData(
$document,
$request,
$this->url->to('api')->route('notifications.index'),
$request->getQueryParams(),
$offset,
$limit,
$areMoreResults ? null : 0
);

Expand Down
10 changes: 8 additions & 2 deletions framework/core/src/Forum/Content/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace Flarum\Forum\Content;

use Flarum\Api\Client;
use Flarum\Api\Controller\ListDiscussionsController;
use Flarum\Frontend\Document;
use Flarum\Http\UrlGenerator;
use Flarum\Locale\TranslatorInterface;
Expand All @@ -25,7 +26,8 @@ public function __construct(
protected Factory $view,
protected SettingsRepositoryInterface $settings,
protected UrlGenerator $url,
protected TranslatorInterface $translator
protected TranslatorInterface $translator,
protected ListDiscussionsController $controller
) {
}

Expand All @@ -39,11 +41,15 @@ public function __invoke(Document $document, Request $request): Document
$filters = Arr::pull($queryParams, 'filter', []);

$sortMap = resolve('flarum.forum.discussions.sortmap');
$limit = $this->controller->limit;

$params = [
'sort' => $sort && isset($sortMap[$sort]) ? $sortMap[$sort] : '',
'filter' => $filters,
'page' => ['offset' => ($page - 1) * 20, 'limit' => 20]
'page' => [
'offset' => ($page - 1) * $limit,
'limit' => $limit
],
];

if ($q) {
Expand Down
Loading