Skip to content

RepeatedType overrides mapped value for the type to work properly #13519

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

Merged
merged 1 commit into from
Sep 25, 2020

Conversation

biozshock
Copy link
Contributor

Add a note to the documentation about overridden mapped option for inner fields.

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Thanks again! Some minor comments

@@ -129,6 +129,11 @@ the label::
'second_options' => ['label' => 'Repeat Password'],
]);

.. note::
Copy link
Contributor

Choose a reason for hiding this comment

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

What about line 100 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me this is most logic place to have a note about options. I would like to have a note about this exactly at the place where documentation describes options for inner fields.

Copy link
Contributor

@HeahDude HeahDude Apr 10, 2020

Choose a reason for hiding this comment

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

I think this information is too important and should be shown before any options. This is needed in order to handle validation, and the line I linked is about that section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I leave that one open for now.

@biozshock biozshock force-pushed the repeated-type_not-mapped-inner branch from 6bc157f to 94980d2 Compare April 10, 2020 18:46
@biozshock biozshock force-pushed the repeated-type_not-mapped-inner branch from 94980d2 to b29335a Compare April 10, 2020 19:26
@HeahDude
Copy link
Contributor

Side note: we should wait next patch of 3.4 before merging here.

@HeahDude HeahDude added Waiting Code Merge Docs for features pending to be merged and removed Status: Needs Review labels Apr 11, 2020
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Apr 13, 2020
…(biozshock)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] RepeatedType should always have inner types mapped

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Doc PR| symfony/symfony-docs#13519 |
| Tickets       | Fix #36410
| License       | MIT

Always set mapped=true to override inner type mapped setting.
Throw an exception if inner types of RepeatedType has mapped=false

Commits
-------

728cd66 RepeatedType should always have inner types mapped
symfony-splitter pushed a commit to symfony/form that referenced this pull request Apr 13, 2020
…(biozshock)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] RepeatedType should always have inner types mapped

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Doc PR| symfony/symfony-docs#13519 |
| Tickets       | Fix #36410
| License       | MIT

Always set mapped=true to override inner type mapped setting.
Throw an exception if inner types of RepeatedType has mapped=false

Commits
-------

728cd66a13 RepeatedType should always have inner types mapped
@javiereguiluz javiereguiluz added Status: Reviewed and removed Waiting Code Merge Docs for features pending to be merged labels Sep 25, 2020
@javiereguiluz
Copy link
Member

Thank you Artem.

@javiereguiluz javiereguiluz merged commit c0fb447 into symfony:3.4 Sep 25, 2020
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.

4 participants