Amp-wp: amp_admin_get_preview_permalink throws query everywhere in Dashboard

[fumikito]

Bug Description

AMP plugin create preview link in admin menu with function amp_admin_get_preview_permalink
But it makes get_posts call which costs a lot especially in WordPress with many posts:

https://github.com/ampproject/amp-wp/blob/746834718109459d06f1510f24d6d83aa4e0e83b/includes/admin/functions.php#L70-L81

As a result, users face slow page loads on every admin page.
In my case, the query costs almost 3 seconds.

Expected Behaviour

1. Customizer link should be always same(e.g. https://example.com/wp-admin/customizer.php)
2. In customizer page of amp, default permalink which amp_admin_get_preview_permalink expected to retrieve will be applied.

Steps to reproduce

1. Install WordPress and put 10,000 posts by 100 users in 3 custom post types.
2. Install and activate AMP & Query Monitor.
3. Open admin page and confirm slow query runs at every page.

Screenshots

Additional context

• WordPress version: 5.2.4
• Plugin version: 1.4.0
• Gutenberg plugin version (if applicable):
• AMP plugin template mode:
• PHP version: 7.2
• OS: Amazon Linux
• Browser: Chrome
• Device: Macbook Pro

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

Implementation brief

QA testing instructions

• Set AMP website mode to 'Reader' (so that the AMP Customizer menu item is shown):

  

• Clicking on it should send you to the homepage, and not the first AMP compatible post

Demo

Changelog entry

[pierlon]

This happens when you navigate to a non-AMP page while in the Customizer and with Reader mode active. I don't think showing the URL to the first AMP compatible post is necessary anymore, and I recommend the notification being removed. Thoughts @westonruter, @schlessera, @kienstra?

Here's a screenshot of the panel notification for a quick revision:

[westonruter]

The problem here I understand is the AMP Customizer link which appears when Reader mode is active:

This has a link

https://example.com/wp-admin/customize.php?autofocus[panel]=amp_panel&url=https%3A%2F%2Fexample.com%2F2019%2F11%2F10%2Fthe-most-recent-post%2Famp%2F&return=https%3A%2F%2Fexample.com%2Fwp-admin%2F

Notice it is deep-linking to the most recently-published post and expanding the AMP panel by default.

@pierlon Are you saying simply that we should remove the url argument here, like so:

--- a/includes/admin/functions.php
+++ b/includes/admin/functions.php
@@ -101,7 +101,6 @@ function amp_add_customizer_link() {
    $menu_slug = add_query_arg(
        [
            'autofocus[panel]' => AMP_Template_Customizer::PANEL_ID,
-           'url'              => rawurlencode( amp_admin_get_preview_permalink() ),
            'return'           => rawurlencode( admin_url() ),
        ],
        'customize.php'

The result is that the Customizer would default to the homepage, and if AMP is not enabled for pages and enabled for the homepage, then the user would see the notice as you showed above (https://github.com/ampproject/amp-wp/issues/3684#issuecomment-552131886).

So you're saying this: remove url argument and leave the notice to direct the user to a location where they can see the changes?

This seems fine to me.

[pierlon]

Yes, that would solve the reported problem, but I still cannot find a reason for having a link to navigate to the first AMP compatible post, especially since it has such a negative impact on performance.

A notice simply asking the user to navigate to an AMP compatible page would be OK for me.

[westonruter]

@fumikito Please test: https://github.com/ampproject/amp-wp/releases/tag/1.4.1-RC1

[csossi]

Verified in QA