Problem/Motivation
In the settingsForm which is a child of ConfigFormBase, \Drupal\siteimprove\Form\SettingsForm::buildForm is not access config correctly for the settings form as described in Working with Configuration Forms
It is currently calling the configFactory to get the config. Which is fine except is it allow overriden values to be saved in config when depending on how these are set these could be loaded from secrets depending on the system.
Steps to reproduce
in yours settings.php add the following lines.
$config['siteimprove.settings']['api_username'] = 'xxx';
$config['siteimprove.settings']['api_key'] = 'xxx';
Proposed resolution
on the settings form on line 108 which is
$config = $this->configFactory->get('siteimprove.settings');
this needed to be changed to
$config = $this->config('siteimprove.settings');
Now the value which is stored in config will display and not the overriden values.
For the API checked we will need to get the values from the real config factory so that they can be run.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | after.png | 44.82 KB | keshavv |
| #4 | before.png | 46.7 KB | keshavv |
| #4 | settings.png | 40.72 KB | keshavv |
Issue fork siteimprove-3381807
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:
Comments
Comment #2
mukhtarm commentedplease review the patch
Comment #3
bhupendra_raykhere commentedComment #4
keshavv commentedConfirmed that the given solution works.
Updated settings.php file

Before patch

After patch

Comment #5
gordon commentedHi
Thanks for this but this change then breaks the republish check.
So in my clients case we override the user and api key and set these fields to ‘’ in config. So when loading this page it will do the checks with user and api key set to null were it should be using the overridden value.
There needs to be a change further down where the check will be done not using $config but using a a different version of the config using configFactory() and then the creds will be loaded correctly if they are overridden.
Thanks for the fast work.
Gordon.
Comment #6
gordon commentedHi,
I have changed this into an issue fork and added the necessary changes so that when it does the API check it will use the real config which includes any overrides.
Comment #8
gordon commentedI have removed the original patch.
Comment #9
bartvig commentedWe have a conflict with https://www.drupal.org/project/siteimprove/issues/3308624
In that issue, the code was specifically changed from
$config = $this->config('siteimprove.settings');to$config = $this->configFactory->get('siteimprove.settings');to allow config overrides.Comment #10
bartvig commentedOh, I think I understand the issue now. You don't want the overridden value to be displayed in the settings form, but the overridden should still be used in API calls.
Comment #12
tlyngej commented