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

[SearchBundle] Allow false value for enabled pre search filter field #2424

Merged
merged 1 commit into from
Feb 3, 2015
Merged

[SearchBundle] Allow false value for enabled pre search filter field #2424

merged 1 commit into from
Feb 3, 2015

Conversation

egeloen
Copy link

@egeloen egeloen commented Feb 2, 2015

Hey!

This PR fixes an issue introduced in #2418. Using cannotBeEmpty on a boolean node seems to not allow us to use false as value.

Using cannotBeEmpty on a boolean node does not allow to use false as value.
@egeloen egeloen changed the title [SearchBundle] Allow false as for pre search filter enabled fied [SearchBundle] Allow false value for enabled pre search filter field Feb 2, 2015
pjedrzejewski pushed a commit that referenced this pull request Feb 3, 2015
[SearchBundle] Allow false value for enabled pre search filter field
@pjedrzejewski pjedrzejewski merged commit 7aeaff2 into Sylius:master Feb 3, 2015
@pjedrzejewski
Copy link
Member

Thank you Eric! 👍

@stloyd
Copy link
Contributor

stloyd commented Feb 3, 2015

I guess better would be to use canBeEnabled() method...

@egeloen
Copy link
Author

egeloen commented Feb 3, 2015

Basically, I don't understand what #2418 tries to do. Currently, the tree forces us to put a taxonomy pre search filter even if we disable the feature... Should I switch it to canBeEnabled? Additionally, I can use a normalizer which verifies if the taxonomy node is needed according to the enabled node.

@egeloen egeloen deleted the search-config branch February 3, 2015 09:35
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

Successfully merging this pull request may close these issues.

None yet

4 participants