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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Fix BC-Break: Default value on multiselect object type introduced by #15871 #16779

Merged
merged 12 commits into from
Mar 14, 2024
Merged

[Bug]: Fix BC-Break: Default value on multiselect object type introduced by #15871 #16779

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e81ca45
remove parent interface is bc break
kingjia90 Mar 13, 2024
7dd882e
fix bc-break reported by Niklas
kingjia90 Mar 13, 2024
4360f19
Update models/DataObject/ClassDefinition/Data/Multiselect.php
kingjia90 Mar 13, 2024
fa1b738
Update models/DataObject/ClassDefinition/Data/Multiselect.php
kingjia90 Mar 13, 2024
93d8389
missing default handling for editmode
kingjia90 Mar 13, 2024
333cc04
missing default handling for editmode
kingjia90 Mar 13, 2024
91a22f5
enrich definition
kingjia90 Mar 13, 2024
c131318
enrich definition
kingjia90 Mar 13, 2024
fed3707
enrich definition
kingjia90 Mar 13, 2024
dc4f9fa
Update models/DataObject/ClassDefinition/Data/Multiselect.php
kingjia90 Mar 13, 2024
e6331b0
Fix SelectionProviderTrait (#16782)
blankse Mar 13, 2024
d1ad341
fix getDataForQueryResource
kingjia90 Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion models/DataObject/ClassDefinition/Data/Multiselect.php
View file
Open in desktop
Expand Up @@ -122,8 +122,16 @@ public function getRenderType(): ?string
return $this->renderType;
}

public function setDefaultValue(array $defaultValue): static
/**
* @return $this
*/
public function setDefaultValue(array|string|null $defaultValue): static
kingjia90 marked this conversation as resolved.
Show resolved Hide resolved
{
if (!$defaultValue) {
$defaultValue = null;
kingjia90 marked this conversation as resolved.
Show resolved Hide resolved
}elseif (is_string($defaultValue)){
$defaultValue = [$defaultValue];
}
Copy link
Contributor

@blankse blankse Mar 13, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this? When it is null, it must not be changed, Empty Array should also no problem or?

Suggested change
if (!$defaultValue) {
$defaultValue = null;
}elseif (is_string($defaultValue)){
$defaultValue = [$defaultValue];
}
if (is_string($defaultValue)) {
$defaultValue = $defaultValue !== '' ? [$defaultValue] : null;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My current project has null and "" and somestring values which broke, I would suspect that perhaps some integers could have made it's way into the default value storage as well. Maybe check for scalar values would make it more forgiving?

And for v12.0 create a migration which forces array type for all while making the type stricter again?

Copy link
Contributor

@blankse blankse Mar 13, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did these defaultValues came from?
Was the field type a single select in the past?
So after the convert in the backend the default value from the single select still exists?

If this is the case, I think we could not make it type strict. This can also possible in v12. We should than also check the case when someone convert the multi select in a single select.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were created using v6 or v10, some could possibly have been selects or other field types at one point as well.

$this->defaultValue = $defaultValue;

return $this;
Expand Down
2 changes: 1 addition & 1 deletion models/DataObject/ClassDefinition/DynamicOptionsProvider/SelectOptionsProviderInterface.php
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@

use Pimcore\Model\DataObject\ClassDefinition\Data;

interface SelectOptionsProviderInterface
interface SelectOptionsProviderInterface extends MultiSelectOptionsProviderInterface
{
public function getOptions(array $context, Data $fieldDefinition): array;

Expand Down