aerni / statamic-advanced-seo

Comprehensive SEO addon for Statamic with flexibility in mind
https://statamic.com/addons/aerni/advanced-seo
10 stars 5 forks source link

seo_social_images_theme: Argument #1 ($theme) must be of type string, null given #147

Closed jonathan-bird closed 1 month ago

jonathan-bird commented 1 month ago

We are getting the error Aerni\AdvancedSeo\SocialImages\SocialImageRepository::route(): Argument #1 ($theme) must be of type string, null given, called in /Users/jonathanbird/Dev/bsphn-statamic/vendor/aerni/advanced-seo/src/SocialImages/SocialImageRepository.php on line 38 when viewing the /cp/collections or /cp/taxonomy pages with Statamic 5 + latest 2.5.2 Advanced SEO library.

It is related to the $entry->seo_social_images_theme line which is returning null, and expecting string to be returned.

We have "Generate Social Images" disabled, and in the /content/seo/collections/xxx yaml files, the data is set to seo_social_images_theme: default

Let me know if you need anything else.

jonathan-bird commented 1 month ago

I am able to get a fix in if I update the method to include ?? SocialImageTheme::fieldtypeDefault():

public function previewTargets(Entry $entry): array
    {
        return [
            [
                'label' => 'Open Graph Image',
                'format' => $this->route(theme: $entry->seo_social_images_theme ?? SocialImageTheme::fieldtypeDefault(), type: 'open_graph', id: '{id}'),
            ],
            [
                'label' => 'Twitter Image',
                'format' => $this->route(theme: $entry->seo_social_images_theme ?? SocialImageTheme::fieldtypeDefault(), type: "twitter_{$entry->seo_twitter_card}", id: '{id}'),
            ],
        ];
    }
aerni commented 1 month ago

I'm unable to reproduce this issue. Did you try clearing the cache?

The route cp/collections should never hit the previewTargets method. This method is only triggered when an entry blueprint is found. In the CP, this only happens on /cp/collections/{collection} and cp/collections/articles/entries/{entry}. And taxonomies are not even supported by the social images generator. So it's very strange that a taxonomy route would throw an exception.

This is where the previewTargets are registered on EntryBlueprintFound:

https://github.com/aerni/statamic-advanced-seo/blob/40d2ec6329eeee19d1237e221c7b7211217020a4/src/Subscribers/SocialImagesGeneratorSubscriber.php#L51-L61

When the social images generator is disabled, it should never even get to adding the targets at the spot where it's throwing an exception for you. I'm curious what the event data looks like when it's passing the SocialImagesGenerator::enabled($data) method and still throwing an exception after that.

jonathan-bird commented 1 month ago

Yeah I'm not cached locally (cleared static, config, compiled, stache etc), and same issue on prod which is fully cached. Fully up to date on Statamic 5 + all libraries too.

It's definitely getting hit on /cp/collections (see screenshot), and /cp/taxonomies. You can see the full stack here: https://flareapp.io/share/DPyjnLz7

SocialImagesGenerator::enabled($data) is true, as I have all of my collections setup within Site > Social Media. My collection entries don't have seo_social_images_theme set directly on the content (as it just inherits from the default) so it's just returning null, throwing the error.