This issue focuses on converting the following variables to configuration:

  • contact_default_status
  • contact_threshold_limit
  • contact_threshold_window

It does not convert the contact form settings to cmi - this is being done by #1588422: Convert contact categories to configuration system
It is a prerequisite for #1496534: Convert account settings to configuration system

It's purpose is to keep both of those issues focussed on their main tasks.

Files: 
CommentFileSizeAuthor
#20 0001-adding-contact-settings-yml-file.patch707 bytesaspilicious
PASSED: [[SimpleTest]]: [MySQL] 39,758 pass(es). View
#15 12-15-interdiff.txt511 bytesalexpott
#15 1704530-15-contact-settings-cmi.patch9.65 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 39,754 pass(es). View
#12 10-12-interdiff.txt2.26 KBalexpott
#12 1704530-12-contact-settings-cmi.patch9.64 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 39,753 pass(es). View
#10 1704530-10-contact-settings-cmi.patch10.62 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 39,751 pass(es), 7 fail(s), and 0 exception(s). View
#10 8-10-interdiff.txt702 bytesalexpott
#8 1704530-8-contact-settings-cmi.patch10.56 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 39,750 pass(es). View
#8 1-8-interdiff.txt4.27 KBalexpott
#1 1704530-1-contact-settings-cmi.patch10.67 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 39,750 pass(es). View

Comments

alexpott’s picture

Status: Active » Needs review
FileSize
10.67 KB
PASSED: [[SimpleTest]]: [MySQL] 39,750 pass(es). View

The new config names are based on comments by @sun and @dawehner in #1588422: Convert contact categories to configuration system

sun’s picture

Hm. I thought we'd merge this into the patch in #1496534: Convert account settings to configuration system ?

cosmicdreams’s picture

Status: Needs review » Needs work
+++ b/core/modules/contact/config/contact.settings.yml
@@ -0,0 +1,5 @@
+user_contact_default_enabled: '1'

How about default_enabled ?

If you have a reason for naming it this way that's fine. Otherwise RTBC.

cosmicdreams’s picture

Status: Needs work » Reviewed & tested by the community

As moshe says, resist the nit-picking.

sun’s picture

Status: Reviewed & tested by the community » Needs review

The "user" part seems appropriate, but I don't understand why we're duplicating "contact" in the key?

@cosmicdreams: moshe's comment wasn't meant universal, but specific to the user pictures issue, which is a major monster patch that has been RTBC a couple of times already.

sun’s picture

Also, what's the deal with default_category? Any particular reason why that is not converted here?

If there's a strong reason for omitting it, then I think we should remove the key from the config file in this patch. Otherwise, let's just convert it here, too?

cosmicdreams’s picture

default_category is in the yml file, so the conversions should probably happen or the setting should also be omitted from that file.

alexpott’s picture

FileSize
4.27 KB
10.56 KB
PASSED: [[SimpleTest]]: [MySQL] 39,750 pass(es). View

@sun: I've um and ahhed about user_default_enabled vs user_contact_default_enabled... I made it user_contact_default_enabled because it is referring to the default setting for the "user contact" form which is provided by the contact module. But to be honest I'm happy either way.

re: default_category - it shouldn't be in the yml file as there is no variable for the contact form default category - currently the default category is determined by a field on the contact table.

re: #2 I made this a separate patch to make reviewing both the user contact and contact machine name patches smaller and focussed on their main aims - there will be less in both on them if we can get this in.

The patch attached removes default_category from the yml file and changes the configuration key to user_default_enabled.

sun’s picture

Status: Needs review » Needs work

This looks great! Only one last remark:

+++ b/core/modules/contact/contact.module
@@ -250,10 +250,23 @@ function contact_form_user_admin_settings_alter(&$form, &$form_state) {
-  $form['contact']['contact_default_status'] = array(
+  $form['contact']['user_default_enabled'] = array(
...
+    ->set('user_default_enabled', $form_state['values']['user_default_enabled'])

Let's leave the original $form array key alone, here. Since the settings form is not using #tree, the submitted form value is not namespaced for contact module, which in turn makes the original form key cleaner.

alexpott’s picture

Status: Needs work » Needs review
FileSize
702 bytes
10.62 KB
FAILED: [[SimpleTest]]: [MySQL] 39,751 pass(es), 7 fail(s), and 0 exception(s). View

How about using #tree on the $form['contact'] fieldset? Afaik the only reason #tree wasn't used in the first place is because the old system_settings_form() did not play nice with it.

sun’s picture

Status: Needs review » Needs work

Sorry, nope, no such premature optimizations, please.

alexpott’s picture

Status: Needs work » Needs review
FileSize
9.64 KB
PASSED: [[SimpleTest]]: [MySQL] 39,753 pass(es). View
2.26 KB

okey-dokey :)

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me...

sun’s picture

Status: Reviewed & tested by the community » Needs work

Almost done! :)

+++ b/core/modules/contact/contact.module
@@ -254,6 +254,19 @@ function contact_form_user_admin_settings_alter(&$form, &$form_state) {
+ * Form builder submit handler; Handle submission for user contact default.

This phpDoc should be:

Form submission handler for user_admin_settings().

@see contact_form_user_admin_settings_alter()
alexpott’s picture

Status: Needs work » Needs review
FileSize
9.65 KB
PASSED: [[SimpleTest]]: [MySQL] 39,754 pass(es). View
511 bytes

Here we go.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! :)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks. Keep 'em coming.

alexpott’s picture

Title: Convert contact module variables to configuration system » [HEAD BROKEN] Convert contact module variables to configuration system
Priority: Normal » Critical
Status: Fixed » Needs work
Issue tags: +New files

We need to add the config directory and files. Creating the "New files" tag to indicate this.

Head will be broken once it retests.

andypost’s picture

Status: Needs work » Reviewed & tested by the community
--- /dev/null
+++ b/core/modules/contact/config/contact.settings.yml

+++ b/core/modules/contact/config/contact.settings.yml
@@ -0,0 +1,4 @@

@@ -0,0 +1,4 @@
+flood:
+    limit: '5'
+    interval: '3600'
+user_default_enabled: '1'

This part

aspilicious’s picture

FileSize
707 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,758 pass(es). View
catch’s picture

Title: [HEAD BROKEN] Convert contact module variables to configuration system » Convert contact module variables to configuration system
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Added the missing file...

(git apply --index ftw.)

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