[D7] Uploadcare

Some Drupal standards missing. see here:
http://ventral.org/pareview/httpgitdrupalorgsandboxgrayhound1905848git

By using external library in drupal module, try to use it with libraries module.

there are two "read me" files. remove README.md

Still Drupal standards missing. see here:
http://ventral.org/pareview/httpgitdrupalorgsandboxgrayhound1905848git

A few points concerning your module:

When installing i get the following error:
Warning: require_once(C:\xampp\htdocs\review/modules/uploadcare/uploadcare.field.inc) [function.require-once]: failed to open stream: No such file or directory in require_once() (line 8 of C:\xampp\htdocs\review\sites\all\modules\review\uploadcare\uploadcare.module).

This is because you use:

require_once DRUPAL_ROOT . '/modules/uploadcare/uploadcare.field.inc';

have a look at drupal_get_path at: http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_get...
You should then use this:

require_once drupal_get_path('module', 'uploadcare') . '/uploadcare.field.inc';

About your commenting:

/**
 * Admin menu.
 */
function uploadcare_menu() {

Since you are using hook_menu() comments should read Implements hook_menu(), you are using this right in uploadcare.field.inc.

Also, have a look at the coder module: http://drupal.org/project/coder
This module reviews your code and gives you an idea about problems that may exist.

Using the coder module it gives me 2 potential problems in uploadcare.field.inc.

Line 187: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized. (Drupal Docs) [security_fapi_title]

        '#title' => $element['#title'],

And

Line 188: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized. (Drupal Docs) [security_fapi_title]

        '#description' => $element['#description'],

Like the error above shows, have a look at the check_plain function.

In line 185 of uploadcare.field.inc. you call function uploadcare_field_widget_form_postfix with the @ to hide errors. If well coded this should not be necessary, could you explain why you use this?

Besides your comments about functions you do not use any inline comments, these are a great way to comment code blocks inside a function, try to use these because they can save you and other programmers a great deal of time figuring out what the piece of code in a function does.

Hello grayhound ,

Thank you for great module.

I have detected some issues.

1. Then I Enable this module I get "HTTP Error 500 (Internal Server Error)".
PHP Fatal error: require_once(): Failed opening required '/var/www/drupal-7.18/modules/uploadcare/uploadcare.field.inc' (include_path='.:/usr/share/php:/usr/share/pear') in /var/www/drupal-7.18/sites/all/modules/uploadcare/uploadcare.module on line 8, referer: http://drupal7.loc/radmin/modules
2. In install file remove uploadcare_public_key and uploadcare_secret_key variables http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/variable_del/7
3. Regarding 3rd party libraries http://drupal.org/node/422996

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Here goes the review for 'uploadcare' 7.x-1.x branch:-

1. Online review report for drupal codeing standard is http://ventral.org/pareview/httpgitdrupalorgsandboxgrayhound1905848git-7...
2. In your project application, link to the corresponding project page is missing.
3. In the project page, default branch is not set as 7.x-1.x
4. On installing the module it gives a fatal error as :-
Fatal error: require_once() [function.require]: Failed opening required '/home/debtcc/docroot/drupal720/modules/uploadcare/uploadcare.field.inc' (include_path='.:/usr/share/pear:/usr/share/php') in /home/debtcc/docroot/drupal720/sites/all/modules/under_review/uploadcare/uploadcare.module on line 8

Few things:

• Your line lengths exceed 80 characters in your README file. This will fail the Drupal coding standards.
• As you are using the field api, you should add a dependency on the field module in to your .info file.
• In your module file, move the uploadcare_menu above uploadcare_admin

broken

When trying to enable the plugin I received the following errors:

Warning: require_once(modules/uploadcare/uploadcare-php/uploadcare/lib/5.2/Uploadcare.php): failed to open stream: No such file or directory in require_once() (line 12 of /var/www/modules/uploadcare/uploadcare.field.inc).
Warning: require_once(modules/uploadcare/uploadcare-php/uploadcare/lib/5.2/Uploadcare.php): failed to open stream: No such file or directory in require_once() (line 12 of /var/www/modules/uploadcare/uploadcare.field.inc).
Warning: require_once(modules/uploadcare/uploadcare-php/uploadcare/lib/5.2/Uploadcare.php): failed to open stream: No such file or directory in require_once() (line 12 of /var/www/modules/uploadcare/uploadcare.field.inc).

Attempted to enable the plugin again and received a different error:

Warning: require_once(modules/uploadcare/uploadcare-php/uploadcare/lib/5.2/Uploadcare.php): failed to open stream: No such file or directory in require_once() (line 12 of /var/www/modules/uploadcare/uploadcare.field.inc).

Automatic review
There are still some errors reported by PAReview: http://ventral.org/pareview/httpgitdrupalorgsandboxgrayhound1905848git-7.... Most of them are long lines warnings. As you said it's not always possible to fix this, but some can be fixed. There are also two errors (according to drupal coding standards).

Now that you added the 7.x-1.x branch you should change the default branch in your project and remove the master branch.

Manual review
I think the module settings page should be under Configuration > Media, and not at the top level of the menu. You can achieve this by changing the path to admin/config/media/uploadcare in uploadcare_menu. Maybe also add a description there.

This is more of a question, but if I set a field as unlimited, when I click on the Add another item button a textfield becomes visible. I don't know if this is desired behaviour or not.

Other than this minor fixes I think this is a good module and should be RTBC.

I think this is RTBC, so I'm changing status, but I will suggest an improvement (although it's not blocking the project application). I think you should consider using Libraries API to load the uploadcare-php library. This way the library can sit in sites/all/libraries, and can be used by other modules, control which version is used, etc. It's a simple change that I think will improve your module.

• It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
• The install docs in your README look like they have the wrong paths. Instead of modules/uploadcare should that be sites/all/modules/uploadcare? To go along with that, the Libraries API module is a recommended method for adding 3rd party dependencies with using hard-coded paths in your module.
• Also, you should avoid having any PHP in global scope at the top of your file - that will get executed on every Drupal page request. Please load your libraries only when they are actually needed.
• You have 'access arguments' => array('administer uploadcare'), but I don't see a hook_permission() anywhere.
• In uploadcare_field_widget_form_prefix(), don't hard-code the version 0.8.1.2 - can you read it from Uploadcare.php instead?
• Use l() instead of
  

    $html = sprintf('<a href="%s">%s</a>', $file->getUrl($filename), $file->getImgTag($filename));
    // ...
    $html = sprintf('<a href="%s">%s</a>', $file->getUrl($filename), $filename);
  

• In uploadcare.module() you should validate those keys if possible - there must be some standard to them.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.