wintercms / wn-blog-plugin

Blog plugin for Winter CMS
https://wintercms.com/
MIT License
18 stars 24 forks source link

Changing :slug page url parameter name breaks the setUrl methods. #25

Open RomainMazB opened 2 years ago

RomainMazB commented 2 years ago

All the four components of this plugin use model helper methods setUrl on Post and Category models which generate the blog or category url from the corresponding page passed in parameter of the method: https://github.com/wintercms/wn-blog-plugin/blob/3fba60378b6ebdef0e69ffdf2af6adbbc6dbcbb2/models/Category.php#L87-L94 https://github.com/wintercms/wn-blog-plugin/blob/3fba60378b6ebdef0e69ffdf2af6adbbc6dbcbb2/models/Post.php#L157-L176

The problem is that as you can see, the slug page's param is hard-coded in the url generation, which make this page url generation to fail:

// post.htm
title = "Blog post"
url = "/blog/:notSlug"
layout = "default"
is_hidden = 0

[blogPost]
slug = "{{ :notSlug }}"
categoryPage = 404
==
{% component 'blogPost' %}

Same is true for the page integrating the blogPosts, blogCategories and blogRssFeed components.

As of today, I don't see any easy way to fix this without adding a new property that would refer the target page url parameter name.
Something like this in Categories component:

// components/Categories.php

public function defineProperties()
{
    return [
        'slug' => [
            // ...
        ],
        'displayEmpty' => [
            // ...
        ],
        'categoryPage' => [
            'title'       => 'winter.blog::lang.settings.category_page',
            'description' => 'winter.blog::lang.settings.category_page_description',
            'type'        => 'dropdown',
            'default'     => 'blog/category',
            'group'       => 'winter.blog::lang.settings.group_links',
        ],
        'categoryPageSlugParam' => [
            'title'       => 'winter.blog::lang.settings.category_page_slug_param',
            'description' => 'winter.blog::lang.settings.category_page_slug_param_description',
            'default'     => 'slug',
            'type'        => 'string',
        ],
    ];
}

This property value could be passed to the setUrl method and generate the good url structure like this:

public function setUrl($pageName, $controller, $slugProp)
{
    $params = [
        'id'   => $this->id,
        $slugProp => $this->slug
    ];

    return $this->url = $controller->pageUrl($pageName, $params, false);
}
RomainMazB commented 2 years ago

Ok, I've found how to solve this issue properly without any modification to the setUrl method, here is the example from blogPosts component:

/*
 * Add a "url" helper attribute for linking to each post and category
 */
// Load post and category pages
$activeTheme = Theme::getActiveTheme();
$postPage = Page::loadCached($activeTheme, $this->property('postPage'));
$categoryPage = Page::loadCached($activeTheme, $this->property('categoryPage'));

// Retrieve the post slug property name
if ($postPage && $postPage->hasComponent('blogPost')) {
    $postPageSlugParam = $postPage->getComponent('blogPost')->property('slug');
    if (preg_match('/^\{\{([^\}]+)\}\}$/', $postPageSlugParam, $postMatches)) {
        $postPageSlugParam = substr(trim($postMatches[1]), 1);
    }
}

// Retrieve the category slug property name
if ($categoryPage && $categoryPage->hasComponent('blogPosts')) {
    $categoryPageSlugParam = $categoryPage->getComponent('blogPosts')->property('slug');
    if (preg_match('/^\{\{([^\}]+)\}\}$/', $categoryPageSlugParam, $categoryMatches)) {
        $categoryPageSlugParam = substr(trim($categoryMatches[1]), 1);
    }
}

// Default to slug if not found
$postPageSlugParam ??= 'slug';
$categoryPageSlugParam ??= 'slug';

$posts->each(function($post) use ($postPageSlugParam, $categoryPageSlugParam, $categorySlug) {
    // Pass them as $params parameter of setUrl method
    $post->setUrl($this->postPage, $this->controller, ['category' => $categorySlug, $postPageSlugParam => $post->slug]);

    $post->categories->each(function($category) use ($categoryPageSlugParam) {
        $category->setUrl($this->categoryPage, $this->controller, [$categoryPageSlugParam => $category->slug]);
    });
});

Basically, I apply the same logic as the sitemap plugin support does here: https://github.com/wintercms/wn-blog-plugin/blob/3fba60378b6ebdef0e69ffdf2af6adbbc6dbcbb2/models/Post.php#L672-L682

I did this directly at the component level to avoid to load 20 times the post and category page object.

If this looks good to @LukeTowers @bennothommo, I could reproduce these steps on all the blog and forum plugin's components.

bennothommo commented 2 years ago

@RomainMazB happy for you to provide a PR with that fix - just change the ??= operators to $variable = $variable ?? 'slug' if you can, so that we can still support Winter 1.1 with these plugins as that operator only works with PHP 7.4 and up.

LukeTowers commented 2 years ago

@RomainMazB are you still interested in submitting a PR?