Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.92%. Comparing base (d94977f) to head (9f89cd4).

@@            Coverage Diff             @@
##            trunk    #1835      +/-   ##
==========================================
+ Coverage   65.89%   65.92%   +0.03%     
==========================================
  Files          88       88              
  Lines        6878     6890      +12     
==========================================
+ Hits         4532     4542      +10     
- Misses       2346     2348       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

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

@westonruter I'm not sure about this change. This seems to be something that helps development, so I think having it in the OD Dev Mode plugin makes more sense to me than making it the default for any administrator. Many administrators of production sites are probably not developers.

On another note, I think the code snippet referenced in the PR description is for something else than described (it mentions viewport, rather than storage lock)?

On another note, I think the code snippet referenced in the PR description is for something else than described (it mentions viewport, rather than storage lock)?

Oops. I copied the wrong code. This has been corrected.

I'm not sure about this change. This seems to be something that helps development, so I think having it in the OD Dev Mode plugin makes more sense to me than making it the default for any administrator. Many administrators of production sites are probably not developers.

Actually, it's not specifically to help development. If someone creates a new post and then views it on the frontend, this should result in a URL Metric being collected right away without having to wait for any TTL storage lock to expire for the author or for another visitor to come by to view the post. A post author will almost always want to view the published post on the frontend after publishing, so this will result in a URL Metric being stored right away to give the collection a head start. Additionally, an administrator browsing around their site should be able to submit URL Metrics as they go around without having to wait for the storage lock TTL to expire. The purpose of the storage lock is to guard against unlimited DB writes from a frontend visitor. This, however, is not relevant to administrators (or even other roles above subscriber) since they can write to the DB at will in the admin.

Actually, come to think of it, manage_options probably isn't the right capability here, now that I think about it. In reality it should be edit_posts. It's about whether or not a user has "write access" to the database.

Your rationale makes sense to me. In that case, maybe a better implementation to achieve that goal would be to invalidate the current user's storage lock transient whenever a post is updated? That way, any fresh content would be able to receive updates immediately, without e.g. "blasting" the server with tons of URL metrics (in case of a very unpopular site, where the administrator visits e.g. 15 URLs quickly to test stuff).

I'm not sure that would work. The storage lock TTL is sent to the client to prevent the client from even attempting to make a POST request to store a new URL Metric.

I don't see a problem with allowing an administrator to make multiple DB writes quickly. Consider bulk actions in the admin or quickly moderating through comments, for example.

Oh, I just realized this won't work as-is since the _wpnonce isn't being included for logged-in users, so the endpoint at present would always see the user as anonymous. Easy to fix.

Oh, I just realized this won't work as-is since the _wpnonce isn't being included for logged-in users, so the endpoint at present would always see the user as anonymous. Easy to fix.

There, this is fixed by 9f89cd4. Note that this could eventually mitigate the need for the Site Health test to check whether anonymous REST API requests are allowed (#1731). We could potentially add a new hook which disables URL Metric collection for anonymous users. This could be attractive to high-traffic sites (cf. #1655) which don't want to expose the REST API to anonymous visitors. Such sites would require logged-in users to collect the URL Metrics by browsing around, but also some automated collection could be implemented in the background, such as by opening the frontend in an IFRAME in the background when saving the post (#1311).