[D7] VOD Infomaniak

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxInfomaniakNetwork2032...

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

Suggestion:

1. Most of the reviewers might not have french knowledge hence request you to provide english translation for your project description.
2. Detailed Project Description will help reviewers understand the project better, hence request your to provide detailed information. You can refer : https://drupal.org/node/1011698

Application blockers

• There are few unused variables in your module, may be those can be removed for improved performance of this module on large sites
• Info file carries version number, normally this is discouraged as drupal.org package manager automatically does this.
• Function Names & Variable names should be prefixed with your module name to avoid clash with other modules
• As per Drupal community guidelines, Git's default master branch should not be used and no downloadable releases can be tied to that branch. Hence you need to remove master branch. Ref : https://drupal.org/empty-git-master
• Info file carries a license tag, which is very unusual. Please refer Writing module .info files (Drupal 7.x). Also Drupal & other contribute content is licensed under GPL by default, hence i request you to check the licensing terms as you have mentioned your application as AGPL.There is only minor differences in these licenses still i request you to double check. Ref : https://drupal.org/licensing/faq#q1
• In those PHP files created for Ajax, you have made an assumptions that this module will reside only in sites/all/modules folder. This is something no one can guarantee, as various developers take difference approaches to contributed module deployment. Looks like this assumption has been made in quite a few places. Those are hard coding, hence please update them.
• Noticed lot of HTML tags in the module, please leverage theme related functions to perform these displays instead of hardcoding.
• Noticed a die statement in vod_infomaniak.callback.inc file. Looks like it was used for debugging & still left there. Please remove this. Also for debugging you can try DSM() function which is part of devel module. Ref : https://api.drupal.org/api/devel/devel.module/function/dsm/7
• vod_infomaniak.module file have a huge inline JS. Please make this external.
• Could not find a valid README.txt. Please follow the guide provide @ https://drupal.org/node/2181737 and create an useful readme file.

Security Concerns

• Approach you have taken to implement ajax calls does not comply with required security standards. Hence i request you to change the ajax implementations. Please follow common Drupal practice for Ajax callback implementation rather than using a Plain PHP file. Some reference : https://drupal.org/node/305747
• Another issue with the PHP code written for Ajax callback is that the enduser posted data are not satanized before evaluating. They are directly used in Database queries.

It looks like you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please note that organization accounts cannot be approved for git commit access. See https://drupal.org/node/1966218 and https://drupal.org/node/1863498 for details on what is/isn't allowed. Please update your user profile so that we don't have to assume that this is a group account.

According to me the below issues are not application blockers, but those are nice to be fixed before releasing.

• PAReview : There are issues reported related to coding standard related. These are not application blockers but its nice to clean them up, if you have time.
• Remove unused codes, instead of commenting them

To Speed Up review Process:
Please manually review 3-4 Projects & have a review bonus tag added to your application. Project applications with review bonus always get priority
Also please go thru the application submission checklist which would be of great help.
Additionally you can also check Apply for permission to create full projectspage for clear instruction on how to have your project description updated.

Hi.

1. Please add appropriate Drupal version tag to the project name.
It should be [D7] VOD Infomaniak.

2. Please provide the description in English.

3. Fix git link in description.
Change it to
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/InfomaniakNetwork/2032985.git vod_infomaniak

The README.txt and docblock comments is still in French. Modules hosted at Drupal.org need to have all its documentation in English. Not being able to read documentation makes it too hard for me to review the module, so I am not even going to try.

You may also want to look at this: https://www.drupal.org/node/997024.

(This is not a review, so I am not changing status.)

Automatic review

Please fix the issues reported by PAReview.sh. As far as I can tell all of these are valid issues and should not be ignored.

Manual review

• You add a @license tag to the file's docblock. All submitted code to drupal.org will be GPL. See https://www.drupal.org/licensing/faq
• All hook docblocks should as the first line state: Implements hook_HOOK().. This is for easier lookup of the hooks api in IDEs. All available hooks are documented in .api.php files in various modules that provide them.
• Hook implementations doesn't require parameter documentation in the docblock.
• function vod_infomaniak_help(): Translating this is a maintenance nightmare. This should be provided as a single string. Using inline style in html is frowned upon. Add classes to list items and provide separate css files, or use semantic html.
• function vod_infomaniak_form_article_node_form_alter():
  • This hook is site instance specific. Sites build from the minimal profile doesn't necessarily have this. Consider using hook_form_BASE_FORM_ID_alter() with BASE_FORM_ID = 'node_form'.
  • You add jquery-2.0 directly from an external resource? How about using Jquery Update and add dependency to it. I believe jquery > 1.9 has feature parity with 2.0, but with support for older versions of IE.
  • And what is this?
    

        drupal_add_js('
    
    
          ', 'inline');
    

    It's not even documented if it were some kind of awful hack around a bizarre problem. Please clean up.

  • Inline style and javascript in HTML. Bad mojo. This needs to be extracted to theme templates or functions and the javascript added separately.
  • $compteur: This is french. Please us english all over.
• function vod_infomaniak_menu():
  • The last menu item, 'admin/config/media/vod_infomaniak/callback' could also have 'page callback' => 'drupal_get_form' as all you have in vod_infomaniak_configuration() is a call to drupal_get_form().
• function vod_infomaniak_configuration_form():
  • define("SALT", "SALT"); This should be prefixed with your module's name to avoid namespace clutter.

  • require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'vod_infomaniak') . '/models/EasyVod_db.inc';
    

    Use module_load_include() instead.

• The files in ajax/*.php seems like they are to be called directly. Why aren't they instead part of the hook_menu() callbacks? It should be documented why not.
• If the js files is part of the integration module, i.e. developed for this project in mind, this needs a lot of attention, and I won't review anything of this, but a keyword is cluttered namespace.

General comments

This is a difficult and rather specific module to review. I haven't tested the integration, only visually looking at the code, its documentation and style. Overall this seems too rough around the edges in terms of using the Drupal API and style. It may be a "prototype" module with working code for the VOD Infomaniak service, but it's not production ready.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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