NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Fixes sprintf(): Too few arguments in form transformer#36907

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:3.4frompedrocasado:patch-sprintf

May 30, 2020

Merged

Fixes sprintf(): Too few arguments in form transformer#36907

nicolas-grekas merged 1 commit intosymfony:3.4frompedrocasado:patch-sprintf

May 30, 2020

Conversation

Copy link

Contributor

pedrocasado commentedMay 22, 2020

Similar to:#29482

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

Fixes the form reverse transformation when the method viewToNorm is called within a value with the character %:

Before : "sprintf(): Too few arguments"

After : Form reverse transformation works.

Reference :http://php.net/manual/function.sprintf.php

pedrocasado requested a review fromxabbuh as acode owner

May 22, 2020 17:43

carsonbot added Status: Needs Review Bug labels

May 22, 2020

xabbuh added the Form label

May 22, 2020

xabbuh added this to the3.4 milestone

May 22, 2020

Copy link

Member

xabbuh commentedMay 22, 2020

Can we add a test case to prevent future regressions (IIRC this was done recently to satisfy fabbot.io)?

nicolas-grekas reviewed

May 23, 2020

View reviewed changes

src/Symfony/Component/Form/Form.php OutdatedShow resolvedHide resolved

Copy link

Contributor

ro0NL commentedMay 25, 2020

Hi@pedrocasado can you also have a look at#36954 maybe :)

stof reviewed

May 25, 2020

View reviewed changes

src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php OutdatedShow resolvedHide resolved

Tobion suggested changes

May 26, 2020

View reviewed changes

Copy link

Contributor

Tobion left a comment

Choose a reason for hiding this comment

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

Seems like most (all?) of this was already fixed, e.g. in#36684
You need to rebase.

carsonbot added Status: Needs Work and removed Status: Needs Review labels

May 26, 2020

pedrocasado requested a review fromTobion

May 26, 2020 01:52

carsonbot added Status: Needs Review and removed Status: Needs Work labels

May 26, 2020

Copy link

Contributor

Tobion commentedMay 26, 2020

Please add a test as suggested by@xabbuh for at least one case like the Form transformation.

Copy link

ContributorAuthor

pedrocasado commentedMay 26, 2020

@Tobion , can you give me some guidance? do you think this test is enough?

public function testDataTransformationEscapesValueCorrectlyInException(){    $this->expectException('Symfony\Component\Form\Exception\TransformationFailedException');    $this->expectExceptionMessage('Unable to transform data for property path "name": No mapping for value "90%"');    $model = new FixedDataTransformer([        'default' => 'foo',    ]);    $view = new FixedDataTransformer([        'foo' => 'bar',    ]);    $config = new FormConfigBuilder('name', null, $this->dispatcher);    $config->addViewTransformer($view);    $config->addModelTransformer($model);    $config->setData('90%');    $form = new Form($config);    $form->getData();}