Options get_group() and get() return different values if using constants

[spenserhale]

Reason for change was Options::init()->get( 'sendgrid', 'api_key' ) and Options::init()->get_group( 'sendgrid' )[ 'api_key' ] returned different values if you were using constants.

So for our instant:
Options::init()->get( 'sendgrid', 'api_key' ) would return the string.
Options::init()->get_group( 'sendgrid' )[ 'api_key' ] would return null and throw undefined index.

So Sendgrid\Mailer::is_mailer_complete would return false even though api_key was setup with constant, since it used the get_group method.

I updated get_group to return defaults and constants, where before it only returned database options.

I also updated to always run apply_filters so developers can filter and return their own value when option is empty, because before filter only ran if group had database option.

Testing procedure

Tested saving admin.
Tested sending mail with external plugin.
Tested sending test mail with plugin.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
  • \WPMailSMTP\Providers\Sendgrid\Mailer::is_mailer_complete now returns correctly when using constants.
• Enhancement
  • \WPMailSMTP\Options::get_group now always runs wp_mail_smtp_options_get_group so developers can always filter and replace with their own values.
• Breaking change (fix or feature that would cause existing functionality to change)
  • \WPMailSMTP\Options::get_group now returns defaults and constants

Checklist:

Important notes on the breaking change, ideally should have no affects if developers are checking values. Only problem is if developers were checking the get_group array. Before would return empty array(), now always has some value.

Example:

<?php
// Before with no option data set
$group_values = Options::init()->get_group('sendgrid') // array();

empty($group_values); // true;

empty($group_values['api_key']); // true;

isset($group_values['api_key']); // false;


// After with no option data set
$group_values = Options::init()->get_group('sendgrid') // array( 'api_key' => '' );

empty($group_values); // false;

empty($group_values['api_key']); // true;

isset($group_values['api_key']); // true;

Also another important note:
There was a local variable, $options, that was being set in foreach but not used anywhere. The get_all() method above passes $options into apply_filters(), but this method instead passes $this->_options[ $group ] into apply_fitlers(). I wanted to know if the goal of plugin was to always update and use $_options instead of local $options. Because either get_all() should be updated to match this style or get_group() should be updated to match get_all()'s style.

[slaFFik]

The approach in get_all() is correct - no direct manipulation with the private _options property.
get_group() should NOT pass a private _options array into a filter - this is a direct manipulation, that can lead to issues later.

So inside get_group() $options = array() should be defined, used in filter and returned as a default fallback.

[slaFFik]

Also, thank you for fixing copy-paste error in Sendgrid/Mailer.php.

[slaFFik]

Minor changes required to not directly modify the class property _options.

[spenserhale]

@slaFFik Thank you for your feedback, I have made the requested changes wIth commit:
bafabfb.

[spenserhale]

@slaFFik I wanted to check in and see if we can get this pull request closed.

[slaFFik]

@spenserhale
I'm currently focusing on some other tasks for the upcoming WP Mail SMTP 1.5.0, but this fix will definitely be included in the next release.

[slaFFik]

This will be included into the next WP Mail SMTP release.

Thank you for your contribution!