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

Bug #9738 Fix nested form collections #11390

Merged
merged 1 commit into from
Apr 27, 2020
Merged

Bug #9738 Fix nested form collections #11390

merged 1 commit into from
Apr 27, 2020

Conversation

vic-blt
Copy link
Contributor

@vic-blt vic-blt commented Apr 22, 2020

If there are nested form collections, the value __name__ of the sub
collection is being replaced by the parent collection index.
Which makes every new sub collection items having the same index.

Q A
Branch? 1.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #9738
License MIT

@vic-blt vic-blt requested a review from a team as a code owner April 22, 2020 10:37
@Zales0123 Zales0123 added the Potential Bug Potential bugs or bugfixes, that needs to be reproduced. label Apr 22, 2020
Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

Hey Victorien,

thanks for your contribution! Your change seems to be ok. We need to fix master build before we will merge your PR.

Welcome to our community! :)

@lchrusciel lchrusciel requested a review from kulczy April 23, 2020 11:25
@vic-blt
Copy link
Contributor Author

vic-blt commented Apr 23, 2020

Hi Lukasz,

Welcome to our community! :)

Thank you :)

@lchrusciel
Copy link
Member

Btw. @vic-blt can you share some screenshots of the change done by this solution? Or an example code?

@vic-blt
Copy link
Contributor Author

vic-blt commented Apr 24, 2020

Without the fix :
without_fix

With the fix :
with_fix

You can see that without the fix, data-form-collection-index in data-prototype attribute, is already set to 0, which makes collection items having the same index because when data-prototype attribute of parent collection is modified by prototype = prototype.replace(/__name__/g, this.count);, data-form-collection-index value of sub collection is also replaced by this.count which is the index of parent collection.

@lchrusciel lchrusciel changed the base branch from 1.7 to 1.6 April 24, 2020 15:31
@lchrusciel lchrusciel changed the base branch from 1.6 to 1.7 April 24, 2020 15:39
@probot-autolabeler probot-autolabeler bot added Documentation Documentation related issues and PRs - requests, fixes, proposals. Maintenance CI configurations, READMEs, releases, etc. labels Apr 24, 2020
If there are nested form collections, the value __name__ of the sub
collection is being replaced by the parent collection index.
Which makes every new sub collection items having the same index.
@lchrusciel lchrusciel changed the base branch from 1.7 to 1.6 April 24, 2020 15:40
@lchrusciel
Copy link
Member

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "issue-9738-fix-nested-form-collections" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@lchrusciel lchrusciel removed Documentation Documentation related issues and PRs - requests, fixes, proposals. Maintenance CI configurations, READMEs, releases, etc. labels Apr 24, 2020
@lchrusciel lchrusciel merged commit 071ac83 into Sylius:1.6 Apr 27, 2020
@lchrusciel
Copy link
Member

Thank you, Victorien! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Potential Bug Potential bugs or bugfixes, that needs to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants