The module should really use the File API instead of storing file details in {site_verify}. It can save its uploaded files to sites/default/files/site_verify

It should even be possible to backport this to Drupal 6 as well.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lathan’s picture

FileSize
10.76 KB

Here is a patch to do this.

lathan’s picture

Status: Active » Needs review
lathan’s picture

FileSize
11.72 KB

I have rerolled this patch with the addition of #1391860: Add validation for meta tags as they conflict.

lathan’s picture

FileSize
11.59 KB

Fixed white space issue.

lathan’s picture

FileSize
11.96 KB

Last time, fixed issue when merged patches.

mrfelton’s picture

Status: Needs review » Needs work

I think this is missing db_drop_field('site_verify', 'file');. The schema module is reporting "file: unexpected column in database".

lathan’s picture

FileSize
12.25 KB

Issues fixed.

- fixed meta verify was on wrong var.
- install schema fixed as pointed out by @mrfelton
- removed unneeded meta output at url.

lathan’s picture

Status: Needs work » Needs review
joachim’s picture

Issue summary: View changes
Status: Needs review » Needs work

Patch no longer applies cleanly I'm afraid.

A few things that need work too:

  1. +++ b/site_verify.admin.inc
    @@ -174,27 +171,66 @@ function site_verify_edit_form($form, &$form_state, $record = array(), $engine =
    + * Helper function to validate meta tags
    + * @param  string $string
    + *   The meta tag string to validate.
    + * @return boolean
    + *   A flag to indicate if validation passed.
    

    Some blank lines are needed here to conform to docs standards.

  2. +++ b/site_verify.admin.inc
    @@ -174,27 +171,66 @@ function site_verify_edit_form($form, &$form_state, $record = array(), $engine =
    +  // Pre pair saving this file.
    

    'Prepare'?

  3. +++ b/site_verify.admin.inc
    @@ -174,27 +171,66 @@ function site_verify_edit_form($form, &$form_state, $record = array(), $engine =
    +  $destination = 'private://site_verify/';
    +  file_prepare_directory($destination, FILE_CREATE_DIRECTORY);
    

    What happens if I have not configured a private directory?

  4. +++ b/site_verify.admin.inc
    @@ -174,27 +171,66 @@ function site_verify_edit_form($form, &$form_state, $record = array(), $engine =
    +/**
    + * Helper function to validate meta tags
    + * @param  string $string
    + *   The meta tag string to validate.
    + * @return boolean
    + *   A flag to indicate if validation passed.
    + */
    +function site_validate_meta_tag($form, &$form_state) {
    

    This is part of the patch at #1391860: Add validation for meta tags.

  5. +++ b/site_verify.admin.inc
    @@ -212,10 +248,25 @@ function site_verify_edit_form_submit($form, &$form_state) {
    -      drupal_write_record('site_verify', $form_state['values'], array('svid'));
    +      db_update('site_verify')
    +        ->fields(array(
    +          'engine' => $form_state['values']['engine'],
    +          'fid' => $form_state['values']['fid'],
    +          'meta' => $form_state['values']['meta'],
    +          'file' => $form_state['values']['file'],
    +        ))
    +        ->condition('svid', $form_state['values']['svid'])
    +        ->execute();
    

    This looks like an unrelated change, at least partly. Why change from drw() to db_update()?

  6. +++ b/site_verify.install
    @@ -128,3 +127,18 @@ function site_verify_import_ghs() {
    + * Add new column for file ID and drop old file_contents file.
    

    s/file/field/ ?

  7. +++ b/site_verify.module
    @@ -177,11 +178,20 @@ function site_verify_get_engines() {
    +    return t('This is a verification page for the @title search engine.', array('@title' => $verification['file']));
    

    I don't follow this change.

joachim’s picture

Also, compare with #1230956: Support for Exportables (Features), which is (I think) incompatible with the direction taken here.