From 02c441542a1364984140323348d396548614cdb1 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 10:48:39 -0600 Subject: [PATCH 01/22] Update array() to the new syntax [] Task: TEC-3232 --- src/Tribe/Aggregator/Cron.php | 25 +++++++++---------- src/Tribe/Aggregator/Record/Abstract.php | 3 +-- src/Tribe/Aggregator/Record/Queue.php | 1 - src/Tribe/Aggregator/Record/Queue_Cleaner.php | 2 +- .../Aggregator/Record/Queue_Realtime.php | 2 -- src/Tribe/Aggregator/Record/Void_Queue.php | 1 - src/Tribe/Aggregator/Tabs/Scheduled.php | 12 ++++----- 7 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index c0f4eef3e2..880dcae684 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -293,14 +293,13 @@ public function verify_child_record_creation() { if ( ! tribe( 'events-aggregator.main' )->is_service_active() ) { return; } - $records = Tribe__Events__Aggregator__Records::instance(); $service = tribe( 'events-aggregator.service' ); - $query = $records->query( array( - 'post_status' => Tribe__Events__Aggregator__Records::$status->schedule, + $query = $records->query( [ + 'post_status' => Tribe__Events__Aggregator__Records::$status->schedule, 'posts_per_page' => -1, - ) ); + ] ); if ( ! $query->have_posts() ) { tribe( 'logger' )->log_debug( 'No Records Scheduled, skipped creating children', 'EA Cron' ); @@ -516,22 +515,22 @@ public function purge_expired_records() { ) ); - $args = array( - 'post_status' => array( + $args = [ + 'post_status' => [ $statuses->pending, $statuses->success, $statuses->failed, $statuses->draft, - ), - 'date_query' => array( - array( + ], + 'date_query' => [ + [ 'before' => date( 'Y-m-d H:i:s', time() - $records->get_retention() ), 'column' => 'post_date_gmt', - ), - ), - 'order' => 'ASC', + ], + ], + 'order' => 'ASC', 'posts_per_page' => 100, - ); + ]; if ( $records_to_retain ) { $args['post__not_in'] = $records_to_retain; diff --git a/src/Tribe/Aggregator/Record/Abstract.php b/src/Tribe/Aggregator/Record/Abstract.php index 8186860515..9342b034c8 100644 --- a/src/Tribe/Aggregator/Record/Abstract.php +++ b/src/Tribe/Aggregator/Record/Abstract.php @@ -655,7 +655,7 @@ public function maybe_add_meta_via_pre_wp_44_method( $id, $meta ) { * post ID if the record had to be re-scheduled due to HTTP request * limit. */ - public function queue_import( $args = array() ) { + public function queue_import( $args = [] ) { $aggregator = tribe( 'events-aggregator.main' ); $is_previewing = ( @@ -1207,7 +1207,6 @@ public function get_last_import_status( $type = 'error', $lookup_children = fals if ( 'error:usage-limit-exceeded' === $status ) { return __( 'When this import was last scheduled to run, the daily limit for your Event Aggregator license had already been reached.', 'the-events-calendar' ); } - return tribe( 'events-aggregator.service' )->get_service_message( $status ); } diff --git a/src/Tribe/Aggregator/Record/Queue.php b/src/Tribe/Aggregator/Record/Queue.php index 805b091834..155baecdfd 100644 --- a/src/Tribe/Aggregator/Record/Queue.php +++ b/src/Tribe/Aggregator/Record/Queue.php @@ -78,7 +78,6 @@ public function __construct( $record, $items = array(), Tribe__Events__Aggregato if ( is_wp_error( $items ) ) { $this->null_process = true; - return; } diff --git a/src/Tribe/Aggregator/Record/Queue_Cleaner.php b/src/Tribe/Aggregator/Record/Queue_Cleaner.php index 7a6b319e39..4a2e152d5f 100644 --- a/src/Tribe/Aggregator/Record/Queue_Cleaner.php +++ b/src/Tribe/Aggregator/Record/Queue_Cleaner.php @@ -94,7 +94,7 @@ public function maybe_fail_stalled_record( Tribe__Events__Aggregator__Record__Ab $post_status = $record->post->post_status; - if ( ! in_array( $post_status, array( $pending, $failed ) ) ) { + if ( ! in_array( $post_status, [ $pending, $failed ], true ) ) { return false; } diff --git a/src/Tribe/Aggregator/Record/Queue_Realtime.php b/src/Tribe/Aggregator/Record/Queue_Realtime.php index 0a1ff8421a..cb60fa3b2b 100644 --- a/src/Tribe/Aggregator/Record/Queue_Realtime.php +++ b/src/Tribe/Aggregator/Record/Queue_Realtime.php @@ -121,7 +121,6 @@ public function ajax() { // Load the queue /** @var \Tribe__Events__Aggregator__Record__Queue_Interface $queue */ $queue = $this->queue ? $this->queue : Tribe__Events__Aggregator__Record__Queue_Processor::build_queue( $this->record_id ); - // We always need to setup the Current Queue $this->queue_processor->set_current_queue( $queue ); @@ -138,7 +137,6 @@ public function ajax() { $current_queue = $this->queue_processor->current_queue; $done = $current_queue->is_empty() && empty( $current_queue->is_fetching ); $percentage = $current_queue->progress_percentage(); - $this->ajax_operations->exit_data( $this->get_progress_message_data( $current_queue, $percentage, $done ) ); } diff --git a/src/Tribe/Aggregator/Record/Void_Queue.php b/src/Tribe/Aggregator/Record/Void_Queue.php index bcf1f75940..d7cf722079 100644 --- a/src/Tribe/Aggregator/Record/Void_Queue.php +++ b/src/Tribe/Aggregator/Record/Void_Queue.php @@ -36,7 +36,6 @@ public function __construct( $error ) { return; } - $this->error = $error; } diff --git a/src/Tribe/Aggregator/Tabs/Scheduled.php b/src/Tribe/Aggregator/Tabs/Scheduled.php index fe21f224dd..68d9fb81c6 100644 --- a/src/Tribe/Aggregator/Tabs/Scheduled.php +++ b/src/Tribe/Aggregator/Tabs/Scheduled.php @@ -261,8 +261,8 @@ private function action_notice( $action, $ids = array(), $error = null ) { private function action_delete_record( $records = array() ) { $record_obj = Tribe__Events__Aggregator__Records::instance()->get_post_type(); $records = array_filter( (array) $records, 'is_numeric' ); - $success = array(); - $errors = array(); + $success = []; + $errors = []; foreach ( $records as $record_id ) { $record = Tribe__Events__Aggregator__Records::instance()->get_by_post_id( $record_id ); @@ -287,7 +287,7 @@ private function action_delete_record( $records = array() ) { $success[ $record->id ] = true; } - return array( $success, $errors ); + return [ $success, $errors ]; } /** @@ -299,12 +299,12 @@ private function action_delete_record( $records = array() ) { * * @return array */ - public function action_run_import( $records = array() ) { + public function action_run_import( $records = [] ) { $service = tribe( 'events-aggregator.service' ); $record_obj = Tribe__Events__Aggregator__Records::instance()->get_post_type(); $records = array_filter( (array) $records, 'is_numeric' ); - $success = array(); - $errors = array(); + $success = []; + $errors = []; foreach ( $records as $record_id ) { $record = Tribe__Events__Aggregator__Records::instance()->get_by_post_id( $record_id ); From 2ce0667d0fc0b25c7e06f92a4901de28aa20fb49 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 10:55:02 -0600 Subject: [PATCH 02/22] Remove non required else blocks As the main block (if) returns a value inmediatly the subsequent else block is non required plus doing that decrease the mental model around thew flow of the calls on those functions. Task: [TEC-3232] --- src/Tribe/Aggregator/Cron.php | 1 - src/Tribe/Aggregator/Record/Abstract.php | 8 ++++---- src/Tribe/Aggregator/Record/Queue_Processor.php | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index 880dcae684..0441879673 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -343,7 +343,6 @@ public function verify_child_record_creation() { // Creating the child records based on this Parent $child = $record->create_child_record(); - tribe( 'logger' )->log_debug( sprintf( 'Creating child record %d for %d', $child->id, $record->id ), 'EA Cron' ); if ( ! is_wp_error( $child ) ) { diff --git a/src/Tribe/Aggregator/Record/Abstract.php b/src/Tribe/Aggregator/Record/Abstract.php index 9342b034c8..edd935a1b5 100644 --- a/src/Tribe/Aggregator/Record/Abstract.php +++ b/src/Tribe/Aggregator/Record/Abstract.php @@ -761,11 +761,11 @@ public function queue_import( $args = [] ) { if ( 'core:aggregator:http_request-limit' === $response->get_error_code() ) { $this->should_queue_import( true ); return $this->set_status_as_pending(); - } else { - $error = $response; - - return $this->set_status_as_failed( $error ); } + + $error = $response; + + return $this->set_status_as_failed( $error ); } // if the Aggregator response has an unexpected format, set this record as failed diff --git a/src/Tribe/Aggregator/Record/Queue_Processor.php b/src/Tribe/Aggregator/Record/Queue_Processor.php index cd17086caa..1b012356ad 100644 --- a/src/Tribe/Aggregator/Record/Queue_Processor.php +++ b/src/Tribe/Aggregator/Record/Queue_Processor.php @@ -192,10 +192,10 @@ public function next_waiting_record( $interactive_only = false ) { if ( empty( $waiting_records ) ) { return $this->current_record_id = 0; - } else { - $next_record = array_shift( $waiting_records ); - return $this->current_record_id = $next_record->ID; } + + $next_record = array_shift( $waiting_records ); + return $this->current_record_id = $next_record->ID; } /** From 39654729ad8bb632c22ae4d697d1065a5eaeebc0 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 10:57:28 -0600 Subject: [PATCH 03/22] Add consistent return values The functions expect a specifc type of value to be returned but on those instances none was returned (meaning null) in order to keep consistency with the expected value the expected return value was used instead. Task: [TEC-3232] --- src/Tribe/Aggregator/Record/Queue_Realtime.php | 2 +- src/Tribe/Aggregator/Tabs/Scheduled.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tribe/Aggregator/Record/Queue_Realtime.php b/src/Tribe/Aggregator/Record/Queue_Realtime.php index cb60fa3b2b..16d535639f 100644 --- a/src/Tribe/Aggregator/Record/Queue_Realtime.php +++ b/src/Tribe/Aggregator/Record/Queue_Realtime.php @@ -63,7 +63,7 @@ public function update_loop_vars() { public function render_update_message() { if ( ! Tribe__Events__Aggregator__Page::instance()->is_screen() ) { - return; + return false; } /** @var Tribe__Events__Aggregator__Record__Queue_Processor $processor */ diff --git a/src/Tribe/Aggregator/Tabs/Scheduled.php b/src/Tribe/Aggregator/Tabs/Scheduled.php index 68d9fb81c6..ec8aaf0ea4 100644 --- a/src/Tribe/Aggregator/Tabs/Scheduled.php +++ b/src/Tribe/Aggregator/Tabs/Scheduled.php @@ -356,7 +356,7 @@ public function action_run_import( $records = [] ) { */ public function maybe_display_aggregator_missing_license_key_message() { if ( tribe( 'events-aggregator.main' )->is_service_active() ) { - return; + return ''; } ob_start(); From 8d668ff9355f750567b8baa69e421403fc4fe94f Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 10:58:50 -0600 Subject: [PATCH 04/22] Replacnig in_array with instanceof instead As if a class, in this particular case: $record is being compared against his parents meaning that if $record is instance of the Tribe__Events__Aggregator__Record__Abstract is the same as comparing that the variable has the class as one of his parents. --- src/Tribe/Aggregator/Record/Queue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Record/Queue.php b/src/Tribe/Aggregator/Record/Queue.php index 155baecdfd..8813ba265b 100644 --- a/src/Tribe/Aggregator/Record/Queue.php +++ b/src/Tribe/Aggregator/Record/Queue.php @@ -70,7 +70,7 @@ public function __construct( $record, $items = array(), Tribe__Events__Aggregato $record = Tribe__Events__Aggregator__Records::instance()->get_by_post_id( $record ); } - if ( ! is_object( $record ) || ! in_array( 'Tribe__Events__Aggregator__Record__Abstract', class_parents( $record ) ) ) { + if ( ! is_object( $record ) || ! $record instanceof \Tribe__Events__Aggregator__Record__Abstract ) { $this->null_process = true; return; From 2d327b4769eac75cc628d4b6d96a29a77270e73a Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:01:05 -0600 Subject: [PATCH 05/22] Show a failure on the UI screen if the import failed --- src/Tribe/Aggregator/Record/List_Table.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Record/List_Table.php b/src/Tribe/Aggregator/Record/List_Table.php index 8f184e5f9e..587a00b0d1 100644 --- a/src/Tribe/Aggregator/Record/List_Table.php +++ b/src/Tribe/Aggregator/Record/List_Table.php @@ -529,12 +529,14 @@ public function column_imported( $post ) { if ( 'scheduled' === $this->tab->get_slug() ) { $last_import_error = $record->get_last_import_status( 'error' ); + $status = 'success'; if ( $last_import_error ) { $html[] = ''; + $status = 'failed'; } - $has_child_record = $record->get_child_record_by_status( 'success', 1 ); + $has_child_record = $record->get_child_record_by_status( $status, 1 ); if ( ! $has_child_record ) { $html[] = '' . esc_html__( 'Unknown', 'the-events-calendar' ) . ''; From a90678a8650e142f9b9e1d60c48a721e02d12b07 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:05:43 -0600 Subject: [PATCH 06/22] Replace constant with a more explicit value Use HOUR_IN_SECONDS constant to present a more meaningful value --- src/Tribe/Aggregator/Record/Queue_Cleaner.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tribe/Aggregator/Record/Queue_Cleaner.php b/src/Tribe/Aggregator/Record/Queue_Cleaner.php index 4a2e152d5f..ef7a526747 100644 --- a/src/Tribe/Aggregator/Record/Queue_Cleaner.php +++ b/src/Tribe/Aggregator/Record/Queue_Cleaner.php @@ -6,10 +6,10 @@ class Tribe__Events__Aggregator__Record__Queue_Cleaner { /** * Default is 12hrs. * - * @var int The time a record is allowed to stall before havint its status set to to failed since its creation in + * @var int The time a record is allowed to stall before have the status set to to failed since its creation in * seconds. */ - protected $time_to_live = 43200; // For pre-PHP 5.6 compat, we do not define as 12 * HOUR_IN_SECONDS + protected $time_to_live = HOUR_IN_SECONDS * 12; // For pre-PHP 5.6 compat, we do not define as 12 * HOUR_IN_SECONDS /** * @var int The time a record is allowed to stall before having From d253784df81cbeb46929a34fa075da958570a247 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:11:41 -0600 Subject: [PATCH 07/22] Make sure a job is marked as incomplete if fails A job is empty if has a null_process or is no longer fetching if has an error --- src/Tribe/Aggregator/Record/Queue.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Record/Queue.php b/src/Tribe/Aggregator/Record/Queue.php index 8813ba265b..bc0636944d 100644 --- a/src/Tribe/Aggregator/Record/Queue.php +++ b/src/Tribe/Aggregator/Record/Queue.php @@ -181,12 +181,18 @@ public function count() { } /** - * Shortcut to check if this queue is empty. + * Shortcut to check if this queue is empty or it has a null process * * @return boolean `true` if this queue instance has acquired the lock and * the count is 0, `false` otherwise. */ public function is_empty() { + if ( $this->null_process ) { + return true; + } + + return true; + return $this->has_lock && 0 === $this->count(); } @@ -289,6 +295,7 @@ public function process( $batch_size = null ) { || is_wp_error( $data ) ) { $this->release_lock(); + $this->is_fetching = false; return $this->activity(); } From bb3048b9b01ca00c6d2816a524efdeb8cd3a9e02 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:12:29 -0600 Subject: [PATCH 08/22] Mark job as failures if an error was present Save the last status as failure as well if the job was not successfull --- src/Tribe/Aggregator/Cron.php | 4 ++++ src/Tribe/Aggregator/Record/Abstract.php | 1 + 2 files changed, 5 insertions(+) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index 0441879673..7e33c2b144 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -588,6 +588,10 @@ protected function maybe_process_immediately( Tribe__Events__Aggregator__Record_ $import_data = $record->prep_import_data(); if ( empty( $import_data ) || $import_data instanceof WP_Error || ! is_array( $import_data ) ) { + + $record->update_meta( 'last_import_status', 'error:import-failed' ); + $record->set_status_as_failed( $import_data ); + return; } diff --git a/src/Tribe/Aggregator/Record/Abstract.php b/src/Tribe/Aggregator/Record/Abstract.php index edd935a1b5..f42fda14c7 100644 --- a/src/Tribe/Aggregator/Record/Abstract.php +++ b/src/Tribe/Aggregator/Record/Abstract.php @@ -1263,6 +1263,7 @@ public function process_posts( $data = array(), $start_immediately = false ) { $items = $this->prep_import_data( $data ); if ( is_wp_error( $items ) ) { + $this->set_status_as_failed( $items ); return $items; } From 3fdf56da7cd6ea67d87afde12357973f4e37b8b2 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:13:01 -0600 Subject: [PATCH 09/22] Remove non used variable --- src/Tribe/Aggregator/Tabs/Scheduled.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tribe/Aggregator/Tabs/Scheduled.php b/src/Tribe/Aggregator/Tabs/Scheduled.php index ec8aaf0ea4..5fdd3df082 100644 --- a/src/Tribe/Aggregator/Tabs/Scheduled.php +++ b/src/Tribe/Aggregator/Tabs/Scheduled.php @@ -301,7 +301,6 @@ private function action_delete_record( $records = array() ) { */ public function action_run_import( $records = [] ) { $service = tribe( 'events-aggregator.service' ); - $record_obj = Tribe__Events__Aggregator__Records::instance()->get_post_type(); $records = array_filter( (array) $records, 'is_numeric' ); $success = []; $errors = []; From c20f053ce0ee119dd6e22b7633985eb045f8052c Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:46:09 -0600 Subject: [PATCH 10/22] Make sure the process of a post does not return error If an error is returned dees not schedule a queue work and mark the job as a failure. Task: [PRMTR-162] --- src/Tribe/Aggregator/Tabs/Scheduled.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Tribe/Aggregator/Tabs/Scheduled.php b/src/Tribe/Aggregator/Tabs/Scheduled.php index 5fdd3df082..c9ecb303e5 100644 --- a/src/Tribe/Aggregator/Tabs/Scheduled.php +++ b/src/Tribe/Aggregator/Tabs/Scheduled.php @@ -336,16 +336,21 @@ public function action_run_import( $records = [] ) { continue; } - $record->update_meta( 'last_import_status', 'success:queued' ); - $child->update_meta( 'import_id', $status->data->import_id ); - $child->finalize(); - $child->process_posts( array(), true ); + $post = $child->process_posts( [], true ); + + if ( is_wp_error( $post ) ) { + $errors[ $record->id ] = $post; + $record->update_meta( 'last_import_status', 'error:import-failed' ); + } else { + $record->update_meta( 'last_import_status', 'success:queued' ); + $child->update_meta( 'import_id', $status->data->import_id ); - $success[ $record->id ] = $record; + $success[ $record->id ] = $record; + } } - return array( $success, $errors ); + return [ $success, $errors ]; } /** From 88020eb860950b50928a734830a1d69539e0fc12 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:47:00 -0600 Subject: [PATCH 11/22] Setup meta query relationships correctly As the default operator of the queries is an AND the queries will remove results that either does not have the key or the value is other than 1 instead the desired behavior is posts where the querie either does not exists or the value is not 1 and where the origin is not CSV. --- src/Tribe/Aggregator/Cron.php | 45 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index 7e33c2b144..8f361f7534 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -399,31 +399,34 @@ public function verify_fetching_from_service() { $records = Tribe__Events__Aggregator__Records::instance(); - $query = $records->query( array( + $query = $records->query( [ 'post_status' => Tribe__Events__Aggregator__Records::$status->pending, - 'posts_per_page' => - 1, + 'posts_per_page' => -1, 'order' => 'ASC', - 'meta_query' => array( - 'origin-not-csv' => array( - 'key' => '_tribe_aggregator_origin', - 'value' => 'csv', - 'compare' => '!=', - ), - // if not specified then assume batch push is not supported - 'no-batch-push-support-specified' => array( - 'key' => '_tribe_aggregator_allow_batch_push', - 'value' => 'bug #23268', - 'compare' => 'NOT EXISTS', - ), - // if specified and not `1` then batch push is not supported - 'explicit-no-batch-push-support' => array( - 'key' => '_tribe_aggregator_allow_batch_push', - 'value' => '1', + 'meta_query' => [ + 'origin-not-csv' => [ + 'key' => '_tribe_aggregator_origin', + 'value' => 'csv', 'compare' => '!=', - ), - ), + ], + [ + 'relation' => 'OR', + // if not specified then assume batch push is not supported + 'no-batch-push-support-specified' => [ + 'key' => '_tribe_aggregator_allow_batch_push', + 'value' => 'bug #23268', + 'compare' => 'NOT EXISTS', + ], + // if specified and not `1` then batch push is not supported + 'explicit-no-batch-push-support' => [ + 'key' => '_tribe_aggregator_allow_batch_push', + 'value' => '1', + 'compare' => '!=', + ], + ], + ], 'after' => '-4 hours', - ) ); + ] ); if ( ! $query->have_posts() ) { tribe( 'logger' )->log_debug( 'No Records Pending, skipped Fetching from service', 'EA Cron' ); From e2227e35b3655aa7450f822d77a68e019b1668ac Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 11:55:50 -0600 Subject: [PATCH 12/22] Remove non required early return Return that was not required as removes the regular flow on the function --- src/Tribe/Aggregator/Record/Queue.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Tribe/Aggregator/Record/Queue.php b/src/Tribe/Aggregator/Record/Queue.php index bc0636944d..1cb3dc1c57 100644 --- a/src/Tribe/Aggregator/Record/Queue.php +++ b/src/Tribe/Aggregator/Record/Queue.php @@ -191,8 +191,6 @@ public function is_empty() { return true; } - return true; - return $this->has_lock && 0 === $this->count(); } From 29c265b3295a257a2f430c8b610a5ea037eb5f61 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 18:47:59 -0600 Subject: [PATCH 13/22] Add test for cron job processing records Make sure records as pending are pick via the cron job as well Task: [TEC-3232] --- .../Tribe/Events/Aggregator/CronTest.php | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/wpunit/Tribe/Events/Aggregator/CronTest.php b/tests/wpunit/Tribe/Events/Aggregator/CronTest.php index bf6212a597..d3eaae3858 100644 --- a/tests/wpunit/Tribe/Events/Aggregator/CronTest.php +++ b/tests/wpunit/Tribe/Events/Aggregator/CronTest.php @@ -52,4 +52,78 @@ public function should_trash_record_posts_not_belonging_to_a_supported_origin() $this->assertEmpty( get_post( $record_post ) ); } + + /** + * It should process all pending records + * + * @test + */ + public function should_process_all_pending_records() { + $first = $this->factory()->post->create( [ + 'post_type' => Records::$post_type, + 'post_status' => Records::$status->pending, + 'post_mime_type' => 'ea/foo-bar' + ] ); + + add_post_meta( $first, '_tribe_aggregator_origin', 'eventbrite' ); + + $second = $this->factory()->post->create( [ + 'post_type' => Records::$post_type, + 'post_status' => Records::$status->pending, + 'post_mime_type' => 'ea/foo-bar' + ] ); + + add_post_meta( $second, '_tribe_aggregator_origin', 'meetup' ); + add_post_meta( $second, '_tribe_aggregator_allow_batch_push', '0' ); + + $batch = $this->factory()->post->create( [ + 'post_type' => Records::$post_type, + 'post_status' => Records::$status->pending, + 'post_mime_type' => 'ea/foo-bar' + ] ); + + add_post_meta( $batch, '_tribe_aggregator_origin', 'meetup' ); + add_post_meta( $batch, '_tribe_aggregator_allow_batch_push', '1' ); + + // By pass the is_active() EA call. + add_filter( 'tribe_aggregator_api', function ( $api ) { + $api->key = 'foo-bar'; + + return $api; + } ); + $cron = $this->make_instance(); + $cron->verify_fetching_from_service(); + + $this->assertEquals( Records::$status->failed, get_post_status( $first ) ); + $this->assertEquals( Records::$status->failed, get_post_status( $second ) ); + $this->assertEquals( Records::$status->pending, get_post_status( $batch ) ); + } + + /** + * Test batch records are not processed by cron task. + * + * @test + */ + public function it_should_bypass_batch_push_records() { + $batch = $this->factory()->post->create( [ + 'post_type' => Records::$post_type, + 'post_status' => Records::$status->pending, + 'post_mime_type' => 'ea/foo-bar' + ] ); + + add_post_meta( $batch, '_tribe_aggregator_origin', 'meetup' ); + add_post_meta( $batch, '_tribe_aggregator_allow_batch_push', '1' ); + + // By pass the is_active() EA call. + add_filter( 'tribe_aggregator_api', function ( $api ) { + $api->key = 'foo-bar'; + + return $api; + } ); + + $cron = $this->make_instance(); + $cron->verify_fetching_from_service(); + + $this->assertEquals( Records::$status->pending, get_post_status( $batch ) ); + } } \ No newline at end of file From fdcbe70c32b62ab91726f27442642dfe5af5221c Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Wed, 5 Feb 2020 19:16:40 -0600 Subject: [PATCH 14/22] Bypass EA is connected call --- .../Tribe/Events/Aggregator/CronTest.php | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/tests/wpunit/Tribe/Events/Aggregator/CronTest.php b/tests/wpunit/Tribe/Events/Aggregator/CronTest.php index d3eaae3858..83b01e8951 100644 --- a/tests/wpunit/Tribe/Events/Aggregator/CronTest.php +++ b/tests/wpunit/Tribe/Events/Aggregator/CronTest.php @@ -16,6 +16,15 @@ public function it_should_be_instantiatable() { $this->assertInstanceOf( Cron::class, $sut ); } + public function _before() { + // By pass the is_active() EA call. + add_filter( 'tribe_aggregator_api', function ( $api ) { + $api->key = 'foo-bar'; + + return $api; + } ); + } + /** * @return Cron */ @@ -43,11 +52,6 @@ public function should_trash_record_posts_not_belonging_to_a_supported_origin() $this->assertEquals( Records::$status->schedule, get_post_status( $record_post ) ); $cron = $this->make_instance(); - add_filter( 'tribe_aggregator_api', function ( $api ) { - $api->key = 'foo-bar'; - - return $api; - } ); $cron->verify_child_record_creation(); $this->assertEmpty( get_post( $record_post ) ); @@ -85,12 +89,6 @@ public function should_process_all_pending_records() { add_post_meta( $batch, '_tribe_aggregator_origin', 'meetup' ); add_post_meta( $batch, '_tribe_aggregator_allow_batch_push', '1' ); - // By pass the is_active() EA call. - add_filter( 'tribe_aggregator_api', function ( $api ) { - $api->key = 'foo-bar'; - - return $api; - } ); $cron = $this->make_instance(); $cron->verify_fetching_from_service(); @@ -114,13 +112,6 @@ public function it_should_bypass_batch_push_records() { add_post_meta( $batch, '_tribe_aggregator_origin', 'meetup' ); add_post_meta( $batch, '_tribe_aggregator_allow_batch_push', '1' ); - // By pass the is_active() EA call. - add_filter( 'tribe_aggregator_api', function ( $api ) { - $api->key = 'foo-bar'; - - return $api; - } ); - $cron = $this->make_instance(); $cron->verify_fetching_from_service(); From 43cf5ee9ef2bec3167ebd61d6303468352004927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Fri, 7 Feb 2020 08:16:11 -0600 Subject: [PATCH 15/22] Update src/Tribe/Aggregator/Record/Queue_Cleaner.php Co-Authored-By: theAverageDev (Luca Tumedei) --- src/Tribe/Aggregator/Record/Queue_Cleaner.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Record/Queue_Cleaner.php b/src/Tribe/Aggregator/Record/Queue_Cleaner.php index ef7a526747..8fe09ccab9 100644 --- a/src/Tribe/Aggregator/Record/Queue_Cleaner.php +++ b/src/Tribe/Aggregator/Record/Queue_Cleaner.php @@ -9,7 +9,7 @@ class Tribe__Events__Aggregator__Record__Queue_Cleaner { * @var int The time a record is allowed to stall before have the status set to to failed since its creation in * seconds. */ - protected $time_to_live = HOUR_IN_SECONDS * 12; // For pre-PHP 5.6 compat, we do not define as 12 * HOUR_IN_SECONDS + protected $time_to_live = HOUR_IN_SECONDS * 12; /** * @var int The time a record is allowed to stall before having From edf9d2e92e6184fee2dc929d74634d5a1b8f10aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Fri, 7 Feb 2020 08:16:18 -0600 Subject: [PATCH 16/22] Update src/Tribe/Aggregator/Record/Queue.php Co-Authored-By: theAverageDev (Luca Tumedei) --- src/Tribe/Aggregator/Record/Queue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Record/Queue.php b/src/Tribe/Aggregator/Record/Queue.php index 1cb3dc1c57..e3518ab917 100644 --- a/src/Tribe/Aggregator/Record/Queue.php +++ b/src/Tribe/Aggregator/Record/Queue.php @@ -181,7 +181,7 @@ public function count() { } /** - * Shortcut to check if this queue is empty or it has a null process + * Shortcut to check if this queue is empty or it has a null process. * * @return boolean `true` if this queue instance has acquired the lock and * the count is 0, `false` otherwise. From 9c8804f1caea07a1e1c39b78a37faec74e671551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Fri, 7 Feb 2020 08:16:25 -0600 Subject: [PATCH 17/22] Update src/Tribe/Aggregator/Cron.php Co-Authored-By: theAverageDev (Luca Tumedei) --- src/Tribe/Aggregator/Cron.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index 8f361f7534..d94af00fe6 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -417,7 +417,7 @@ public function verify_fetching_from_service() { 'value' => 'bug #23268', 'compare' => 'NOT EXISTS', ], - // if specified and not `1` then batch push is not supported + // If specified, and not `1`, then batch push is not supported. 'explicit-no-batch-push-support' => [ 'key' => '_tribe_aggregator_allow_batch_push', 'value' => '1', From 6e7d253ed54274e34bb73786c298ace989f16c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Crisoforo=20Gaspar=20Hern=C3=A1ndez?= Date: Fri, 7 Feb 2020 08:16:33 -0600 Subject: [PATCH 18/22] Update src/Tribe/Aggregator/Cron.php Co-Authored-By: theAverageDev (Luca Tumedei) --- src/Tribe/Aggregator/Cron.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tribe/Aggregator/Cron.php b/src/Tribe/Aggregator/Cron.php index d94af00fe6..37cfecc6d4 100644 --- a/src/Tribe/Aggregator/Cron.php +++ b/src/Tribe/Aggregator/Cron.php @@ -411,7 +411,7 @@ public function verify_fetching_from_service() { ], [ 'relation' => 'OR', - // if not specified then assume batch push is not supported + // If not specified then assume batch push is not supported. 'no-batch-push-support-specified' => [ 'key' => '_tribe_aggregator_allow_batch_push', 'value' => 'bug #23268', From 335638111b8dc5a3627f8c1244e748d8a0c3fce7 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Fri, 7 Feb 2020 10:35:29 -0600 Subject: [PATCH 19/22] Make sure organizer does not exists --- .../wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php index e86d948479..d736dfaf49 100644 --- a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php +++ b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php @@ -449,7 +449,7 @@ public function it_should_return_organizer_error_if_trying_to_insert_organizer_w 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'organizer' => 23, + 'organizer' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { From ad5e528a0cb3f284333fe6f80a623dcc5598c56a Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Fri, 7 Feb 2020 10:43:47 -0600 Subject: [PATCH 20/22] Make sure uses a non existing venue or organizer --- .../Tribe/Events/REST/V1/Endpoints/Single_EventTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php index d736dfaf49..aaba0d746c 100644 --- a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php +++ b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_EventTest.php @@ -190,7 +190,7 @@ public function it_should_return_venue_error_if_trying_to_insert_event_with_inva 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'venue' => 23, + 'venue' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { @@ -221,7 +221,7 @@ public function it_should_return_organizer_error_if_trying_to_insert_organizer_w 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'organizer' => 23, + 'organizer' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { @@ -415,7 +415,7 @@ public function it_should_return_venue_error_if_trying_to_insert_event_with_inva 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'venue' => 23, + 'venue' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { From 6f42e0bdcad91a5bb9c9072aaeede1af104a6343 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Fri, 7 Feb 2020 10:52:12 -0600 Subject: [PATCH 21/22] Make sure the ID of the organizer is not defined --- .../Events/REST/V1/Endpoints/Single_Event_SlugTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_Event_SlugTest.php b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_Event_SlugTest.php index e50afc93f3..65a7f74fc9 100644 --- a/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_Event_SlugTest.php +++ b/tests/wpunit/Tribe/Events/REST/V1/Endpoints/Single_Event_SlugTest.php @@ -199,7 +199,7 @@ public function it_should_return_venue_error_if_trying_to_insert_event_with_inva 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'venue' => 23, + 'venue' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { @@ -230,7 +230,7 @@ public function it_should_return_organizer_error_if_trying_to_insert_organizer_w 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'organizer' => 23, + 'organizer' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { @@ -437,7 +437,7 @@ public function it_should_return_venue_error_if_trying_to_insert_event_with_inva 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'venue' => 23, + 'venue' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { @@ -473,7 +473,7 @@ public function it_should_return_organizer_error_if_trying_to_insert_organizer_w 'description' => 'An event content', 'start_date' => 'tomorrow 9am', 'end_date' => 'tomorrow 11am', - 'organizer' => 23, + 'organizer' => PHP_INT_MAX, ]; foreach ( $params as $key => $value ) { From 9eaf274a6dfcafb3ca5a24a57381c929aac57b38 Mon Sep 17 00:00:00 2001 From: Crisoforo Gaspar Date: Fri, 7 Feb 2020 11:14:31 -0600 Subject: [PATCH 22/22] Use PHP_INT_MAX as record ID instead --- .../Importer/File_Importer_Events_BooleanFieldsTest.php | 2 +- tests/wpunit/Tribe/Events/REST/V1/Validator/BaseTest.php | 4 ++-- tests/wpunit/Tribe/Events/Validator/BaseTest.php | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/wpunit/Tribe/Events/Importer/File_Importer_Events_BooleanFieldsTest.php b/tests/wpunit/Tribe/Events/Importer/File_Importer_Events_BooleanFieldsTest.php index f12abfbde9..f8ab524a3a 100644 --- a/tests/wpunit/Tribe/Events/Importer/File_Importer_Events_BooleanFieldsTest.php +++ b/tests/wpunit/Tribe/Events/Importer/File_Importer_Events_BooleanFieldsTest.php @@ -20,7 +20,7 @@ public function boolean_fields() { } - /** + /**z * @test * it should not mark record as invalid if boolean field is missing * @dataProvider boolean_fields diff --git a/tests/wpunit/Tribe/Events/REST/V1/Validator/BaseTest.php b/tests/wpunit/Tribe/Events/REST/V1/Validator/BaseTest.php index af4a526376..79f3da010f 100644 --- a/tests/wpunit/Tribe/Events/REST/V1/Validator/BaseTest.php +++ b/tests/wpunit/Tribe/Events/REST/V1/Validator/BaseTest.php @@ -9,9 +9,9 @@ class BaseTest extends Events_Testcase { public function linked_post_bad_inputs() { return [ - [ 23 ], + [ PHP_INT_MAX ], [ 'foo' ], - [ '23' ], + [ "{PHP_INT_MAX}" ], [ [ 'website' => 'http://example.com' ] ], ]; } diff --git a/tests/wpunit/Tribe/Events/Validator/BaseTest.php b/tests/wpunit/Tribe/Events/Validator/BaseTest.php index 3d37defc27..215faebe2f 100644 --- a/tests/wpunit/Tribe/Events/Validator/BaseTest.php +++ b/tests/wpunit/Tribe/Events/Validator/BaseTest.php @@ -133,7 +133,7 @@ public function it_should_not_validate_an_empty_organizer_id_as_organizer() { public function it_should_not_validate_a_non_existing_post_id_as_organizer() { $sut = $this->make_instance(); - $this->assertFalse( $sut->is_organizer_id( 23 ) ); + $this->assertFalse( $sut->is_organizer_id( PHP_INT_MAX ) ); } /** @@ -260,7 +260,7 @@ public function test_is_organizer_id_list() { $this->assertTrue( $sut->is_organizer_id_list( implode( ', ', $organizers ) ) ); $this->assertTrue( $sut->is_organizer_id_list( implode( ' , ', $organizers ) ) ); $this->assertTrue( $sut->is_organizer_id_list( implode( ' ,', $organizers ) ) ); - $this->assertFalse( $sut->is_organizer_id_list( implode( ' ,', array_merge( $organizers, [ 23 ] ) ) ) ); + $this->assertFalse( $sut->is_organizer_id_list( implode( ' ,', array_merge( $organizers, [ PHP_INT_MAX ] ) ) ) ); } public function post_id_bad_inputs() { @@ -269,8 +269,8 @@ public function post_id_bad_inputs() { [ null ], [ false ], [ 'foo' ], - [ '23' ], - [ 23 ], + [ "{PHP_INT_MAX}" ], + [ PHP_INT_MAX ], [ 0 ], [ '0' ], ];