Javek Uploader

First take a look at: PAReview
Read Drupal coding standards.

Hi and welcome,

As installation and usage instructions are quite important, so please take a moment to create a README.txt that follows the guidelines for in-project documentation.

Here is some manual review :

1) only list files in the info file that contain classes or interfaces
2) you should create a .install file to delete all variables you are using (javek_uploader_subscription_url).
3) you are not running check_plain() on the '$subscription_name'. Not a big deal as the value is set up by trusted users but still..
See http://drupal.org/node/28984

We do really need more hands in the application queue and highly recommend to get a review bonus so we will(/can) come back to your application sooner :
See also #1410826: [META] Review bonus

Best,

Hello,

You could add the package where the module has to be displayed too.

You delete the "javek_uploader_subscription_url" variable on uninstall step. This variable could be defined on the hook_install() I think.

Finally, you can put your JS code into a JS file and passes you Drupal parameter to JS ($subscription_name) with Drupal JS settings.

Regards,

Hi Javek,

It's true that the package information is optional, but, in my opinion, it's a pity to see a module in the "Other" package you know.
Maybe it could be attached to an existing package (I don't know if a "Javek" or "Uploader" package exist) ?

I really do not know if this module is big enough to give you access to do full projects on drupal.org (without the help text it is 107 lines of code - see http://groups.drupal.org/node/195848), but you could apply for promoting it as a single project.

With that said a short manual review:

Odd URL?

Why is the URL added like drupal_add_js('//widgets.javek.com/widget.public-bucket.v1.min.js', should you not have http?

Company account?

Are your user account a personal or a company account?

Manual review of your module:

- The module is quite small-ish, however it seems like it adds a significant amount of functionality to warrant a new module.
- Your use of the t-function is a little abusive and just plain wrong in some cases.

function javek_uploader_help($path, $arg) {
   switch ($path) {
     case "admin/help#javek_uploader":
       $message = '<p>' . t('This module displays a file upload form.') . '</p>';
       $message .= '<p>' . t('The form allows people to securely send you
         large files using the Public Buckets feature of a Javek client
         portal. In order to use the plugin, you must have a Javek subscription.
         You can sign up for Javek here:') .
         ' <a href="https://www.javek.com/sign_up.htm" target="_blank">https://www.javek.com/sign_up.htm</a>.</p>';
       $message .= '<p>' . t('For information on Javek, please visit') . '
         <a href="http://www.javek.com" target="_blank">http://www.javek.com</a>.</p>';
       $message .= '<p>' . t('To display the upload form on your site, go
         to Structure > Blocks. You will see a Javek Uploader block under
         Disabled at the bottom of the page. Drag the block into the region you
         would like the block to show in.') . '<p>';
       return $message;
       break;
   }
 }

You should try to create a single translatable string out of these. You are currently adding four extra strings that need to be translated to the system; you also don't really need to create a new paragraph so often. Links shouldn't be translatable, see here.

The following is even worse; you are actually adding partial sentences to the system:

$form['javek_uploader_subscription_url'] = array(
     '#type' => 'textfield',
     '#title' => t('URL of your Javek subscription'),
     '#default_value' => variable_get('javek_uploader_subscription_url', NULL),
     '#size' => 20,
     '#maxlength' => 50,
     '#description' => '<p>' . t('The URL for your Javek subscription, minus
       "https://www.javek.com".') . '</p><p>' . t('If your subscription is located at') . '
       <strong>https://www.javek.com/example</strong>, ' . t('enter') . '
       <strong>"example"</strong> ' . t('(without quotes).') . '</p>',
     '#required' => TRUE,
 );

You also don't need to wrap the description in another paragraph; that's more the responsibility of the theme. Your default value will return NULL if it is not set, might not be the best option.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.