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 array keys of an element type collection #192

Closed
wants to merge 1 commit into from

Conversation

hussonkevin
Copy link

When you get an element with form type CollectionType and you create 3 items, you'll get the following indices [0 => data, 1 => data, 2 => data]
If you remove the index 1 then you get the following indices [0 => data, 2 => data]
When you save the page, all elements are saved into json object but the format of this sub element is converted into object instead of array.
To fix that you need to apply array_values() to reset keys. This function cannot be applied on a Form Event because you override form data with the request into this method processFormDataWithoutChild due to this:

if ($form->getConfig()->getType()->getInnerType() instanceof NativeFileType && !empty($requestData)) {
    // Check if we have a string value for this fields which is the file path (During edition for example)
    return $requestData; // Will return the current filename string
}

@madamebiz madamebiz added the quick win Easy to pick, everybody will be happy. label Nov 24, 2022
@maximehuran
Copy link
Member

Hi Kevin,

Thank you for your contribution.
We know this case and we actually correct it on Sylius directly using this patch :
Sylius/Sylius#13666

But it does not work since Sylius 1.11 and we haven't go another patch at this moment.
We had a discussion about this issue before on #174 and we have specific case when indexes are not integer as said @jacquesbh in #174 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quick win Easy to pick, everybody will be happy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants