Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple repeaters with similar names stored in the viewBag on the page. #4808

Closed
pkurg opened this issue Dec 6, 2019 · 5 comments
Closed

Comments

@pkurg
Copy link

pkurg commented Dec 6, 2019

I am trying to add 2 repeaters to the page.
Everything works fine, but if the name of the second viewBag (viewBag [dogy]) starts with the name of the first (viewBag [dog]) we get the error "" Undefined offset: 0 "on line 398 of / srv / www / devv / modules / backend / formwidgets /Repeater.php ".

<?php namespace Pkurg\Reapeatertest;

use Cms\Classes\Theme;
use Event;
use System\Classes\PluginBase;

class Plugin extends PluginBase
{

    public function boot()
    {

    	//First repeater
        Event::listen('backend.form.extendFields', function ($widget) {

            if (!$widget->model instanceof \Cms\Classes\Page) {
                return;
            }

            if (!($theme = Theme::getEditTheme())) {
                throw new ApplicationException(Lang::get('cms::lang.theme.edit.not_found'));
            }

            if (false === $widget->isNested) {
                $widget->addTabFields([
                    'viewBag[dog]' => [
                        'label' => 'My Dogs',
                        'type' => 'repeater',
                        'tab' => 'First repeater',
                        'form' => [
                            'fields' => [
                                'name' => [
                                    'label' => 'Name',
                                    'type' => 'text',
                                ],
                                'text' => [
                                    'label' => 'Text',
                                    'type' => 'text',
                                ],
                            ],
                        ],
                    ],
                ]);

            }

        });


        //Second repeater
        Event::listen('backend.form.extendFields', function ($widget) {

            if (!$widget->model instanceof \Cms\Classes\Page) {
                return;
            }

            if (!($theme = Theme::getEditTheme())) {
                throw new ApplicationException(Lang::get('cms::lang.theme.edit.not_found'));
            }

            if (false === $widget->isNested) {
                $widget->addTabFields([
                    'viewBag[dogy]' => [
                        'label' => 'My Dogs',
                        'type' => 'repeater',
                        'tab' => 'Second repeater',
                        'form' => [
                            'fields' => [
                                'name' => [
                                    'label' => 'Name',
                                    'type' => 'text',
                                ],
                                'text' => [
                                    'label' => 'Text',
                                    'type' => 'text',
                                ],
                            ],
                        ],
                    ],
                ]);

            }

        });

    }
}

If you change viewBag [dogy] to viewBag [ddogy], everything starts working fine again.

The error occurs here.
file - /modules/backend/formwidgets/Repeater.php

   / **
     * Determines the repeater that has triggered an AJAX request to add an item.
     *
     * @return void
     * /
    protected function checkAddItemRequest ()
    {
        $ handler = $ this-> getParentForm ()
            -> getController ()
            -> getAjaxHandler ();

        if ($ handler === null || strpos ($ handler, '::') === false) {
            return
        }

        list ($ widgetName, $ handlerName) = explode ('::', $ handler);
        if ($ handlerName! == 'onAddItem') {
            return
        }

        if ($ this-> alias === $ widgetName) {
            // This repeater has made the AJAX request
            self :: $ onAddItemCalled = true;
        } else if (strpos ($ widgetName, $ this-> alias) === 0) {
            // A child repeater has made the AJAX request

            // Get index from AJAX handler
            $ handlerSuffix = str_replace ($ this-> alias. 'Form', '', $ widgetName);
            preg_match ('/ ^ [0-9] + /', $ handlerSuffix, $ matches);

            $ this-> childAddItemCalled = true;
HERE-> $ this-> childIndexCalled = (int) $ matches [0];
        }
    }

Screencast -> https://www.youtube.com/watch?v=WrI9OSLTcLo

@bennothommo bennothommo added this to the v1.0.461 milestone Dec 7, 2019
@bennothommo
Copy link
Contributor

@pkurg Thanks for the report. Confirmed bug. I believe build 460 is now locked, so the fix for this will need to be done in 461.

@bennothommo bennothommo self-assigned this Dec 7, 2019
bennothommo added a commit that referenced this issue Dec 9, 2019
Similarly named repeater fields being used in viewBag variables were being assigned aliases which succeeded the `strpos` check on line 407. This will more clearly look for a child repeater form and index.

Fixes #4808
@bennothommo
Copy link
Contributor

@pkurg Would you mind testing the fix in #4814 and let us know if that resolves the issue for you?

@mjauvin
Copy link
Contributor

mjauvin commented Dec 9, 2019

@bennothommo you could launch an octodock instance with this PR for testing, I think this should be used for all PRs since it puts everyone on the same environment/libraries for testing.

What do you think?

@bennothommo
Copy link
Contributor

@mjauvin I believe @LukeTowers has discussed that with @petehalverson as a possible thing to do down the track, but for the moment, I've only been posting links to it very sparingly (generally via DM) as there's no control over what someone may get up to with the instance if it's made public.

@pkurg
Copy link
Author

pkurg commented Dec 10, 2019

@pkurg Would you mind testing the fix in #4814 and let us know if that resolves the issue for you?

Yes, it solves the problem.

@daftspunk daftspunk modified the milestones: v1.0.461, v1.0.462 Dec 10, 2019
@LukeTowers LukeTowers modified the milestones: v1.0.462, v1.0.461 Dec 10, 2019
daftspunk pushed a commit that referenced this issue Dec 12, 2019
Similarly named repeater fields being used in viewBag variables were being assigned aliases which succeeded the `strpos` check on line 407. This will more clearly look for a child repeater form and index.

Fixes #4808
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants