Problem/Motivation

In all places throughout the module where the 'spammaster.settings' need to be used, they are loaded as editable config, for example:

$spammaster_settings = $this->configFactory->getEditable('spammaster.settings');

In most places, subsequent code is only calling the get method for individual values and never setting or saving the configuration. As a result, if you want to override either the license key or subtype settings for a specific environment in the settings.php file you can't, because editable configuration does not respect those overrides.

I consider this a bug because it's not really a best practice for Drupal modules given there are many use-cases for overriding configuration in the settings.php file. The only time it should be loaded as editable is for the configuration form (which is automatically loaded as editable) or in any part of the code that needs to programmatically modify the settings and save them. For this module that makes perfect sense in the install hook where it uses the API to generate a license, or in the updater service where it sets the license key and saves it again.

In all other places that use the config settings in a read-only fashion, it should call $this->configFactory->get('spammaster.settings'); to fetch immutable config that respects any overrides in the settings.php file.

Proposed resolution

Modify all the calls to getEditable on the config factory for spammaster.settings and spammaster.settings_protection to us "get" instead, where it is not setting any values and saving the configuration.

Issue fork spammaster-3541464

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

teknocat created an issue. See original summary.

teknocat’s picture

teknocat commented
Issue summary: View changes

teknocat opened merge request !5

pedro-alves made their first commit to this issue’s fork.

  • pedro-alves committed d9f550cf on 8.x-2.x authored by teknocat 
    Issue #3541464 by teknocat: Spammaster settings cannot be overridden via...
pedro-alves’s picture

pedro-alves commented
Assigned: Unassigned » pedro-alves

Hi @teknokat

Thanks for the commit. Yes, absolutely right.
There's an issue with the cleanup service not related to your commit. Currently trying to store these as state when in fact they are editable

        // Store editable values.
        $this->configFactory->getEditable('spammaster.settings_protection')
          ->set('spammaster.spam_master_is_cloudflare', '0')
          ->set('spammaster.signature', '1')
          ->save();

Not a major issue, we will fix this soon.

pedro-alves’s picture

pedro-alves commented
Status: Active » Fixed
pedro-alves’s picture

pedro-alves commented
Status: Fixed » Closed (fixed)