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

Duplicate configs on certain situations #578

Open
xavier83ar opened this issue Dec 15, 2021 · 2 comments
Open

Duplicate configs on certain situations #578

xavier83ar opened this issue Dec 15, 2021 · 2 comments

Comments

@xavier83ar
Copy link

I was dealing with an issue that duplicates items when I passed nameCallback as an array like this [$object, 'methodName'], for example, with this config:

    // inside some Table::initialize()
    $this->addBehavior('Josegonzalez/Upload.Upload', [
        'imagen' => [
            'path' => 'webroot{DS}files{DS}mayoristas{DS}',
            'nameCallback' => [$this, 'buildName'],
        ],
    ]);

nameCallback is never called, to understand why, I read Behavior config after adding it, and nameCallback had this value

[
    0 => \MyObject\Instance,
    1 => "buildName",
    2 => \MyObject\Instance,
    3 => "buildName",
]

Digging into the code I find out that \Josegonzalez\Upload\Model\Behavior\UploadBehavior doesn't have a constructor, so it inherits from \Cake\ORM\Behavior, so the setConfig is called twice:

and in both cases is called with merge = true which is the default, I've tried replace that last line with $this->setConfig($configs, null, 'shallow'); and it works ok, but I don't know if that's the best solution as it is calling setConfig twice anyway, maybe a better solution is to add a constructor and call parent constructor with an empty array as configs.

xavier83ar added a commit to soluciones-ypunto/cakephp-upload that referenced this issue Dec 15, 2021
ADmad added a commit that referenced this issue Dec 16, 2021
@ADmad
Copy link
Member

ADmad commented Dec 16, 2021

I had made a PR to overwrite the config instead of merging again but closed it to avoid any potential issue I might have not considered.

Your issue can be fixed by using 'nameCallback' => \Closure::fromCallable([$this, 'buildName']).

@ADmad
Copy link
Member

ADmad commented Dec 16, 2021

In future the fields config should be nested inside a fields key so that it can be overwritten without worrying about affecting other configs. There's currently another issue too where className key needs to be explicitly unset due to having fields at top level of config array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants