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

REST API: new endpoint to fetch post counts by post status #66294

Closed
wants to merge 8 commits into from

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Oct 22, 2024

What?

Creates a new endpoint to fetch the total number of post counts by post status using wp_count_posts.

Why?

The need for a performant post counts check for the block editor. See: #65223 (comment)

Contrast this with $query->found_posts which returns the total count for a specific query only and would degrade performance given n posts.

How?

Following the simplicity of wp/v2/statuses, which wraps get_post_stati, creating a basic route to return post type counts, e.g., wp/v2/counts/<post_type>

Testing Instructions

Run the tests: npm run test:unit:php:base -- --filter Gutenberg_Test_REST_Post_Counts_Controller

Open up the site editor and have a play with the console:

Examples:

await wp.apiFetch( { path: '/wp/v2/counts/page' } )

/*
{
    "publish": 1,
    "future": 0,
    "draft": 2,
    "pending": 0,
    "private": 0,
    "trash": 0,
    "auto-draft": 0
}
*/

await wp.apiFetch( { path: '/wp/v2/counts/wp_block' } )

/*
{
    "publish": 2,
    "future": 0,
    "draft": 0,
    "pending": 0,
    "private": 0,
    "trash": 0,
    "auto-draft": 0
}
*/

Also add a custom post type and ensure it appears in the response:

function add_my_custom_post_type() {
	register_post_status(
		'sweaty-palms',
		array(
			'label'                     => _x( 'Sweaty palms', 'post' ),
			'label_count'               => _n_noop(
				'Sweaty palms <span class="count">(%s)</span>',
				'Sweaty palms <span 
class="count">(%s)</span>'
			),
			'public'                    => true,
			'exclude_from_search'       => false,
			'show_in_admin_all_list'    => true,
			'show_in_admin_status_list' => true,
		)
	);
}
add_action( 'init', add_my_custom_post_type );

@ramonjd ramonjd self-assigned this Oct 22, 2024
@ramonjd ramonjd added [Type] Enhancement A suggestion for improvement. REST API Interaction Related to REST API [Status] In Progress Tracking issues with work in progress labels Oct 22, 2024
@ramonjd
Copy link
Member Author

ramonjd commented Oct 22, 2024

For future reference, a quick and dirty implementation to get #65223 working again with this PR:

Diff
diff --git a/packages/edit-site/src/components/sidebar-dataviews/dataview-item.js b/packages/edit-site/src/components/sidebar-dataviews/dataview-item.js
index 1e12d6706d8..1a96fdc0591 100644
--- a/packages/edit-site/src/components/sidebar-dataviews/dataview-item.js
+++ b/packages/edit-site/src/components/sidebar-dataviews/dataview-item.js
@@ -27,6 +27,7 @@ export default function DataViewItem( {
 	isActive,
 	isCustom,
 	suffix,
+	navigationItemSuffix,
 } ) {
 	const {
 		params: { postType },
@@ -55,6 +56,7 @@ export default function DataViewItem( {
 			<SidebarNavigationItem
 				icon={ iconToUse }
 				{ ...linkInfo }
+				suffix={ navigationItemSuffix }
 				aria-current={ isActive ? 'true' : undefined }
 			>
 				{ title }
diff --git a/packages/edit-site/src/components/sidebar-dataviews/default-views.js b/packages/edit-site/src/components/sidebar-dataviews/default-views.js
index 658fa319e9c..dae640fcf07 100644
--- a/packages/edit-site/src/components/sidebar-dataviews/default-views.js
+++ b/packages/edit-site/src/components/sidebar-dataviews/default-views.js
@@ -14,7 +14,8 @@ import {
 } from '@wordpress/icons';
 import { useSelect } from '@wordpress/data';
 import { store as coreStore } from '@wordpress/core-data';
-import { useMemo } from '@wordpress/element';
+import { useMemo, useState, useEffect } from '@wordpress/element';
+import apiFetch from '@wordpress/api-fetch';
 
 /**
  * Internal dependencies
@@ -68,6 +69,51 @@ const DEFAULT_POST_BASE = {
 	layout: defaultLayouts[ LAYOUT_LIST ].layout,
 };
 
+export function useDefaultViewsWithItemCounts( { postType } ) {
+	const defaultViews = useDefaultViews( { postType } );
+	const [ counts, setCounts ] = useState( null );
+	useEffect( () => {
+		if ( ! postType ) {
+			return;
+		}
+		const abortController =
+			typeof window.AbortController === 'undefined'
+				? undefined
+				: new window.AbortController();
+
+		apiFetch( {
+			path: `/wp/v2/counts/${ postType }`,
+			signal: abortController?.signal,
+		} ).then( ( response ) => {
+			setCounts( response );
+		} );
+
+		return () => abortController?.abort();
+	}, [ postType, setCounts ] );
+
+	return useMemo( () => {
+		if ( ! defaultViews ) {
+			return [];
+		}
+
+		// If there are no records, return the default views with no counts.
+		if ( ! counts ) {
+			return defaultViews;
+		}
+
+		// All items excluding trashed items as per the default "all" status query.
+		counts.all =
+			counts.publish + counts.future + counts.draft + counts.pending;
+
+		return defaultViews.map( ( _view ) => {
+			if ( counts?.[ _view.status ] ) {
+				_view.count = counts[ _view.status ];
+			}
+			return _view;
+		} );
+	}, [ defaultViews, counts ] );
+}
+
 export function useDefaultViews( { postType } ) {
 	const labels = useSelect(
 		( select ) => {
@@ -80,6 +126,7 @@ export function useDefaultViews( { postType } ) {
 		return [
 			{
 				title: labels?.all_items || __( 'All items' ),
+				status: 'all',
 				slug: 'all',
 				icon: pages,
 				view: DEFAULT_POST_BASE,
@@ -87,6 +134,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Published' ),
 				slug: 'published',
+				status: 'publish',
 				icon: published,
 				view: DEFAULT_POST_BASE,
 				filters: [
@@ -100,6 +148,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Scheduled' ),
 				slug: 'future',
+				status: 'future',
 				icon: scheduled,
 				view: DEFAULT_POST_BASE,
 				filters: [
@@ -113,6 +162,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Drafts' ),
 				slug: 'drafts',
+				status: 'draft',
 				icon: drafts,
 				view: DEFAULT_POST_BASE,
 				filters: [
@@ -126,6 +176,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Pending' ),
 				slug: 'pending',
+				status: 'pending',
 				icon: pending,
 				view: DEFAULT_POST_BASE,
 				filters: [
@@ -139,6 +190,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Private' ),
 				slug: 'private',
+				status: 'private',
 				icon: notAllowed,
 				view: DEFAULT_POST_BASE,
 				filters: [
@@ -152,6 +204,7 @@ export function useDefaultViews( { postType } ) {
 			{
 				title: __( 'Trash' ),
 				slug: 'trash',
+				status: 'private',
 				icon: trash,
 				view: {
 					...DEFAULT_POST_BASE,
diff --git a/packages/edit-site/src/components/sidebar-dataviews/index.js b/packages/edit-site/src/components/sidebar-dataviews/index.js
index 86420c4eec1..c4bd4e3eb30 100644
--- a/packages/edit-site/src/components/sidebar-dataviews/index.js
+++ b/packages/edit-site/src/components/sidebar-dataviews/index.js
@@ -7,7 +7,7 @@ import { privateApis as routerPrivateApis } from '@wordpress/router';
 /**
  * Internal dependencies
  */
-import { useDefaultViews } from './default-views';
+import { useDefaultViewsWithItemCounts } from './default-views';
 import { unlock } from '../../lock-unlock';
 import DataViewItem from './dataview-item';
 import CustomDataViewsList from './custom-dataviews-list';
@@ -18,8 +18,9 @@ export default function DataViewsSidebarContent() {
 	const {
 		params: { postType, activeView = 'all', isCustom = 'false' },
 	} = useLocation();
-	const defaultViews = useDefaultViews( { postType } );
-	if ( ! postType ) {
+	const defaultViews = useDefaultViewsWithItemCounts( { postType } );
+	console.log( { defaultViews } );
+	if ( ! postType || ! defaultViews ) {
 		return null;
 	}
 	const isCustomBoolean = isCustom === 'true';
@@ -34,6 +35,9 @@ export default function DataViewsSidebarContent() {
 							slug={ dataview.slug }
 							title={ dataview.title }
 							icon={ dataview.icon }
+							navigationItemSuffix={
+								<span>{ dataview.count }</span>
+							}
 							type={ dataview.view.type }
 							isActive={
 								! isCustomBoolean &&

@Mamaduka
Copy link
Member

Do we need a new endpoint for this? I think the data is available via headers, and you can query post types by status.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 22, 2024

I think the data is available via headers, and you can query post types by status.

That's right.

But the use case here is: "Gimmie the total counts for a particular post type grouped by status".

wp_count_posts returns total counts for all statuses using a more performant COUNT query.

Furthermore the header value (X-WP-Total) represents the total number of posts found matching the current query parameters.

Quoting myself:

While X-WP-Total is available in the headers for single entity requests, for this use case, and given the current set up, there'd have be a separate request for each status to grab the totals. There could [also] be n statuses.

@Mamaduka
Copy link
Member

It might be worth checking with the REST API team for suggestions early.

Also, instead of a new API, we could extend the posts controller and support something like wp/v2/<post_type>/total.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 22, 2024

It might be worth checking with the REST API team for suggestions early.

Yep, I'm just experimenting for now, but that's on the cards. Cheers!

@ramonjd
Copy link
Member Author

ramonjd commented Oct 22, 2024

we could extend the posts controller and support something like wp/v2/<post_type>/total.

Oh yes, I actually started doing it that way, but the templates controller doesn't inherit the posts controller and I didn't want to duplicate things for the sake of an experiment.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 24, 2024

Asked for advice in the REST API slack channel: https://wordpress.slack.com/archives/C02RQC26G/p1729806293986179

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

Please find my notes below. Thanks.

@ramonjd ramonjd force-pushed the add/rest-api-posts-counts-controller branch from acfc77f to 9bc9fd8 Compare October 28, 2024 03:37
// translators: %s: Post status.
'description' => sprintf( __( 'The number of posts with the status %s.' ), $post_status ),
'type' => 'integer',
'context' => array( 'view', 'edit', 'embed' ),
Copy link
Member Author

Choose a reason for hiding this comment

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

Allows embedding in, for example, WP_Posts_Controller, e.g.,

		$links['counts'] = array(
			'href'       => rest_url( 'wp/v2/counts/' . $post->post_type ),
			'embeddable' => true,
		);

Output of /wp/v2/posts?_embed=counts

[
   {
	....,
	_embedded: {
	    "counts": [
	        {
	            "publish": 1,
	            "future": 0,
	            "draft": 0,
	            "pending": 0,
	            "private": 0,
	            "trash": 0
	        }
	},
	...
    ]
},
...
]

@ramonjd ramonjd force-pushed the add/rest-api-posts-counts-controller branch from 1a9deb4 to d15ce71 Compare October 31, 2024 23:47
@ramonjd ramonjd marked this pull request as ready for review October 31, 2024 23:48
@ramonjd ramonjd requested a review from spacedmonkey as a code owner October 31, 2024 23:48
Copy link

github-actions bot commented Oct 31, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: desrosj <[email protected]>
Co-authored-by: Mamaduka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

*/
public function __construct() {
$this->namespace = 'wp/v2';
$this->rest_base = 'counts';
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm not sure that I love an endpoint specific for post counting. I am not against potentially adding this capability, but I think it feels more natural to be available under the wp/v2/<post_type>/count endpoint.

I am not sure if there's a technical reason why that would be difficult. But wondering if that could support retrieving the overall count for a specific post type (no arguments) OR with any other passed specific query arguments since they should already be supported within that post type's controller.

@TimothyBJacobs Do you have any opinion on any of these approaches?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @desrosj This is exactly the feedback I was after 🙇🏻

@Mamaduka also hinted in that direction above

I am not sure if there's a technical reason why that would be difficult

In principle, I don't see any barriers. In fact, that's what I started with locally at least.

I arrived here because the templates controller doesn't inherit the posts controller, and I didn't want to duplicate things, but rather, make the response embeddable.

However, if the consensus is that it'd be better for users to have a route on post-type endpoints, e.g., wp/v2/<post_type>/count, then I'm happy to switch things around.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've thrown up an alternative that adds a /count route to WP_REST_Posts_Controller, so wp/v2/<post_type>/count as @desrosj mentions above.

I'm feeling more comfortable with that version 😄

For Gutenberg, it's only needed for the page post type, but in the Core sync it'd be available for all post types that inherit from the controller.

Sync with changes from Core backport
@ramonjd ramonjd force-pushed the add/rest-api-posts-counts-controller branch from 35d277b to 927ac87 Compare December 9, 2024 04:27
Copy link

github-actions bot commented Dec 9, 2024

Flaky tests detected in 927ac87.
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/12228945892
📝 Reported issues:

ramonjd added a commit that referenced this pull request Dec 9, 2024
Extend post type controller to return total post count
ramonjd added a commit that referenced this pull request Dec 9, 2024
Extend post type controller to return total post count
ramonjd added a commit that referenced this pull request Dec 9, 2024
Extend post type controller to return total post count

Alternative to #66294
Extend post type controller to return total post count
@ramonjd
Copy link
Member Author

ramonjd commented Dec 9, 2024

@ramonjd ramonjd closed this Dec 9, 2024
@ramonjd ramonjd deleted the add/rest-api-posts-counts-controller branch December 9, 2024 06:45
@ramonjd
Copy link
Member Author

ramonjd commented Dec 10, 2024

Closing in favor of

I'm not entirely convinced that integrating a new schema into WP_REST_Posts_Controller is a good idea.

  1. It doesn't work in the first place (the schema property of the routes, if custom, is never fired except for OPTIONS as far as I can tell), and my instinct is that an endpoint shouldn't have two separate schemas.
  2. Integrating it into the current item schema isn't appropriate as it's not an item, it's related to the collection.
  3. Considering 2, this leaves the possibility of adding it to $response->header.

More info: https://github.com/WordPress/wordpress-develop/pull/7773/files#r1877033539

Which makes me think I should do a complete backflip and reinstate this PR and close #67719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REST API Interaction Related to REST API [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants