Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming default 'post' post_type #36

Open
mike-sheppard opened this issue Sep 6, 2021 · 3 comments
Open

Renaming default 'post' post_type #36

mike-sheppard opened this issue Sep 6, 2021 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@mike-sheppard
Copy link
Contributor

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
Copy link

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

@Log1x Log1x added the help wanted Extra attention is needed label Sep 10, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

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.

protected function modifyPostType($name, $config)
{
$object = get_post_type_object($name);
if (! $this->isPostType($object)) {
return;
}
foreach ($config as $key => $value) {
$object->{$key} = $value;
}
}

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants