DorsetDigital / silverstripe-canonical

src/SiteTreeCanonicalExtension.php

<?php

namespace DorsetDigital\SilverStripeCanonical;

use SilverStripe\View\HTML;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\TextField;
use SilverStripe\Forms\LiteralField;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\CMS\Model\SiteTreeExtension;
use SilverStripe\CMS\Model\VirtualPage;

class SiteTreeCanonicalExtension extends SiteTreeExtension
{
    private static $db = [

The private property $db is not used, and could be removed.

        'CanonicalURL' => 'Text'
    ];

    public function updateCMSFields(FieldList $fields)
    {
        if ($MetaToggle = $fields->fieldByName('Root.Main.Metadata')) {

Are you sure the assignment to $MetaToggle is correct as $fields->fieldByName('Root.Main.Metadata') targeting SilverStripe\Forms\FieldList::fieldByName() seems to always return null.

            if ($url = $this->getorsetCanonicalURL()) {

Are you sure the assignment to $url is correct as $this->getorsetCanonicalURL() targeting DorsetDigital\SilverStripeCanonical\SiteTreeCanonicalExtension::getorsetCanonicalURL() seems to always return null.

The assignment to $url is dead and can be removed.

                $MetaToggle->push($MetaCanonical = TextField::create('CanonicalURL', _t(__CLASS__ . '.LinkOverride', "Override canonical URL")));
                $MetaCanonical
                    ->setAttribute('placeholder', $this->getorsetCanonicalURL())

Are you sure the usage of $this->getorsetCanonicalURL() targeting DorsetDigital\SilverStripeCanonical\SiteTreeCanonicalExtension::getorsetCanonicalURL() seems to always return null.

                    ->setDescription(_t(__CLASS__ . '.LinkOverrideDesc', 'Only set this if another URL should count as the original (e.g. of reposting a blog post from another source).'));
                if ($this->owner->ClassName == VirtualPage::class) {
                    $MetaCanonical
                        ->setReadonly(true)
                        ->setDescription(_t(__CLASS__ . '.LinkOverrideVirtualDesc', 'Linked page will be used.'));
                }
            } else {
                $MetaToggle->push($MetaCanonical = LiteralField::create("CanonicalURL", '<p class="form__field-label">' . _t(__CLASS__ . '.LinkFieldPlaceholder', 'Canonical-URLs needs a Canonical domain in <a href="/admin/settings">SiteConfig</a>') . '</p>'));
            }
            $MetaCanonical->setRightTitle(_t(__CLASS__ . '.LinkFieldRightTitle', 'Used to identify the original resource (URL) to prevent being considered as "duplicate content".'));
        }
    }

    function getorsetCanonicalURL()
    {
        $siteConfig = SiteConfig::current_site_config();
        if (filter_var($siteConfig->CanonicalDomain, FILTER_VALIDATE_URL)) {
            $canonicalBase = trim($siteConfig->CanonicalDomain, '/');

It seems like $siteConfig->CanonicalDomain can also be of type null; however, parameter $string of trim() does only seem to accept string, maybe add an additional type check?

            // dynamic value
            if (method_exists($this->owner, 'CanonicalLink')) {
                $link = $this->owner->CanonicalLink();
            }

            // canonical link on Page
            if (isset($this->owner->CanonicalURL) && $this->owner->CanonicalURL != null) {
                $link = $this->owner->CanonicalURL;
            }

            // use CopyContentFrom()->Link() for VirtualPage
            if ($this->owner->ClassName == VirtualPage::class) {
                $link = $this->owner->CopyContentFrom()->Link();
            }

            // add canonicalBase if relative URL
            if (isset($link)) {
                $urlArray = parse_url($link);
                if (!isset($urlArray['host']) && !isset($urlArray['scheme'])) {
                    $link = $canonicalBase . $link;
                }
            } else {
                // default link with base
                $link = $canonicalBase . $this->owner->Link();
            }

            return $link;

The variable $link does not seem to be defined for all execution paths leading up to this point.

        }
    }

    public function MetaComponents(array &$tags)
    {
        if ($canonLink = $this->getorsetCanonicalURL()) {

Are you sure the assignment to $canonLink is correct as $this->getorsetCanonicalURL() targeting DorsetDigital\SilverStripeCanonical\SiteTreeCanonicalExtension::getorsetCanonicalURL() seems to always return null.

            $tags['canonical'] = [
                'tag' => 'link',
                'attributes' => [
                    'rel' => 'canonical',
                    'href' => $canonLink
                ]
            ];
        }
    }
}