Problem/Motivation

1. AddressProvider plugin config is unnecessarily being serialized and un-serialized

It is difficult to override config for an AddressProvider plugin when the configuration is serialized.

Instead of:

$config['address_autocomplete.settings']['google_maps']['api_key'] = 'foo';

One must specify a serialized array:

$config['address_autocomplete.settings']['google_maps'] = 'a:2:{s:9:"plugin_id";s:11:"google_maps";s:7:"api_key";s:3:"foo";}';

Additionally, it does not appear that the serialization and un-serialization of the plugin config is actually necessary.

2. Immutable (potentially overridden) config values are displayed in config admin screens for edit

This can lead to confusion and is not standard practice in module config forms.

3. We appear to be lacking a configuration schema file for our module settings

Proposed resolution

- Add a configuration schema file
- Write an update hook to un-serialize and re-save the AddressProvider plugin configuration and remove the serialization and un-serialization steps when saving and loading config.
- Display mutable/editable config values in module configuration forms

Remaining tasks

User interface changes

DX: Easier / standard configuration overrides, easier to read config in config files.
UX: Less confusion in settings forms when configuration overrides are present.

API changes

Data model changes

CommentFileSizeAuthor
#4 address_autocomplete-config-issues-3311019-4.patch6.19 KBvaish
#2 address_autocomplete-config-issues-3311019-2.patch4.59 KBchrisolof

Issue fork address_autocomplete-3311019

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

chrisolof created an issue. See original summary.

chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new address_autocomplete-config-issues-3311019-2.patch4.59 KB

The attached patch fixes the configuration issues outlined above. Needs review and testing.

routinet’s picture

routinet commented

Patch at #2 did not work for me. After applying patch, with no override configured, I receive a serialized value for, e.g., 'google_maps'. With an override configured for api_key, I receive an array with 'api_key' as the only key.

Before override: 

php > $config = \Drupal::config('address_autocomplete.settings');
php > $value = $config->get('google_maps');
php > var_export($value);
'a:2:{s:9:"plugin_id";s:11:"google_maps";s:7:"api_key";s:0:"";}'

After override: 

php > $config = \Drupal::config('address_autocomplete.settings');
php > $value = $config->get('google_maps');
php > var_export($value);
array (
  'api_key' => 'some-api-key',
)
vaish’s picture

vaish commented
StatusFileSize
new address_autocomplete-config-issues-3311019-4.patch6.19 KB

Here is a new version of the patch from #2. Tested both on Drupal 9.5 and Drupal 10.1. Main difference is a fix for an error

TypeError: Drupal\Component\Plugin\PluginManagerBase::createInstance(): Argument #2 ($configuration) must be of type array, null given

which occurs when attempting to edit provider settings for the very first time while they are still undefined. In addition to that, I converted variable into a constant and injected Guzzle Http client instead of instantiating it in the constructor.

Note: If you want to use this patch on Drupal 10, you also need to apply #3349849-12: Drupal 10 compatibility.

vaish opened merge request !12

  • vaish committed 17738397 on 1.0.x 
    Issue #3311019 by vaish, chrisolof: Module configuration issues
vaish’s picture

vaish commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.