This patch adds exportables support for Mollom form configurations through loose coupling with the CTools export API. If CTools is available, Mollom form configurations can be exported and any defaults can be overridden and reverted. By virtue of providing CTools integration, Mollom also can be used with Features (see screenshots).

If CTools is not present, Mollom falls back gracefully to the current behavior with no advanced storage handling.

Patched against DRUPAL-6--1.

Files: 
CommentFileSizeAuthor
#42 717874-42-exportables_for_mollom.patch13.09 KBstella
PASSED: [[SimpleTest]]: [MySQL] 6,710 pass(es).
[ View ]
#31 717874-31-exportables-for-mollom_reroll.patch12.53 KBnetsensei
PASSED: [[SimpleTest]]: [MySQL] 4,884 pass(es).
[ View ]
#29 717874-29-exportables-for-mollom_reroll.patch12.08 KBnetsensei
PASSED: [[SimpleTest]]: [MySQL] 4,883 pass(es).
[ View ]
#23 717874-23-exportables-for-mollom_reroll.patch12.52 KBcam8001
PASSED: [[SimpleTest]]: [MySQL] 4,371 pass(es).
[ View ]
#18 717874-18-exportables-for-mollom.patch11.89 KBGarrett Albright
FAILED: [[SimpleTest]]: [MySQL] 4,226 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#16 717874-16-exportables-for-mollom.patch11.99 KBGarrett Albright
FAILED: [[SimpleTest]]: [MySQL] 2,876 pass(es), 3 fail(s), and 0 exception(es).
[ View ]
#15 717874-15-exportables-for-mollom.patch12.14 KBGarrett Albright
FAILED: [[SimpleTest]]: [MySQL] 2,876 pass(es), 3 fail(s), and 0 exception(es).
[ View ]
#14 717874-14-exportables-for-mollom.patch12.85 KBGarrett Albright
FAILED: [[SimpleTest]]: [MySQL] 106 pass(es), 58 fail(s), and 2 exception(es).
[ View ]
#9 mollom-n717874-9.patch802 bytesDamienMcKenna
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch mollom-n717874-9.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#7 mollom-DRUPAL-6--1.export.7.patch12.71 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch mollom-DRUPAL-6--1.export.7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 717874_mollom_ctools_export_2.patch11.58 KByhahn
PASSED: [[SimpleTest]]: [MySQL] 1,289 pass(es).
[ View ]
screenshot4.png8.06 KByhahn
screenshot3.png14.83 KByhahn
screenshot2.png20.05 KByhahn
mollom_ctools_export.patch7.72 KByhahn
FAILED: [[SimpleTest]]: [MySQL] 1,307 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, mollom_ctools_export.patch, failed testing.

Thanks! That test failure is caused by a different bug unrelated this patch, #716250: Blacklisting test

+++ mollom.admin.inc 18 Feb 2010 06:32:09 -0000
@@ -21,21 +21,46 @@ function mollom_admin_form_list() {
+  if (module_exists('ctools')) {
...
+    if (module_exists('ctools')) {
...
+    $rows[] = array(array('data' => l(t('Add form'), 'admin/settings/mollom/add'), 'colspan' => module_exists('ctools') ? 5 : 4));

Can we do a single

$exportable = module_exists('ctools');

at the beginning?

+++ mollom.admin.inc 18 Feb 2010 06:32:09 -0000
@@ -21,21 +21,46 @@ function mollom_admin_form_list() {
+      // Remove protection.
+      if ($mollom_form['export_type'] & EXPORT_IN_DATABASE) {
+        $row[] = l(t('Unprotect'), 'admin/settings/mollom/unprotect/' . $form_id);
+      }
+      else {
+        $row[] = '';
+      }
+      // Storage operations.
+      if ($mollom_form['export_type'] & EXPORT_IN_DATABASE && $mollom_form['export_type'] & EXPORT_IN_CODE) {
+        $row[] = t('Overridden (!revert)', array('!revert' => l(t('revert'), 'admin/settings/mollom/revert/' . $form_id)));
+      }
+      else if ($mollom_form['export_type'] & EXPORT_IN_DATABASE) {
+        $row[] = t('Normal (!export)', array('!export' => l(t('export'), 'admin/settings/mollom/export/' . $form_id)));
+      }
+      else if ($mollom_form['export_type'] & EXPORT_IN_CODE) {
+        $row[] = t('Default');
+      }

1) minor: Can we rename 'export_type' to 'storage'?

2) major: It seems like I cannot unprotect or disable a form protection that lives in code? Or do I get this code wrong?

+++ mollom.install 18 Feb 2010 06:32:09 -0000
@@ -126,6 +126,16 @@ function mollom_schema() {
     'primary key' => array('form_id'),
+    'export' => array(
+      'key' => 'form_id',
+      'identifier' => 'mollom_form',

"key" and "identifier" seem to duplicate schema information - are they required?

+++ mollom.module 18 Feb 2010 06:32:10 -0000
@@ -528,13 +548,40 @@ function mollom_get_form_info($form_id =
+function mollom_form_load_all() {
...
+      $mollom_forms[] = $form_id;
...
+    $result = db_query("SELECT form_id FROM {mollom_form}");
+    while ($form_id = db_result($result)) {
+      $mollom_forms[] = $form_id;
...
+  return $mollom_forms;

Hm. It's a bit strange that this function is named mollom_form_load_all() and lives next to mollom_form_load(), but returns something completely different than mollom_form_load().

Powered by Dreditor.

Status:Needs work» Needs review
StatusFileSize
new11.58 KB
PASSED: [[SimpleTest]]: [MySQL] 1,289 pass(es).
[ View ]

Thanks for the review. New version of patch attached.

  • Can we do a single $exportable = module_exists('ctools');
    • Yes
  • 1) minor: Can we rename 'export_type' to 'storage'?
    • Short answer: no. Long answer: CTools uses this key for this metadata by default, we could make a copy of the information on load but it wouldn't really make sense to.
  • 2) major: It seems like I cannot unprotect or disable a form protection that lives in code? Or do I get this code wrong?
    • I updated the patch to take advantage of CTools built-in ->disabled flag. Default or Overridden forms can now be disabled (behaves the same as an unprotected form).
  • "key" and "identifier" seem to duplicate schema information - are they required?
    • I've removed identifier which defaults to the name of the table. While CTools could probably derive key in many cases from the primary key of a table, it currently doesn't handle this so we must declare it.
  • It's a bit strange that this function is named mollom_form_load_all() and lives next to mollom_form_load(), but returns something completely different
    • Revised to return an array of loaded mollom forms.

Thank you! This looks pretty good now. I'll try to come back to this issue and actually test the code before approving it.

Speaking of tests, we probably need a new test case for this feature, which leverages the new 'dependencies' property in getInfo() for CTools, and ensures that form protections can be defined in code, disabled/enabled, and overridden.

Status:Needs review» Needs work

After applying this patch, I get the following PHP notices + warnings on the front page:

* Notice: Undefined index: name in ctools_export_load_object() (line 122 of sites\all\modules\ctools\includes\export.inc).
* User warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'type for db_type_placeholder)' at line 1 query: SELECT * FROM mollom_form WHERE name IN (unsupported type for db_type_placeholder) in _db_query() (line 128 of includes\database.mysqli.inc).
* Notice: Undefined variable: mollom_form in mollom_form_load() (line 643 of sites\all\modules\mollom-DRUPAL-6--1\mollom.module).

The first one looks like I need to update ctools on my Mollom test site - do we require a certain minimum API version of CTools?

After updating CTools to latest CVS DRUPAL-6--1, those notices and warnings seem to disappear.

However, exporting a form protection defines:

$mollom_form = new stdClass;
$mollom_form->disabled = FALSE; /* Edit this to true to make a default mollom_form disabled initially */
$mollom_form->api_version = 1;
$mollom_form->form_id = 'comment_form';
$mollom_form->mode = TRUE;
$mollom_form->enabled_fields = array(
  '0' => 'subject',
  '1' => 'comment',
);
$mollom_form->module = 'comment';

$mollom_form->mode is not a boolean, but instead one of the constants MOLLOM_MODE_DISABLED, MOLLOM_MODE_CAPTCHA, or MOLLOM_MODE_ANALYSIS.

StatusFileSize
new12.71 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch mollom-DRUPAL-6--1.export.7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

- Fixed some minor issues.
- Additionally implemented Bulk exporter support to be able to actually test this more easily.

Remaining issues:
- After exporting a form into code, it is properly displayed as such in the form administration. However, after clicking on "revert", the form suddenly appears at the end of the list.

- A disabled form cannot be enabled. (link points to 'disable')

EDIT: The 'mode' bug mentioned in #6 also remains.

There are two tables:

Given it has been eight months since the last update, and CTools has greatly improved the exportables support, could we just start with some basic exporting via the extensions on hook_schema and build from there through multiple releases?

StatusFileSize
new802 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch mollom-n717874-9.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Here's the bare hook_schema() for 'mollom_form' extracted from the patches above for D6.

subscribe - any luck with this?

Version:6.x-1.x-dev» 7.x-2.x-dev

Bumping this to 7.x-2.x as this would require an additional dependency.

To clarify, we're not going to add a hard dependency on another module. However, we can implement optional support for ctools - which, I think, would still look similar to #7 (although that's a lot of code for this functionality and I seriously hope we can cut it down). I'm not sure what kind of effect a schema-definition-only patch like #9 would have?

Nobody has worked on this further?

StatusFileSize
new12.85 KB
FAILED: [[SimpleTest]]: [MySQL] 106 pass(es), 58 fail(s), and 2 exception(es).
[ View ]

My mostly brute force attempt at a D7 version of the patch. Should apply, but will cause a "unsupported operand" error that I'm not sure how to fix without being more familiar with what the codebase looked like in the past (and possibly other errors when that one is fixed), but I'm hoping this will provide a kick in the pants for this to be picked up again.

StatusFileSize
new12.14 KB
FAILED: [[SimpleTest]]: [MySQL] 2,876 pass(es), 3 fail(s), and 0 exception(es).
[ View ]

Powered through it some more. No more fatal errors. In fact, I think this might even… work.

Status:Needs work» Needs review
StatusFileSize
new11.99 KB
FAILED: [[SimpleTest]]: [MySQL] 2,876 pass(es), 3 fail(s), and 0 exception(es).
[ View ]

Fix whitespace complaints when patch is applied. Also, NR.

Status:Needs review» Needs work

The last submitted patch, 717874-16-exportables-for-mollom.patch, failed testing.

StatusFileSize
new11.89 KB
FAILED: [[SimpleTest]]: [MySQL] 4,226 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Fix some lingering D6 menu paths and such.

Status:Needs work» Needs review

Let's have a bot review.

Status:Needs review» Needs work

The last submitted patch, 717874-18-exportables-for-mollom.patch, failed testing.

The patch doesn't change any tests, so that's probably why they're failing, right? Since the admin forms have changed?

#18 applies cleanly and works on 7.x-2.1 but needs to be rerolled for 2.2 and committed.

Status:Needs work» Needs review
StatusFileSize
new12.52 KB
PASSED: [[SimpleTest]]: [MySQL] 4,371 pass(es).
[ View ]

Here's a reroll against 7.x.2.x/7.x-2.2.

Is it me or the protection mode keeps reverting back to CAPTCHA? When I set a form definition to Text analysis and then export it into a feature, upon rebuild it pops back up set on CAPTCHA. Driving me type wonkers. Even when I blow away the mollom.inc manually and re-export a clean one... still no joy.

Can anyone else recreate?

edit:
Confirmed, protection mode isn't exporting.

Does the patch in #23 work for 7.x-2.3? Just wondering if anyone tried it...

Downloaded 7.x-2.2 just to try the patch...and I get these errors:
Notice: Undefined index: module in mollom_admin_form_list() (line 37 of /home/pfresonke/public_html/public_html/sites/all/modules/contrib/mollom/mollom.admin.inc).
Notice: Undefined index: mode in mollom_admin_form_list() (line 54 of /home/pfresonke/public_html/public_html/sites/all/modules/contrib/mollom/mollom.admin.inc).
Notice: Undefined index: export_type in mollom_admin_form_list() (line 70 of /home/pfresonke/public_html/public_html/sites/all/modules/contrib/mollom/mollom.admin.inc).

Plus a bunch more of the same on different lines...anyone else run into this?

You've also left some trailing whitespace around line 341 in this patch.

+++ b/mollom.moduleundefined
@@ -1042,6 +1134,7 @@ function mollom_form_load($form_id) {
   }
+  ¶
   return $mollom_form;
}

Status:Needs review» Needs work

#25: I don't think this code applies cleanly for 2.3. You'll need to backport parts of it to make it all fit. It does apply to the 2.x-dev branch as intended. The issue reported in #24 does warrant further investigation though.

Also. You can only export a configuration once. When it is already exported, there is no way to export the overridden configuration again. You are only able to
revert back to the original exported configuration.

I think we need to split the two the overridden configuration + the export functionality.

I'm also wary about the navigational path from the exported code preview back to the list of forms. The breadcrumb doesn't seem to follow nor is there a "back to form configurations" button and/or link.

Status:Needs work» Needs review
StatusFileSize
new12.08 KB
PASSED: [[SimpleTest]]: [MySQL] 4,883 pass(es).
[ View ]

And a reroll against 7.x-2.x. This patch:

* Removes the whitespace error
* UI change: the export functionality is now always available.
* UI change: I've recycled the "Database overrides code", "In code" or "In database" flags which are also used in the Views UI.

I've also discovered the cause of the issue reported in #717874-24: Provide exportables for Mollom forms. The protection mode is stored in a tinyint type db field. ctools_export_object does not make a distinction between tinyint values and boolean values. The function turns any tinyint value (except 0) into a TRUE. It's a CTools issue reported here: #1760752: Tiny int should not be exported as a boolean

I'm going to reroll the patch with a field specific export callback to get around this problem.

The other solution would be to change the field type size from tiny to small. I don't favor changing the schema here because of a code issue in external API module though.

StatusFileSize
new12.53 KB
PASSED: [[SimpleTest]]: [MySQL] 4,884 pass(es).
[ View ]

Okay. Patch should fix the mismatch problem. Go testbot!

Thanks for the work so far, but I've installed the patch from #31, made some changes, and tried to revert my feature. The revert doesn't do anything at all; it says overridden, and there is no green status text at the top of the page like there usually is during a revert!

Where did you make your changes? In the code or in the database (configuration)?

So the changes were made on a local copy in the UI, and then the feature was downloaded. The code was deployed to a production system and the feature was reverted, but this is where the issue came in, as the revert apparently had no effect.

Closer inspection using the diff module reveals that the left side of the diff doesn't have anything at all in it, just the word 'FALSE', which would be an obvious reason for the revert to fail, but I can't seem to see why the left side of the diff would be 'FALSE'.

Check these two things:
1. this patch is against mollom 7.x-2.x, make sure to upgrade to 7.x-2.4 if you are still using a 7.x-1.x version
2. Make sure you have the following code in your MYFEATURE.features.inc:

  list($module, $api) = func_get_args();
  if ($module == "mollom" && $api == "mollom") {
    return array("version" => "1");
  }

For the records, the patch #31 works for me with 7.x-2.4.

If #34 can confirm #35, can we put this to RTBC?

This is #34. I can confirm #35!

Status:Needs review» Reviewed & tested by the community

Putting this into RTBC. :-)

Version:7.x-2.x-dev» 7.x-2.4

Hi, I'm having the same issue as #34. I'm on version 7.x-2.4
Looking at the [my_module].mollom.inc file, it is implementing this hook: hook_default_mollom_form() but when I search for it, I can't find it. Maybe that's causing the issue?

Regards.

Version:7.x-2.4» 7.x-2.6

Can confirm that the patch still works with 7.x-2.6

Status:Reviewed & tested by the community» Needs work

Update: while the configuration gets deployed, and caches cleared, etc, the captcha does not appear until the settings are saved to the database for each configured form :(

Status:Needs work» Needs review
StatusFileSize
new13.09 KB
PASSED: [[SimpleTest]]: [MySQL] 6,710 pass(es).
[ View ]

Patch re-roll against 7.x-2.6 and also fixes the issue I described in #42. Basically the problem was that mollom_form_cache() was still only loading from the database and not looking at the versions in code. I changed it so it now calls mollom_form_load_all() instead.

Please, can someone update this patch for 2.7 and provide instructions how to use it?

Version:7.x-2.6» 7.x-2.7

+1 to #44. I applied the patch from #42 and it worked on 2.7. Can we get this into a release, please? Happy to mark this RTBC.

#44 works when you I the admin interface, but it's failing for me when I try to update a feature with drush fu.

The patch works on #42 for 2.7 and no higher - so it was unable to patch with 2.8