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

Allow bulk article deletions from Apple News list table #1207

Merged
merged 12 commits into from
Dec 4, 2024
Merged

Allow bulk article deletions from Apple News list table #1207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a4d4c93
Remove now-redundant filter
dlh01 Nov 30, 2024
8e278de
Use more-standard escaping and encoding
dlh01 Nov 30, 2024
96e1e6a
De-duplicate selector
dlh01 Nov 30, 2024
2daa84a
Delete checksum when deleting article
dlh01 Nov 30, 2024
958b09d
Correct check for successful DELETE response
dlh01 Nov 30, 2024
6e69aa5
Set up delete bulk action
dlh01 Nov 30, 2024
7265549
Allow more time for DELETE requests
dlh01 Nov 30, 2024
6b0d022
Set page text depending on the action
dlh01 Dec 2, 2024
640f0d6
Disable submit button after completion
dlh01 Dec 2, 2024
e701463
Restore PHPCS ignores
dlh01 Dec 2, 2024
83b8bec
Spacin'
dlh01 Dec 3, 2024
0ab4d9c
Undo button text change
dlh01 Dec 4, 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
1 change: 1 addition & 0 deletions admin/apple-actions/class-api-action.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,6 @@ protected function delete_post_meta( $post_id ): void {
delete_post_meta( $post_id, 'apple_news_api_created_at' );
delete_post_meta( $post_id, 'apple_news_api_modified_at' );
delete_post_meta( $post_id, 'apple_news_api_share_url' );
delete_post_meta( $post_id, 'apple_news_article_checksum' );
}
}
151 changes: 88 additions & 63 deletions admin/class-admin-apple-bulk-export-page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public function __construct( $settings ) {

add_action( 'admin_menu', [ $this, 'register_page' ] );
add_action( 'admin_enqueue_scripts', [ $this, 'register_assets' ] );
add_action( 'wp_ajax_push_post', [ $this, 'ajax_push_post' ] );
add_filter( 'admin_title', [ $this, 'set_title' ] );
add_action( 'wp_ajax_apple_news_push_post', [ $this, 'ajax_push_post' ] );
add_action( 'wp_ajax_apple_news_delete_post', [ $this, 'ajax_delete_post' ] );
}

/**
Expand All @@ -66,45 +66,52 @@ public function register_page() {
);
}

/**
* Fix the title since WordPress doesn't set one.
*
* @param string $admin_title The title to be filtered.
* @access public
* @return string The title for the screen.
*/
public function set_title( $admin_title ) {
$screen = get_current_screen();
if ( 'admin_page_apple_news_bulk_export' === $screen->base ) {
$admin_title = __( 'Bulk Export', 'apple-news' ) . $admin_title;
}

return $admin_title;
}

/**
* Builds the plugin submenu page.
*
* @access public
*/
public function build_page() {
$ids = isset( $_GET['ids'] ) ? sanitize_text_field( wp_unslash( $_GET['ids'] ) ) : null; // phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected, WordPress.Security.NonceVerification.Recommended
if ( ! $ids ) {
$post_ids = isset( $_GET['post_ids'] ) ? sanitize_text_field( wp_unslash( $_GET['post_ids'] ) ) : null; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$action = isset( $_GET['action'] ) ? sanitize_text_field( wp_unslash( $_GET['action'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended

if ( ! $post_ids ) {
wp_safe_redirect( esc_url_raw( menu_page_url( $this->plugin_slug . '_index', false ) ) ); // phpcs:ignore WordPressVIPMinimum.Security.ExitAfterRedirect.NoExit

if ( ! defined( 'APPLE_NEWS_UNIT_TESTS' ) || ! APPLE_NEWS_UNIT_TESTS ) {
exit;
}
}

// Populate $articles array with a set of valid posts.
$articles = [];
foreach ( explode( '.', $ids ) as $id ) {
$post = get_post( absint( $id ) );
// Allow only specific actions.
if ( ! in_array( $action, [ 'apple_news_push_post', 'apple_news_delete_post' ], true ) ) {
wp_die( esc_html__( 'Invalid action.', 'apple-news' ), '', [ 'response' => 400 ] );
}

// Populate articles array with a set of valid posts.
$apple_posts = [];
foreach ( explode( ',', $post_ids ) as $post_id ) {
$post = get_post( (int) $post_id );

if ( ! empty( $post ) ) {
$articles[] = $post;
$apple_posts[] = $post;
}
}

// Override text within the partial depending on the action.
$apple_page_title = match ( $action ) {
'apple_news_push_post' => __( 'Bulk Export Articles', 'apple-news' ),
'apple_news_delete_post' => __( 'Bulk Delete Articles', 'apple-news' ),
};
$apple_page_description = match ( $action ) {
'apple_news_push_post' => __( 'The following articles will be exported.', 'apple-news' ),
'apple_news_delete_post' => __( 'The following articles will be deleted.', 'apple-news' ),
};
$apple_submit_text = match ( $action ) {
'apple_news_push_post' => __( 'Publish All', 'apple-news' ),
'apple_news_delete_post' => __( 'Delete All', 'apple-news' ),
};

require_once __DIR__ . '/partials/page-bulk-export.php';
}

Expand All @@ -123,41 +130,25 @@ public function ajax_push_post() {
// Ensure the post exists and that it's published.
$post = get_post( $id );
if ( empty( $post ) ) {
echo wp_json_encode(
[
'success' => false,
'error' => __( 'This post no longer exists.', 'apple-news' ),
]
);
wp_die();
wp_send_json_error( __( 'This post no longer exists.', 'apple-news' ) );
}

// Check capabilities.
if ( ! current_user_can(
/** This filter is documented in admin/class-admin-apple-post-sync.php */
apply_filters( 'apple_news_publish_capability', self::get_capability_for_post_type( 'publish_posts', $post->post_type ) )
) ) {
echo wp_json_encode(
[
'success' => false,
'error' => __( 'You do not have permission to publish to Apple News', 'apple-news' ),
]
);
wp_die();
wp_send_json_error( __( 'You do not have permission to publish to Apple News', 'apple-news' ) );
}

if ( 'publish' !== $post->post_status ) {
echo wp_json_encode(
[
'success' => false,
'error' => sprintf(
// translators: token is a post ID.
__( 'Article %s is not published and cannot be pushed to Apple News.', 'apple-news' ),
$id
),
]
wp_send_json_error(
sprintf(
/* translators: %s: post ID */
__( 'Article %s is not published and cannot be pushed to Apple News.', 'apple-news' ),
$id
)
);
wp_die();
}

$action = new Apple_Actions\Index\Push( $this->settings, $id );
Expand All @@ -168,22 +159,56 @@ public function ajax_push_post() {
}

if ( $errors ) {
echo wp_json_encode(
[
'success' => false,
'error' => $errors,
]
);
} else {
echo wp_json_encode(
[
'success' => true,
]
);
wp_send_json_error( $errors );
}

wp_send_json_success();
}

/**
* Handles the ajax action to delete a post from Apple News.
*
* @access public
*/
public function ajax_delete_post() {
// Check the nonce.
check_ajax_referer( self::ACTION );

// Sanitize input data.
$id = isset( $_GET['id'] ) ? (int) $_GET['id'] : -1;

$post = get_post( $id );

if ( empty( $post ) ) {
wp_send_json_error( __( 'This post no longer exists.', 'apple-news' ) );
}

/** This filter is documented in admin/class-admin-apple-post-sync.php */
$cap = apply_filters( 'apple_news_delete_capability', self::get_capability_for_post_type( 'delete_posts', $post->post_type ) );

// Check capabilities.
if ( ! current_user_can( $cap ) ) {
wp_send_json_error( __( 'You do not have permission to delete posts from Apple News', 'apple-news' ) );
}

$errors = null;

// Try to sync only if the post has a remote ID. Ref `Admin_Apple_Post_Sync::do_delete()`.
if ( get_post_meta( $id, 'apple_news_api_id', true ) ) {
$action = new Apple_Actions\Index\Delete( $this->settings, $id );

try {
$errors = $action->perform();
} catch ( Apple_Actions\Action_Exception $e ) {
$errors = $e->getMessage();
}
}

if ( $errors ) {
wp_send_json_error( $errors );
}

// This is required to terminate immediately and return a valid response.
wp_die();
wp_send_json_success();
}

/**
Expand Down
54 changes: 44 additions & 10 deletions admin/class-admin-apple-index-page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function admin_page() {
*
* @since 0.4.0
* @access public
* @return mixed The result of the requested action.
* @return mixed|void The result of the requested action.
*/
public function page_router() {
$id = isset( $_GET['post_id'] ) ? absint( $_GET['post_id'] ) : null; // phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected, WordPress.Security.NonceVerification.Recommended
Expand All @@ -130,22 +130,56 @@ public function page_router() {
return $this->export_action( $id );
case self::namespace_action( 'reset' ):
return $this->reset_action( $id );
case self::namespace_action( 'push' ): // phpcs:ignore PSR2.ControlStructures.SwitchDeclaration.TerminatingComment
if ( ! $id ) {
case self::namespace_action( 'push' ):
if ( $id ) {
$this->push_action( $id );
} else {
$url = menu_page_url( $this->plugin_slug . '_bulk_export', false );
if ( isset( $_GET['article'] ) ) { // phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected, WordPress.Security.NonceVerification.Recommended
$ids = is_array( $_GET['article'] ) ? array_map( 'absint', $_GET['article'] ) : absint( $_GET['article'] ); // phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected, WordPress.Security.NonceVerification.Recommended
$url .= '&ids=' . implode( '.', $ids );

if ( isset( $_GET['article'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$post_ids = is_array( $_GET['article'] ) ? array_map( 'intval', $_GET['article'] ) : (int) $_GET['article']; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$url = add_query_arg(
[
'action' => 'apple_news_push_post',
'post_ids' => implode( ',', $post_ids ),
],
$url,
);
}

wp_safe_redirect( esc_url_raw( $url ) ); // phpcs:ignore WordPressVIPMinimum.Security.ExitAfterRedirect.NoExit

if ( ! defined( 'APPLE_NEWS_UNIT_TESTS' ) || ! APPLE_NEWS_UNIT_TESTS ) {
exit;
}
} else {
return $this->push_action( $id );
}

break;
case self::namespace_action( 'delete' ):
return $this->delete_action( $id );
if ( $id ) {
$this->delete_action( $id );
} else {
$url = menu_page_url( $this->plugin_slug . '_bulk_export', false );

if ( isset( $_GET['article'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$post_ids = is_array( $_GET['article'] ) ? array_map( 'intval', $_GET['article'] ) : (int) $_GET['article']; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$url = add_query_arg(
[
'action' => 'apple_news_delete_post',
'post_ids' => implode( ',', $post_ids ),
],
$url,
);
}

wp_safe_redirect( esc_url_raw( $url ) ); // phpcs:ignore WordPressVIPMinimum.Security.ExitAfterRedirect.NoExit

if ( ! defined( 'APPLE_NEWS_UNIT_TESTS' ) || ! APPLE_NEWS_UNIT_TESTS ) {
exit;
}
}

break;
}
}

Expand Down Expand Up @@ -401,7 +435,7 @@ private function delete_action( $id ) {
$action = new Apple_Actions\Index\Delete( $this->settings, $id );
try {
$action->perform();
$this->notice_success( __( 'Your article has been removed from apple news.', 'apple-news' ) );
$this->notice_success( __( 'Your article has been removed from Apple News.', 'apple-news' ) );
} catch ( Apple_Actions\Action_Exception $e ) {
$this->notice_error( $e->getMessage() );
}
Expand Down
5 changes: 3 additions & 2 deletions admin/class-admin-apple-news-list-table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,9 @@ public function get_bulk_actions() {
return apply_filters(
'apple_news_bulk_actions',
[
Admin_Apple_Index_Page::namespace_action( 'push' ) => __( 'Publish', 'apple-news' ),
]
Admin_Apple_Index_Page::namespace_action( 'push' ) => __( 'Publish', 'apple-news' ),
Admin_Apple_Index_Page::namespace_action( 'delete' ) => __( 'Delete', 'apple-news' ),
],
);
}

Expand Down
19 changes: 11 additions & 8 deletions admin/partials/page-bulk-export.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,28 @@
/**
* Publish to Apple News partials: Bulk Export page template
*
* phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
*
* @global array $articles
*
* @package Apple_News
*/

// Expect these variables to be defined in Admin_Apple_Bulk_Export_Page::build_page() but make sure they're set.
$apple_page_title ??= __( 'Bulk Export', 'apple-news' );
$apple_page_description ??= __( 'The following articles will be affected.', 'apple-news' );
$apple_posts ??= [];
$apple_submit_text ??= __( 'Go', 'apple-news' );
Comment on lines +10 to +12
Copy link

Choose a reason for hiding this comment

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

the old hard-coded text tells me that "Bulk Export" will publish the posts to Apple News. This new text does not tell me that. If I saw "Bulk Export", I would expect it to download a file vs publishing. Maybe "Bulk Export" needs to be "Bulk Publish"?

Copy link
Member Author

Choose a reason for hiding this comment

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

The current language was retained and is supposed to be applied here:

apple-news/admin/class-admin-apple-bulk-export-page.php

Lines 86 to 115 in 83b8bec

// Allow only specific actions.
if ( ! in_array( $action, [ 'apple_news_push_post', 'apple_news_delete_post' ], true ) ) {
wp_die( esc_html__( 'Invalid action.', 'apple-news' ), '', [ 'response' => 400 ] );
}
// Populate articles array with a set of valid posts.
$apple_posts = [];
foreach ( explode( ',', $post_ids ) as $post_id ) {
$post = get_post( (int) $post_id );
if ( ! empty( $post ) ) {
$apple_posts[] = $post;
}
}
// Override text within the partial depending on the action.
$apple_page_title = match ( $action ) {
'apple_news_push_post' => __( 'Bulk Export Articles', 'apple-news' ),
'apple_news_delete_post' => __( 'Bulk Delete Articles', 'apple-news' ),
};
$apple_page_description = match ( $action ) {
'apple_news_push_post' => __( 'The following articles will be exported.', 'apple-news' ),
'apple_news_delete_post' => __( 'The following articles will be deleted.', 'apple-news' ),
};
$apple_submit_text = match ( $action ) {
'apple_news_push_post' => __( 'Export', 'apple-news' ),
'apple_news_delete_post' => __( 'Delete', 'apple-news' ),
};
require_once __DIR__ . '/partials/page-bulk-export.php';

With the changes in this PR, this partial should only ever be rendered when a known bulk action is being invoked, and the text should reflect that action (which happens in the lines linked above).

There isn't any circumstance where the default strings seen here should actually be rendered, but I included them so that there would be something on the page if it ever happened. You're right that the strings are vague, but how the page would behave at that point is also not known, so I wasn't sure what else to do.

Copy link

Choose a reason for hiding this comment

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

cool, I was kind of picking that up from the code. this makes sense. I still wonder about using Export vs Upload or Publish, but maybe that's a question for project maintainers or stakeholders. The button text was Publish All and I don't see that it is retained, but maybe I'm missing it. Anyway, I'm good with whatever, so feel free to proceed as you see fit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that the button text wasn't actually kept! Whoops. Fixed in 0ab4d9c.

Re. the word Export, I agree that it isn't as clear as it could be. I'm reluctant to change it here, but, as it happens, @scottnelle has been working on refining that language in #1204 (see also https://l.alley.dev/a3a948a289), so maybe we could expand that PR to include language for this page that will be consistent with the change being made on the list table.


?>
<div class="wrap">
<h1><?php esc_html_e( 'Bulk Export Articles', 'apple-news' ); ?></h1>
<p><?php esc_html_e( "The following articles will be published to Apple News. Once started, it might take a while, please don't close the browser window.", 'apple-news' ); ?></p>
<h1><?php echo esc_html( $apple_page_title ); ?></h1>
<p><?php echo esc_html( $apple_page_description ); ?></p>
<p><?php esc_html_e( "Once started, it might take a while. Please don't close the browser window.", 'apple-news' ); ?></p>
<?php
/**
* Allows for custom HTML to be printed before the bulk export table.
*/
do_action( 'apple_news_before_bulk_export_table' );
?>
<ul class="bulk-export-list" data-nonce="<?php echo esc_attr( wp_create_nonce( Admin_Apple_Bulk_Export_Page::ACTION ) ); ?>">
<?php foreach ( $articles as $apple_post ) : ?>
<?php foreach ( $apple_posts as $apple_post ) : ?>
<li class="bulk-export-list-item" data-post-id="<?php echo esc_attr( $apple_post->ID ); ?>">
<span class="bulk-export-list-item-title">
<?php echo esc_html( $apple_post->post_title ); ?>
Expand All @@ -39,5 +42,5 @@
?>

<a class="button" href="<?php echo esc_url( menu_page_url( $this->plugin_slug . '_index', false ) ); ?>"><?php esc_html_e( 'Back', 'apple-news' ); ?></a>
<a class="button button-primary bulk-export-submit" href="#"><?php esc_html_e( 'Publish All', 'apple-news' ); ?></a>
<a class="button button-primary bulk-export-submit" href="#"><?php echo esc_html( $apple_submit_text ); ?></a>
</div>
Loading
Loading