[D7] Variable File [#2072377]

Variable File module will provide uploadfile type for Variable module.
Variable File allow you upload file as configuration, support thumbnail when
file has filemime is image/jpeg.

Note: when you load vairable with type uploadfile, it just a file id, you need
use file_load to load other properties of file.

How?
===========================
Implement hook_variable_info() with type is uploadfile.

/**
 * Implements hook_variable_info().
 */
function hook_variable_info($options) {
  $variables['myuploadfile'] = array(
    'type'              => 'uploadfile',
    'title'             => t('My Upload File', array(), $options),
    'description'       => t('Upload my config file.', array(), $options),
    'required'          => TRUE,
    'upload_location'   => 'public://myuploaddir',
    'upload_validators' => array(
      'file_validate_extensions' => array('gif png jpg jpeg'),
    ),
    'group'             => 'variable_example',
  );

  return $variables;
}

More information about hook_variable_info and config you can look at Variable module.

Project Page: https://drupal.org/sandbox/flashvnn/2072321
Git: git clone http://git.drupal.org/sandbox/flashvnn/2072321.git variable_file

Comments

Comment #1

zipme_hkt CreditAttribution: zipme_hkt commented 24 August 2013 at 00:29

Status:

Active

» Needs review

Change status to need review

Comment #2

xiukun.zhou CreditAttribution: xiukun.zhou commented 24 August 2013 at 03:43

Status:

Needs review

» Needs work

Review of the 7.x-1.x branch:

Coder Sniffer has found some issues with your code (please check the Drupal coding standards).


FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 1 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 32 | WARNING | Line exceeds 80 characters; contains 85 characters
 35 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Source: http://pareview.sh/ - PAReview.sh online service

Comment #3

zipme_hkt CreditAttribution: zipme_hkt commented 24 August 2013 at 06:10

Status:

Needs work

» Needs review

Fix PAReview.sh

Comment #4

zipme_hkt CreditAttribution: zipme_hkt commented 31 August 2013 at 09:49

Issue tags:

+PAreview: review bonus

Add tag PAReview: review bonus

Comment #5

sergei_brill CreditAttribution: sergei_brill commented 1 September 2013 at 15:19

Status:

Needs review

» Needs work

Hi. I did manual review for your module.
1. As I know, module.api.php file is used to describe hooks which provides the module. Since your module do not provides a hooks, may be it is better to move description of additional options for hook_variable_info to README and remove variables_file.api.php?
2. Could you display the thumbnail for other mime types? At least for png and gif. Files of this types are used often.
3. I define custom variable of type 'uploadfile' via hook_variable_info. I added 'options' for the variable. When I uploaded file via variable form and clicked 'Save configuratio' I got error 'An illegal choice detected'. Now I understand the reason. But may be it would be nice to describe an available options which developer can use in the hook.
4. just a feature request, do not fix for now. Try to integrate with views. It could be useful to display a variable as a file, not file id.

Comment #6

zipme_hkt CreditAttribution: zipme_hkt commented 6 September 2013 at 11:29

Status:

Needs work

» Needs review

Update points 1-3 of @sergei_brill.

Comment #7

Jay.Chen CreditAttribution: Jay.Chen commented 6 September 2013 at 21:32

Status:

Needs review

» Needs work

One issue on the hook_help().
$output = file_get_contents(drupal_get_path('module', 'variable_file') . '/README.txt');
You should wrap the output by using the t function. Otherwise, the string can not be translated.

Comment #8

zipme_hkt CreditAttribution: zipme_hkt commented 7 September 2013 at 12:44

Status:

Needs work

» Needs review

Fix @Jay.Chen issue.
Now easy translate README help by define README file with format "README.{lang_code}.txt".

Comment #9

bohart

Lutsk, Ukraine

CreditAttribution: bohart commented 7 September 2013 at 13:45

Status:	Needs review	» Needs work
Issue tags:	-PAreview: review bonus

Hi zipme_hkt
I found next problems:

You should use file_build_uri() function instead of hardcoded uri "public://vfile" because file default scheme may be different.
In file_usage_add() (line: 164) you set global user uid as $id parameter. So maybe you should use the same global user uid in file_usage_delete() (line: 139) as $id.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Comment #10

zipme_hkt CreditAttribution: zipme_hkt commented 7 September 2013 at 14:17

Status:

Needs work

» Needs review

Fixed issue reported by @bohart, thank you.

Comment #11

Jay.Chen CreditAttribution: Jay.Chen commented 11 September 2013 at 13:48

Status:

Needs review

» Needs work

For the #8, I think your solution is no good.
1. It is not a standard Drupal way to translate string.
2. User can't search the string via the admin UI.
3. The string can't be imported or exported via the admin UI.
4. It is difficult to update your module.
5. In your way, user has to manually create a translated file like "README.{lang_code}.txt" in your module folder. But not all end users have FTP permission to do that or they even don't know how to do that.

My suggestion:
Why don't you use the t() function? If you think the help text is too long, you can make it shorter as possible.

Comment #12

zipme_hkt CreditAttribution: zipme_hkt commented 23 September 2013 at 06:07

Status:

Needs work

» Needs review

Fixed #8

Thanks

Comment #13

kscheirer

English

Vallejo

CreditAttribution: kscheirer commented 23 September 2013 at 23:37

Status:

Needs review

» Reviewed & tested by the community

Can you talk a little more about why someone would want to use this module on your project page? From reading the description it sounds interesting, but I'm not sure what use case it solves.
You have a couple minor errors reported here: http://pareview.sh/pareview/httpgitdrupalorgsandboxflashvnn2072321git.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Comment #14

zipme_hkt CreditAttribution: zipme_hkt commented 24 September 2013 at 15:42

Fixed pareview.sh warning.
Thanks.

Comment #15

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 23 October 2013 at 19:31

Status:

Reviewed & tested by the community

» Fixed

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

manual review:

variable_file.module: @file doc block is wrong.

Otherwise looks good to me, so ...

Thanks for your contribution, zipme_hkt!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.