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.

Sign up for GitHub

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

Issue-1154: A different approach to reindex the deleted article #1177

Merged
merged 2 commits into from
Oct 14, 2024
+57 −81
Merged

Issue-1154: A different approach to reindex the deleted article #1177

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2028130
A different approach to reindex the deleted article
renatonascalves Oct 11, 2024
24c23d6
Resolving merge conflict
renatonascalves Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions admin/apple-actions/index/class-get.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,7 @@ public function perform() {
}

// Get the article from the API.
try {
Copy link
Author

Choose a reason for hiding this comment

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

I removed this so that we keep the data removal intentional. Since GET requests are performed in more unrelated places than I anticipated.

Copy link

Choose a reason for hiding this comment

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

interesting here

$article = $this->get_api()->get_article( $apple_id );
} catch ( \Apple_Push_API\Request\Request_Exception $e ) {
$article = $e->getMessage();

// Reset the API postmeta if the article is deleted in Apple News.
if ( is_string( $article ) && str_contains( $article, 'NOT_FOUND (keyPath articleId)' ) ) {
$this->delete_post_meta( $this->id );
}
}
$article = $this->get_api()->get_article( $apple_id );

if ( empty( $article->data ) ) {
return null;
Expand Down
64 changes: 44 additions & 20 deletions admin/apple-actions/index/class-push.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
use Admin_Apple_Async;
use Admin_Apple_Notice;
use Admin_Apple_Sections;
use Apple_Actions\Action_Exception;
use Apple_Actions\API_Action;
use Apple_Exporter\Exporter;
use Apple_Exporter\Settings;
use Apple_Actions\API_Action;
use Apple_Actions\Action_Exception;
use Apple_Push_API\Request\Request_Exception;

/**
Expand Down Expand Up @@ -212,11 +212,12 @@ private function get(): void {
/**
* Push the post using the API data.
*
* @param int $user_id Optional. The ID of the user performing the push. Defaults to current user.
* @param int $user_id Optional. The ID of the user performing the push. Defaults to current user.
* @param bool $display_notices Optional. Whether to display notices. Defaults to true.
*
* @throws Action_Exception If unable to push.
*/
private function push( $user_id = null ): void {
private function push( $user_id = null, $display_notices = true ): void {
if ( ! $this->is_api_configuration_valid() ) {
throw new Action_Exception( esc_html__( 'Your Apple News API settings seem to be empty. Please fill in the API key, API secret and API channel fields in the plugin configuration page.', 'apple-news' ) );
}
Expand Down Expand Up @@ -387,6 +388,9 @@ private function push( $user_id = null ): void {
);
}

$original_error_message = null;
$error_message = null;

try {
if ( $remote_id ) {
// Update the current article from the API in case the revision changed.
Expand Down Expand Up @@ -450,32 +454,52 @@ private function push( $user_id = null ): void {
} else {
$error_message = __( 'There has been an error with the Apple News API: ', 'apple-news' ) . esc_html( $original_error_message );
}
} finally {
/**
* Reindex the article if it was deleted in the iCloud News Publisher dashboard.
*
* @see https://github.com/alleyinteractive/apple-news/issues/1154
*/
if ( $original_error_message && str_contains( $original_error_message, 'NOT_FOUND (keyPath articleId)' ) ) {
try {
self::push(
user_id: $user_id,
display_notices: false
);
} catch ( Action_Exception $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch
// Do nothing, even if the second push fails.
}
}

throw new Action_Exception( esc_html( $error_message ) );
if ( $error_message ) {
throw new Action_Exception( esc_html( $error_message ) );
}
}

// If we're not supposed to display notices, bail out.
if ( false === $display_notices ) {
return;
}

// Print success message.
$post = get_post( $this->id );

$success_message = sprintf(
// translators: token is the post title.
__( 'Article %s has been pushed successfully to Apple News!', 'apple-news' ),
$post->post_title
);

if ( $remote_id ) {
Admin_Apple_Notice::success(
sprintf(
$success_message = sprintf(
// translators: token is the post title.
__( 'Article %s has been successfully updated on Apple News!', 'apple-news' ),
$post->post_title
),
$user_id
);
} else {
Admin_Apple_Notice::success(
sprintf(
// translators: token is the post title.
__( 'Article %s has been pushed successfully to Apple News!', 'apple-news' ),
$post->post_title
),
$user_id
__( 'Article %s has been successfully updated on Apple News!', 'apple-news' ),
$post->post_title
);
}

Admin_Apple_Notice::success( $success_message, $user_id );

$this->clean_workspace();
}

Expand Down
39 changes: 0 additions & 39 deletions tests/admin/apple-actions/index/test-class-get.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,43 +57,4 @@ public function test_get_action(): void {
$this->assertSame( $response['data']['revision'], $data->data->revision );
$this->assertSame( $response['data']['type'], $data->data->type );
}

/**
* Test the behavior of the get action with a deleted Apple News article assigned to the post.
*/
public function test_get_deleted_article(): void {
$api_id = 'def456';
$post_id = self::factory()->post->create();
$action = new Apple_Actions\Index\Get( $this->settings, $post_id );

$this->assertNull( $action->perform() );

add_post_meta( $post_id, 'apple_news_api_id', $api_id );

// Fake the API response for the GET request.
$this->add_http_response(
verb: 'GET',
url: 'https://news-api.apple.com/articles/' . $api_id,
body: wp_json_encode(
[
'errors' => [
[
'code' => 'NOT_FOUND',
'keyPath' => [ 'articleId' ],
'value' => $api_id,
],
],
]
),
response: [
'code' => 404,
'message' => 'Not Found',
]
);

$action = new Apple_Actions\Index\Get( $this->settings, $post_id );

$this->assertNull( $action->perform() );
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_id', true ) );
}
}
13 changes: 6 additions & 7 deletions tests/rest/test-class-rest-post-published-state.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,12 @@ public function test_get_post_published_state_of_an_invalid_id_when_authenticate

$this->get( rest_url( '/apple-news/v1/get-published-state/' . $post_id ) )
->assertOk()
->assertJsonPath( 'publishState', 'N/A' );
->assertJsonPath( 'publishState', 'NOT_FOUND (keyPath articleId)' );

// Ensure that the API postmeta _was_ reset.
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_created_at', true ) );
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_id', true ) );
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_modified_at', true ) );
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_revision', true ) );
$this->assertEmpty( get_post_meta( $post_id, 'apple_news_api_share_url', true ) );
$this->assertEquals( 'abc123', get_post_meta( $post_id, 'apple_news_api_created_at', true ) );
$this->assertEquals( $api_id, get_post_meta( $post_id, 'apple_news_api_id', true ) );
$this->assertEquals( 'ghi789', get_post_meta( $post_id, 'apple_news_api_modified_at', true ) );
$this->assertEquals( 'jkl123', get_post_meta( $post_id, 'apple_news_api_revision', true ) );
$this->assertEquals( 'mno456', get_post_meta( $post_id, 'apple_news_api_share_url', true ) );
}
}