Allow private pages to display in Nav block on front of site #49223

getdave · 2023-03-21T10:32:13Z

What?

Allows private Posts to display in the Nav block if they are part of a menu.

Why?

In the Classic Menus system it was not possible to add Private posts to a menu from the Menus screen. However, if the Post was added to a Menu and then subsequently made Private it would render on the front of the site and remain in the Menu on the Menus screen.

The Nav Link/Submenu blocks were not mirroring this behaviour. This PR fixes things to display the same behaviour as for Classic Menus.

How?

There were two tasks that needed to be addressed.

Check that private posts cannot be found in the Link creation UI.

I checked by:

✅ making a Private Post and then trying to find that post using the Link UI search. I could not find it.
✅ verified that the Post Search endpoint is set to publish status only.

Add private to the list of allowed post types on the front of the site.

This PR implements this part by adding private to the list of post statuses that are allowed.

Note: it should not be possible to find/add a private post to a menu from the Link UI. That part has not changed.

Testing Instructions

Create a Post/Page and publish it as Private.
In a Navigation block add a new menu item and try and find that post in the search UI. You shouldn't be able to find it.
Now make that Post "Public" again (Publish status).
Now go back to the Nav block and try and find your Post and add it to your menu.
Save the menu.
View the front of your site and check that the menu item is visible.
Now return to the Post and make it Private again.
Now reload the front of your site. Check the menu item is still rendered.
Test with other Post Types.

Testing Instructions for Keyboard

Screenshots or screencast

getdave · 2023-03-21T10:36:26Z

cc @Chrico who raised the original Issue.

github-actions · 2023-03-21T11:03:08Z

Flaky tests detected in c60b763.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4489008590
📝 Reported issues:

[Flaky Test] should filter results properly #44462 in specs/site-editor/global-styles-sidebar.test.js

scruffian

The code looks good and does what it says.

My concern is that this could potentially change the front end of sites which already have a link to a private post/page in them. Do you think that's something to be careful about?

getdave · 2023-03-21T13:05:14Z

Do you think that's something to be careful about?

100%. If this lands then we need to communicate the change. Do you have any ideas about how we could do that?

jeryj · 2023-03-21T15:09:45Z

My concern is that this could potentially change the front end of sites which already have a link to a private post/page in them. Do you think that's something to be careful about?

To make sure I follow, is this the scenario that would be changed?

Someone adds a public page to the navigation block menu (private ones are not allowed)
They save the page/nav
They set one of the pages from the nav to private
Before this PR, the private page would not show on the frontend of their site, but now it will?

getdave · 2023-03-21T15:15:11Z

phpunit/blocks/render-block-navigation-link-test.php

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


This can be removed as the test class is inherting from the base WP test class which does this clean up.

See:

https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#shared-fixtures

https://core.trac.wordpress.org/browser/trunk/tests/phpunit/includes/abstract-testcase.php#L91

https://github.com/WordPress/wordpress-develop/blob/95826a28f17bda68092926bb6c33a05cc9459964/tests/phpunit/includes/functions.php#L110

getdave · 2023-03-21T15:16:13Z

Before this PR, the private page would not show on the frontend of their site, but now it will?

Correct. It's consistent with Classic Menus system but it's not how Block menus have behaved until this PR.

aristath · 2023-03-22T09:06:36Z

I'm not sure if the old behavior (classic themes) was a feature, or a bug. It sounds to me like what classic themes were doing, was just wrong, and the block fixed the issue.
I'm a bit skeptical about allowing private posts to show in navigation - regardless of whether their status was changed after the menu item was added.

scruffian

I'm not sure if the old behavior (classic themes) was a feature, or a bug. It sounds to me like what classic themes were doing, was just wrong, and the block fixed the issue.

I was thinking more about this and I agree with this. In the same way that we'd like page slugs to update when the page updates, I think this is actually a feature not a bug. IMO we should close this.

getdave · 2023-03-22T09:31:15Z

I'm happy to close it. It did seem strange even though that's how @talldan said Classic Menus behave.

I'll close this out and at least we have a paper trail to follow if it's ever raised in future.

talldan · 2023-03-22T10:00:32Z

It did seem strange even though that's how @talldan said Classic Menus behave.

I double checked and it is how they work. 🤷

I didn't and still don't think it's how the nav block should work.

draganescu · 2023-03-22T10:48:33Z

There still is the need to provide to the use case where you have content which is accessible only to logged in users but logged out users should still see the link. This thing of "private pages should not appear in nav" is similar to Google not linking to Gmail just because you're logged out. Which of course is not true. Links are there pointing to things so users know things exist.

Chrico · 2023-03-22T11:02:36Z

Why not just adding a filter and let the developer decide which "post_status" should be allowed to see in "Navigation"-Block?

aristath · 2023-03-22T11:08:24Z

Does it not work if the user adds the menu items as custom links instead of selecting the post from a dropdown? 🤔 I think that method should work and cover the scenario described above...

draganescu · 2023-03-22T12:42:45Z

@Chrico I think the filter is "yet another filter", seems like too powerful a solution for such a corner case. From the issue the problem is a disconnect between the editor and the front end of the site.

I don't think the logic to not render a link that appears in the navigation block in the editor because it is private makes sense. Particularly when, as the issue describes, the page is made private after it was added to the editor. I have doubts about draft pages as well, but private pages/posts are even worse.

In #27207 I did mention that draft page/post links should show some kind of "indicator that the link will not be visible on the front end". If that indicator were to exist I think things would become clearer, but I would still have doubts, but at least we could explain what is going on.

For now though, as a user, having a link in my navigation not render because I made the page or post private is unexpected and impossible to figure out. I don't agree much with the idea to use "custom links", because, as a user, not finding things is super frustrating. For instance, I have a page which I linked to in my footer. In the mean time I made the page private and want to link to it in the header. I do the exact same actions and the page is not found - because I made it private, but how can I know that? I can't. Also a user that wants to link to their private page will never ever discover that they need to open the inserter and choose custom link.

It's one thing to be smart like having dynamic links which update when the slugs change, and it's another to not render things based on strict heuristics, which we may not be even right about. On another note, I also don't think it's good to encourage "leftover content" in the editor, whicih is there, is visible but does not render. Users should assume that what they see in the editor renders. Hidden content, should exist via special blocks (like annotations or editorial todos etc).

Chrico · 2023-03-22T12:57:09Z

I think the filter is "yet another filter", seems like too powerful a solution for such a corner case. From the issue the problem is a disconnect between the editor and the front end of the site.

I just proposed a filter to find a middle ground between "we don't want to change", but still allow developers to change it on their own.

Overall the restriction to "only post_status === publish" is incorrect here.

Why? Because when inserting a new Navigation Item, you can search via LinkControl on /wp-json/wp/v2/search?search={YOUR TERM}&per_page=20&type=post&_locale=user which uses internally the WP_REST_Post_Search_Handler. This Handler has a filter called rest_post_search_query [1] which allows to add aside from the default "post_type" => "publish"[2] every post_type you want.

So stripping off [3], what before can be allowed, is quiet frustating for the User who manages the navigation menue(s).

[1] https://github.com/WordPress/WordPress/blob/93a4991c018a562e9cb8298d997be21ffa85e9d1/wp-includes/rest-api/search/class-wp-rest-post-search-handler.php#L90

[2] https://github.com/WordPress/WordPress/blob/93a4991c018a562e9cb8298d997be21ffa85e9d1/wp-includes/rest-api/search/class-wp-rest-post-search-handler.php#L62

[3] https://github.com/WordPress/gutenberg/blob/v15.3.1/packages/block-library/src/navigation-link/index.php#L167

talldan · 2023-03-27T03:40:16Z

In #27207 I did mention that draft page/post links should show some kind of "indicator that the link will not be visible on the front end". If that indicator were to exist I think things would become clearer, but I would still have doubts, but at least we could explain what is going on.

That's a good point. There is an indicator for draft posts:

Something similar should at least exist for private pages when they're hidden.

getdave added 2 commits March 21, 2023 10:24

Allow private posts to render in Nav Link block

3f37245

Allow private posts in Submenus

5e52cd2

getdave requested a review from draganescu March 21, 2023 10:32

getdave self-assigned this Mar 21, 2023

getdave added [Block] Navigation Link Affects the Navigation Link Block [Block] Submenu Affects the Submenu Block - for submenus in navigation labels Mar 21, 2023

Lint

8c07bea

getdave marked this pull request as ready for review March 21, 2023 10:35

getdave requested a review from tellthemachines as a code owner March 21, 2023 10:35

getdave requested a review from talldan March 21, 2023 10:36

scruffian approved these changes Mar 21, 2023

View reviewed changes

Move tests to correct location and add test for private post render

f0285d2

Remove unneeded cleanup handled by Base Test Classes

c60b763

getdave commented Mar 21, 2023

View reviewed changes

getdave requested a review from scruffian March 21, 2023 15:15

scruffian requested changes Mar 22, 2023

View reviewed changes

getdave closed this Mar 22, 2023

getdave mentioned this pull request Mar 22, 2023

Navigation-Link does not render post_status==="private" #33215

Open

draganescu reopened this Mar 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow private pages to display in Nav block on front of site #49223

Allow private pages to display in Nav block on front of site #49223

getdave commented Mar 21, 2023 •

edited

Loading

getdave commented Mar 21, 2023

github-actions bot commented Mar 21, 2023 •

edited

Loading

scruffian left a comment

getdave commented Mar 21, 2023

jeryj commented Mar 21, 2023

getdave Mar 21, 2023

getdave commented Mar 21, 2023

aristath commented Mar 22, 2023

scruffian left a comment

getdave commented Mar 22, 2023

talldan commented Mar 22, 2023

draganescu commented Mar 22, 2023

Chrico commented Mar 22, 2023

aristath commented Mar 22, 2023

draganescu commented Mar 22, 2023

Chrico commented Mar 22, 2023 •

edited

Loading

talldan commented Mar 27, 2023

Allow private pages to display in Nav block on front of site #49223

Are you sure you want to change the base?

Allow private pages to display in Nav block on front of site #49223

Conversation

getdave commented Mar 21, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

getdave commented Mar 21, 2023

github-actions bot commented Mar 21, 2023 • edited Loading

scruffian left a comment

Choose a reason for hiding this comment

getdave commented Mar 21, 2023

jeryj commented Mar 21, 2023

getdave Mar 21, 2023

Choose a reason for hiding this comment

getdave commented Mar 21, 2023

aristath commented Mar 22, 2023

scruffian left a comment

Choose a reason for hiding this comment

getdave commented Mar 22, 2023

talldan commented Mar 22, 2023

draganescu commented Mar 22, 2023

Chrico commented Mar 22, 2023

aristath commented Mar 22, 2023

draganescu commented Mar 22, 2023

Chrico commented Mar 22, 2023 • edited Loading

talldan commented Mar 27, 2023

getdave commented Mar 21, 2023 •

edited

Loading

github-actions bot commented Mar 21, 2023 •

edited

Loading

Chrico commented Mar 22, 2023 •

edited

Loading