craftcms / craftcms/cms

Reuse of Field names results in stale data properties

Open
#2,800 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
3.6k
Forks
705
Avg merge
1d 5h
Merged PRs (30d)
134

Description

I had an issue reported to me today where someone had an existing field from another plugin named "SEO" -- they installed SEOmatic, and changed the Field type from the old plugin to SEOmatic's SEO Settings field type, and they received this error:

2018-04-24 16:45:27 [75.50.55.134][1][39d75da9258539b5d0389dce43b111c5][error][yii\base\UnknownPropertyException] yii\base\UnknownPropertyException: Setting unknown property: nystudio107\seomatic\models\MetaBundle::title in /home/web/vhosts/stoneridge/vendor/yiisoft/yii2/base/Component.php:209
Stack trace:
#0 /home/web/vhosts/stoneridge/vendor/yiisoft/yii2/BaseYii.php(546): yii\base\Component->__set('title', '')
#1 /home/web/vhosts/stoneridge/vendor/yiisoft/yii2/base/BaseObject.php(107): yii\BaseYii::configure(Object(nystudio107\seomatic\models\MetaBundle), Array)
#2 /home/web/vhosts/stoneridge/vendor/nystudio107/craft-seomatic/src/models/MetaBundle.php(133): yii\base\BaseObject->__construct(Array)
#3 /home/web/vhosts/stoneridge/vendor/nystudio107/craft-seomatic/src/fields/SeoSettings.php(180): nystudio107\seomatic\models\MetaBundle::create(Array)
#4 /home/web/vhosts/stoneridge/vendor/craftcms/cms/src/base/Element.php(1904): nystudio107\seomatic\fields\SeoSettings->normalizeValue('{"title":"","de...', Object(craft\elements\Entry))
#5 /home/web/vhosts/stoneridge/vendor/craftcms/cms/src/base/Element.php(761): craft\base\Element->normalizeFieldValue('seo')

The issue is that the old "SEO" field has properties like title and description that my field does not, so Yii2 rightly throws an error when it tries to set properties that don't exist.

Now, I can do something like:

        foreach ($config as $propName => $propValue) {
            if (!property_exists($class, $propName)) {
                unset($config[$propName]);
            }
        }

To sanitize the incoming data into something that my Field is expecting, by removing any non-existent properties... but this feels like a core issue, because this type of thing will happen with any field name re-use.

Thoughts?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the stack-trace paths in src/models/MetaBundle.php, src/fields/SeoSettings.php, and craft/base/Element.php, then reproduce normalization after reusing a field name. Determine the expected handling for stale properties and verify that reused-field data no longer raises UnknownPropertyException, with regression coverage for the case.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.