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

Set default $currentOrder in Sort class #212

Open
Tigrov opened this issue Nov 27, 2024 · 7 comments
Open

Set default $currentOrder in Sort class #212

Tigrov opened this issue Nov 27, 2024 · 7 comments

Comments

@Tigrov
Copy link
Member

Tigrov commented Nov 27, 2024

$currentOrder can be set to default value after normalizing config.

data/src/Reader/Sort.php

Lines 114 to 126 in 15ddd6e

$normalizedConfig[$fieldName] = array_merge(
[
'asc' => [$fieldName => SORT_ASC],
'desc' => [$fieldName => SORT_DESC],
'default' => 'asc',
],
$fieldConfig,
);
}
/** @psalm-var TConfig $normalizedConfig */
$this->config = $normalizedConfig;
}

This will help avoid an exception if $currentOrder is not set manually.

if (empty($sort->getOrder())) {
throw new InvalidArgumentException('Data should be always sorted to work with keyset pagination.');
}

@vjik
Copy link
Member

vjik commented Nov 27, 2024

How to understand which order is default?

@Tigrov
Copy link
Member Author

Tigrov commented Nov 27, 2024

Usually it's in ascending order.

@vjik
Copy link
Member

vjik commented Nov 27, 2024

Order in Sort object is field name + order direction, e. g. "id asc". Configuration may contains a lot of fields, how we can understand which field should use by default?

@Tigrov
Copy link
Member Author

Tigrov commented Nov 28, 2024

By default sort by all fields in ascending order

@vjik
Copy link
Member

vjik commented Nov 28, 2024

For set default order we should select field also, not only direction. How we can understand which field should use by default?

For example, configuration: ['id' => ..., 'name' => ..., 'create_date' => ...]. Order may be id asc or create_date asc or name asc ...

@Tigrov
Copy link
Member Author

Tigrov commented Nov 28, 2024

If we use KeysetPaginator, then sorting is required. In case $currentOrder is not set, we can use the first field from the configuration. If multiple fields are required for sorting, the user must initialize $currentOrder.

@samdark
Copy link
Member

samdark commented Dec 6, 2024

Makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants