Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form] Add getErrorsAsArray method#7512

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

Closed

Conversation

raziel057
Copy link
Contributor

Usefull to return form errors through JsonResponse.

See#7205

QA
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets7205
LicenseMIT
Doc PRIs it needed for this kind of addition?
modified:   src/Symfony/Component/Form/Form.phpmodified:   src/Symfony/Component/Form/Tests/CompoundFormTest.phpmodified:   src/Symfony/Component/Form/Tests/SimpleFormTest.php

Usefull to return form errors through JsonResponsemodified:   src/Symfony/Component/Form/Form.phpmodified:   src/Symfony/Component/Form/Tests/CompoundFormTest.phpmodified:   src/Symfony/Component/Form/Tests/SimpleFormTest.php
modified:   src/Symfony/Component/Form/Tests/CompoundFormTest.php
@raziel057
Copy link
ContributorAuthor

For information, Scrutinizer errors are not due to my commits.

I can't see what is wrong with Travis check on PHP 5.4. Is it possible to have the detail of the failure?

When I click here:https://travis-ci.org/symfony/symfony/jobs/5879397, I simply have an endless "loading" message.

@stof
Copy link
Member

stof commentedApr 2, 2013

the travis interface seems broken currently (I see a JS error in the console). You should report it in their issue tracker. And waiting for a fix, you could try to get the logs through their CLI or their API.

@raziel057
Copy link
ContributorAuthor

@stof Thanks. In fact I get an error with Firefox 19.0.2 but not in Chrome. I opened a ticket on travis-web tracker:https://github.com/travis-ci/travis-web/issues/171

The failure concern this test, but I didn't touch it.
There was 1 failure:

  1. Symfony\Component\Process\Tests\SigchildDisabledProcessTest::testRestart
    Failed asserting that true is false.

$template = $error->getMessageTemplate();
if (null !== $template) {
$parameters = $error->getMessageParameters();
$errors[$key] = strtr($template, $parameters);
Copy link
Contributor

Choose a reason for hiding this comment

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

How it pissible translate this messages?

Copy link
Member

Choose a reason for hiding this comment

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

It is not possible. :(
may be, we can introduce a littleoptional coupling by allow the end user to give a translatorInterface togetErrorsAsArray

so from the controller$form->getErrorsAsArray($translator);

Copy link
Contributor

Choose a reason for hiding this comment

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

@raziel057 Does one$key can contain more than 1 error? If yes - you should append they but not override

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@Koc As you can see it on the tests I added, a field can contain multiple errors, but in this case there is only one error per $key. Do you have use cases with multiples error per $key?

@lyrixx Yes, I also thought about this solution. I think it's not so bad. I can add this option if@fabpot is ok for that.

@fabpot
Copy link
Member

@bschussek What do you think about this addition?

@webmozart
Copy link
Contributor

The current implementation here is a bit flawed. It combines both local errors (indexed by integers) and nested errors (indexed by field names, which can be integers too) in one single data structure, which cannot work.

I think this functionality should be merged intogetErrors() by introducing a new parameter$deep that isfalse by default.getErrors() should then return some sort of array-compatible (for BC) object (FormErrorList,FormErrorIterator, not sure yet) that supports recursive iteration (\RecursiveIterator). Also, this object should implement__toString(), which should return the same output asgetErrorsAsString() (which then could be deprecated).

I can't really give any advice yet on further details. This needs to be thought through and it is definitely not trivial.

@tijuan
Copy link

If I can give my opinion on the expected format, this should look like
$errors[field_name] = ['label' => 'field_label', 'errors' => ['error_msg_1', 'error_msg_2'] ];

@TomiS
Copy link

@bschussek That seems like a nice idea to me. However, it seems to meFormError class currently does not contain any information about the form field name the error is related to. So some additional data about the field name/key should be added to FormError too, I think.

About the array compatibility, would implementingArrayAccess interface do the trick?

@stof
Copy link
Member

stof commentedMay 9, 2013

@TomiS FormError instances are attached to a Form instance. So getting the field name is possible as you need to Form to get the errors.

@TomiS
Copy link

Ok, I spent yesterday playing with this thing and first tried to implement something similar to what @bschussek had in mind.Here's a diff of what I ended up with. There are a couple of problems with this approach, though. 1) The code becomes quite complex if we really want array compatibility and if it should be fully array serializable (to get json too). 2) I ended up repeating the tree structure already introduced byForm class and I feel this adds unnecessary complexity too. 3) Getting a nice looking json output out of the very complex class structure is tedious (at least before PHP 5.4 and its JsonSerializable inteface)

So... I also ended up trying something much more simple. Basically what I did was repeating the same implementation introduced in this PR but just in thegetErrors() method. You can see that implementationin here. I tried to serialize the output of this with JMSSerializer to json and the result was "just what the api developer needs", imo :) The benefit of this approach is that it's very simple as it utilizes the tree structure provided by Form class itself. What comes to indexes of local and nested errors, I really can't see the problem. Is there really a risk for index collision because I truly can't see it. (but on the other hand, I didn't write the component like some of us, so what do I know ;)

ps. Good point@stof. And it's also kind of related to what I say here about duplicating tree structure.

@webmozart
Copy link
Contributor

replaced by#9099

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@raziel057@stof@fabpot@webmozart@tijuan@TomiS@Koc@lyrixx

[8]ページ先頭

©2009-2025 Movatter.jp