Checkbox checked on activation #220
Conversation
|
|
||
| if ( Event::POST_TYPE !== $post->post_type || 1 !== intval( $gp_settings['pages']['post_or_event_date'] ) ) { | ||
| if ( Event::POST_TYPE !== $post->post_type || 1 !== intval( $event_date_format ) ) { |
There was a problem hiding this comment.
make $post->post_type get_post_type( $post )
| @@ -78,6 +78,8 @@ protected function setup_hooks() { | |||
| * @return void | |||
| */ | |||
| public function activate_gatherpress_plugin() { | |||
| $gp_settings['general']['post_or_event_date'] = true; | |||
| add_option( 'gp_general', $gp_settings ); | |||
There was a problem hiding this comment.
You're gonna have to do better than that. Does it matter if it works? It's only going to run once on initial activation.
There was a problem hiding this comment.
$gp_settings doesn't appear to be defined here is what I mean.
There was a problem hiding this comment.
It is defined. How would you do it?
There was a problem hiding this comment.
?
you are setting $gp_settings['general']['post_or_event_date'] = true; and you did not define $gp_settings in the method. You need to set something like $gp_settings = get_option( 'gp_general' ); first and also check that it's an array and such so you don't throw any notices.
| @@ -78,6 +78,8 @@ protected function setup_hooks() { | |||
| * @return void | |||
| */ | |||
| public function activate_gatherpress_plugin() { | |||
There was a problem hiding this comment.
| public function activate_gatherpress_plugin() { | |
| public function activate_gatherpress_plugin() { | |
| $gp_settings = []; |
To @mauteri 's point, the variable should be defined(Or I think you'll see a bunch of undefined offset warnings in the logs) and in this case, I think it just needs a default value. I'm pretty sure get_option( 'gp_general' ) won't work at this point because this happens on plugin activation. So I think just setting the default to an empty array is the best solution here.
There was a problem hiding this comment.
Ok got it... I don't think this should be on activation hook as new options will then require the same sort of treatment and won't be backwards compatible. This should use the default if not saved to the database. I can take a look at this later and make a recommendation.
There was a problem hiding this comment.
@jmarx @mauteri The variable is set on activation. No errors/warnings/notices are thrown. Don't need to declare en empty array. Just set what you can. The other settings can't be set until the pages exist and will require that the settings page is visited and options updated. The checkbox, however, will be checked and then saved again when the other settings are saved.
|
@mauteri This looks great. I appreciate the cleanup work to check for settings or set variable to empty. Can you merge? |
Sure, it is a different PR, so going to close this one. When I checked out the branch it created a new PR. |
Also placed in its own section on initial settings page