diff --git a/includes/classes/IndexHelper.php b/includes/classes/IndexHelper.php index d3b84aa742..9a59e4b7d9 100644 --- a/includes/classes/IndexHelper.php +++ b/includes/classes/IndexHelper.php @@ -465,15 +465,7 @@ protected function get_objects_to_index() { */ do_action( "ep_pre_{$this->args['method']}_index", $this->index_meta, ( $this->index_meta['start'] ? 'start' : false ), $indexable ); - /** - * Filter number of items to index per cycle in the dashboard - * - * @since 2.1 - * @hook ep_index_default_per_page - * @param {int} Entries per cycle - * @return {int} New number of entries - */ - $per_page = apply_filters( 'ep_index_default_per_page', Utils\get_option( 'ep_bulk_setting', 350 ) ); + $per_page = $this->get_index_default_per_page(); if ( ! empty( $this->args['per_page'] ) ) { $per_page = $this->args['per_page']; @@ -1219,6 +1211,24 @@ protected function on_error_update_and_clean( $error ) { ); } + /** + * Return the default number of documents to be sent to Elasticsearch on each batch. + * + * @since 4.4.0 + * @return integer + */ + public function get_index_default_per_page() : int { + /** + * Filter number of items to index per cycle in the dashboard + * + * @since 2.1 + * @hook ep_index_default_per_page + * @param {int} Entries per cycle + * @return {int} New number of entries + */ + return (int) apply_filters( 'ep_index_default_per_page', Utils\get_option( 'ep_bulk_setting', 350 ) ); + } + /** * Return singleton instance of class. * diff --git a/includes/classes/Indexable/Post/SyncManager.php b/includes/classes/Indexable/Post/SyncManager.php index 5ac943b6e3..b795adf6d2 100644 --- a/includes/classes/Indexable/Post/SyncManager.php +++ b/includes/classes/Indexable/Post/SyncManager.php @@ -11,6 +11,8 @@ use ElasticPress\Elasticsearch as Elasticsearch; use ElasticPress\Indexables as Indexables; use ElasticPress\SyncManager as SyncManagerAbstract; +use ElasticPress\Utils; +use ElasticPress\IndexHelper; if ( ! defined( 'ABSPATH' ) ) { // @codeCoverageIgnoreStart @@ -61,13 +63,17 @@ public function setup() { // Called just because we need to know somehow if $delete_all is set before action_queue_meta_sync() runs. add_filter( 'delete_post_metadata', array( $this, 'maybe_delete_meta_for_all' ), 10, 5 ); add_action( 'deleted_post_meta', array( $this, 'action_queue_meta_sync' ), 10, 4 ); - add_action( 'set_object_terms', array( $this, 'action_set_object_terms' ), 10, 6 ); - add_action( 'edited_term', array( $this, 'action_edited_term' ), 10, 3 ); - add_action( 'deleted_term_relationships', array( $this, 'action_deleted_term_relationships' ), 10, 3 ); add_action( 'wp_initialize_site', array( $this, 'action_create_blog_index' ) ); add_filter( 'ep_sync_insert_permissions_bypass', array( $this, 'filter_bypass_permission_checks_for_machines' ) ); add_filter( 'ep_sync_delete_permissions_bypass', array( $this, 'filter_bypass_permission_checks_for_machines' ) ); + + // Conditionally update posts associated with terms + add_action( 'ep_admin_notices', [ $this, 'maybe_display_notice_edit_single_term' ] ); + add_action( 'ep_admin_notices', [ $this, 'maybe_display_notice_term_list_screen' ] ); + add_action( 'set_object_terms', array( $this, 'action_set_object_terms' ), 10, 6 ); + add_action( 'edited_term', array( $this, 'action_edited_term' ), 10, 3 ); + add_action( 'deleted_term_relationships', array( $this, 'action_deleted_term_relationships' ), 10, 3 ); } /** @@ -359,6 +365,74 @@ public function action_sync_on_update( $post_id ) { } } + /** + * Depending on the number of posts associated with the term display an admin notice + * + * @since 4.4.0 + * @param array $notices Current ElasticPress admin notices + * @return array + */ + public function maybe_display_notice_edit_single_term( $notices ) { + global $pagenow, $tag; + + /** + * Make sure we're on a term-related page in the admin dashboard. + */ + if ( ! is_admin() || 'term.php' !== $pagenow || ! $tag instanceof \WP_Term ) { + return $notices; + } + + if ( IndexHelper::factory()->get_index_default_per_page() >= $tag->count ) { + return $notices; + } + + $notices['edited_single_term'] = [ + 'html' => sprintf( + /* translators: Sync Page URL */ + __( 'Due to the number of posts associated with this term, you will need to resync after editing or deleting it.', 'elasticpress' ), + Utils\get_sync_url() + ), + 'type' => 'warning', + 'dismiss' => true, + ]; + + return $notices; + } + + /** + * Depending on the number of posts display an admin notice in the Dashboard Terms List Screen + * + * @since 4.4.0 + * @param array $notices Current ElasticPress admin notices + * @return array + */ + public function maybe_display_notice_term_list_screen( $notices ) { + global $pagenow, $tax; + + /** + * Make sure we're on a term-related page in the admin dashboard. + */ + if ( ! is_admin() || 'edit-tags.php' !== $pagenow || ! $tax instanceof \WP_Taxonomy ) { + return $notices; + } + + if ( ! $this->is_tax_max_count_bigger_than_items_per_cycle( $tax ) ) { + return $notices; + } + + $notices['too_many_posts_on_term'] = [ + 'html' => sprintf( + /* translators: Sync Page URL */ + __( 'Depending on the number of posts associated with a term, you may need to resync after editing or deleting it.', 'elasticpress' ), + Utils\get_sync_url() + ), + 'type' => 'warning', + 'dismiss' => true, + ]; + + return $notices; + } + /** * When a post's terms are changed, re-index. * @@ -473,6 +547,9 @@ public function action_edited_term( $term_id, $tt_id, $taxonomy ) { return; } + // If we have more items to update than the number set as Content Items per Index Cycle, skip it. + $should_skip = count( $object_ids ) > IndexHelper::factory()->get_index_default_per_page(); + /** * Filter to allow skipping this action in case of custom handling * @@ -484,12 +561,10 @@ public function action_edited_term( $term_id, $tt_id, $taxonomy ) { * @param {array} $object_ids IDs of the objects attached to the term id. * @return {bool} New value of whether to skip running action_edited_term or not */ - if ( apply_filters( 'ep_skip_action_edited_term', false, $term_id, $tt_id, $taxonomy, $object_ids ) ) { + if ( apply_filters( 'ep_skip_action_edited_term', $should_skip, $term_id, $tt_id, $taxonomy, $object_ids ) ) { return; } - $indexable = Indexables::factory()->get( $this->indexable_slug ); - // Add all of them to the queue foreach ( $object_ids as $post_id ) { if ( ! $this->should_reindex_post( $post_id, $taxonomy ) ) { @@ -652,6 +727,54 @@ protected function should_reindex_post( $post_id, $taxonomy ) { return false; } - return true; + // If we have more items to update than the number set as Content Items per Index Cycle, skip it to avoid a timeout. + $single_ids_queued = array_unique( array_keys( $this->sync_queue ) ); + $has_too_many_queued = count( $single_ids_queued ) > IndexHelper::factory()->get_index_default_per_page(); + + return ! $has_too_many_queued; + } + + /** + * Given a taxonomy, check if the term with most posts is under or above the number set as Content Items per Index Cycle. + * + * The result will be cached in a transient. Its TTL will depend on the result: + * If it is determined we have a term with more posts, cache it for more time. + * + * @since 4.4.0 + * @param \WP_Taxonomy $tax The taxonomy object + * @return boolean + */ + protected function is_tax_max_count_bigger_than_items_per_cycle( \WP_Taxonomy $tax ) : bool { + $transient_name = "ep_term_max_count_{$tax->name}"; + $cached_max_count = get_transient( $transient_name ); + + if ( is_integer( $cached_max_count ) ) { + return $cached_max_count > IndexHelper::factory()->get_index_default_per_page(); + } + + $max_count = get_terms( + [ + 'taxonomy' => $tax->name, + 'orderby' => 'count', + 'order' => 'DESC', + 'number' => 1, + 'count' => true, + ] + ); + + if ( ! is_array( $max_count ) || ! $max_count[0] instanceof \WP_Term || ! is_integer( $max_count[0]->count ) ) { + set_transient( $transient_name, 0, HOUR_IN_SECONDS ); + return false; + } + + $is_max_count_bigger = $max_count[0]->count > IndexHelper::factory()->get_index_default_per_page(); + + set_transient( + $transient_name, + $max_count[0]->count, + $is_max_count_bigger ? DAY_IN_SECONDS : HOUR_IN_SECONDS + ); + + return $is_max_count_bigger; } } diff --git a/includes/utils.php b/includes/utils.php index 759f10bd92..fabfcf5f59 100644 --- a/includes/utils.php +++ b/includes/utils.php @@ -671,3 +671,20 @@ function get_asset_info( $slug, $attribute = null ) { return $asset; } + +/** + * Return the Sync Page URL. + * + * @since 4.4.0 + * @param boolean $do_sync Whether the link should or should not start a resync. + * @return string + */ +function get_sync_url( bool $do_sync = false ) : string { + $page = 'admin.php?page=elasticpress-sync'; + if ( $do_sync ) { + $page .= '&do_sync'; + } + return ( defined( 'EP_IS_NETWORK' ) && EP_IS_NETWORK ) ? + network_admin_url( $page ) : + admin_url( $page ); +} diff --git a/tests/cypress/integration/dashboard-sync.cy.js b/tests/cypress/integration/dashboard-sync.cy.js index b7b7f40178..5f0a8bf935 100644 --- a/tests/cypress/integration/dashboard-sync.cy.js +++ b/tests/cypress/integration/dashboard-sync.cy.js @@ -1,11 +1,6 @@ /* global indexNames */ describe('Dashboard Sync', () => { - function setPerIndexCycle(number = null) { - const newValue = number || 350; - cy.wpCli(`option set ep_bulk_setting ${newValue}`); - } - function canSeeIndexesNames() { cy.visitAdminPage('admin.php?page=elasticpress-health'); cy.get('.metabox-holder') @@ -145,7 +140,7 @@ describe('Dashboard Sync', () => { }); it('Can pause the dashboard sync, can not activate a feature during sync nor perform a sync via WP-CLI', () => { - setPerIndexCycle(20); + cy.setPerIndexCycle(20); cy.visitAdminPage('admin.php?page=elasticpress-sync'); @@ -176,6 +171,6 @@ describe('Dashboard Sync', () => { cy.visitAdminPage('admin.php?page=elasticpress'); cy.get('.error-overlay').should('not.have.class', 'syncing'); - setPerIndexCycle(); + cy.setPerIndexCycle(); }); }); diff --git a/tests/cypress/integration/features/comments.cy.js b/tests/cypress/integration/features/comments.cy.js index eeed4bc2ce..acb402fe93 100644 --- a/tests/cypress/integration/features/comments.cy.js +++ b/tests/cypress/integration/features/comments.cy.js @@ -107,11 +107,7 @@ describe('Comments Feature', () => { cy.get('#submit').click(); // Check if the new comment was indexed - cy.wpCliEval( - ` - $comments_index = \\ElasticPress\\Indexables::factory()->get( "comment" )->get_index_name(); - WP_CLI::runcommand("elasticpress request {$comments_index}/_refresh --method=POST");`, - ).then(() => { + cy.refreshIndex('comment').then(() => { cy.wpCli('wp elasticpress stats') .its('stdout') .should('contain', `Documents: ${defaultApprovedComments + 1}`); diff --git a/tests/cypress/integration/indexables/post.cy.js b/tests/cypress/integration/indexables/post.cy.js new file mode 100644 index 0000000000..c9719171f9 --- /dev/null +++ b/tests/cypress/integration/indexables/post.cy.js @@ -0,0 +1,53 @@ +describe('Post Indexable', () => { + it('Can conditionally update posts when a term is edited', () => { + /** + * At this point, using the default content: + * - the `Classic` (ID 29) term has 36 posts + * - the `Block` (ID 54) term has 7 posts + * Important: There is no post with both categories, as that would skew results. + */ + cy.setPerIndexCycle(); + cy.visitAdminPage('edit-tags.php?taxonomy=category'); + cy.get('div[data-ep-notice="too_many_posts_on_term"]').should('not.exist'); + + cy.setPerIndexCycle(35); + cy.visitAdminPage('edit-tags.php?taxonomy=category&orderby=count&order=desc'); + cy.get('div[data-ep-notice="too_many_posts_on_term"]').should('exist'); + + // Change the `Classic` term, should not index + cy.visitAdminPage('term.php?taxonomy=category&tag_ID=29'); + cy.get('div[data-ep-notice="edited_single_term"]').should('exist'); + cy.get('#name').clearThenType('totallydifferenttermname'); + cy.get('input.button-primary').click(); + + // Change the `Block` term, should index + cy.visitAdminPage('term.php?taxonomy=category&tag_ID=20'); + cy.get('div[data-ep-notice="edited_single_term"]').should('not.exist'); + cy.get('#name').clearThenType('b10ck'); + cy.get('input.button-primary').click(); + + // Make sure the changes are processed by ES + cy.refreshIndex('post'); + + cy.visit('/?s=totallydifferenttermname'); + cy.get('.hentry').should('not.exist'); + + cy.visit('/?s=b10ck'); + cy.get('.hentry').should('exist'); + cy.get('#debug-menu-target-EP_Debug_Bar_ElasticPress .ep-query-debug').should( + 'contain.text', + '"name": "b10ck",', + ); + + // Restore + cy.visitAdminPage('term.php?taxonomy=category&tag_ID=29'); + cy.get('#name').clearThenType('Classic'); + cy.get('input.button-primary').click(); + + cy.visitAdminPage('term.php?taxonomy=category&tag_ID=20'); + cy.get('#name').clearThenType('Block'); + cy.get('input.button-primary').click(); + + cy.setPerIndexCycle(); + }); +}); diff --git a/tests/cypress/support/commands.js b/tests/cypress/support/commands.js index 0e716b4e76..2bfbd7310a 100644 --- a/tests/cypress/support/commands.js +++ b/tests/cypress/support/commands.js @@ -444,3 +444,15 @@ Cypress.Commands.add('createUser', (userData) => { cy.get('#user_pass').clear().type(`${newUserDate.password}{enter}`); } }); + +Cypress.Commands.add('setPerIndexCycle', (number = 350) => { + cy.wpCli(`option set ep_bulk_setting ${number}`); +}); + +Cypress.Commands.add('refreshIndex', (indexable) => { + cy.wpCliEval( + ` + $index = \\ElasticPress\\Indexables::factory()->get( "${indexable}" )->get_index_name(); + WP_CLI::runcommand("elasticpress request {$index}/_refresh --method=POST");`, + ); +}); diff --git a/tests/php/TestIndexHelper.php b/tests/php/TestIndexHelper.php new file mode 100644 index 0000000000..ebc84eb92a --- /dev/null +++ b/tests/php/TestIndexHelper.php @@ -0,0 +1,50 @@ +assertEquals( 350, $index_helper->get_index_default_per_page() ); + + /** + * Test changing the option value (most likely how users will have this changed.) + */ + $change_via_option_filter = function() { + return 10; + }; + if ( defined( 'EP_IS_NETWORK' ) && EP_IS_NETWORK ) { + add_filter( 'pre_site_option_ep_bulk_setting', $change_via_option_filter ); + } else { + add_filter( 'pre_option_ep_bulk_setting', $change_via_option_filter ); + } + $this->assertEquals( 10, $index_helper->get_index_default_per_page() ); + + /** + * Test the `ep_index_default_per_page` filter. + */ + $change_via_ep_filter = function() { + return 15; + }; + add_filter( 'ep_index_default_per_page', $change_via_ep_filter ); + $this->assertEquals( 15, $index_helper->get_index_default_per_page() ); + } +} diff --git a/tests/php/TestUtils.php b/tests/php/TestUtils.php index 978f4f7a21..94ddb01c12 100644 --- a/tests/php/TestUtils.php +++ b/tests/php/TestUtils.php @@ -188,4 +188,32 @@ public function testIsIndexing() { $this->assertFalse( ElasticPress\Utils\is_indexing() ); } + + /** + * Test the get_sync_url method + * + * @since 4.4.0 + */ + public function testGetSyncUrl() { + /** + * Test without the $do_sync parameter + */ + $sync_url = ElasticPress\Utils\get_sync_url(); + $this->assertStringNotContainsString( '&do_sync', $sync_url ); + if ( defined( 'EP_IS_NETWORK' ) && EP_IS_NETWORK ) { + $this->assertStringContainsString( 'wp-admin/network/admin.php?page=elasticpress-sync', $sync_url ); + } else { + $this->assertStringContainsString( 'wp-admin/admin.php?page=elasticpress-sync', $sync_url ); + } + + /** + * Test with the $do_sync parameter + */ + $sync_url = ElasticPress\Utils\get_sync_url( true ); + if ( defined( 'EP_IS_NETWORK' ) && EP_IS_NETWORK ) { + $this->assertStringContainsString( 'wp-admin/network/admin.php?page=elasticpress-sync&do_sync', $sync_url ); + } else { + $this->assertStringContainsString( 'wp-admin/admin.php?page=elasticpress-sync&do_sync', $sync_url ); + } + } }