Exportables support for CAPTCHA points

Hi Jeff,

Thanks for your contribution.
I think it's difficult to know if there is much interest in ctools exportables support. As CAPTCHA maintainer, I've experienced that questions like this sometimes get very few response (if any) in the CAPTCHA issue queue.
There is also a CAPTCHA drupal group (http://groups.drupal.org/captcha) where you could ask this, but the traffic over there is also pretty low.

About the possibility to rename a the type column: there is this critical issue #810534: Fix CAPTCHA session reuse, for which the fix involves a database update (in the captcha_sessions table). Maybe it's a good idea for end user friendliness to do both database updates in the same release, if the timing allows this. The critical issue should be fixed as soon as possible of course, so waiting on #825088: Exportables support for CAPTCHA points would be a bad idea, but on the other hand, the column rename could be done before the ctools-exportables patch is ready. Just an idea.

I'm a fan of making captcha points exportable. I want to export everything!

Yes, exportables support is very important. E.g., I'm wanting to include CAPTCHA support in the Debut feature set, and lack of exportables support prevents doing so.

This is certainly worth an update to the existing 'type' column name.

ok then, I'll try to rename the column before for the CAPTCHA 6.x-2.3 release

Is there any documentation on this " 'type' as reserved column name" feature of ctools?

Yes, the reserved column names are documented in the ctools Advanced Help--see the help page at help/ctools/export.

Here's a patch with the column name change. Export now works via features, but captcha configuration from enabled features doesn't yet show up on the captcha config screen.

Great!
Let's see what the testbot thinks

The patch actually works fine, as far as it goes. Captchas can be exported into features and, when a feature with captchas in it is enabled, they show up in the list of captchas.

However, there are remaining problems that stem from the existing install function and the UI for editing captchas.

1. The first problem I found in testing relates to the install function. Because the captcha_points table is populated with an initial set of records, if there are code-based defaults for the same forms, they are overridden. Since the defaults include some of the forms most likely to be assigned captchas, this is a significant problem. For example, if a feature module added a captcha challenge to the comment_form, that captcha would be overridden (removed) in the initial install state.

2. The second problem stems from the fact that data are saved for all forms at once. If you enable a feature module with captcha information, the data are not initially overridden (unless they were already on the site). But submitting the settings form even once saves a record for each captcha in the database, meaning that all default captchas are overridden (even if no change was made). This doesn't break anything, but makes it hard for an admin to determine if s/he is using the default or has overridden it.

Re problem 1, a possibility would be to remove the inserts from the install script and instead alter the array of available captcha data in hook_captcha_default_points_alter(), providing captchas for given forms if none was already provided by a module. Draft:

/**
 * Implementation of hook_captcha_default_points_alter().
 *
 * Provide some default captchas only if defaults are not already
 * provided by other modules.
 */
function captcha_captcha_default_points_alter(&$items)  {
  $modules = array(
    'comment' => array(
      'comment_form',
    ),
    'contact' => array(
      'contact_mail_user',
      'contact_mail_page',
    ),
    'forum' => array(
      'forum_node_form',
    ),
    'user' => array(
      'user_register',
      'user_pass',
      'user_login',
      'user_login_block',
    ),
  );
  foreach ($modules as $module => $form_ids) {
    // Only give defaults if the module exists.
    if (module_exists($module)) {
      foreach ($form_ids as $form_id) {
        // Ensure a default has not been provided already.
        if (!isset($items[$form_id])) {
          $captcha = new stdClass;
          $captcha->disabled = FALSE;
          $captcha->api_version = 1;
          $captcha->form_id = $form_id;
          $captcha->module = '';
          $captcha->captcha_type = 'default';
          $items[$form_id] = $captcha;
        }
      }
    }
  }
}

As an added advantage, we can test module_exists() and provide only suitable suggestions.

Re problem 2, we could detect in the form submit handler whether there is a default version of a particular captcha and, if so, save a record to the db only if the data are different from the default.

Alternately, we could consider switching to using Ctools exportables UI, but that would mean bigger changes.

Here's a patch with the two fixes described in #8.

There are some details still missing, e.g., enabling settings that are disabled by default (as noted in a code comment by jmiccolis), but the basics are here and - with a look over from someone else - this should be ready to go.

soxofaan, please let me know if you have further questions or concerns.

I suggest after this patch goes in that any further changes begin with consideration of switching to CTools exportables UI.

FYI: sorry about the radio silence on my part, for the moment I don't have a lot of time to review/followup this thread/patch. I hope to give it some love soon.

Oh great, wish I would have found this yesterday :)
I used the patch and was able to export a setting for a custom form_id.
It seems all of the default form_ids that come with captcha are now using a captcha by default, correct?

Hi all,

Because I couldn't allocate a long enough time slot to review patch #9 properly in full (I'm not very experienced with ctools exportables), I decided to chop this task in parts and already committed a part of the patch #9: the rename of the column name from 'type' to 'captcha_type': http://drupal.org/cvs?commit=457682

In attachment: the remainder of the patch (all the Ctools export stuff)

In case this could be interested to anybody: I maintain the module with Git (easier to work with per feature branches and small incremental commits) and push regularly to https://github.com/soxofaan/drupal-captcha-dev (CVS branch DRUPAL-6--2 corresponds to git branch "master-d6")
The branch for this issue is "825088-ctools-export" (https://github.com/soxofaan/drupal-captcha-dev/tree/825088-ctools-export) and already contains the remainder of patch #9

FYI: ported and committed the patch from http://drupal.org/cvs?commit=457682 for D7: http://drupal.org/cvs?commit=471330

subscribing

Here's a patch for the D7 version.

@aschiwi, "It seems all of the default form_ids that come with captcha are now using a captcha by default, correct?"

Yes, this is the one significant change from the D6 functionality. From an exportables perspective, it seems incorrect to force the captchas to none when the point is the opposite--that these are forms that likely should have captchas. Probably what we want here is to set them to use captchas but to have an initial state of disabled, allowing users to enable them--but we don't yet have support for the ctools exportables disabled flag.

To retain the D6 functionality, we would change this line in captcha_captcha_default_points_alter()

+          $captcha->captcha_type = 'default';

to

+          $captcha->captcha_type = '';

cross post

Fix one glaring error at least.

The patch sort of works, but has some remaining issues related to the defaults.

The patch replaces the current db inserts that create an initial set of form data with an alter. As a result, these forms appear to have default settings on an initial install. This makes it non-intuitive to add a captcha setting for these forms to a feature. To do so, you have to:

* Change the setting for the form from the default to another setting, e.g., disable the captcha, and save the configuration. This creates a record in the database.
* Now, if you actually wanted the default setting, select it and save again.
* Only now will the form be available for export.

So, it works, but it's awkward.

However, it's not clear what would be better. Options:

* Drop all the initial data population, relying instead on the regular methods of adding presets to forms. But this would raise problems for the user register and login forms, which aren't available to logged in users and so can't be effectively selected by admins for adding captchas.
* Drop all but forms that don't appear for logged in users.
* Convert the initial population into a regular hook_captcha_default_points() implementation, meaning that (a) any features wanting to affect these forms would need to do so in an alter hook and (b) any sites wanting to have something different would need to either use an alter hook or override through the UI.

The last option might be the best.

Besides these considerations, there is also the question of whether CTools is still the best way to do this. Entity API is another option and may be a better choice. See #624018: Exportables and Features support for WYSIWYG 7.x comment #119 and later.

subscribing

#19: 825088-19-captcha_ctools_export.patch queued for re-testing.

subscribe

Subscribe

Simple change to remove warnings that occur in strict error reporting.

Ah, never mind - was a version behind with the warnings ... sorry for the confusion.

Hi all,

I finally found a bit of time to look into this issue a bit more.
I committed the patch for Drupal6 from #9 and #12 (with some extra tweaks) in commits
http://drupalcode.org/project/captcha.git/commit/afda4a3
http://drupalcode.org/project/captcha.git/commit/92cf795
http://drupalcode.org/project/captcha.git/commit/e4988d1
http://drupalcode.org/project/captcha.git/commit/f528630

I included the suggestion from #17 to default to no CAPTCHA. This also makes the issue from #20 almost inexistent.
I hope this closes the issue for the Drupal6 version.

Still to be ported to D7

@pwhiteside please do not change the issue status

@scottrigby Sorry pal I didn't mean to.

I clicked a re-test link and it automatically did it and I don't know how to delete comments.

I've been using the patch in #19 for D7 and it worked fine. I've done an upgrade of drupal core (7.12) and features (1.0-beta6) and now, when I do a feature update, it removes my captcha points exports.

Something new here?

Thanks.-

Here is a reroll of the patch in #25 which fixes this warning with newer PHP versions:

Strict warning: Only variables should be passed by reference in captcha_get_form_id_setting() (line 74 of /home/ao2/public_html/test_site/sites/all/modules/contrib/captcha/captcha.inc).

the incremental change is just this one:

diff --git a/captcha.inc b/captcha.inc
index 2e5ece7..72e44bf 100644
--- a/captcha.inc
+++ b/captcha.inc
@@ -71,7 +71,8 @@ function captcha_set_form_id_setting($form_id, $captcha_type) {
 function captcha_get_form_id_setting($form_id, $symbolic=FALSE) {
   if (module_exists('ctools')) {
     ctools_include('export');
-    $captcha_point = array_pop(ctools_export_load_object('captcha_points', 'names', array($form_id)));
+    $object = ctools_export_load_object('captcha_points', 'names', array($form_id));
+    $captcha_point = array_pop($object);
   }
   else {
     $result = db_query("SELECT module, captcha_type FROM {captcha_points} WHERE form_id = :form_id",

but I am attaching the full patch here; easier for drush-make users.

Just wondering what the status of CAPTCHA points being exportable to features?

I tried the last patch (#33) and it just set EVERY form to use the default challenge...

What does the testbot say of the latest patch?

Hi:

Path in #19 worked for me for some time but in some point it stopped working. I've try patch #33 with beta2 and last dev and it doesn't seem to work. When I make "drush fu" the line of my info file that instructs features to export captcha points gets removed:

features[captcha_points][] = "contact_site_form"

and no captcha points are exported.

Any news about this?

Thanks.-

Check attached patch working fine with Captcha 7.x-1.0-beta2 version.

Please check fixed version.

please check patch attached for Captcha 7.x-1.x-dev. I've tested it properly. its working now.

Patch well formated for applying (path fixed)

Patch had problem with filename becouse of the "#" character. File renamed and reuploaded

I'm getting weird behaviour where my Feature is reporting it's overridden, with this as the diff:

Component: captcha_points
  array(
<   'user_login' => array(
<     'disabled' => FALSE,
<     'api_version' => 1,
<     'form_id' => 'user_login',
<     'module' => '',
<     'captcha_type' => 'default',
<   ),
<   'user_login_block' => array(
<     'disabled' => FALSE,
<     'api_version' => 1,
<     'form_id' => 'user_login_block',
<     'module' => '',
<     'captcha_type' => 'default',
<   ),
<   'user_pass' => array(
<     'disabled' => FALSE,
<     'api_version' => 1,
<     'form_id' => 'user_pass',
<     'module' => '',
<     'captcha_type' => 'default',
<   ),
    'user_register_form' => array(
      'disabled' => FALSE,

The only exported captcha point is the 'user_register_form'; all others are set to 'none' in the UI and their DB rows have NULLs.

Reverting the feature is not fixing the problem.

On the plus side, my feature deployed to the staging site perfectly :) -- though the staging site is showing the same problem.

The problem is caused by captcha_captcha_default_points_alter():

 * Implementation of hook_captcha_default_points_alter().
 *
 * Provide some default captchas only if defaults are not already
 * provided by other modules.

This takes the default captcha points that have come from the default hook (ie, from features), and adds defaults.

So:

- install captcha module
- set login captcha to 'none'
- export another captcha point to a feature
- when the feature is reverted or diff'ed, hook_captcha_default_points_alter() fires, and because the feature has nothing to say about the login captcha, it enables it

This seems pretty weird to me.

One way to deal with the problem is to just add the captcha points captcha_points:user_login captcha_points:user_login_block captcha_points:user_pass to the feature, even though they are set to no captcha. This seems very weird -- nobody would think to try that, so this would need documenting.

I think the problem is really that while this module sets those forms up to have a captcha in hook_install(), these should be taken to be installation defaults, not permanent defaults. Hence the behaviour in captcha_captcha_default_points_alter() in incorrect. Though even removing that would still mean that on deployment you'd get hook_install() adding those in.

I'm inclined to say that having defaults forced in hook_install() isn't really compatible with supporting exportables. But that would be a fairly big change in how this module works.

so needswork as of #52...

additional note: i enabled captcha module by rolling out a premade captcha feature, without this patch.
user forms did NOT get captcha-auto-enabled (which i wanted...) using 7.x-1.0-beta2. did not investigate that yet.

IIRC, without this patch, this module has no Feature support at all - so no forms will get enabled.

IIRC, without this patch, this module has no Feature support at all - so no forms will get enabled.

#50: captcha-patch-on-44-well-formated-825088-47.patch queued for re-testing.

subscribe

Having defaults via hook_install is fairly usual I think -- media does it now tmk. The problem is realizing when those defaults should be gone.

Patch #61 failed to apply successfully in the latest '7.x-1.x-dev' version of CAPTCHA module.

Got the following message when trying to apply patch #61 (825088-captcha-export-60.patch)

patching file captcha.admin.inc
Hunk #1 succeeded at 64 (offset 1 line).
Hunk #2 FAILED at 80.
Hunk #3 succeeded at 288 with fuzz 2 (offset -3 lines).
1 out of 3 hunks FAILED -- saving rejects to file captcha.admin.inc.rej
patching file captcha.inc
Hunk #1 FAILED at 70.
Hunk #2 succeeded at 214 with fuzz 1 (offset 103 lines).
1 out of 2 hunks FAILED -- saving rejects to file captcha.inc.rej
patching file captcha.install
Hunk #2 FAILED at 146.
1 out of 2 hunks FAILED -- saving rejects to file captcha.install.rej
patching file captcha.module
Hunk #1 FAILED at 300.
1 out of 1 hunk FAILED -- saving rejects to file captcha.module.rej

The latest patch does not apply successfully to the latest 7.x-1.x-dev version. This latest patch needs to be rerolled for it to apply successfully.

Rerolled the patch #61 to apply successfully in the latest 7.x-1.x-dev version.

We're using the patch in #66 successfully.

Were the issues from #51 and #52 ever addressed? It appears they were perhaps overlooked? See the related follow-up:

#2552279: Feature is overridden always