Make WordPress Core

Opened 6 years ago

Last modified 22 months ago

#44042 new enhancement

Hook for ‘delete_option’ behaviour required

Reported by: robertodonpedro's profile RobertoDonPedro Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 1.2
Component: Options, Meta APIs Keywords: good-first-bug has-patch dev-feedback has-unit-tests
Focuses: Cc:

Description

Hi, I posted this about one month ago in the wordpress support forum: https://wordpress.org/support/topic/hook-for-delete_option-behaviour-required/

I did not receive any answers there but referred to this forum, so I post it here again:

Hi, I would need to prevent and change the deletion of certain options by the WP core function ‘delete_option’.
There is a hook

do_action( 'delete_option', $option );

available here: https://core.trac.wordpress.org/browser/tags/4.9.4/src/wp-includes/option.php#L532

But this does neither provide a way to exit the delete_option function before the option is deleted nor to change the option name to be deleted. In fact this existing hook seems to be pretty useless.
Therefore I would need a filter in the first line of the delete_option core function like

$option = apply_filters( 'delete_option_name', $option );

.
Or change the line 535 from

$option = trim( $option );

to

$option = trim( apply_filters( 'delete_option_name', $option ));

Any chances to get this into core immediately?

Thx, Robert

Attachments (3)

44042.diff (505 bytes) - added by sebastien@… 6 years ago.
44042.1.diff (1.2 KB) - added by palmiak 6 years ago.
Patch with added documentation
44042.2.diff (1.9 KB) - added by donmhico 5 years ago.
Changed the new filter name to pre_delete_option and added unit tests.

Download all attachments as: .zip

Change History (12)

#1 @danieltj
6 years ago

  • Component changed from General to Options, Meta APIs
  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement
  • Version set to 1.2

Hi @RobertoDonPedro, welcome to Core Trac.

It won't be possible to add this to Core straight away, but after reviewing the other similar functions such as add_option and update_option, they do both have methods are bailing out early through a filter.

I think this is a good idea and something worth exploring considering what is already available within Core, it would definitely bring the delete_option inline with the others.

I've tagged this as good-first-bug because I think this would be quite a nice one for someone new to have a go at.

Thanks!

@sebastien@…
6 years ago

#2 @sebastien@…
6 years ago

  • Keywords has-patch added; needs-patch removed

#3 @sebastien@…
6 years ago

  • Keywords dev-feedback added

#4 @RobertoDonPedro
6 years ago

Hi, it seams that the changes did not make it into 4.9.6.
Any reasons why? Is there something I need to do?
Thx, Robert

@palmiak
6 years ago

Patch with added documentation

#5 @kkarpieszuk
6 years ago

@RobertoDonPedro in case it never is applied, you can use existing action to achieve what you want:

  • get option into a variable which you know will be deleted in the moment
  • modify it as you want
  • add new option from a modified variable, if name is the same, the best will be to add it again when it is actually deleted (in deleted_option action, you can store variable as static to pass between actions)

So this is also a thing for code maintainers if it really must be merged into the code (still, it is better to add filter like this to modify option upfront than run additional SQL queries to delete and put option back in DB)

#6 @RobertoDonPedro
6 years ago

@kkarpieszuk My software is a highly available virtualization layer, therefore I cannot accept a solution where a required option gets deleted, even if it is just for a fraction of a second.

And I really do not understand why it is so hard to get this hook into core. The source is available yet, it does no harm and above this, it is completely in line with the already available hooks for add_option and update_option...

#7 @kkarpieszuk
6 years ago

Bear in mind I am not a core developer here :) That was just my side note how you could achieve it while waiting for merge :)

@donmhico
5 years ago

Changed the new filter name to pre_delete_option and added unit tests.

#8 @donmhico
5 years ago

  • Keywords has-unit-tests added

#9 @yannielsen
22 months ago

I read and tested @donmhico patch and it works and looks to me as a viable solution.

Note: See TracTickets for help on using tickets.