Skip to content

Fix selection of Reader theme causing validated URLs to always be stale #6779

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

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Dec 9, 2021

Summary

I've selected Reader mode and chose a Reader theme:

image

Upon doing a Site Scan, the stale message goes away. But after I reload, the stale message appears:

image

Going to the AMP Validated URLs screen shows that all the just-scanned URLs are stale:

image

My active theme is twentyseventeen and my Reader theme is twentythirteen, and I can see that AMP_Validated_URL_Post_Type::get_post_staleness() is returning unexpectedly:

{
    "theme": {
        "new": [
            "twentyseventeen"
        ],
        "old": [
            "twentythirteen"
        ]
    }
}

Clearly \AMP_Validated_URL_Post_Type::get_validated_environment() is not taking Reader themes into account. This fixes that.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added Bug Something isn't working RME Reader Mode Expansion labels Dec 9, 2021
@westonruter westonruter added this to the v2.2 milestone Dec 9, 2021
@westonruter westonruter force-pushed the fix/reader-theme-validated-env branch from f758995 to 42eea65 Compare December 9, 2021 06:48
@westonruter westonruter force-pushed the fix/reader-theme-validated-env branch from 42eea65 to d375b93 Compare December 9, 2021 06:53
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2021

Plugin builds for d375b93 are ready 🛎️!

Copy link
Collaborator

@dhaval-parekh dhaval-parekh left a comment

Choose a reason for hiding this comment

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

Changes look good to me. ✅

QA Passed
It no longer shows stale results after reloading the page after site scanning when the reader theme is selected.

@delawski
Copy link
Collaborator

delawski commented Dec 9, 2021

Works as expected for me, too. After doing a Site Scan in the Reader mode and then reloading the page, I get the "No changes since your last scan" message which is correct:

Screenshot 2021-12-09 at 15 47 57

@westonruter westonruter merged commit 5ab4412 into develop Dec 9, 2021
@westonruter westonruter deleted the fix/reader-theme-validated-env branch December 9, 2021 16:32
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. RME Reader Mode Expansion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants