8000 Symfony/form CollectionType prototype_options not applied to new fields · Issue #49786 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Symfony/form CollectionType prototype_options not applied to new fields #49786

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.

Sign up for GitHub

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

Jump to bottom
Closed
Thorry84 opened this issue Mar 23, 2023 · 3 comments
Closed

Symfony/form CollectionType prototype_options not applied to new fields #49786

Thorry84 opened this issue Mar 23, 2023 · 3 comments
Labels
Bug Form Status: Needs Review Status: Waiting feedback

Comments

@Thorry84
Copy link
Contributor

Symfony version(s) affected

6.1+

Description

The CollectionType from Symfony/form core allows setting prototype_options since 6.1. However new fields in the collection don't get the prototype_options applied. The option does exactly what it says, it applies options to the prototype and that part works well. Those prototype options won't get passed to the ResizeFormListener, which means new fields won't get the prototype_options applied.

This isn't a problem in the case mentioned in the testcase, where a help text gets applied to new fields. This doesn't change the way the field is handled, so not a problem.
But in the blog another case is mentioned where existing fields get disabled and new fields will be enabled. This allows the user to add new data, but not edit existing data. This was exactly my use case, but I found out it doesn't work. When the fields get rendered for the user, the behavior is correct, the existing fields are disabled and the new fields are enabled. But when submitting the form, the new fields aren't being applied to the data.

The reason is in the ResizeFormListener new fields get added in the preSubmit event and uses entry_options instead of prototype_options. Since disabled is set to true in entry_options, the new fields get added as being disabled and won't accept the Request data.

As an aside, the blog post is also incorrect where the disabled key is missing from the prototype_options. Since prototype_options works as an override to entry_options, all the fields would be disabled.

See

How to reproduce

$builder->add('names', CollectionType::class, [
    'allow_add' => true,
    'entry_type'   => TextType::class,

    // this is used when editing items in the collection
    'entry_options'  => [
        'attr' => ['class' => 'item-edit'],
        'help'  => 'You cannot edit existing names.',
        'disabled' => true,
    ],

    // this is used when adding new items to the collection
    'prototype_options'  => [
        'attr' => ['class' => 'item-add'],
        'help'  => 'Check out the <a href="...">rules to create new names</a>',
        'help_html' => true,
        'disabled' => false,
    ],
]);

Possible Solution

The ResizeFormListener should be passed both the entry_options as the prototype_options arrays and use the prototype_options array when adding new lines.

See: https://github.com/symfony/form/blob/6.2/Extension/Core/EventListener/ResizeFormListener.php#L99

Additional Context

No response
@Thorry84 Thorry84 added the Bug label Mar 23, 2023
@xabbuh xabbuh added the Form label Mar 25, 2023
@xabbuh
Copy link
Member
xabbuh commented Mar 25, 2023

Can you create a fully working example application that allows to reproduce and debug your issue or alternatively send a PR with the fix that you have in mind and test that would fail without your changes?

@Thorry84
Copy link
Contributor Author

Yes I've created a very simple app to show the issue.

https://github.com/Thorry84/symfony_issue_49786

Steps to reproduce:

  • Open the application, test page is index (/)
  • Expected result:
    Array ( [lines] => Array ( [0] => existingLine - should be disabled [1] => newLine - should not be disabled ) )
  • Actual result:
    Array ( [lines] => Array ( [0] => existingLine - should be disabled ) )

I will try to create a pull request with a fix, but writing a unittest for an issue like this is a bit beyond my skill level. Normally I test things like forms with Katalon (E2E test), not unit tests. But I'll try and see if it's up to snuff.

This was referenced Mar 27, 2023
Closed
Closed
Merged
@Thorry84
Copy link
Contributor Author

I've created a pull request to fix this (sorry about the incorrect ones, this is my first pull request for Symfony). #49835

nicolas-grekas added a commit that referenced this issue Mar 31, 2023
@nicolas-grekas
bug #49835 [Form] CollectionType apply prototypeOptions to ResizeForm…
6b9538b 
…Listener new fields (Thorry84)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[Form] CollectionType apply prototypeOptions to ResizeFormListener new fields

Fixes issue 49786

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #49786
| License       | MIT

This pull request provides a fix for issue 49786.

By passing the prototypeOptions to the ResizeFormListener from the CollectionType and applying it to new fields it makes the prototypeOptions function as intended. Otherwise the use case such as in this blog aren't possible: https://symfony.com/blog/new-in-symfony-6-1-customizable-collection-prototypes

I've updated the existing ResizeFormListener tests to handle the new signature and added a new test to the CollectionType which would fail without this fix and pass with the fix.

Commits
-------

85167da [Form] CollectionType apply prototypeOptions to ResizeFormListener new fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Status: Needs Review Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix issue 49786
4 participants
@nicolas-grekas @xabbuh @Thorry84 @carsonbot
0