Inconsistent MS API: delete_site_option action hook is only called AFTER deletion, unlike delete_option

[danielnashnz]

Unlike the delete_option action hook, the delete_site_option action hook is only called AFTER deletion of the row from the sitemeta table - making it difficult to capture the expiring value (for instance), unless you know its precise option key name ( and hook it using pre_delete_site_option_{$option} ).

Suggest introducing a pre-deletion, non-specific action hook call named "pre_delete_option" for options and "pre_delete_site_option" for site options, for consistency.

[scribu]

What a mess. 20651.diff​:

• add 'pre_delete_site_option' and call it only if the option exists
• add 'pre_delete_option_{$option}'

[scribu]

Similar efforts: [13148]

[danielnashnz]

To keep it consistent with delete_option and delete_site_option, shouldn't $option be passed as well in the do_action calls?

[scribu]

Obviously, 'pre_delete_site_option' is useless if we don't pass the $option. Sergey already submitted a fixed patch.

[jeremyfelt]

20651.3.diff​ is a refresh of Sergey's patch for current trunk. Moving to 3.7 to add the action or close the ticket.

[nacin]

Option 1:

• I wonder if we could just break things slightly here, move the delete_site_option hook to be a pre-delete hook, introduce a new deleted_site_option hook, and just ignore that pre_delete_site_option_$option exists.

Option 2:

• This isn't just in delete_site_option(). The same pattern exists in the other site_option() functions. While this pattern differs from the regular option() functions, it is at least consistent across the site_option API. Maybe wontfix? If/when we rename these to network_option we could use the other hook pattern.

[jeremyfelt]

Beyond pattern, the weird parts here are:

1. pre_site_delete_option_$option runs even if delete_site_option() is run in single site mode.
2. The value of $option is required in advance to use pre_site_delete_option_$option

I think a patch is worth it to fix at least the pass of $option to the action, which is the original description of this ticket. See 20651.4.diff​

In the long run, I think Nacin's option 2 is probably best. In some ways I find the pattern of the hooks in the site_option functions to be more intuitive.

[nacin]

20651.4.diff​ is OK. Again, though, add_site_option() and update_site_option() similarly follow the existing pattern and should also thus get new pre_*_site_option, $option hooks.

[jeremyfelt]

Replying to nacin:

Maybe wontfix? If/when we rename these to network_option we could use the other hook pattern.

I'm leaning this way now. We're adding at least 3 hooks (if not another for get_site_option()), that will end up being misnamed in the future.

[jeremyfelt]

Closing as wontfix due to varying parts of the discussion. If we add a new pre_site_delete_option filter, then we would want to make sure the pattern is consistent across all *_site_option() functions. In the future, we'd like to have *_network_option() and we would be stuck with maintaining more confusing names.