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

Ignore not existing columns in the ListBuilder #7207

Open
Prokyonn opened this issue Nov 22, 2023 · 4 comments
Open

Ignore not existing columns in the ListBuilder #7207

Prokyonn opened this issue Nov 22, 2023 · 4 comments
Labels
Bug Error or unexpected behavior of already existing functionality

Comments

@Prokyonn
Copy link
Member

Q A
Sulu Version 2.5

Actual Behavior

When you have a page with multiple tabs with listviews, e.g. the Automation tab from the AutomationBundle and the Data tab from the FormBundle, the query parameters for sorting/filtering the lists are copied from one tab to the other.
Therefore it can happen, that you are on the Automation tab, sort after schedule and then move to the Data tab, the request sent to the server has then sortBy=schedule which ends in a 500, because the Data listview configuration does not have a column schedule.

Expected Behavior

The request doesn't end in a 500 and returns the data with default sorting, ignoring the query parameters that define non-existing columns.

Steps to Reproduce

see above

Possible Solutions

Ignore query parameters that reference not existing columns

@Prokyonn Prokyonn added the Bug Error or unexpected behavior of already existing functionality label Nov 22, 2023
@MrSrsen
Copy link

MrSrsen commented May 28, 2024

It looks like bug in the vendor/sulu/sulu/src/Sulu/Component/Rest/RestHelper.php class in the initializeListBuilder method. This condition:

        $sortBy = $this->listRestHelper->getSortColumn();
        if (null != $sortBy) {
            $listBuilder->sort($fieldDescriptors[$sortBy], $this->listRestHelper->getSortOrder());
        }

should look like this:

        $sortBy = $this->listRestHelper->getSortColumn();
        if (null !== $sortBy && \array_key_exists($sortBy, $fieldDescriptors)) {
            $listBuilder->sort($fieldDescriptors[$sortBy], $this->listRestHelper->getSortOrder());
        }

I copied the original code and fixed it in my own class and decorated the original like this:

    App\Sulu\RestHelper:
        decorates: sulu_core.rest_helper
        arguments:
            - '@sulu_core.list_rest_helper'

and it fixed my problem. Good-enough hotfix for now.

@alexander-schranz
Copy link
Member

@MrSrsen Sounds like a good solution. Do you want to create a pull request for this?

@mamazu
Copy link
Contributor

mamazu commented May 28, 2024

Alternatively we could also allow the sort to allow null as sorting then we could remove the if statement entirely.

@MrSrsen
Copy link

MrSrsen commented May 29, 2024

@MrSrsen Sounds like a good solution. Do you want to create a pull request for this?

@alexander-schranz to which repository should I make the PR? sulu/sulu? And shouldn't this rather be fixed in the JavaScript?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error or unexpected behavior of already existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants