Abhoryo / APYJsFormValidationBundle

This bundle performs validations of a form in javascript. (i18n compatible and several javascript frameworks supported)
92 stars 23 forks source link

Added ReapeatedValidation + tweaked CacheWarmer #2

Closed guillaumepotier closed 12 years ago

guillaumepotier commented 12 years ago

Hi, I had a nice time with your bundle today ;)

I faced a tricky problem today with the js validation of a repeated password field. Neither the first password nor the second password (confirmation) had validation cause once repeated the fields took a different name:

user_password becomes user_password_first and user_password_second by default or event different if you give special names like:

    $builder->add('password', 'repeated', array(
        'type' => 'password',
        'invalid_message' => 'The password fields must match..',
        'first_name' => 'password',
        'second_name' => 'password_confirm',
    ));`

I added a RepeatedValidator.js.twig to check if second password equals the first one.

I had also to rework a bit on your code. You used to foreach the constraints and check if they were in fields Asserts, I did the other way, by doing a foreach on fields and checking then their constraints.

My unsolved problem with this PR is that I was not able to get the invalid_message for this repeated constraint.. It is currently hard coded in english in the CacheWarmer... If you have a better idea to enhance that ;)

Best

Abhoryo commented 12 years ago

The problem is that the repeated field's validator is store in the field itself. It's a dynamic constraint.

I think I found a way to handle this case properly. I keep you in touch.

guillaumepotier commented 12 years ago

Great, didn't thought about that! Must be a better handling rather that PR. Can't wait to hear news about you on this ;)

Best

Abhoryo commented 12 years ago

I'll add a pre-processing(tag service support) with formChildren and metadata input arguments so we can manipulate constraints.

Do you think that the cache warmer is realy usefull for creating javascript files? If yes, maybe we can call an array of route to cachewarmed these scripts?

guillaumepotier commented 12 years ago

Most definitely, a pre-processing service manipulating children and metadata would be very handfull (that's why I thought in my PR that was more interesting giving the children directly to the CacheWarmer instead of just the fieldNames)

Cache warmer is great too I think. although I'm not sure to understand what you mean and would like to do with the array of route ;)

Abhoryo commented 12 years ago

If we use the formChildren, we can't use cachewarmer anymore because we don't have the form to get its children.

But if we call directly the route that create the wanted form, the processing whould be the same.

In the simple example doc, the cache warmer will call the myform route to generate the script. Enjoy! And with this system, the configuration of the cache warmer is easier.

# app/config.yml

apy_js_form_validation:
    ...
    assets_warmer:
        - { route : myform }
        - { route : myform2, params : { category : food } }
guillaumepotier commented 12 years ago

That's sounds great! A bit of rework on the warmup should allow more flexibility for generateFormValidationScript!

You should for example give directly the $form to it to have access to everything needed, instead of passing 3 args ;)

Abhoryo commented 12 years ago

Yes, I'll create a pre-processing with form + metadata input arguments and a post-processing with form and constraints input arguments.

With these, we can do everything.

guillaumepotier commented 12 years ago

Great! I'm really looking forward to it! Your bundle is just awesome and saves a lot of time, bugs and pia ;)

I currently don't have much time right now but I will be glad to improve it very soon ;)

Abhoryo commented 12 years ago

You have a todo list, if you want to help ;)

I have a test controller with all constraints but no phpunit tests.

pia stands for ?

guillaumepotier commented 12 years ago

Pain In the Ass ;)

Abhoryo commented 12 years ago

Ok je sors => [ ] -_-.

Pourquoi s'embêter a parler anglais quand on est tous les deux français :D

guillaumepotier commented 12 years ago

Last thing here and I stop bothering you: won't you prefer in the processing scripts (the pre-processing one at least) to foreach fields first and then their constraints rather than constraints and look for the fields with them as you're doing so?

It seems more logical for me. But it is just my personal feeling ;)

Abhoryo commented 12 years ago

Yes it's more logical. With the pre-processing, we can also remove some constraints which have not the right validation_group instead of doing it in the general processing.

guillaumepotier commented 12 years ago

Je pense qu'on peut raisonnablement close cette PR étant donné le rework que tu es entrain de faire. En effet, c'est pas mal aussi en français ;)

Abhoryo commented 12 years ago

En parlant du loup, je viens de le commit il y a 10 minutes. Dis-moi si tu perçois des problèmes.

Il manque encore de la doc vis à vis des events. Ce sera pour plus tard.

Abhoryo commented 12 years ago

Sinon pour le message invalid_message, si tu utilises un message dont la traduction n'existe pas dans le domaine validators (si tu utilises ce domaine pour le bundle bazinga_exposetranslation_js), il restera tel quel.

guillaumepotier commented 12 years ago

Great!

Je regarde ça 'credi, j'ai un début de semaine assez chargé --'

guillaumepotier commented 12 years ago

Yop,

Au cas oùhehe mets ici vu que t'as pas renseigné ton email sur Github et que je t'ai pas trouvé sur twitter: balloon.github.com

Si cela te concerne ou des connaissances à toi ;)

+

Abhoryo commented 12 years ago

Je serai bien sûr intéressé mais j'habite sur Montpellier avec maison achetée + famille donc Paris c'est mort :D

guillaumepotier commented 12 years ago

Ahaha ok, quelle chance, Niçois exilé à Paris, je t'envie pas mal ;)

Bonne continuation!

Abhoryo commented 12 years ago

J'ai habité quasi 20 ans à Paris, dès que j'ai eu mon diplôme je suis parti :)