Make WordPress Core

Opened 9 years ago

Closed 7 years ago

#30448 closed enhancement (invalid)

Customizer Manager/Settings classes do not provide public save method to use outside of customize.php

Reported by: crazyjaco's profile CrazyJaco Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

I'm making a customizer setting exporter/importer. Trying to do it by instantiating the Customizer Manager and then looping through each of the settings.

When I want to save in the import, there are no public save methods in the Customizer Setting class that can be used outside the context of the customize.php interface.

Please create a method or mechanism in the customizer setting class (wp-includes\class-wp-customize-setting.php) to facilitate saving settings outside the context of customize.php.

I'd like to help if possible.

Thanks

Attachments (3)

30448-customizer-patch.diff (824 bytes) - added by CrazyJaco 8 years ago.
New filter in customizer setting class.
30448-customizer-patch-2.diff (615 bytes) - added by CrazyJaco 8 years ago.
Add parameter to setting->save() method
30448-customizer-patch-3.diff (548 bytes) - added by CrazyJaco 8 years ago.
Making update function public

Download all attachments as: .zip

Change History (19)

#1 @ocean90
9 years ago

  • Keywords needs-patch added
  • Type changed from feature request to enhancement
  • Version changed from trunk to 4.0

@CrazyJaco
8 years ago

New filter in customizer setting class.

#2 follow-up: @CrazyJaco
8 years ago

I was able to do what I needed with just this added filter rather than a whole new public function.

Here is a snippet of code I used in a plugin to use this new filter.

add_filter( 'customize_save_value', array( $this, 'filter_customize_save_value' ), 10, 2 );
$wp_setting->save();
remove_filter( 'customize_save_value', array( $this, 'filter_customize_save_value' ) );

Feedback would be appreciated.

Thank you.

#3 @CrazyJaco
8 years ago

  • Keywords has-patch added; needs-patch removed

#4 @westonruter
8 years ago

If a new filter is to be added, then I think it should actually be located within WP_Customize_Setting::update() instead. This is what the save method calls anyway, and it is what is actually responsible for persisting the value. So the filter could then be customize_pre_update, to be analogous with the pre_update_option filter.

#5 in reply to: ↑ 2 ; follow-up: @westonruter
8 years ago

Replying to CrazyJaco:

I was able to do what I needed with just this added filter rather than a whole new public function.

What are you trying to do, exactly? What does your filter_customize_save_value function do?

Also, instead of adding a new filter to change the value being saved, I think what you actually want is to allow the WP_Customize_Setting::save() method to allow an argument to be supplied, whereas right now it always gets its value from $_POST. So you could do this:

foreach ( $wp_customize->settings() as $setting ) {
    $setting->save( $my_override_values[ $setting->id ] );
}

When a value is passed in to the save method, then it could bypass the call to $this->post_value().

#6 in reply to: ↑ 5 @CrazyJaco
8 years ago

Replying to westonruter:

What are you trying to do, exactly? What does your filter_customize_save_value function do?

Also, instead of adding a new filter to change the value being saved, I think what you actually want is to allow the WP_Customize_Setting::save() method to allow an argument to be supplied, whereas right now it always gets its value from $_POST. So you could do this:

foreach ( $wp_customize->settings() as $setting ) {
    $setting->save( $my_override_values[ $setting->id ] );
}

When a value is passed in to the save method, then it could bypass the call to $this->post_value().

I agree. That is a much better approach.

As for what I'm trying to do:
I have a plugin that takes all the existing customizer settings for a theme and exports them to json.
In a second plugin that I install on a different blog/server/etc, it reads that json and saves those settings to the new blog.

A pretty basic export/import process.

The callback function just returns the value of the setting being saved from the imported data.

I'll update the patch for the new approach.

Thank you for the feedback! Its greatly appreciated.

@CrazyJaco
8 years ago

Add parameter to setting->save() method

#7 @CrazyJaco
8 years ago

New patch attached.

Passing in parameter to save().
If parameter is populated, bypass post_value() call.

This ticket was mentioned in Slack in #core-customize by crazyjaco. View the logs.


8 years ago

#9 @westonruter
8 years ago

For the work I've been doing on #30937 for Customizer Transactions, I've also ended up modifying the WP_Customize_Setting::save() method: https://github.com/xwp/wordpress-develop/pull/61/files#diff-8582bb972a2a8283eebe2050f95f3c19L170

In my change, the settings are pre-saved in the transaction and the transaction does a capability check itself to ensure that the user is authorized to update the setting before storing the value in the transaction: https://github.com/xwp/wordpress-develop/pull/61/files#diff-4fb8a477f559bdfad2c1e6db6d1c8b06R999

If my patch were to be applied, then the way you could programmatically update a setting would be:

<?php
$setting = new WP_Customize_Setting( 'whatever_the_setting_name_is', $args );
$value = get_value_from_wherever();
$wp_customize->transaction->set( $setting, $value );
$setting->save();

But this still seems a bit round-about, and the WP_Customize_Setting::save() method would ideally take an argument which would override what is currently in the post_value (or transaction_value).

My concern now with the changes to the save method to remove the capability check is whether existing plugins would by relying on there being capability checks in the method to ensure proper authorization. I wonder if a better approach might be to instead just make the update method public instead of private so that plugins would be able to access that method? That would be a patch that could go in now without waiting for a decision on transactions.

@CrazyJaco
8 years ago

Making update function public

#10 @CrazyJaco
8 years ago

New patch uploaded. Simply changes the update function from protected to public.

#11 @westonruter
8 years ago

I just realized a big problem with what I had suggested above. In PHP, a parent class can have a protected method and a child class can override the method with a public one. But if a parent class has a public method then the child class must also define that method as public. So here's the problem: there are existing subclasses of WP_Customize_Setting that may define update as protected, as opposed to public. Although the existing subclasses of WP_Customize_Setting, do indeed for some reason use public instead of protected. But if any subclass retains the protected visibility, then a fatal error will be triggered:

<?php

class WP_Customize_Setting {
        public function update() { // before this was protected
                // ...
        }
}
class WP_Customize_Subsetting extends WP_Customize_Setting {
        protected function update() { // PHP Fatal error:  Access level to WP_Customize_Subsetting::update() must be public (as in class WP_Customize_Setting)
                // ...
        }
}

So the question is this: is anyone subclassing WP_Customize_Setting and defining update as protected? If so, we may need to figure out another alternative solution.

#12 follow-up: @CrazyJaco
8 years ago

How can I tell if anyone is subclassing the WP_Customize_Setting class? Would we need to do a search of the plugin repository?

#13 in reply to: ↑ 12 @westonruter
8 years ago

Replying to CrazyJaco:

How can I tell if anyone is subclassing the WP_Customize_Setting class? Would we need to do a search of the plugin repository?

Looking at all plugins in the repo would be a good start, but it is certainly not exhaustive. There many instances which may not be published at all. I think what will have to be done is introduce a new method entirely, or wait for Customizer transactions to land so that you can do what I describe in comment:9 above.

#14 @CrazyJaco
8 years ago

Are Transactions in a featured plugin? I could install it and do some testing with your aforementioned approach. I'd rather not introduce unnecessary code if what is needed can be accomplished via a different (forthcoming) method.

#15 @westonruter
8 years ago

Transactions (#30937) are currently in an alpha patch, currently available on a GitHub fork and in a PR there: https://github.com/xwp/wordpress-develop/pull/61

There are too many low-level changes for Customizer transactions to be bundled up as a feature plugin. If you can check out the branch from GitHub, you can do your testing on it.

#16 @westonruter
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

@CrazyJaco The most basic way to do this is just to call $wp_customize->set_post_value( $setting_id, $setting_value ) and then do $wp_customize->get_setting( $setting_id )->save(). Note that in doing this you will need to make sure you have set the current user to one who can do the setting's capability.

There is a better approach now with changesets which allows you to get access to validation errors and also to ensure that the update is transactional (that all get saved rather than only some):

<?php
$manager = new WP_Customize_Manager();
do_action( 'customize_register', $manager );
foreach ( $exported as $setting_id => $setting_value ) {
    $manager->set_post_value( $setting_id, $setting_value );
}
$manager->save_changeset_post( array( 'status' => 'publish' ) );

This is similar to what is done now with importing starter content in 4.7 via WP_Customize_Manager::import_theme_starter_content().

Let me know how you go with that.

Note: See TracTickets for help on using tickets.