search

doublesecretagency / craft-adwizard

Ad Wizard plugin for Craft CMS
Other
7 stars 8 forks source link

Bug: If a Field Layout exists, can no longer create group without selecting a layout #51

Closed cballenar closed 5 months ago

cballenar commented 6 months ago

Steps to replicate

  1. Create an AdWizard Field Layout
  2. Go to create a Group
  3. Attempt to leave Field Layout dropdown empty
  4. A Type Error will interrupt the operation

This issue doesn't occur if no Field Layouts have been created in AdWizard since the Field Layout dropdown is not displayed.

Expected behavior

When creating a group, the Field Layout should allow for empty value even if Field layouts exist.

Stack Trace

TypeError: Cannot assign string to property doublesecretagency\adwizard\models\AdGroup::$fieldLayoutId of type ?int in /var/www/html/vendor/doublesecretagency/craft-adwizard/src/controllers/AdGroupsController.php:142
Stack trace:
#0 [internal function]: doublesecretagency\adwizard\controllers\AdGroupsController->actionSaveGroup()
#1 /var/www/html/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#2 /var/www/html/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array)
#3 /var/www/html/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('save-group', Array)
#4 /var/www/html/vendor/craftcms/cms/src/web/Application.php(341): yii\base\Module->runAction('ad-wizard/ad-gr...', Array)
#5 /var/www/html/vendor/craftcms/cms/src/web/Application.php(642): craft\web\Application->runAction('ad-wizard/ad-gr...', Array)
#6 /var/www/html/vendor/craftcms/cms/src/web/Application.php(303): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#7 /var/www/html/vendor/yiisoft/yii2/base/Application.php(384): craft\web\Application->handleRequest(Object(craft\web\Request))
#8 /var/www/html/web/index.php(12): yii\base\Application->run()
#9 {main}
cballenar commented 6 months ago

Workarounds

If you create all your ad groups before creating the field layout this won't be an issue.

Because I happen to have some groups that require custom fields and others that don't , I ended up creating a Default layout which is blank and using that for all other ad groups. I suppose if at some point I need to add a custom field this will end up saving me some time.

lindseydiloreto commented 5 months ago

Hmm, not sure what's happening there. I'll dive in and take a look.

Thanks for reporting it! 👍

lindseydiloreto commented 5 months ago

This is fixed in v3.4.0 (for Craft 4) and v4.1.0 (for Craft 5). Thanks again for reporting it! 🍺

cballenar commented 5 months ago

Awesome! I'll check it out this week 👍