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

Set defaults for new options on update #734

Merged
merged 3 commits into from
Jun 5, 2015

Conversation

wjhdev
Copy link

@wjhdev wjhdev commented Jun 1, 2015

This is a fix for #727, but in a way that we won't run into future problems when adding new defaults for options.

It took me a bit to figure out what was going on here, so here's a walkthrough for posterity.

  1. The options framework only saves default values to the database once (here). So if new items of the $option array are introduced, their default values never hit the database.
  2. During the update process, several functions interact with of_set_option() to update various settings. When this function is called, three things happen:
    • The array of options is grabbed from the database. The correct key is set to to the option value and the options framework saves this back to the database with a call to update_option().
    • Before hitting the database, WordPress applies a sanitization callback provided by the options framework called optionsframework_validate().
    • As part of this, any option of type checkbox or multicheck that is absent gets a default value of false. Since this list is originally from the database, it doesn't include any newly added options. Thus all newly added options of type checkbox or multicheck default to zero on the first of_set_option call.

The tl;dr is for the optionsframework to behave properly, the defaults have to be saved to the database. This pull request does that with the new update function largo_set_new_option_defaults(). This is the first function that runs because any other update functions that call of_set_option() will override the defaults.

@rnagle
Copy link

rnagle commented Jun 1, 2015

This looks great.

Pretty please, add a new test for the new function you added to update.php and we can merge this.

@wjhdev
Copy link
Author

wjhdev commented Jun 4, 2015

I added a stub function, but don't think there is a way to write a test currently as this relies on things the options framework does internally.

rnagle added a commit that referenced this pull request Jun 5, 2015
Set defaults for new options on update
@rnagle rnagle merged commit e9519e7 into develop Jun 5, 2015
@rnagle rnagle deleted the 727-social-links-sticky-footer branch July 13, 2015 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants