Log1x / poet

Configuration-based post type, taxonomy, block category, and block registration for Sage 10.
https://github.com/Log1x/poet
MIT License
205 stars 16 forks source link

Renaming default 'post' post_type #36

Open mike-sheppard opened 3 years ago

mike-sheppard commented 3 years ago

Hey @Log1x 👋🏻 not an urgent one, but wanted to drop a note in case anyone else is having/fixed this issue.

I haven't had a chance to dig in completely to figure out why, but since upgrading to WP 5.8 + latest poet (v2.0.4) renaming the default post_type throws this notice + the 'Articles' label disappears in the admin sidebar.

Screenshot 2021-09-06 at 17 58 38

E_NOTICE Trying to get property 'name_admin_bar' of non-object
File: site/web/wp/wp-includes/admin-bar.php
Line: 851 

Poet config

<?php
return [
    'post' => [
        'post' => [
            'labels' => [
                'singular' => 'Article',
                'plural'   => 'Articles',
            ],
        ],
    ],
];

^ removing this label override gets rid of the notice + Posts appears in the admin sidebar as you'd expect.

Cheers!

intelligence commented 3 years ago

Getting Trying to get property 'menu_name' of non-object when trying to do the same!

ghost commented 3 years ago

So this was a bugger to track. Turns out that when an existing post type is modified (such as the post type), the config values that are passed to the object replace what's already there.

In practice, what this means is that when you go to modify the post type with your labels config, a whole host of values (such as menu_name) get wiped away. WordPress expects to find parameters such as what is specified here.

// Default post type labels
[
        'name' => array( _x( 'Posts', 'post type general name' ), _x( 'Pages', 'post type general name' ) ),
        'singular_name'            => array( _x( 'Post', 'post type singular name' ), _x( 'Page', 'post type singular name' ) ),
        'add_new'                  => array( _x( 'Add New', 'post' ), _x( 'Add New', 'page' ) ),
        'add_new_item'             => array( __( 'Add New Post' ), __( 'Add New Page' ) ),
        'edit_item'                => array( __( 'Edit Post' ), __( 'Edit Page' ) ),
        'new_item'                 => array( __( 'New Post' ), __( 'New Page' ) ),
        'view_item'                => array( __( 'View Post' ), __( 'View Page' ) ),
        'view_items'               => array( __( 'View Posts' ), __( 'View Pages' ) ),
        'search_items'             => array( __( 'Search Posts' ), __( 'Search Pages' ) ),
        'not_found' 
        ...
]

Those labels get replaced with what the config specified, i.e.:

[
        'singular' => 'Article',
        'plural' => 'Articles',
]

This is caused by modifyPostType, which overwrites the values instead of merging them.

https://github.com/Log1x/poet/blob/7bb298c4d79d4fb14cc88dce77c02dc0f50e1fad/src/Modules/PostTypeModule.php#L74-L85

One possible fix is to merge config values with the existing label values, like so:

   protected function modifyPostType($name, $config)
    {
       ...
        foreach ($config as $key => $value) {
            $merged = (object) collect($object->{$key})->merge($value)->toArray();            
            $object->{$key} = $merged;        
        }  
    }

You'll see some changes in the admin panel, but not all labels will use your configured values. If you want to get them all changed, you need to individually specify the labels in accordance with what get_post_type_labels expects here.

Another possible fix (and the easiest, from what I can tell, but also the riskiest, because it's not clear to me what the drawbacks could be) is to just re-register the post type.

    public function handle()
    {
        $this->config->each(function ($value, $key) {

            if (empty($key) || is_int($key)) {
                return register_extended_post_type(...Arr::wrap($value));
            }

            if ($this->hasPostType($key)) {                
                if ($value === false) {
                    return $this->unregisterPostType($key);
                }
               // Remove next line so existing post types are re-registered
               // return $this->modifyPostType($key, $value);
            } 

            return register_extended_post_type(
                $key,
                $value,
                Arr::get($value, 'labels', [])
            );

        });
    }

WordPress used to condone using this method to modify post types (as evidenced here), but I'm not sure that still stands.

Somewhere in the middle, there's this next solution, which robs a set of labels that get generated when a WP_Post_Type object is instantiated and passed to get_post_type_labels. A few labels still need to be set in Poet's config.php, but not nearly as many. From what I can tell, you can get away with something like this:

'post' => [
    'labels' => [
        'name' => 'Article',
        'search_items' => 'Search Articles'
   ]
]

To implement this solution, modify the modifyPostType as follows:


    protected function modifyPostType($name, $config)
    {
        $object = get_post_type_object($name);

        if (! $this->isPostType($object)) {
            return;
        }       

        foreach ($config as $key => $value) {
            if($key == 'labels') {
                $new_labels = new WP_Post_Type($name, [
                    'labels' => $value
                ]);
                $object->{$key} = get_post_type_labels($new_labels);                
            }

        }        
    }

This comment is a bit long winded, I know. :) If any of these solutions should be pushed to the repo I'd be happy to make a PR.

Jamiewarb commented 1 year ago

@Log1x Any chance you can feed in on the above approaches and make a decision so we can submit a PR for this please?