Skip to content

Conversation

@xabbuh
Copy link
Member

@xabbuh xabbuh commented May 19, 2020

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #36852
License MIT
Doc PR

// in different steps without breaking early enough
$this->resolvedGroups[$field] = (array) $group;
$validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $formConstraint);
$fieldFormConstraint = new Form();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed for now to prevent hash collisions (see also #36415).

/* @var $metadata ClassMetadata */
$metadata->addConstraint(new Form());
$metadata->addPropertyConstraint('children', new Valid());
$metadata->traversalStrategy = TraversalStrategy::NONE;
Copy link
Member Author

Choose a reason for hiding this comment

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

traversal happens explicitly inside the FormValidator

@xabbuh xabbuh force-pushed the issue-36852 branch 3 times, most recently from d17a4d2 to 2fa63a5 Compare May 21, 2020 11:40
@xabbuh xabbuh marked this pull request as ready for review May 21, 2020 11:56
@xabbuh
Copy link
Member Author

xabbuh commented May 21, 2020

This is ready to be reviewed.

@xabbuh xabbuh force-pushed the issue-36852 branch 3 times, most recently from 68fcedb to 27d9f82 Compare May 21, 2020 11:59
@xabbuh
Copy link
Member Author

xabbuh commented May 21, 2020

/cc @HeahDude

@lyrixx
Copy link
Member

lyrixx commented May 29, 2020

Hello,

I ran our full test suite and I confirm this patch fixed everything. It's all 💚

Thanks @xabbuh and @stof

@nicolas-grekas
Copy link
Member

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 2e8ae40 into symfony:3.4 May 30, 2020
This was referenced May 31, 2020
@bendavies
Copy link
Contributor

This breaks property mapping in some way.
Investigating.

@xabbuh
Copy link
Member Author

xabbuh commented Jun 1, 2020

Maybe the same as #37027?

@bendavies
Copy link
Contributor

@xabbuh yes looks like it. also #37025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants