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 submit filters #6877

Merged
merged 16 commits into from
Feb 22, 2021
Merged

Fix submit filters #6877

merged 16 commits into from
Feb 22, 2021

Conversation

willemverspyck
Copy link
Contributor

@willemverspyck willemverspyck commented Feb 17, 2021

Subject

Closes #6865

Changelog

### Added
- Added `AbstractAdmin::mergeParameters()` to merge parameters but replace them when it's a subarray.

### Fixed
- Fix merge problem for filters with array value

@VincentLanglet VincentLanglet requested a review from a team February 17, 2021 12:24
@VincentLanglet
Copy link
Member

Can you rebase in order to fix the conflict @willemverspyck ?

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

src/Resources/public/Admin.js Outdated Show resolved Hide resolved
src/Util/ParametersManipulator.php Outdated Show resolved Hide resolved
src/Util/ParametersManipulator.php Outdated Show resolved Hide resolved
tests/Util/ParametersManipulatorTest.php Outdated Show resolved Hide resolved
@willemverspyck
Copy link
Contributor Author

Rebase and fix the conflict is done. Only I'm still having a problem during testing that I don't know how to fix:

For example:

If I set the default values like this in the Admin class:

protected function configureDefaultFilterValues(array &$filterValues): void
{
    $filterValues['status'] = [
        'value' => [
            Content::STATUS_ACCEPT,
            Content::STATUS_DECLINE,
            Content::STATUS_REVIEW,
        ],
    ];
}

And you remove this "status" filter and click on "Filter", then the filter will be back, because you don't pass this filter anymore with the GET parameters, but they are retrieved again from the "configureDefaultFilterValues" method. How would you solve this? Maybe create separate issue for this.

@VincentLanglet
Copy link
Member

Rebase and fix the conflict is done. Only I'm still having a problem during testing that I don't know how to fix:

For example:

If I set the default values like this in the Admin class:

protected function configureDefaultFilterValues(array &$filterValues): void
{
    $filterValues['status'] = [
        'value' => [
            Content::STATUS_ACCEPT,
            Content::STATUS_DECLINE,
            Content::STATUS_REVIEW,
        ],
    ];
}

And you remove this "status" filter and click on "Filter", then the filter will be back, because you don't pass this filter anymore with the GET parameters, but they are retrieved again from the "configureDefaultFilterValues" method. How would you solve this? Maybe create separate issue for this.

This is another issue indeed. This should be handled by the JS and I think it's already solved.
https://github.com/sonata-project/SonataAdminBundle/pull/6871/files#diff-8859a7d22354e11ba589b1f6d5b43b704276a2cbfe9899ecdd0ce0cb41e8dd63R341-R345
We now keep all the changed value.

@VincentLanglet VincentLanglet merged commit dd31045 into sonata-project:3.x Feb 22, 2021
@VincentLanglet
Copy link
Member

Thanks @willemverspyck

@SonataCI
Copy link
Collaborator

Ok @phansys,

I requested a new release for SonataAdminBundle via Slack in #releases channel 👍

If you want to get notified about new releases, make sure to follow SonataNews on Twitter!

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

Successfully merging this pull request may close these issues.

Advanced filter value lost when changing default value type
5 participants