PublishDateTimePicker: Retrieve all future posts in a given month

[GeoJunkie]

What?

Updates PublishDateTimePicker to retrieve all posts scheduled for the month shown on the date picker.

Why?

Currently, when retrieving scheduled posts for a given month, not all days of the month with scheduled posts are flagged. If there are more than 10 posts scheduled, it will only mark days for 10 posts, and will also omit some posts scheduled for the first or last day of the month (this behavior was reported at https://core.trac.wordpress.org/ticket/54784 ).

This has two causes:

• When making the call to /v2/posts, the per_page parameter is omitted. Since this defaults to 10, at most 10 scheduled posts for the given month are returned in the JSON response, and those are the only ones the PostSchedule component will see.
• getDayOfTheMonth() doesn't specify a time, so the current time is used and items scheduled before that time on the first day of the month and after that time on the last day of the month won't be included.

How?

• Adds a per_page parameter to the call to getEntityRecords(). This will be set to the number of days in the month.
• Updates getDayOfTheMonth() to set the first day's time to 00:00:00 and the last day's time to 23:59:59.

Testing Instructions

• Create at least 11 new posts with different scheduled publish dates within a future month (I used this bash script to automatically generate a scheduled post for each day of the next 90)
• Create a new post
• Click on Post settings, then "Immediately" next to "Publish"
• Navigate to the month with the scheduled posts
• Confirm that a dot appears below each day with a scheduled post

Screenshots or screencast

Testing with a post scheduled each day from October 1st - October 31st

Before

After

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @GeoJunkie! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[getdave]

@noisysocks I think you've been working in and around the Publish panel in 6.1 cycle. I wonder if you are able to review?

[paaljoachim]

Thanks for creating the PR @GeoJunkie Mike!
I will ping @retrofox Damian so that he can take a look as he has earlier worked on this feature.

[noisysocks]

• Adds a per_page parameter to the call to getEntityRecords(). This will be set to the number of days in the month.

Could you explain this logic? If there's 30 days in the month, then it isn't necessarily the case that fetching 30 posts will fill the calendar. It might be that there are multiple posts scheduled for a single day. I think it would make sense to simply update per_page to be a large value (e.g. per_page=100) and compensate for the increased size of the response by selecting only the fields that we need (e.g. _fields=id,date).

[noisysocks]

Thanks for the PR! ❤️

[GeoJunkie]

Thanks for the review @noisysocks :)

Could you explain this logic? If there's 30 days in the month, then it isn't necessarily the case that fetching 30 posts will fill the calendar. It might be that there are multiple posts scheduled for a single day. I think it would make sense to simply update per_page to be a large value (e.g. per_page=100) and compensate for the increased size of the response by selecting only the fields that we need (e.g. _fields=id,date).

That's true. So that we don't run into any issues with high-volume posting, should we set per_page to -1? If someone happened to post more than three times per day, that could break again even with it set to 100.

I'll definitely set the fields arg, which should help performance quite a bit.

[noisysocks]

If we set per_page to -1 then the frontend is permitted to make multiple requests which isn't great performance wise. I'm not sure that the value of this feature is worth the cost of multiple requests firing off. I'd say it's enough to have a large but finite limit of 100.

[GeoJunkie]

Adding a note here that this fix also exposed a performance issue with how Jetpack handles the /v2/posts: Automattic/jetpack#26955

Hopefully that will be fixed before this makes it out, but wanted to identify a potential impact on sites with Jetpack Publicize.

[noisysocks]

This looks great to me. Thanks for working on this @GeoJunkie.

I did a little testing locally and all seemed well. Since I pushed my own changes though, could you please double check that everything is working as you expected? Then I can merge for you.

[GeoJunkie]

I did a little testing locally and all seemed well. Since I pushed my own changes though, could you please double check that everything is working as you expected? Then I can merge for you.

Thanks @noisysocks, that looks great! I tested it out and it's working very well — and much more performant.

I did update package-lock.json to get the static analysis failure fixed. If that's OK, then this is ready to merge 🥳

[noisysocks]

Perfect, thank you! And congrats on the first PR.