[D7] WebEmailProtector

Git clone command for the sandbox is missing in the issue summary, please add it.

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.

Hi dsrodzin,

Please provide Project URL and GIT clone URL in the Issue summary.

Here is the process to apply for permission to create full projects: https://www.drupal.org/node/1011698

Regards,
Neeraj Singh

Automated Review:

Pareview is showing lots of issues: http://pareview.sh/pareview/httpgitdrupalorgsandboxdsrodzin2489936git. You should start fixing them

Manual Review:

• hook_help() has a heap of return text unformatted. You should format them.
• No comment block is there for any custom functions. You should also consider adding @param and @return.
• There are also many commented codes in the module. Please finalize and remove them.
• when using require that it is a statement, not a function. It's not necessary to write: require('file.php') instead use require 'file.php'

Hi dsrodzin,

1) Remove INSTALL.txt file from your module
(it only contains 'Please see README.txt').

2) No need to use quote in *.info file
It would be better if your *.info file looks like

name = WebEmailProtector
description = Stop your website email addresses from being scraped.
package = Input filters
core = 7.x
configure = admin/settings/webemailprotector

dependencies[] = filter

Hi dsrodzin,
First of all, I would like to inform you that your git link is wrongly given in your project page. It is git clone --branch 7.x-1.x http://git.drupal.org/sandbox/dsrodzin/2498545.git webemailprotector instead of what is given on the project page. Please correct it.

I have not gone through the functionality of the module. But found these in your code.
1. Please remove INSTALL.txt from your project as it has no purpose.

2. it seems that Webemailprotector is a 3rd party software. So instead of including it as you have done, please follow the guidelines for 3rd party assets/code.

[P.S. Please comment me if I am wrong. ]

3. You have used this piece of code multiple times in your module.
drupal_get_path('module', 'webemailprotector');
Instead store it in a constance, and use it wherever needed.
define('WEBEMAIL_PROTECTOR_MODULE', drupal_get_path('module', 'webemailprotector'));

4. I could not get the use of the function call webemailprotector_proc() just below itself. Does it solve any purpose?

5. Instead of adding js and css outside, you can put them in a function and call it in the place. (line 391-398)

$path = drupal_get_path('module', 'webemailprotector');
$js_pathname = $path . '/scripts/webemailprotector_headerscripts.js';
drupal_add_js($js_pathname);
// INCLUDE THE CSS.
$wep_css = $path . '/css/webemailprotector_emailstyle.css';
drupal_add_css($wep_css, array('weight' => CSS_THEME));

These are some petty issues:

6. You have used some unnecessary declarations/definitions which could be avoided by wrapping the two lines in one.
(a) $wep_css : This does not seem to have any use except

$wep_css = $path . '/css/webemailprotector_emailstyle.css';
drupal_add_css($wep_css, array('weight' => CSS_THEME));

Instead you can have this drupal_add_css( $path . '/css/webemailprotector_emailstyle.css', array('weight' => CSS_THEME));
(b) You have already done

variable_set('wep_ver', 'v1.1.0');
  $wep_ver = variable_get('wep_ver');

in hook_admin(). So no need to again define it as $wep_ver = 'v1.0.0'; in the next line. (line 81).

7. Please add proper comments to hook_filter_preprocess() and hook_info() and hook_info().

8. Please make sure you have deleted all the set variables when their task is complete or when the user uninstalls the module. You can refer to hook_uninstall()

Please comment me out if I am wrong anywhere. Looking forward to learn new things.

Automated Review

Fix JS errors from ES Lint

Manual Review

Individual user account

Yes: Follows guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

- Handle errors for ajax calls like the validate button: if there's an error during the call, the popup stays and the user cannot access the form.

- Images can be loaded. Check the URL of your images to be sure they are correctly loaded to the form.

- Add "<?php" open tag to your .install file.

- In your webemailprotector.module try to create the drupal form arrays instead of long markups with HTML.

Project doesn't even install because .install misses "<?php" open tag (as pointed out in #18) 10 months ago.

I am setting this to "Needs work". Will do a proper review when the errors pointed out by previous reviewers are fixed.

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.