[D7] Contact form extra validation

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 ravi.khetri
There are some thing you need to do with your issue:

• Change your git clone command because it is personal git clone command. It should be git clone --branch 7.x-1.x http://git.drupal.org/sandbox/ravi.kumawat/2276281.git
• Provide more detail in your sandbox project (Requirement, Installation, ....)
• Add PA Review result link to your issue http://pareview.sh/pareview/httpgitdrupalorgsandboxravikumawat2276281git

Manual review:

Line 10,11,12 file contact_validation.admin.inc function contact_validation_admin_configure()

You should provide default value in variable_get().

variable_get('numeric', FALSE);

Line 15,32, file contact_validation.admin.inc function contact_validation_admin_configure()

You should use t() function in #title property.

Line 38,39 file contact_validation.module, function contact_validation_form_alter()

You shouldn't use function drupal_add_js() to add js, js settings to form/form element. You should use property #attached.

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.

Automated Review

[Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]

Manual Review

Individual user account

[Yes: Follows].

No duplication

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

Master Branch

[No: Does not follow] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[No: Does not follow] the guidelines for 3rd party assets/code.

README.txt/README.md

[No: Does not follow] 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.

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*) "variable_get('numeric');,": all variables defined by your module must be deleted in hook_uninstall().
2. (+) in hook_form_alter(), your code affect all forms, so check the if statement.
3. configuration page can't be found on the menu, just in the modules page. so add attribute 'type'. ex "'type' => MENU_NORMAL_ITEM,"

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

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.

Hi Ravi,
Here is the automated review for your sandbox.
Please correct all the druaplCS issues mentioned below.

Review of the 7.x-1.x branch (commit 7ce04cc):

• Remove all backup files from your repository:
  

  ./contact_extra_validation.install~
  

• ./contact_extra_validation.module: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming
  

  function _contact_validation_extra(&$form, &$form_state) {
  

• ./contact_validation.admin.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming
  

  function contact_validation_admin_configure() {
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

FILE: .../www/drupal-7-pareview/pareview_temp/contact_extra_validation.install
---------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 3 LINES
---------------------------------------------------------------------------
 13 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
 13 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 1
 14 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
 14 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 1
 15 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
 15 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 1
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------


FILE: ...r/www/drupal-7-pareview/pareview_temp/contact_extra_validation.module
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
 23 | ERROR | [x] Expected 1 space after "=>"; 2 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Time: 84ms; Memory: 5.5Mb

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

Removing review bonus tag, you have not done any manual review, you just repeated the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

And please add references to your review comments to the issue summary next time.

• The last commit message is just one word, you should provide a meaningful short summary what you changed. See https://www.drupal.org/node/52287

Review of the 7.x-1.x branch (commit 490e36c):

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

  
  FILE: /home/klausi/pareview_temp/contact_extra_validation.info
  ----------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  ----------------------------------------------------------------------
   1 | ERROR | Duplicate entry for "core" in info file
  ----------------------------------------------------------------------
  
  Time: 97ms; Memory: 6.25Mb
  

• ESLint has found some issues with your code (please check the JavaScript coding standards). See attachment.
• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

manual review:

1. project page is too short, please describe the use case in more detail what your module does. See https://www.drupal.org/node/997024
2. contact_extra_validation.install: that function there should be named contact_extra_validation_install() and the docblock is wrong. Please test your uninstall hook next time!
3. "variable_get('title-field', 0);": all variables used by your module need to be prefixed with your module's name to avoid name clashes with others.
4. contact_extra_validation_form_alter(): doc block is wrong, this is hook_form_alter().
5. contact_extra_validation_form_alter(): so this will add the $form['message'] entry to every form on the Drupal site? You should implement hook_form_FORM_ID_alter() if you are only targeting one form. See https://api.drupal.org/api/drupal/modules!system!system.api.php/function...
6. This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

So the wrong variable names and the alteration of any form on the Drupal site are blockers right now. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects. And please add all your manual review comments to the issue summary, so that we can better track them.

admin.js: don't use jQuery(document).ready(), use Drupal.behaviors instead. See https://www.drupal.org/node/756722

Looks like you forgot to push your changes to the git repository?

Please add all your reviews of other projects to the issue summary so that we can better track them.

manual review:

1. looks like you forgot to fix the eslint errors I pointed out in my previous comment?
2. The Git commits are not connected to your user account. You need to specify an email address. See https://www.drupal.org/node/1022156 and https://www.drupal.org/node/1051722
3. The line "; Information added by drupal.org packaging script on 2014-05-26" should be removed from the info file, this is a comment by the automated packaging script and should not be added by you.
4. contact_extra_validation_form_contact_site_form_alter(): do not call drupal_add_js() in a form constructor, use #attached instead as you are already doing for the JS file. The same works for settings, too.
5. contact_extra_validation_() is not a good function name. It should describe what the function does, example "contact_extra_validation_validate_contact_request" or similar.
6. "'Numeric latter is not allow in this field.'" should be "'Numeric letters are not allowed in this field.'"
7. "'Special character is not allow in this field.'" should be "'Special characters are not allowed in this field.'".

But that are not critical application blockers, otherwise looks RTBC to me.

Assigning to stBorchert as he might have time to take a final look at this.

no objections for more than a week, so ...

Thanks for your contribution, ravi.khetri!

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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.