While in general I am completely convinced the need of and using this module, it lacks one particular feature: it completely lacks validation. First time I have encountered with this issue was when I had to realize that a "number" typed variable can even hold "0asd123". Digging into the module I had to realize that it completely lacks support of any validation.

My proposal is twofold.

1. Add validation to the variable types that the module already ships and validation is needed (eg. the aforementioned "number" type).

2. Add support for a "validate callback" (just like for "element callback" and "group callback" and others) for both levels: hook_variable_type_info() and hook_variable_info.

Whilst the first one seems to be easier to solve/code, the second one would bring more general solution, as it would solve the first one as well.

Comments

boobaa’s picture

Version: 7.x-1.0-beta3 » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new1.21 KB

Attached patch solves this in the 2nd way mentioned above.

q0rban’s picture

This makes sense to me, however a temporary workaround is to of course use a form_alter and add the validation there yourself.

q0rban’s picture

Oh, actually, now that I'm getting more familiar with the module, I see you can specify customizations to the form in the 'element' item for the variable. Awesome!

jose reyero’s picture

Title: Module competely lacks support for validation » Implement 'validate callback' for some variables / variable types.
Category: feature » task
Status: Needs review » Needs work

Yes, this makes sense. Committed the first part of the patch, simple support for validate callback.

Now we should be adding some of such callbacks for known variable types.

coleman.sean.c’s picture

Status: Needs work » Needs review
StatusFileSize
new913 bytes

The commit in #4 doesn't quite line up to the patch in #2- 'validate callback' is being added to $element['#validate'] instead of $element['#element_validate'], so it is never actually called. One line patch attached.

jose reyero’s picture

StatusFileSize
new3.13 KB

Though the patch is right, I wish we had some more powerful validate callbacks for variables that also could be used without forms.

You always can add some '#element_validate' using the variable['element'] property.

This patch adds a validate callback that takes the full variable data as parameter. Please take a look at it and let me know what you think.

(Implemented sample validate for 'number' variables).

wilco’s picture

It should be noted, for some reason, this feature made it into 7.x-1.2 already and has NOT BEEN DOCUMENTED. I just spent 2 hours trying to understand why some of the output from my variables where wrapped in a set of PRE tags. This has been a rather frustrating set of time with no reason for this stringent output.

Please do one of two things:

  • document the change on the main example, OR
  • remove the PRE tags.

Whatever solution you come up with, forcing that output broke my site. BIG TIME.

Thanks for the heads up.

jose reyero’s picture

Status: Needs review » Fixed
Issue tags: +Needs change record

Committed the pending patch #6 with some minor changes.

(In branch 2.x)

Automatically closed -- issue fixed for 2 weeks with no activity.