Problem/Motivation

Initially saved form configurations not updating when changed again

Steps to reproduce

  1. Save Settings for the first time
  2. Try to update debug and debug log fields, for example
  3. Save
  4. values not changed

Proposed resolution

Update in BuildForm method:
this line: $config = $this->configFactory()->get('phpmailer_smtp.settings');

to be this: $config = $this->config('phpmailer_smtp.settings');

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 fix-settings-phpmailer-smtp-form-3292302-2.patch1.83 KBvlyalko
#2 fix-settings-phpmailer-smtp-form-3292302.patch535 bytesvlyalko

Comments

vlyalko created an issue. See original summary.

vlyalko’s picture

vlyalko commented
StatusFileSize
new fix-settings-phpmailer-smtp-form-3292302.patch535 bytes
vlyalko’s picture

vlyalko commented
StatusFileSize
new fix-settings-phpmailer-smtp-form-3292302-2.patch1.83 KB

Settings were still not respected and debugging was still printing when email is sent. Made a few more changes in the PhpMailerSmtp.php to get the correct config values.

imclean’s picture

imclean commented
Status: Active » Postponed (maintainer needs more info)

I can't replicate the problem with 2.0.5 or 2.1.0. 

+++ b/src/Form/SettingsForm.php
@@ -94,7 +94,7 @@ class SettingsForm extends ConfigFormBase {
   public function buildForm(array $form, FormStateInterface $form_state) {
     // Get immutable config.
-    $config = $this->configFactory()->get('phpmailer_smtp.settings');
+    $config = $this->config('phpmailer_smtp.settings');

This change isn't necessary as the form is only being populated here, not saving the values, so it doesn't need to be writable.

I'm not sure what your changes to /src/Plugin/Mail/PhpMailerSmtp.php do either.

Can you try a fresh install of Drupal with only PHPMailer SMTP installed. You don't need the mailsystem module to send a test email.

Additional information may be useful if that doesn't work. What versions of PHP and Drupal are you using? What hosting environment (web server etc.).

vlyalko’s picture

vlyalko commented

Hi imclean,
First, thank you for the great module. Great alternative for the symfony_mailer.

This $config = $this->configFactory()->get('phpmailer_smtp.settings'); does not get the updated config value. WHat ever value was set the first time the form was saved remains. This way $config = $this->config('phpmailer_smtp.settings'), i get the new updated config value.

In this plugin $config = $this->config('phpmailer_smtp.settings');, it also not getting the correct config value for me. That is why i had to make that change. Now, with the above changes, I am getting the correct values and debug information not being printed is disabled. (this was my issue. Have not checked other config values)

I will try on the fresh install without the patch and will let you know.

imclean’s picture

imclean commented

Have you got any related config overrides in settings.php or elsewhere? It's possible these are being loaded into the active configuration. If so, this is indeed a bug which which needs to be fixed here.

imclean’s picture

imclean commented

The form class should probably implement getEditableConfigNames(). 

  /**
   * {@inheritdoc}
   */
  protected function getEditableConfigNames() {
    return [
      'phpmailer_smtp.settings',
    ];
  }
imclean’s picture

imclean commented

Ignore #7, the method is already present.

imclean’s picture

imclean commented
Status: Postponed (maintainer needs more info) » Needs work

I can see how the patch in #2 would help, but I'm not sure why the changes in /src/Plugin/Mail/PhpMailerSmtp.php are necessary.

  1. +++ b/src/Plugin/Mail/PhpMailerSmtp.php
@@ -190,7 +190,7 @@ class PhpMailerSmtp extends PHPMailer implements MailInterface, ContainerFactory
 
-    $this->drupalDebug = $this->config->get('smtp_debug', 0);
+    $this->drupalDebug = $this->config->getRawData()['smtp_debug'];
     if ($this->drupalDebug > $this->SMTPDebug && \Drupal::currentUser()->hasPermission('administer phpmailer smtp settings')) {

    What values do you get before and after this change? 

  2. +++ b/src/Plugin/Mail/PhpMailerSmtp.php
@@ -222,7 +222,7 @@ class PhpMailerSmtp extends PHPMailer implements MailInterface, ContainerFactory
         $this->messenger->addMessage(Markup::create($this->drupalDebugOutput));
-        if ($this->config->get('smtp_debug_log', 0)) {
+        if ($this->config->getRawData()['smtp_debug_log'] ) {
           $this->loggerFactory->get('phpmailer_smtp')->debug('Output of communication with SMTP server:<br /><pre>{output}</pre>', [

    What values do you get before and after this change?

Just to confirm for my testing, are you using config overrides?

vlyalko’s picture

vlyalko commented

Hi imclean,
because $this->config->get('smtp_debug_log', 0)) should have given me the correct value, but it did not.
and this $this->config->getRawData()['smtp_debug_log'] did give me the correct updated most recent value.

imclean’s picture

imclean commented

@vlyalko, can you be more specific?

  1. Are you using config overrides?
  2. What exact value does $this->config->get('smtp_debug_log', 0) return and where does it get the value from? (e.g. 0, 1, 2 etc.)
  3. What value does $this->config->getRawData()['smtp_debug_log'] return and where does it get the value from?
  4. What version of Drupal are you using?
  5. What PHP version are you using?
imclean’s picture

imclean commented
Status: Needs work » Postponed (maintainer needs more info)
imclean’s picture

imclean commented

From the config override docs I can see getting mutable config avoids overrides, which is what might be causing a problem here.

  • imclean committed 58c79b2 on 2.0.x 
    Issue #3292302 by vlyalko: phpmailer_smtp form settings not updating

  • imclean committed f0e1a9a on 2.1.x 
    Issue #3292302 by vlyalko: phpmailer_smtp form settings not updating
imclean’s picture

imclean commented
Status: Postponed (maintainer needs more info) » Needs review

@vlyalko can you please try the dev version?

imclean’s picture

imclean commented
Status: Needs review » Fixed
imclean’s picture

imclean commented
Status: Fixed » Closed (fixed)