silverstripe / cwp

Common Web Platform (CWP) features module. We strongly recommend using it for all new CWP projects. Future features will be delivered here.
https://www.cwp.govt.nz
BSD 3-Clause "New" or "Revised" License
10 stars 26 forks source link

Conflicting Author field #209

Closed elliot-sawyer closed 5 years ago

elliot-sawyer commented 5 years ago

CWP News Page has one of these as a $db field:

    private static $db = [
        'Author' => 'Varchar(255)',
    ];

But it's a Page type, which is Versioned.... which has a public function Author() method

The result is that Author is not shown at all... how would you go about resolving this?

elliot-sawyer commented 5 years ago

Suggest changing the method on Versioned to this, to avoid conflicting with any existing field named "Author"

    /**
     * Get author of this record.
     * Note: Only works on records selected via Versions()
     *
     * @return Member|null
     */
    public function Author()
    {
        //return the contents of the Author field, if one exists
        if($author = $this->owner->getField('Author')) {
            return $author;
        }

        if (!$this->owner->AuthorID) {
            return null;
        }

        /** @var Member $member */
        $member = DataObject::get_by_id(Member::class, $this->owner->AuthorID);
        return $member;
    }
robbieaverill commented 5 years ago

It's unlikely we'll be changing either Versioned's API or the DB field name to fix this issue, but we could add a method to the CWP page that returns the value with a different name, e.g.

public function getNewsPageAuthor()
{
    return $this->dbObject('Author');
}

You could add something like that to an extension as a workaround until it is fixed in CWP itself.

elliot-sawyer commented 5 years ago

Does that mean that "Author" is effectively a reserved name when working with Pages?

robbieaverill commented 5 years ago

Yep, I think so

michalkleiner commented 5 years ago

I'd vote for this being catered for at the Versioned module's end to interfere with custom dataobject db field names as little as possible.

robbieaverill commented 5 years ago

Thanks for the input. We can't easily change the Versioned method name due to semantic versioning restrictions, but even if we could, this would only fix this particular case.

You can track the general problem you're describing here: https://github.com/silverstripe/silverstripe-framework/issues/6329

We will fix this in CWP by adding a non-conflicting accessor for the Author property, since we also can't rename that due to semantic versioning restrictions.

robbieaverill commented 5 years ago

Fixed in CWP 2.4