[D7] parsley.js

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,

Some errors can be easily fixed in README.txt.

Line exceeds 80 characters

Fix:
The lines are too long. Maximum of 80 characters per line is allowed.

A comma should follow the last multiline array item. Found:
| | 'another_form':

Fix:

  array(
    'example_form',
    'another_form',
  );

Avoid backslash escaping in translatable strings when possible,
| | use "" quotes instead

Fix:

t("It's my field, only I get to fill it in")

Hi,

I noticed that Parsley.js requires least version 1.6 of jQuery. So you could check the current version of jQuery in hook_requirements(), like this:

$version = variable_get('jquery_update_jquery_version', 'default');
if ($version == 'default' || $version == '1.5') {
// Running an old version of jQuery...
}

Interesting module. You state that it is a lot more lightweight than a similar module, Clientside Validation. In what way is it more lightweight? Presumably not the js itself because jquery.validate appears to be a smaller file than parsley.js. The most recent version of parsley.js is even larger.

I would include a link to parsleyjs.org, so that prospective users of your module can more quickly assess the parsley.js library.

Parsely.js seems to be rapidly evolving and 2.0.0 was released in April. I don't know how much effort it would be for you to ensure your module is compatible with 2.0.0 but all the docs and downloads on parsleyjs.org are for the latest version. [Note: I see you have it on your list under Todo and you specifically reference the version your module is compatible with in your .module file.]

There are a few typos in your project page and project files:

-- On the sandbox page...
- under Usage, you might want to include some inline documentation that makes it clear the return array comprises form ids.
- under Rules "same rule name as the parsley.js documentation"

-- parsley_js.api.php
- "Hook too set which forms will have the parsley.js validation on them"

-- parsley_js.info
- maybe use a more explanatory name like "Parsley Validation"
- suggest you change description to "Enables developers to integrate the parsley.js client-side validation library with forms"

-- parsley_js.install
- "Provides the requirements, install and uninstall functions for parsley_js"

-- parsley_js.module
- "Currently supports parsley.js 1.x"
- "Declaring the parsley_js_form hook..."
- "First check whether the there is a cache entry"
- "The rule with either params or no params, it follows the parsley.js syntax
- "The rules [space] tring parsed from #rules"
- "@see http://parsleyjs.org/documentation.html#basic-constraints" should be "@see http://parsleyjs.github.io/Parsley-1.x/documentation.html#basic-constraints"

Are you sure that jQuery 1.9 is required? The higher the version of jQuery you require the higher the potential for conflicts with other modules that may not be compatible with that recent a version of jQuery. This test for jquery version might be more versatile:

$min_jquery_version_required_by_parsley_js = 1.6
if (version_compare(variable_get('jquery_update_jquery_version', '1.5'),$min_jquery_version_required_by_parsley_js, '<')) {
// Running an old version of jQuery....
}

Setting to needs work based on above comments.

A few of my own...

    // Rebuild list of known hooks for parsley_js_form.
    $declared_forms = module_invoke_all('parsley_js_forms');

This seems like a really weird way round. You're already implementing hook_form_alter(), so why not detect a property in the $form_state array, say $form_state['parsley_validation'] = TRUE?

              // Borrowed from FAPI module.

You mean https://drupal.org/project/FAPI ?

define('PARSLEY_JS_RULE_NOT_FOUND', 1);

You only use this constant once, and never check for the return value. What's it for?

> At it's core it's simply injects the correct attributes into the relevant FAPI

Typos. "At its core, it ... "

> 2. jQuery Update (Tested with jQuery 1.9, millage may vary with earlier

Mileage.

The README maybe needs a run through a spellchecker?

Interesting module! Might use it in my projects when it gets a stable release :)

Hi botto,

The overall impression of your module is very good to me.

I have tried to add a validation rule to a node form on the title, which worked after a few tweaks.

First, I had to adjust the module weight of parsley_js to run after my custom module. You should consider giving you module a weight by default, and/or add a note the README which explains the situation.

Second, I had a fatal error because not every element in the node $form array is an array. This can easily be fixed by an is_array check - I have a patch attached with this change.

As I final remark I would strongly encourage you to add an parsley_js_example module, which showcases most (or even all?) validations on an example form. this would allow easy testing of the module and also developers can easily copy/paste rules.

PS: I would be glad about a review of my project application here: https://drupal.org/node/2288389

Fix these pareview errors: http://pareview.sh/pareview/httpgitdrupalorgsandboxbotto2140265git

I did a manual review and your seems alright.

Installed your module and found several issues. I tested this module in Drupal 7.30. Downloaded the library in libraries directory, but it was unable to detect the library and status report was showing error that it cannot find library. I have kept the library inside sites/all/libraries/parsleyjs. Also It was unable to detect jQuery version, jquery version was set to 1.10 but it was showing error. Tried clearing cache but did not worked.

- You should provide options in field settings to enable field validation using parsleyjs, as user would want a UI setting rather than enabling validation by writing code. Or even better, provide an admin configuration where admin can select whether they want site wide form validation handled by parsleyjs.
- You should provide the link for example parsleyjs validation somewhere, somewhere in help or module configuration would be good.

Automated Review

Best practice issues identified by pareview.sh

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
48 | ERROR | [x] Expected 1 space before "<"; newline found
48 | ERROR | [x] Expected 1 space after "<"; 0 found
48 | ERROR | [x] Expected 1 space before "?"; 0 found
48 | ERROR | [x] Expected 1 space after "?"; 0 found
89 | ERROR | [x] Expected 1 space after "*"; 0 found
90 | ERROR | [x] Expected 1 space before "*"; 0 found
90 | ERROR | [x] Expected 1 space after "*"; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Manual Review

Individual user account

Yes: Follows the 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

List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance.

1. Found some minor issues in pareview,please resolve.

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.

Sorry for the delay. Make sure to review more project applications and complete your review bonus and this will get finished faster.

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

1. I think '#rules" in form arrays is not a good name, I would rather go with #parsley_rules or similar. This has nothing to do with the Rules module and prefixing such things with your module's name is always a good idea.
2. parsley_js_form_alter(): so this will only work on the first level elements of the form? What if I have a nested form with elements further down the tree? I think you should use element_children() here to go through the elements instead and then call element_children() recursively again to get really all form elements in the tree.
3. parsley_js_form_alter(): I don't think you should show the error message to the end user since that is completely useless information for them. Either only show it to users with a certain admin permission or use watchdog() to log the error. And you should include the form ID in the log message. Same in _parsley_js_numeric_args_parser().
4. You could also read the #maxlength property automatically and potentially add a parsley rule for that, just an idea.
5. "if (!empty($form['#parsley_js']) && $form['#parsley_js'] === TRUE) {": the second condition is not really necessary? Same for the #required condition below.

But otherwise looks RTBC to me.

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

Thanks for your contribution!

Looks good to me.
In addition to Klausi's notes I would highly recommend using element_children() to loop through the form elements. I've tried to add parsley-rules to a node form but this isn't possible since only the first level of the form hierarchy is scanned.

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.