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

Fix Navigation Links when post type is not Page or Post #28892

Merged
merged 2 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/block-library/src/navigation-link/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ function block_core_navigation_link_render_submenu_icon() {
*/
function render_block_core_navigation_link( $attributes, $content, $block ) {
// Don't render the block's subtree if it is a draft.
if ( isset( $attributes['id'] ) && is_numeric( $attributes['id'] ) ) {
if (
isset( $attributes['id'] ) && is_numeric( $attributes['id'] ) &&
( 'post' === $attributes['type'] || 'page' === $attributes['type'] )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder here if we should exclude what get_post does not support (tags, categories) because CPT's (e.g. a book) should also follow this draft logic but we can't add them all to this condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point @draganescu. Thanks for taking a look! How about we follow up on it in the next PR, since I'm working on #24814 which adds variations for CPTs and taxonomies?

At the moment I'm not sure type is enough to encapsulate what something is. It looks like we're using the post-type and taxonomy slug as a type value, and I'm not sure we enforce uniqueness across the two strings. For example can someone create a poorly named custom taxonomy that collides with a custom post type? I'll need to experiment. We'll also need to respect show_in_nav_menus on both entities.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as this remains on the radar, I am merging this 🚀

) {
$post = get_post( $attributes['id'] );
if ( 'publish' !== $post->post_status ) {
return '';
Expand Down
149 changes: 149 additions & 0 deletions phpunit/class-block-library-navigation-link-test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<?php
/**
* Tests server side rendering of core/navigation-link
*
* @package Gutenberg
* @subpackage block-library
*/

/**
* Tests for various cases in Navigation Link rendering
*/
class Block_Library_Navigation_Link_Test extends WP_UnitTestCase {
private static $category;
private static $page;
private static $draft;

private static $pages;
private static $terms;

public static function wpSetUpBeforeClass() {

self::$draft = self::factory()->post->create_and_get(
array(
'post_type' => 'page',
'post_status' => 'draft',
'post_name' => 'ceilingcat',
'post_title' => 'Ceiling Cat',
'post_content' => 'Ceiling Cat content',
'post_excerpt' => 'Ceiling Cat',
)
);
self::$pages[] = self::$draft;

self::$page = self::factory()->post->create_and_get(
array(
'post_type' => 'page',
'post_status' => 'publish',
'post_name' => 'tabby',
'post_title' => 'Tabby cats',
'post_content' => 'Tabby cat content',
'post_excerpt' => 'Tabby cat',
)
);
self::$pages[] = self::$page;

self::$category = self::factory()->category->create_and_get(
array(
'taxonomy' => 'category',
'name' => 'cats',
'slug' => 'cats',
'description' => 'Cats Category',
)
);

self::$terms[] = self::$category;
}

public static function wpTearDownAfterClass() {
foreach ( self::$pages as $page_to_delete ) {
wp_delete_post( $page_to_delete );
}
foreach ( self::$terms as $term_to_delete ) {
wp_delete_term( $term_to_delete->term_id, $term_to_delete->taxonomy );
}
}

function test_returns_link_when_post_is_published() {
$page_id = self::$page->ID;

$parsed_blocks = parse_blocks(
"<!-- wp:navigation-link {\"label\":\"Sample Page\",\"type\":\"page\",\"id\":{$page_id},\"url\":\"http://localhost:8888/?page_id={$page_id}\"} /-->"
);
$this->assertEquals( 1, count( $parsed_blocks ) );

$navigation_link_block = new WP_Block( $parsed_blocks[0], array() );
$this->assertEquals(
true,
strpos(
gutenberg_render_block_core_navigation_link(
$navigation_link_block->attributes,
array(),
$navigation_link_block
),
'Sample Page'
) !== false
);
}

function test_returns_empty_when_label_is_missing() {
$page_id = self::$page->ID;

$parsed_blocks = parse_blocks(
"<!-- wp:navigation-link {\"type\":\"page\",\"id\":{$page_id},\"url\":\"http://localhost:8888/?page_id={$page_id}\"} /-->"
);
$this->assertEquals( 1, count( $parsed_blocks ) );

$navigation_link_block = new WP_Block( $parsed_blocks[0], array() );
$this->assertEquals(
'',
gutenberg_render_block_core_navigation_link(
$navigation_link_block->attributes,
array(),
$navigation_link_block
)
);
}

function test_returns_empty_when_draft() {
$page_id = self::$draft->ID;

$parsed_blocks = parse_blocks(
"<!-- wp:navigation-link {\"label\":\"Draft Page\",\"type\":\"page\",\"id\":{$page_id},\"url\":\"http://localhost:8888/?page_id={$page_id}\"} /-->"
);
$this->assertEquals( 1, count( $parsed_blocks ) );

$navigation_link_block = new WP_Block( $parsed_blocks[0], array() );

$this->assertEquals(
'',
gutenberg_render_block_core_navigation_link(
$navigation_link_block->attributes,
array(),
$navigation_link_block
)
);
}

function test_returns_link_for_category() {
$category_id = self::$category->term_id;

$parsed_blocks = parse_blocks(
"<!-- wp:navigation-link {\"label\":\"Cats\",\"type\":\"category\",\"id\":{$category_id},\"url\":\"http://localhost:8888/?cat={$category_id}\"} /-->"
);
$this->assertEquals( 1, count( $parsed_blocks ) );

$navigation_link_block = new WP_Block( $parsed_blocks[0], array() );
$this->assertEquals(
true,
strpos(
gutenberg_render_block_core_navigation_link(
$navigation_link_block->attributes,
array(),
$navigation_link_block
),
'Cats'
) !== false
);
}
}