Cannot unselect all additional webspaces

[thomaskanzig]

Q	A
Sulu Version	2.1
Bug fix?	Yes
New feature?	No

Actual Behavior

When I select a webspace, the article is still displayed for all webspaces.

Expected Behavior

The article should only be displayed in the respective webspace selected.

Steps to Reproduce

On settings tab of article, the main webspace dropdown.

Possible Solutions

On additionalWebspaces should be send a multiselect array, instead null.

[alexander-schranz]

It totally make sense for me that this should be an empty array when saved once. We need to test the Analytics UI as there this was introduced that its null instead of empty array. https://github.com/sulu/sulu/pull/4484

[niklasnatter]

For completion: The problem is that you cannot unselect all options in the additional webspaces multi-select of the article bundle, right?

I think that the article bundle will fallback to use the default_additional_webspaces configuration if you unselect all options, because the MultiSelect will send null to the server if no option is selected.

[thomasduenser]

It is possible to unselect all options (in the additional webspaces multi-select) in the UI, but null is sent to the server instead of an empty array.

[niklasnatter]

Yeah that is what I suspected. The ArticleBundle then will probably fallback to use the default_additional_webspaces instead of handling null as "nothing selected".

I think the correct behaviour would be sending [] instead of null to the server from the MultiSelect. That should also fix the problem in the ArticleBundle 🙂

[thomaskanzig]

Any updates?

[luca-rath]

@nnatter @alexander-schranz I think we should not change the behaviour to send [] instead of null, if no option is selected, because there are many parts in the system expecting that the empty value of a field is always null. An example would be json schema validation. I would rather say, if the Customize Webspace Settings toggler is active, the article bundle should ignore the default values from the sulu_article config, it should only use those defaults if the toggler is disabled. Therefore this is not a sulu issue IMO but an ArticleBundle issue.

[alexander-schranz]

I definitly see it that a multi select should send a empty array always. If we did build on top of that something we need to have change there the behaviour, that it don't crash when not longer null is. Maybe mandatory does not longer work in 2.1, 2.2 but with the new json schema introducing in 2.3 this should hopefully be no problem to fix that there.

[luca-rath]

But I don't see the benefit of changing that behaviour.

Let's run through this example, imagine empty multi selects send []:

• When a user first creates an article, the additionalWebspaces property isn't sent to the server, because the details form doesn't know about it.
• Then, if the user navigates to the settings tab, changes something (but doesn't change the additionalWebspaces property) and then saves the form, [] is sent as value for the additional webspaces, just because the form knows, that that field exists and [] is the default value.
• So as soon as the user saves the settings tab, the defaults from the sulu_article config will stop working for that article, because the value of additionalWebspaces is [] now instead of null, even if the user didn't change that field.

[niklasnatter]

I think we should not change the behaviour to send [] instead of null, if no option is selected, because there are many parts in the system expecting that the empty value of a field is always null

I think the other selection field-types (eg. contact_selection) will send [] (at least when there was a selection before), right? I think I would expect the select to behave similar to the *_selection field-types.

But yeah, it is possible that we need to adjust some parts of the system to the new behaviour. In the case of the json schema validation, we should handle the mandatory attribute on the 2.x branch

[luca-rath]

I think the other selection field-types (eg. contact_selection) will send [] (at least when there was a selection before), right? I think I would expect the select to behave similar to the *_selection field-types.

Then I would rather adjust the selection field types ... Because another problem with [] as default value is that visibleCondition and disabledCondition will not work correctly, because !null evaluates to true but ![] evaluates to false

[niklasnatter]

Then, if the user navigates to the settings tab, changes something (but doesn't change the additionalWebspaces property) and then saves the form, [] is sent as value for the additional webspaces, just because the form knows, that that field exists and [] is the default value.

This should not happen because the the additionalWebspaces property is set to the configured value during serialization in the ArticleSubscriber (it looks like that is not working because the value is written to additionalWebspace instead of additionalWebspaces at the moment 🙈 ): https://github.com/sulu/SuluArticleBundle/blob/2.x/Document/Serializer/ArticleSubscriber.php#L144-L149

Then I would rather adjust the selection field types ... Because another problem with [] as default value is that visibleCondition and disabledCondition will not work correctly, because !null evaluates to true but ![] evaluates to false

To me, it is semantically wrong to use null instead of [] if nothing is selected in a multi-selection. Furthermore, I dont think we should decide which value to use based on the implementation of the jexl library. I am quite sure that this library supports a way to check if an array is empty 🙂

[luca-rath]

@thomaskanzig this should be fixed by sulu/SuluArticleBundle#556, but I'll keep this issue open to keep the discussion above alive.

[mario-fehr]

@luca-rath the provided fix from sulu/SuluArticleBundle#556 seems not to work for production.

[mario-fehr]

@nnatter any news how this should be fixed?

[thomasduenser]

@luca-rath @nnatter (@mfehr94 ) The fix in the SuluArticleBundle seems to only work if no default_additional_webspaces are defined in the configuration. I've had a short look at this with @alexander-schranz.

In our case the config looks like this:

sulu_article:
    default_main_webspace: 'main_webspace'
    default_additional_webspaces:
        - 'webspace_additional_one'
        - 'webspace_additional_two'

If we save the settings with no selected additional webspaces, it uses the config of the default_additional_webspaces instead of really setting it to null.

I think that the toggler Customize (Webspace settings) should be considered when defining these additional webspaces and when to use the default_additional_webspaces or not.

[luca-rath]

@mfehr94 @thomasduenser Could you please check, if sulu/SuluArticleBundle#590 fixes your problem?