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

Sorting doesn't work with 2 items symfony 4 #527

Open
fraserr opened this issue Dec 20, 2018 · 14 comments

Comments

@fraserr
Copy link

commented Dec 20, 2018

I'm using symfony4 and sorting works as expected unless I <3 items, has anyone else experienced this?

By the way, what a great package, has saved me loads of time - thanks very much!

@nicolasmure

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

Hello,
Can you provide some more details please ? Where are your items coming from ? What are you trying to sort? Maybe you can make a repository with your bug so we can clone it and investigate on this ;)

@ThomasRueckert

This comment has been minimized.

Copy link

commented Feb 12, 2019

Hi, i can confirm this problem. There is really nothing special about the data coming into the paginator:

this is our controller action:

/**
     * @Route("/", name="category_index", methods={"GET"})
     * @param CategoryRepository $categoryRepository
     * @return Response
     */
    public function index(
        CategoryRepository $categoryRepository,
        Request $request,
        \Knp\Component\Pager\PaginatorInterface $paginator
    ): Response
    {
        $pagination = $paginator->paginate(
            $categoryRepository->findAll(),
            $request->query->getInt('page', 1),
            50
        );

        return $this->render(
            'category/index.html.twig',
            [
                'pagination' => $pagination
            ]
        );
    }

and here is our template:

<table class="table table-striped table-bordered table-hover table-sm table-fixed-head small">
        <thead>
        <tr>
            <th>{{ knp_pagination_sortable(pagination, 'Id', 'id') }}</th>
            <th>{{ knp_pagination_sortable(pagination, 'Name', 'name') }}</th>
            <th>{{ knp_pagination_sortable(pagination, 'Sorting', 'sorting') }}</th>
            <th>actions</th>
        </tr>
        </thead>
        <tbody>
        {% for category in pagination %}
            <tr>
                <td>{{ category.id }}</td>
                <td>{{ category.name }}</td>
                <td>{{ category.sorting }}</td>
                <td>
                    <a href="{{ path('category_show', {'id': category.id}) }}">show</a>
                    <a href="{{ path('category_edit', {'id': category.id}) }}">edit</a>
                </td>
            </tr>
        {% else %}
            <tr>
                <td colspan="4">no records found</td>
            </tr>
        {% endfor %}
        </tbody>
    </table>

There is no further customization made to the repositoy etc. The problem occurs on any data we put in.

We are using version v2.8.0. The symfony/framework-bundle is on version v4.2.3.

@garak

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2019

@ThomasRueckert can you try with latest version (v4)?

@armellin

This comment has been minimized.

Copy link

commented Jul 5, 2019

I am not an expert, but I had the same problem and I think that it happens when you want to paginate an array made by exactly two items. I think that the bug is in knplabs/knp-components/src/Knp/Component/Pager/Event/Subscriber/Sortable/SolariumQuerySubscriber.php.
This subscriber has this test:

if (is_array($event->target) && 2 == count($event->target)) {
            $event->setCustomPaginationParameter('sorted', true);
            ..............

So, if the target is an array made by exactly two items, the SolariumQuerySubscriber triggers the sorting, preventing the ArraySubscriber to do his job.

As a quick-and-dirty hack I disabled the subscribers I don't use commenting them out in SortableSubscriber.php (in the same directory). If the bug is confirmed, there should be a more elegant fix making the tests that trigger the sort subscribers more robust.

@garak

This comment has been minimized.

Copy link
Collaborator

commented Jul 5, 2019

I tried to paginate an array of two items, I can't see any problem.

@ThomasRueckert you're not using the paginator correctly: you should paginate a query or a querybuilder, not all the results.

@armellin that's a special case, please open an issue on knp-components

@armellin

This comment has been minimized.

Copy link

commented Aug 9, 2019

@garak Issue opened.
Anyway, even if paginating the query or the queryBuilder is preferred, the paginator is supposed to work with an ArrayCollection or even a simple array as well, so @ThomasRueckert's code should work anyway (and he said that it does when there are more than two elements in the array).

@ThomasRueckert

This comment has been minimized.

Copy link

commented Aug 10, 2019

Sorry for the very delayed answer guys. Thank for the input.

I am no longer working on the project i had this problem. But i can give you some additional information for clarification:

When simplifying my source code for the example i made a mistake. As @garak stated correctly, one should use a query for pagination. I did so in the project. I changed it into a call to findAll() for shortening the example on here.

Unfortunately i only have another project with using symfony v3.

@garak

This comment has been minimized.

Copy link
Collaborator

commented Aug 10, 2019

@ThomasRueckert you can use version 4 of this bundle with Symfony 3

@ThomasRueckert

This comment has been minimized.

Copy link

commented Aug 12, 2019

Hey @garak,
i tried it on the other project. I wasn't able to reproduce the problem here. Seems fine to me now.
Thank you :)

@garak

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2019

@fraserr do you still experience this problem?

@johonunu

This comment has been minimized.

Copy link

commented Aug 13, 2019

Hey @garak,
i tried it on the other project. I wasn't able to reproduce the problem here. Seems fine to me now.
Thank you :)

Have you tried it with array items? If you are paginating array (not doctrine collection) and it has 2 items, it does not work.
@garak I already added comment to KnpLabs/knp-components#225

@garak

This comment has been minimized.

Copy link
Collaborator

commented Aug 13, 2019

@johonunu that case is limited to Solarium

@johonunu

This comment has been minimized.

Copy link

commented Aug 13, 2019

@garak The issue is that SolariumQuerySubscriber is before ArraySubscriber, so ArraySubscriber sorting logic would not be called at all, as check for SolariumQuerySubscriber is too generic, and in this case fails.

@garak

This comment has been minimized.

Copy link
Collaborator

commented Aug 13, 2019

It should be fixed in main library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.