Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Decouple xml loader from Form #303

Open
florianv opened this issue Dec 3, 2013 · 15 comments
Open

Decouple xml loader from Form #303

florianv opened this issue Dec 3, 2013 · 15 comments

Comments

@florianv
Copy link
Contributor

florianv commented Dec 3, 2013

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
Copy link
Contributor

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

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
Copy link
Contributor

👍

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

@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
Copy link
Contributor

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

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

@florianv
Copy link
Contributor Author

florianv commented Dec 3, 2013

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
Copy link
Contributor

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

@florianv
Copy link
Contributor Author

florianv commented Dec 3, 2013

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

@eddieajau
Copy link
Contributor

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
Copy link
Contributor Author

florianv commented Dec 3, 2013

It looks nice. 👍

@florianv
Copy link
Contributor Author

florianv commented Dec 8, 2013

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
Copy link
Contributor

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();
}

@piotr-cz piotr-cz mentioned this issue Mar 4, 2014
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants