schmunk42 / yii2-giiant

Yii 2 Framework Code Generator Gii on Steroids
271 stars 120 forks source link

Error using --providerList and possible fix #265

Open bwragg opened 4 years ago

bwragg commented 4 years ago

Hi,

when I run this command:

./yii gii/giiant-crud --modelClass='common\models\MyModel' --enablePjax=1 --tidyOutput=1 --controllerClass='backend\controllers\MyModelController' --indexWidgetType='grid' --searchModelClass='common\models\MyModelSearch' --viewPath='@backend/views' --baseControllerClass='backend\controllers\base\BaseController' --accessFilter=0 --enableI18N=0 --indexWidgetType='grid' --providerList=['\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider'] --overwriteControllerClass=0

I get the following error:

Running 'Giiant CRUD'...

PHP Warning 'yii\base\ErrorException' with message 'Invalid argument supplied for foreach()'

in /Users/me/projects/testside/vendor/schmunk42/yii2-giiant/src/generators/crud/ProviderTrait.php:62

If found that if I change line 62 to the following it fixes it.

if ($this->providerList && is_array($this->providerList)) {

Seems it was trying to loop over a non array. I don't know the code super well so if this isn't breaking anything else could it be added to the source?

Thanks,

schmunk42 commented 4 years ago

Can you try

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider'

usually Yii CLI applications should parse a comma separated list as an array.

bwragg commented 4 years ago

Thanks for the quick reply @schmunk42 ! I think I tried that already but to be sure I just tried that again and get the same error.

If I add my code in it works, but in saying that I haven't successfully got the date time callback to work. At the moment just outputting text field still. Maybe my addition is screwing it up? Will keep looking into it

schmunk42 commented 4 years ago

It looks like the one (1) provider in the options is treated as a string rather than an array. With your check it would (also) not be added to the provider list.

For debugging: Could you add another provider as a second element?

A fix could be to cast the option into an array, but this could also be a framework issue (CC: @cebe)

bwragg commented 4 years ago

Thanks. I tried this:

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider','\schmunk42\giiant\generators\crud\providers\extensions\EditorProvider','schmunk42\giiant\generators\crud\providers\core\OptsProvider'

and it still didn't work. also tried it like:

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider,\schmunk42\giiant\generators\crud\providers\extensions\EditorProvider,schmunk42\giiant\generators\crud\providers\core\OptsProvider'

To get around it I did what you suggested and did this before the for loop:

$this->providerList=(array)$this->providerList;

bwragg commented 4 years ago

Still can't get the DateTimeProvider working. Traced it back to this in DateTimeProvider.php

switch (true) { case in_array($attribute, $this->columnNames):

the columnNames array is empty. Im setting it like the instructions say:

\Yii::$container->set( schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider::class, [ 'columnNames' => ['proposed_disposal_at','type'], ] );

Can't see anywhere in the code where columnNames actually gets set?

schmunk42 commented 4 years ago

Looks good. the check should be here: https://github.com/schmunk42/yii2-giiant/blob/af8574f4be72dda7404c6c3f588ae2f5b21db124/src/generators/crud/providers/extensions/DateTimeProvider.php#L10

bwragg commented 4 years ago

If I put a var_dump($this->columnNames); above the switch it prints out an empty array for every field. so it never makes it into that section of the case.

When I said "Can't see anywhere in the code where columnNames actually gets set?" i meant I can't see where $this->columnNames gets the array from the bootstrap but I think its me not understanding DI. Need to do a bit more reading on it.

schmunk42 commented 4 years ago

Where you use use DI?

Can be done in the config i.e.

bwragg commented 4 years ago

Where you use use DI? I thought your plugin uses DI In the bootstrap.php Eg $container->set ...I understand that is how you set the columnName field in the DateTimeProvider?

Since I’m finding the columnName property is empty In the DateTimeProvider I’m trying to understand where the values in the columnName array in bootstrap.php get loaded into the DateTimeProvider object.

Can be done in the config i.e. You mean in the bootstrap? That is where I am doing it. Or is there another way?

schmunk42 commented 4 years ago

Can you post the file where you are setting it? Can be done in several places. Did you turn on logging? That should also help seeing what's going on.

bwragg commented 4 years ago

Thanks.

bootstrap.php has:

<?php include_once('giiant.php');

and giiant.php has

<?php
\Yii::$container->set(
    'schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider', 
    [
        'columnNames' => ['proposed_disposal_at','type'],
    ]
);

will do some extra logging now and see what it shows.

schmunk42 commented 4 years ago

Where is bootstrap.php included? Is from the tests?

bwragg commented 4 years ago

Im using the advanced template. Running it from the console using

yii gii/giiant-crud

So in answer to your question I believe the yii command does this:

` /**

defined('YII_DEBUG') or define('YII_DEBUG', true); defined('YII_ENV') or define('YII_ENV', 'dev');

require DIR . '/vendor/autoload.php'; require DIR . '/vendor/yiisoft/yii2/Yii.php'; require DIR . '/common/config/bootstrap.php'; require DIR . '/console/config/bootstrap.php';`

So done some more digging into a few of the issues:

1) With the \ --providersList issue I was having. No matter what way I enter the providers they don't get inserted into the model as an array, always a string. To fix this I propose the following code changes to Generator.php line 313:

public function generate() { $this->providerList=explode(',',$this->providerList);

This forces an input like this:

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider,\schmunk42\giiant\generators\crud\providers\extensions\EditorProvider'

or like:

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider','\schmunk42\giiant\generators\crud\providers\extensions\EditorProvider'

2) The code that DateTimeProvider.php creates here (around line 14):

https://github.com/schmunk42/yii2-giiant/blob/3d00e9c03d7d19a0366cca09d2825c099b1ec589/src/generators/crud/providers/extensions/DateTimeProvider.php#L14

doesn't work, should be: \$form->field(\$model, '{$attribute}')->widget(\zhuravljov\yii\widgets\DateTimePicker::className(), [

(notice the \yii added in)

3) Still am not getting the columnNames array to be passed correctly to DateTimeProvider. To my test to work I hardcoded may field names into the object:

$this->columnNames=['proposed_disposal_at'];

just before the switch here:

https://github.com/schmunk42/yii2-giiant/blob/3d00e9c03d7d19a0366cca09d2825c099b1ec589/src/generators/crud/providers/extensions/DateTimeProvider.php#L9

widget began to generate when I did this. Looking into why its not being passed in correctly.

bwragg commented 4 years ago

Found the issue for why the columnName was not being passed into the DateTimeProvider..missing a freaking backslash!

I noticed in ProviderTrait.php it calls this:

$obj = \Yii::createObject(['class' => $class]);

Made me thing that if the $class value didn't match exactly it would fail. Checked my code and my bootstrap had no backslash:

\Yii::$container->set( 'schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider',

but my --providerList did:

--providerList='\schmunk42\giiant\generators\crud\providers\extensions\DateTimeProvider','\schmunk42\giiant\generators\crud\providers\extensions\EditorProvider'

added a \ to the bootstrap and it worked....as long as I applied changes 1 and 2 in my last post.

schmunk42 commented 4 years ago

Might be better this use ::class...

So, is this still and issue with giiant or was it only DI?

bwragg commented 4 years ago

Thanks for the response schmunk42. I believe there are a few issues with Giiant.

1) Code here is wrong:

https://github.com/schmunk42/yii2-giiant/blob/3d00e9c03d7d19a0366cca09d2825c099b1ec589/src/generators/crud/providers/extensions/DateTimeProvider.php#L14

doesn't work. Maybe the external library has changed its structure. It should be:

\$form->field(\$model, '{$attribute}')->widget(\zhuravljov\yii\widgets\DateTimePicker::className(), [

2) --providerList doesn't work from CLI unless I add this:

$this->providerList=explode(',',$this->providerList);

above this line:

https://github.com/schmunk42/yii2-giiant/blob/3d00e9c03d7d19a0366cca09d2825c099b1ec589/src/generators/crud/Generator.php#L315

Is it possible to add these changes to the code?

I'm having issues with custom templates for crud and schema with models but will start another thread on these issues.

bwragg commented 4 years ago

actually adding

$this->providerList=explode(',',$this->providerList);

messes up if you are using the Gii web form. This works:

if (Yii::$app instanceof \yii\console\Application){ $this->providerList=explode(',',$this->providerList); }

schmunk42 commented 4 years ago

Hmm, wouldn't this break, when using a real array via DI in a config, since an array would be exploded?

We should have tests for all cases, CLI, DI, Web with one and multiple items.