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

Jump to bottom

Fix: Remove old records #1076

Closed
wants to merge 12 commits into from
Closed

Fix: Remove old records #1076

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
e98850c
Don’t restrict where the purge can run
kasparsd Mar 25, 2020
15128b1
This should depend on the plugin state instead
kasparsd Mar 25, 2020
64f4ac3
Use the helper for fetching the plugin settings
kasparsd Mar 25, 2020
e67e1ee
Describe why we might bail early
kasparsd Mar 25, 2020
85b1815
Introduce helpers for getting certain settings
kasparsd Mar 25, 2020
342e664
Use the new helpers instead
kasparsd Mar 25, 2020
9418eff
Simplify the timestamp offset generation
kasparsd Mar 25, 2020
69e0906
Per coding standards
kasparsd Mar 25, 2020
3444d85
Use the API instead
kasparsd Mar 25, 2020
2336970
This is already handled by the methods calling is_plugin_active_for_n…
kasparsd Mar 25, 2020
d874487
Use the API in tests too
kasparsd Mar 25, 2020
58bf268
Test that fresh records are still there
kasparsd Mar 25, 2020
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
36 changes: 6 additions & 30 deletions classes/class-admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,6 @@ public function __construct( $plugin ) {

add_action( 'init', array( $this, 'init' ) );

// Ensure function used in various methods is pre-loaded.
if ( ! function_exists( 'is_plugin_active_for_network' ) ) {
require_once ABSPATH . '/wp-admin/includes/plugin.php';
}

// User and role caps.
add_filter( 'user_has_cap', array( $this, 'filter_user_caps' ), 10, 4 );
add_filter( 'role_has_cap', array( $this, 'filter_role_caps' ), 10, 3 );
Expand Down Expand Up @@ -686,36 +681,17 @@ public function purge_schedule_setup() {
public function purge_scheduled_action() {
global $wpdb;

// Don't purge when in Network Admin unless Stream is network activated
if (
is_multisite()
&&
is_network_admin()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why is_network_admin() would need to be checked on cron runs.

&&
! $this->plugin->is_network_activated()
) {
return;
}

if ( is_multisite() && $this->plugin->is_network_activated() ) {
$options = (array) get_site_option( 'wp_stream_network', array() );
} else {
$options = (array) get_option( 'wp_stream', array() );
}
$days = $this->plugin->settings->records_ttl();

if ( ! empty( $options['general_keep_records_indefinitely'] ) || ! isset( $options['general_records_ttl'] ) ) {
// Ensure we don't want to keep the records indefinitely.
if ( $this->plugin->settings->keep_records_indefinitely() || empty( $days ) ) {
return;
}

$days = $options['general_records_ttl'];
$timezone = new DateTimeZone( 'UTC' );
$date = new DateTime( 'now', $timezone );

$date->sub( DateInterval::createFromDateString( "$days days" ) );

$where = $wpdb->prepare( ' AND `stream`.`created` < %s', $date->format( 'Y-m-d H:i:s' ) );
$timestamp = strtotime( sprintf( '%d days ago', $days ) );
$where = $wpdb->prepare( ' AND `stream`.`created` < %s', gmdate( 'Y-m-d H:i:s', $timestamp ) );

// Multisite but NOT network activated, only purge the current blog
// Multisite but NOT network activated, only purge the current blog.
if ( is_multisite() && ! $this->plugin->is_network_activated() ) {
$where .= $wpdb->prepare( ' AND `blog_id` = %d', get_current_blog_id() );
}
Expand Down
42 changes: 35 additions & 7 deletions classes/class-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,7 @@ public function add_display_name_search_columns( $search_columns, $search, $quer
public function get_option_key() {
$option_key = $this->option_key;

$current_page = wp_stream_filter_input( INPUT_GET, 'page' );

if ( ! $current_page ) {
$current_page = wp_stream_filter_input( INPUT_GET, 'action' );
}

if ( 'wp_stream_network_settings' === $current_page ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the network settings if the plugin is network-activated. Is there a situation we would like to use network settings when the plugin is not network activated?

if ( $this->plugin->is_network_activated() ) {
$option_key = $this->network_options_key;
}

Expand Down Expand Up @@ -1167,4 +1161,38 @@ public function get_settings_translations( $labels ) {

return $labels;
}

/**
* Get the record TTL sanitized.
*
* @return integer|null Returns the time-to-live in seconds or null if empty or not set.
*/
public function records_ttl() {
$options = $this->get_options();

if ( isset( $options['general_records_ttl'] ) ) {
$ttl = abs( $options['general_records_ttl'] );

if ( $ttl > 0 ) {
return $ttl;
}
}

return null;
}

/**
* Should the records be stored indefinitely.
*
* @return boolean
*/
public function keep_records_indefinitely() {
$options = $this->get_options();

if ( ! empty( $options['general_keep_records_indefinitely'] ) ) {
return true;
}

return false;
}
}
30 changes: 18 additions & 12 deletions tests/tests/test-class-admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ public function test_construct() {
$this->assertNotEmpty( $this->admin->plugin );
$this->assertInstanceOf( '\WP_Stream\Plugin', $this->admin->plugin );

$this->assertTrue( function_exists( 'is_plugin_active_for_network' ) );

if ( is_multisite() && is_plugin_active_for_network( $this->plugin->locations['plugin'] ) && ! is_network_admin() ) {
if ( is_multisite() && $this->admin->plugin->is_network_activated() && ! is_network_admin() ) {
$this->assertTrue( $this->admin->disable_access );
} else {
$this->assertFalse( $this->admin->disable_access );
Expand Down Expand Up @@ -268,8 +266,8 @@ public function test_purge_schedule_setup() {
}

public function test_purge_scheduled_action() {
// Set the TTL to one day
if ( is_multisite() && is_plugin_active_for_network( $this->plugin->locations['plugin'] ) ) {
// Set the TTL to one day.
if ( $this->admin->plugin->is_network_activated() ) {
$options = (array) get_site_option( 'wp_stream_network', array() );
$options['general_records_ttl'] = '1';
update_site_option( 'wp_stream_network', $options );
Expand All @@ -281,29 +279,37 @@ public function test_purge_scheduled_action() {

global $wpdb;

// Create (two day old) dummy records
// Create a fresh stream record that shouldn't be deleted.
$stream_data = $this->dummy_stream_data();
$stream_data['created'] = date( 'Y-m-d h:i:s' );
$wpdb->insert( $wpdb->stream, $stream_data );
$fresh_stream_id = $wpdb->insert_id;
$this->assertNotFalse( $fresh_stream_id );

// Create (two day old) dummy records.
$stream_data = $this->dummy_stream_data();
$stream_data['created'] = date( 'Y-m-d h:i:s', strtotime( '2 days ago' ) );
$wpdb->insert( $wpdb->stream, $stream_data );
$stream_id = $wpdb->insert_id;
$this->assertNotFalse( $stream_id );

// Create dummy meta
// Create dummy meta.
$meta_data = $this->dummy_meta_data( $stream_id );
$wpdb->insert( $wpdb->streammeta, $meta_data );
$meta_id = $wpdb->insert_id;
$this->assertNotFalse( $meta_id );

// Purge old records and meta
// Purge old records and meta.
$this->admin->purge_scheduled_action();

// Check if the old records have been cleared
$fresh_stream_results = $wpdb->get_row( "SELECT * FROM {$wpdb->stream} WHERE ID = $fresh_stream_id" );
$this->assertNotEmpty( $fresh_stream_results, 'fresh record from now is still there' );

$stream_results = $wpdb->get_row( "SELECT * FROM {$wpdb->stream} WHERE ID = $stream_id" );
$this->assertEmpty( $stream_results );
$this->assertEmpty( $stream_results, 'Old records have been cleared' );

// Check if the old meta has been cleared
$meta_results = $wpdb->get_row( "SELECT * FROM {$wpdb->streammeta} WHERE meta_id = $meta_id" );
$this->assertEmpty( $meta_results );
$this->assertEmpty( $meta_results, 'Old meta has been cleared' );
}

public function test_plugin_action_links() {
Expand Down