some more Form tests and some patches proposals

Le 09/09/2014 20:51, Richard Griffith a écrit :
>
> In htdocs/xoops_lib/Xoops/Form/GroupFormCheckbox.php:
>
> > +        $this->optionTree[$itemId]['parent'] = $itemParent;
> > +        $this->optionTree[$itemId]['name'] = $itemName;
> > +        $this->optionTree[$itemId]['id'] = $itemId;
> > +    }
> > +
> > +    /**
> > +     * Loads all child ids for an item to be used in javascript
> > +     *
> > +     * @param int   $itemId    item id
> > +     * @param array &$childIds child ids
> > +     *
> > +     * @return void
> > +     */
> > +    private function loadAllChildItemIds($itemId, &$childIds)
> > +    {
> > +        if (!empty($this->itemTree[$itemId]['children'])) {
>
> Since there is no itemTree property, this won't ever execute. 
> Obviously you saw a need here, but I'm confused about what these 
> changes are accomplishing. Can you please enlighten me?
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/XOOPS/XoopsCore/pull/214/files#r17319656>.
>

Good catch, there is a mistake in my patch.

I have seen analogies between GroupFormCheckbox.php and 
GroupPermissionForm.php (the former have optionTree and the latter 
itemTree).

Maybe they could share some common methods in a superclass. At the 
moment, I only duplicate the method to add an Option (similar to add an 
item) and then compute the 'allchild' field (in a similar way as itemTree).

My concern is to suppress the setOptionTree because the structure of an 
option is not describe. I'd rather to have an add option method.

Somewhere in files, there will be interesting to describe option and 
item structure in a comment.

Alain91