joomla / joomla-framework

[READ-ONLY] This repo is no longer in active development. Please see https://github.com/joomla-framework for the individual Framework packages.
http://framework.joomla.org
GNU General Public License v2.0
189 stars 140 forks source link

Decouple xml loader from Form #303

Open florianv opened 10 years ago

florianv commented 10 years ago

The xml format as form definition is hardcoded in the current Form implementation. It should be decoupled so it's possible for example to define forms in json or yaml format.

dongilbert commented 10 years ago

I worked on an implementation that would allow you to use any Registry supported format, but it's hard, because the registry XML format is lacking in many areas.

florianv commented 10 years ago

Also some other classes must be changed, for example https://github.com/joomla/joomla-framework/blob/staging/src/Joomla/Form/Rule.php#L61 depends on SimpleXMLElement

eddieajau commented 10 years ago

:+1:

I guess there are a couple of steps:

  1. Define an internal data structure for a "form".
  2. Devise importers for different data structures.

I'd also argue that we should separate validation out into a new package. I've done a little work on it but I'm not completely happy with it.

florianv commented 10 years ago

A simple interface like this could replace the Rule class I think

<?php

interface RuleInterface
{
    public function isValid($data);
}

Also the Rule class should be called RegexRule

florianv commented 10 years ago

@dongilbert @eddieajau Also we need to add the ability to load namespaced fields and rules maybe with a '.' as separator ?

<field type="MyProject.Fields.Integer" />
eddieajau commented 10 years ago

This is what I've used so far:

/**
 * Validator interface.
 *
 * @since  1.0
 */
interface ValidatorInterface
{
    /**
     * Gets the errors if the value was invalid.
     *
     * @return  array[]  An associative array of "Error Code" => "Error Data" values, where "data" is an array.
     *
     * @since   1.0
     */
    public function getErrors();

    /**
     * Checks that the value is valid.
     *
     * @param   scalar  $value  The value to validate.
     *
     * @return  boolean
     *
     * @since   1.0
     */
    public function isValid($value);
}
florianv commented 10 years ago

A good feature would be to be able to retrieve the validation error per field (which is currently impossible). It can be handy to display the error message close to the field.

florianv commented 10 years ago

@eddieajau I like this interface. Also, we need translatable error messages.

florianv commented 10 years ago

A XSD schema for the xml files could be handy. It would greatly simplify the validation when loading the form and provide autocompletion in most IDE when writing your forms.

dongilbert commented 10 years ago

We have XSD's for our manifests, I think it would be easy to build one for the forms.

florianv commented 10 years ago

Yes. I know PHPStorm can generate a schema from an xml file (it will just need some quick adjustments).

eddieajau commented 10 years ago

So, a sample rule looks like this:

class RangeValidator extends AbstractValidator
{
    /**
     * Error code if the value is invalid.
     *
     * @var    string
     * @since  1.0
     */
    const INVALID = 'RangeInvalid';

    /**
     * Error code for a number that is too low.
     *
     * @var    string
     * @since  1.0
     */
    const TOO_LOW = 'RangeTooLow';

    /**
     * Error code for a number that is too high.
     *
     * @var    string
     * @since  1.0
     */
    const TOO_HIGH = 'RangeTooHigh';

    /**
     * Validates the value.
     *
     * @param   scalar  $value  The value to validate.
     *
     * @return  boolean
     *
     * @since   1.0
     */
    protected function doIsValid($value)
    {
        if (!is_numeric($value))
        {
            $this->addError(self::INVALID);

            return false;
        }

        $min = $this->getOption('min');
        $max = $this->getOption('max');

        if ($min && $value < $min)
        {
            $this->addError(self::TOO_LOW);

            return false;
        }
        elseif ($max && $value > $max)
        {
            $this->addError(self::TOO_HIGH);

            return false;
        }

        return true;
    }
}

You get fixed error strings but it would be trivial for your L10N package to convert them.

florianv commented 10 years ago

It looks nice. :+1:

florianv commented 10 years ago

For information there is this library https://github.com/Respect/Validation which has already a lot of rules. https://github.com/Respect/Validation/tree/develop/library/Respect/Validation/Rules It also provides an adapter for zend framework rules.

piotr-cz commented 10 years ago

What do you think about having an interface for errors. We could use for Form/ Field or Model

/**
 * Describes an error-aware interface
 */
interface ErrorAwareInterface
{
    /**
     * Set the error.
     *
     * @param  object|Exception
     */
    public function addError($error);

    /**
     * Get the error.
     *
     * @return  object|Exception
     */
    public function popError();
}