Problem/Motivation

FloodControlSettingsForm loads and saves data to a contact.settings config object that belongs to the core Contact module. This can lead to issues with e.g. config sync as sync will fail if Flood Control is enabled and Contact is not enabled.

Steps to reproduce

1. Install Flood Control module.
2. Disable core Contact module.
3. Save the Flood Control module settings form.
4. Export the site config.
5. Attempt to import the site config.

This should fail with an error similar to :

 [error]  Drupal\Core\Config\ConfigImporterException: There were errors validating the config synchronization.
Configuration <em class="placeholder">contact.settings</em> depends on the <em class="placeholder">Contact</em> module that will not be installed after import. in Drupal\Core\Config\ConfigImporter->validate() (line 755 of /[...]/docroot/core/lib/Drupal/Core/Config/ConfigImporter.php). 

In ConfigImportCommands.php line 357:
                                                                                                                                             
  The import failed due to the following reasons:                                                                                            
  Configuration <em class="placeholder">contact.settings</em> depends on the <em class="placeholder">Contact</em> module that will not be i  
  nstalled after import.

Proposed resolution

Only offer and save Contact specific settings if the Contact module is enabled.

Incidentally -- why are these config settings available but not configurable in the core Contact module config object anyway? Weird.

Remaining tasks

Create patch.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 3192020-2.patch5.17 KBwells

Issue fork flood_control-3192020

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wells created an issue. See original summary.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
CreditAttribution: wells at Cascade Public Media commented
Status: Active » Needs review
FileSize
3192020-2.patch5.17 KB

Attaching patch to prevent relevant form fields from appearing and the config object from saving if the Contact module is not enabled.

cdesautels’s picture

cdesautels CreditAttribution: cdesautels commented

Tried the patch 3192020-1. Made no difference. Config export still exports the contact.setting.yml.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
CreditAttribution: wells at Cascade Public Media commented
Status: Needs review » Needs work

Did you already have the config saved? If so, this patch will indeed not do anything. It only prevents saving the config if the Contact module is not enabled. I.e. if you are already using the Flood Control module and have already saved the form before applying this patch, it will have no effect on future saves of the form so the contact config will still exist.

I'll see this back to Needs work for anyone who wants to take it further. I imagine a post update hook would be needed.

bobbygryzynger made their first commit to this issue’s fork.

bobbygryzynger opened merge request !10

bobbygryzynger’s picture

bobbygryzynger
he/him
Primary language English
Location United States
CreditAttribution: bobbygryzynger at Mediacurrent commented

I'm also encountering this issue. I added the patch from #2 to the issue fork and added a post_update hook to remove the contact module settings if the module is not enabled.

bobbygryzynger’s picture

bobbygryzynger
he/him
Primary language English
Location United States
CreditAttribution: bobbygryzynger at Mediacurrent commented
Status: Needs work » Needs review

Marking for review.

brandonratz’s picture

brandonratz CreditAttribution: brandonratz as a volunteer commented

Patch #2 works to prevent export if config doesn't initially exist

1. drush cdel contact.settings
2. Patch
3. drush cex

No contact.settings.yml exists in exported config

bygeoffthompson’s picture

bygeoffthompson CreditAttribution: bygeoffthompson commented

Similar (but different!) issue and test.

I am moving a site (with Core's Contact disabled) from flood_unblock to flood_control. Without Patch #2 here I received an Undefined Index: flood error noted in this closed issue https://www.drupal.org/project/flood_control/issues/3195497

Patch #2 here resolved my issue re: the undefined index: flood, exactly as batigolix theorized.

Additionally I no longer see Contact related configuration being exported with Patch #2 applied (as expected because my site does not have the module enabled). Thanks for this improvement!

bygeoffthompson’s picture

bygeoffthompson CreditAttribution: bygeoffthompson commented
Status: Needs review » Reviewed & tested by the community

JeroenT made their first commit to this issue’s fork.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented

The code in the MR no longer applied, so I merged 2.0.x into that branch and I added some test coverage for the settings form.

milovan’s picture

milovan CreditAttribution: milovan commented

I confirm #13 works.
Thanks!

VladimirAus’s picture

VladimirAus
Primary language English
Location Brisbane 🇦🇺
CreditAttribution: VladimirAus at Tomato Elephant Studio for Terem commented

I tested the patch and can confirm #13 is working.
+1 for release.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody as a volunteer and commented

Confirming RTBC. Can we have a new stable release please @maintainer? :) Great work!

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix at Finalist commented
Status: Reviewed & tested by the community » Fixed

Thanks everybody for helping out.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody as a volunteer and commented

Thank YOU very very much for the commit and new release @batigolix :)

Status: Fixed » Closed (fixed)

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