Problem/Motivation
I found this bug while looking into the following php warning I was getting.
Invalid argument supplied for foreach() in Drupal\search_api_federated_solr\Form\SearchApiFederatedSolrSearchAppSettingsForm->buildForm() (line 325)
Changes
Update the getting of that config
src/Form/SearchApiFederatedSolrSearchAppSettingsForm.php:321 uses global $config
to check for overrides.
This is a great idea to make sure we don't ignore the overrides but it may be better to use \Drupal::config('')->get()
.
This way, we still check for overrides but if there are none in settings.php, it loads whatever is in the saved search_api_federated_solr.search_app.settings.yml configuration.
Comment | File | Size | Author |
---|---|---|---|
#2 | search_api_federated_solr-invalidArgument-3096072-2.patch | 1.64 KB | jonnyeom |
Comments
Comment #2
jonnyeom CreditAttribution: jonnyeom as a volunteer commentedThis works great for me!
Comment #3
jonnyeom CreditAttribution: jonnyeom as a volunteer commentedComment #4
agentrickardHow did you first encounter this error? I'd like to be able to reproduce it.
Comment #5
agentrickardComment #6
agentrickardTo do that, we'd have to save
site_list
in config as well, which I am hesitant to do. The problem here -- see the comment on line 43 -- is that $this->config in the form context is immutable and _not_ subject to overrides.We have to have a consistent means of telling all sites in the network what the potential values are. The safest way to do that is to force loading in settings.php as a config override.
So I agree we should get rid of the
foreach()
error but disagree that the proposed patch solves the root issue.Comment #7
agentrickardApologies. I should have looked at the patch more closely. It's loading the mutable config.
Comment #8
agentrickardWith this patch, I also changed $configuration back to $config for consistency.
Comment #10
agentrickard