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 file upload in collections #276

Closed
wants to merge 1 commit into from
Closed

Conversation

jwaschkau
Copy link

No description provided.

@coveralls
Copy link

coveralls commented Mar 6, 2017

Coverage Status

Coverage remained the same at 99.751% when pulling 2211e79 on BlazingSun:master into aaca075 on craue:master.

@craue
Copy link
Owner

craue commented Mar 17, 2017

It would be nice to have a test proving that there's an issue at all, as well as proving that this change fixes it, also to avoid future regressions.

@craue
Copy link
Owner

craue commented Mar 22, 2017

Could you set up such a failing scenario, e.g. by forking the demo bundle and modifying the photo upload flow?

@jwaschkau
Copy link
Author

@craue Yes, i will see what i can do.

@InTimesIT
Copy link

InTimesIT commented Apr 10, 2017

Hi, I just met this issue.
Thank you for your solution, I've override the method in my form flow. :)

@craue
Copy link
Owner

craue commented Aug 7, 2017

@jwaschkau, any progress?

@syeikhanugrah
Copy link

This should be merged.

@craue
Copy link
Owner

craue commented May 23, 2018

A test is still needed.

@craue
Copy link
Owner

craue commented May 23, 2018

What exactly is the issue here? Could you explain?

@craue
Copy link
Owner

craue commented May 31, 2018

I can't fix something that's not proven to be broken.

@jwaschkau
Copy link
Author

Hi @craue,
i won't be able to provide a testcase, because i am not developing PHP anymore.
In some cases the file upload was not working. I think currentStepData was filled with default values and array_merge just appends uploads as a list and does not overwrite existing values.

@craue
Copy link
Owner

craue commented Jun 6, 2018

@jwaschkau, alright. Never mind then. 😏

@syeikhanugrah, could you give some more details on how to reproduce the issue?

@syeikhanugrah
Copy link

syeikhanugrah commented Jun 11, 2018

@craue I've created repo for this issue: https://github.com/syeikhanugrah/CraueFormFlowBundle276, and try to replace with this: https://gist.github.com/syeikhanugrah/906406e47ec3d763d1fd7d270919e77b.

Because array_merge_recursive doesnt overwrite existing value, but appended to the new ArrayCollection.

@blump
Copy link

blump commented Mar 24, 2021

@syeikhanugrah 's solution is correct.

Is it possible to correct this problem?

@craue
Copy link
Owner

craue commented Mar 25, 2021

Unfortunately, the repo @syeikhanugrah mentions is unavailable. We got no test or code exposing the problem and I'm still unable to see the issue. Please enlighten me, @blump.

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

6 participants