Since Site Verification stores its data in its own table {site_verify}, it isn't possible to add them to a Feature module.

Would be very useful to be able to do so on an individual basis so that each entry can be put in different feature modules when so is required.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jdleonard’s picture

I am also interested in this feature. Thanks!

tobias.grasse’s picture

Count me in.

Deciphered’s picture

Assigned: Unassigned » Deciphered

I've begun work on this and will supply a patch ASAP.

It is being done using CTools with the CTools Export UI which means it isn't a minor task, but it's really the only logical way to do it.

Deciphered’s picture

Status: Active » Needs work

Patch attached (sponsored by CustomFormatters.com).

The only thing that currently isn't working is the 'Delete' portion of the simpletest, all the functionality works as expected and an upgrade path is included.

Deciphered’s picture

FileSize
31.22 KB

Oops, helps if I attach the patch...

Deciphered’s picture

FileSize
1.33 KB
31.22 KB

Updated patch for a minor issue with the disabled status flag.

indytechcook’s picture

Status: Needs work » Needs review

The features integration works fine and so does the UI. I didn't do tons of testing so I can't mark RTBC but it probably is.

@deciphered, for future reference, you can have ctools exportables without using the ctools exportables UI. Great work!

Deciphered’s picture

I'm aware of that, but CTools Export UI means we don't have to re-invent the wheel, so why not take advantage of it.

Deciphered’s picture

Issue summary: View changes
FileSize
31.22 KB
1.33 KB

As per #2136309: Upgrade path issue, which I accidentally opened not identifying this patch as the issue, the upgrade path needs work. Which this patch does.

joachim’s picture

Status: Needs review » Needs work

Patch looks very comprehensive -- tests too! :)

But also huge and scary, especially as I'm a temporary maintainer who's not familiar with the module code. So probably not one for me to deal with, sorry.

I think this is going to clash with #696436: Use File API and file_transfer(). In particular, am I right in guessing that when you upload a file to be a verification file, its contents get put into the table? That's completely at odds with what #696436: Use File API and file_transfer() is seeking to do -- and furthermore, using File API would actively prevent verification items from being exportable! So there's a question to be resolved about the direction this module should take. Which again, is not one for me, sorry!

Here's a review with a few things that need work:

  1. +++ b/plugins/export_ui/site_verify.inc
    @@ -0,0 +1,180 @@
    +    '#ajax' => array(
    +      'callback' => 'site_verify_export_ui_form_js',
    +      'wrapper' => 'verification-wrapper',
    +    ),
    

    Could we have comments here to explain what the ajax does? When I select an engine, I just get the AJAX spinner and then the exact same form comes back.

  2. +++ b/plugins/export_ui/site_verify.inc
    @@ -0,0 +1,180 @@
    +    '#element_validate' => $engines[$engine]['meta_validate'],
    

    This needs documenting.

  3. +++ b/plugins/export_ui/site_verify.inc
    @@ -0,0 +1,180 @@
    +/**
    + * Submission callback; send form to the next step or save the verification.
    + */
    +function site_verify_export_ui_form_submit(&$form, &$form_state) {
    +  _site_verify_cache_clear();
    +}
    

    Comment and code do not agree.

  4. +++ b/site_verify.install
    @@ -89,6 +108,31 @@ function site_verify_update_3() {
    +function site_verify_update_4() {
    

    This hook_update_N() is numbered incorrectly. It should be 7001.

  5. +++ b/site_verify.test
    @@ -1,5 +1,4 @@
     <?php
    -
     /**
    

    Quite a few of these unrelated whitespace changes in this patch.

realityloop’s picture

Status: Needs work » Needs review
FileSize
20.66 KB

Rerolled for current dev version

netw3rker’s picture

@realityloop,

I tried this patch & it blew away my ability to get back to http://example.com/admin/config/search/verifications is that by design? If so, what's the new admin URL?

netw3rker’s picture

FileSize
4.57 KB

In reviewing the work from @realityloop's patch, it looks to be a pretty extensive overhaul of the site_verify module in order to support the full ctools exportables framework. This might be overkill for what's need in order to meet the goals of the issue (specifically: providing features support for this module).

I've gone ahead and implemented the specific code required just for features support. this involves simply adding the export value to the db_schema, and then implementing the features_render and features_revert hooks.

The only downside to the attached patch is that it relies on "engine" as the key name, so you can't have more than one verification with the exact same engine name. That should be resolvable with the "custom" option though.

Anyhow, sorry @realityloop for the competing patch option, but this much less impactful to the current versions of the module.

netw3rker’s picture

FileSize
5.82 KB

Turns out a machine name is kinda important to the function of this :). I've added a machine name field, and an update hook to populate it. the default machine name for existing entries will be [engine]_[svid]. that can be change before export in the UI by editing the machine name.

carolpettirossi’s picture

#13 works for me.
#14 does not work.

realityloop’s picture

@netw3rker. I was just re-rolling an earlier patch.. no harm or foul. :)