Skip to content

Commit

Permalink
AN-183 Hotfix—autopublishing not working on some platforms (#678)
Browse files Browse the repository at this point in the history 
* Moves all publishing back to the `save_post` hook rather than having a separate workflow for Gutenberg.
* Introduces a checksum system to prevent double publishing of the same content as a more robust check than date in `is_post_in_sync`.
* Adds a test for the behavior of `is_post_in_sync`.
  • Loading branch information
@kevinfodness
kevinfodness authored Sep 30, 2019
1 parent 57a524f commit 43df1c4
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 63 deletions.
166 changes: 116 additions & 50 deletions admin/apple-actions/index/class-push.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@
*/
class Push extends API_Action {

/**
* Checksum for current article being exported.
*
* @var string
* @access private
*/
private $checksum;

/**
* Current content ID being exported.
*
Expand Down Expand Up @@ -85,27 +93,82 @@ public function perform( $doing_async = false, $user_id = null ) {
}
}

/**
* Generate a checksum against the article JSON with certain fields ignored.
*
* @param string $json The JSON to turn into a checksum.
* @param array $meta Optional. Metadata for the article. Defaults to empty array.
* @param array $bundles Optional. Any bundles that will be sent with the article. Defaults to empty array.
* @param bool $force Optional. Allows bypass of local cache for checksum.
*
* @return string The checksum for the JSON.
*/
private function generate_checksum( $json, $meta = [], $bundles = [], $force = false ) {
// Use cached checksum, if it exists, and if force is false.
if ( ! $force && ! empty( $this->checksum ) ) {
return $this->checksum;
}

// Try to decode the JSON object.
$json = json_decode( $json, true );
if ( empty( $json ) ) {
return '';
}

// Remove any fields from JSON that might change but not affect the article itself, like dates and plugin version.
unset( $json['metadata']['dateCreated'] );
unset( $json['metadata']['dateModified'] );
unset( $json['metadata']['datePublished'] );
unset( $json['metadata']['generatorVersion'] );

// Add meta and bundles so we can checksum the whole thing.
$json['checksum']['meta'] = $meta;
$json['checksum']['bundles'] = $bundles;

// Calculate the checksum as a hex value and cache it.
$this->checksum = dechex( absint( crc32( wp_json_encode( $json ) ) ) );

return $this->checksum;
}

/**
* Check if the post is in sync before updating in Apple News.
*
* @access private
* @param string $json The JSON for this article to check if it is in sync.
* @param array $meta Optional. Metadata for the article. Defaults to empty array.
* @param array $bundles Optional. Any bundles that will be sent with the article. Defaults to empty array.
* @return boolean
* @throws \Apple_Actions\Action_Exception If the post could not be found.
*/
private function is_post_in_sync() {
$post = get_post( $this->id );
private function is_post_in_sync( $json, $meta = [], $bundles = [] ) {
$in_sync = true;

// Ensure the post (still) exists. Async operations might result in this function being run against a non-existent post.
$post = get_post( $this->id );
if ( ! $post ) {
throw new \Apple_Actions\Action_Exception( __( 'Could not find post with id ', 'apple-news' ) . $this->id );
}

$api_time = get_post_meta( $this->id, 'apple_news_api_modified_at', true );
$api_time = strtotime( get_date_from_gmt( date( 'Y-m-d H:i:s', strtotime( $api_time ) ) ) );
$local_time = strtotime( $post->post_modified );

$in_sync = $api_time >= $local_time;
// Compare checksums to determine whether the article is in sync or not.
$current_checksum = get_post_meta( $this->id, 'apple_news_article_checksum', true );
$new_checksum = $this->generate_checksum( $json, $meta, $bundles );
if ( empty( $current_checksum ) || $current_checksum !== $new_checksum ) {
$in_sync = false;
}

return apply_filters( 'apple_news_is_post_in_sync', $in_sync, $this->id, $api_time, $local_time );
/**
* Allows for custom logic to determine if a post is in sync or not.
*
* @since 2.0.2 Added the $post_id, $json, $meta, and $bundles parameters.
*
* @param bool $in_sync Whether the current post is in sync or not.
* @param int $post_id The ID of the post being checked.
* @param string $json The JSON for the current article.
* @param array $meta Metadata for the current article.
* @param array $bundles Any bundles that will be sent with the current article.
*/
return apply_filters( 'apple_news_is_post_in_sync', $in_sync, $this->id, $json, $meta, $bundles );
}

/**
Expand Down Expand Up @@ -159,17 +222,6 @@ private function push( $user_id = null ) {
);
}

// Ignore if the post is already in sync.
if ( $this->is_post_in_sync() ) {
throw new \Apple_Actions\Action_Exception(
sprintf(
// Translators: Placeholder is a post ID.
__( 'Skipped push of article %d because it is already in sync.', 'apple-news' ),
$this->id
)
);
}

/**
* The generate_article function uses Exporter->generate, so we MUST
* clean the workspace before and after its usage.
Expand All @@ -195,46 +247,57 @@ private function push( $user_id = null ) {
$bundles = array();
}

try {
// If there's an API ID, update, otherwise create.
$remote_id = get_post_meta( $this->id, 'apple_news_api_id', true );
$result = null;
// If there's an API ID, update, otherwise create.
$remote_id = get_post_meta( $this->id, 'apple_news_api_id', true );
$result = null;

do_action( 'apple_news_before_push', $this->id );
do_action( 'apple_news_before_push', $this->id );

// Populate optional metadata.
$meta = array(
'data' => array(),
);
// Populate optional metadata.
$meta = array(
'data' => array(),
);

// Set sections.
if ( ! empty( $this->sections ) ) {
sort( $this->sections );
$meta['data']['links'] = array( 'sections' => $this->sections );
}
// Set sections.
if ( ! empty( $this->sections ) ) {
sort( $this->sections );
$meta['data']['links'] = array( 'sections' => $this->sections );
}

// Get the isPreview setting.
$is_paid = (bool) get_post_meta( $this->id, 'apple_news_is_paid', true );
$meta['data']['isPaid'] = $is_paid;
// Get the isPreview setting.
$is_paid = (bool) get_post_meta( $this->id, 'apple_news_is_paid', true );
$meta['data']['isPaid'] = $is_paid;

// Get the isPreview setting.
$is_preview = (bool) get_post_meta( $this->id, 'apple_news_is_preview', true );
$meta['data']['isPreview'] = $is_preview;
// Get the isPreview setting.
$is_preview = (bool) get_post_meta( $this->id, 'apple_news_is_preview', true );
$meta['data']['isPreview'] = $is_preview;

// Get the isHidden setting.
$is_hidden = (bool) get_post_meta( $this->id, 'apple_news_is_hidden', true );
$meta['data']['isHidden'] = $is_hidden;
// Get the isHidden setting.
$is_hidden = (bool) get_post_meta( $this->id, 'apple_news_is_hidden', true );
$meta['data']['isHidden'] = $is_hidden;

// Get the isSponsored setting.
$is_sponsored = (bool) get_post_meta( $this->id, 'apple_news_is_sponsored', true );
$meta['data']['isSponsored'] = $is_sponsored;
// Get the isSponsored setting.
$is_sponsored = (bool) get_post_meta( $this->id, 'apple_news_is_sponsored', true );
$meta['data']['isSponsored'] = $is_sponsored;

// Get the maturity rating setting.
$maturity_rating = get_post_meta( $this->id, 'apple_news_maturity_rating', true );
if ( ! empty( $maturity_rating ) ) {
$meta['data']['maturityRating'] = $maturity_rating;
}
// Get the maturity rating setting.
$maturity_rating = get_post_meta( $this->id, 'apple_news_maturity_rating', true );
if ( ! empty( $maturity_rating ) ) {
$meta['data']['maturityRating'] = $maturity_rating;
}

// Ignore if the post is already in sync.
if ( $this->is_post_in_sync( $json, $meta, $bundles ) ) {
throw new \Apple_Actions\Action_Exception(
sprintf(
// Translators: Placeholder is a post ID.
__( 'Skipped push of article %d because it is already in sync.', 'apple-news' ),
$this->id
)
);
}

try {
if ( $remote_id ) {
// Update the current article from the API in case the revision changed.
$this->get();
Expand Down Expand Up @@ -265,6 +328,9 @@ private function push( $user_id = null ) {
// Clear the cache for post status.
delete_transient( 'apple_news_post_state_' . $this->id );

// Update the checksum for the article JSON version.
update_post_meta( $this->id, 'apple_news_article_checksum', $this->generate_checksum( $json, $meta, $bundles ) );

do_action( 'apple_news_after_push', $this->id, $result );
} catch ( \Apple_Push_API\Request\Request_Exception $e ) {

Expand Down
12 changes: 0 additions & 12 deletions admin/class-admin-apple-post-sync.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public function __construct( $settings = null ) {
|| 'yes' === $this->settings->get( 'api_autosync_update' )
) {
// This needs to happen after meta boxes save.
add_action( 'rest_after_insert_post', [ $this, 'do_publish_from_rest' ] );
add_action( 'save_post', [ $this, 'do_publish' ], 99, 2 );
}

Expand All @@ -58,16 +57,6 @@ public function __construct( $settings = null ) {
}
}

/**
* An action callback for rest_after_insert_post. Handles the publish action.
*
* @since 2.0.0
* @param WP_Post $post The post object to publish.
*/
public function do_publish_from_rest( $post ) {
$this->do_publish( $post->ID, $post );
}

/**
* When a post is published, or a published post updated, trigger this function.
*
Expand All @@ -81,7 +70,6 @@ public function do_publish( $id, $post ) {
|| ! in_array( $post->post_type, $this->settings->post_types, true )
|| ( ! current_user_can( apply_filters( 'apple_news_publish_capability', Apple_News::get_capability_for_post_type( 'publish_posts', $post->post_type ) ) )
&& ! ( defined( 'DOING_CRON' ) && DOING_CRON ) )
|| ( function_exists( 'has_blocks' ) && has_blocks( $post ) )
) {
return;
}
Expand Down
65 changes: 64 additions & 1 deletion tests/admin/apple-actions/index/test-class-push.php
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

use \Apple_Actions\Action_Exception;
use \Apple_Actions\Index\Push as Push;
use \Apple_Actions\Action_Exception as Action_Exception;
use \Apple_Exporter\Settings as Settings;
use \Prophecy\Argument as Argument;

Expand Down Expand Up @@ -567,4 +567,67 @@ public function testJSONErrorsFail() {
array( $this, 'filterAppleNewsGetErrors' )
);
}

/**
* Tests the behavior of the is_post_in_sync function to ensure that
* posts are only sync'd when necessary.
*/
public function test_is_post_in_sync() {
// Mock the API.
$response = $this->dummy_response();
$api = $this->prophet->prophesize( '\Apple_Push_API\API' );
$api->post_article_to_channel( Argument::cetera() )
->willReturn( $response )
->shouldBeCalled();

// Create a test post.
$post_id = $this->factory->post->create();

// Test the initial push.
$action = new Push( $this->settings, $post_id );
$action->set_api( $api->reveal() );
$action->perform();

// Ensure that the push completed and the data was saved.
$this->assertEquals( $response->data->id, get_post_meta( $post_id, 'apple_news_api_id', true ) );
$this->assertEquals( $response->data->createdAt, get_post_meta( $post_id, 'apple_news_api_created_at', true ) );
$this->assertEquals( $response->data->modifiedAt, get_post_meta( $post_id, 'apple_news_api_modified_at', true ) );
$this->assertEquals( $response->data->shareUrl, get_post_meta( $post_id, 'apple_news_api_share_url', true ) );
$this->assertEquals( null, get_post_meta( $post_id, 'apple_news_api_deleted', true ) );
$this->assertNotEmpty( get_post_meta( $post_id, 'apple_news_article_checksum', true ) );

// Run the push again, and this time it should bail out because it is in sync.
$action = new Push( $this->settings, $post_id );
$action->set_api( $api->reveal() );
try {
$action->perform();
} catch ( Action_Exception $e ) {
$this->assertEquals(
sprintf( 'Skipped push of article %d because it is already in sync.', $post_id ),
$e->getMessage()
);
}

// Mock the response for updating the article.
$api = $this->prophet->prophesize( '\Apple_Push_API\API' );
$api->get_article( Argument::cetera() )
->willReturn( $response )
->shouldBeCalled();
$api->update_article( Argument::cetera() )
->willReturn( $response )
->shouldBeCalled();

// Update the post and ensure it posts and updates the checksum.
$previous_checksum = get_post_meta( $post_id, 'apple_news_article_checksum', true );
$post = get_post( $post_id );
$post->post_title = 'Updated post title.';
wp_update_post( $post );
$action = new Push( $this->settings, $post_id );
$action->set_api( $api->reveal() );
$action->perform();
$new_checksum = get_post_meta( $post_id, 'apple_news_article_checksum', true );
$this->assertNotEmpty( $previous_checksum );
$this->assertNotEmpty( $new_checksum );
$this->assertNotEquals( $previous_checksum, $new_checksum );
}
}

0 comments on commit 43df1c4

Please sign in to comment.