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

Fix detection for whether AMP is available to allow new accepted errors #1485

Merged
merged 1 commit into from Oct 3, 2018

Conversation

Projects
None yet
2 participants
@westonruter
Copy link
Member

westonruter commented Oct 3, 2018

In paired mode, I noticed when there were new-accepted validation errors (but no rejected ones) there was no amphtml link provided, with instead a comment:

<!--
There are 19 validation errors that are blocking the amphtml version from being available.
-->

Likewise, with auto-accepting sanitization disabled, I would also see the admin bar as indicating AMP is not available when it actually is:

image

This PR fixes those issues by ensuring that ack-accepted and new-accepted aren't being accounted for together, including elimination of obsolete is_sanitization_auto_accepted checks.

This is a regression from #1429.

Fix detection for whether AMP is available to allow new accepted errors
Update other instances where ack-accepted and new-accepted aren't being accounted for together, including elimination of obsolete is_sanitization_auto_accepted checks

@westonruter westonruter requested a review from kienstra Oct 3, 2018

@westonruter westonruter added this to the v1.0 milestone Oct 3, 2018

@kienstra

This comment has been minimized.

Copy link
Collaborator

kienstra commented Oct 3, 2018

Approved

Hi @westonruter,
This looks good, and I also see that this fixes the 2 cases you mentioned.

Before this PR, in Paired mode with 1 'New Accepted' error:
link-rel-missing

With this PR:
link-rel-present

@westonruter westonruter merged commit c72452a into 1.0 Oct 3, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the fix/amp-availability-check branch Oct 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.