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.

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

edd_delete_option() does not work #5647

Closed
joelworsham opened this issue Apr 7, 2017 · 2 comments · Fixed by #6001
Closed

edd_delete_option() does not work #5647

joelworsham opened this issue Apr 7, 2017 · 2 comments · Fixed by #6001

Comments

@joelworsham
Copy link

This function unsets the key you pass and then attempts to update the new settings array to the database.

// First let's grab the current settings
$options = get_option( 'edd_settings' );

// Next let's try to update the value
if( isset( $options[ $key ] ) ) {

	unset( $options[ $key ] );

}

$did_update = update_option( 'edd_settings', $options );

This is all fine and well, but the issue is in updating the option. edd_settings_sanitize() is hooked into updating this option and there's a line in there that is problematic:

// Merge our new settings with the existing
$output = array_merge( $edd_options, $input );

The issue is that the global $edd_options used here still contains the value we're trying to delete. So by merging, we're simply adding that value back into the settings. This completely overrides what edd_delete_option() is trying to do. It seems that the simple fix is to unset this key from the global within edd_delete_option(). So like this:

global $edd_options;

// First let's grab the current settings
$options = get_option( 'edd_settings' );

// Next let's try to update the value
if( isset( $options[ $key ] ) ) {

	unset( $options[ $key ] );

}

if( isset( $edd_options[ $key ] ) ) {

        unset( $edd_options[ $key ] );

}

$did_update = update_option( 'edd_settings', $options );
@cklosowski
Copy link
Contributor

Quick way to test this out with this code in console:

<?php
edd_update_option( 'testing_setting', 'this is a test' );
var_dump(edd_get_option( 'testing_setting' ));
edd_delete_option( 'testing_setting' );
var_dump(edd_get_option( 'testing_setting' ));

On master you'll see:
this is a test twice

On issue/5647 you'll see:
this is a test
false

cklosowski added a commit that referenced this issue Sep 11, 2017
cklosowski added a commit that referenced this issue Sep 11, 2017
@SeanTOSCD
Copy link
Contributor

Replicated bug on master. Confirmed fixed on issue/5674. 👍

cklosowski added a commit that referenced this issue Sep 11, 2017
* Unset the option being deleted in edd_delete_option #5647

* Adding unit test #5647

* Remove debug code #5647
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants