[D7] Autocomplete Dependent Population

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

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.

I think you should consider creating submodule with your example form.
Otherwise anyone who enables your module gets www.example.com/autocomplete_dependent_test_one url exposing your testing form to every user and there's no way to disable it without disabling the whole module.

Another thing which is not clear from your example is should all possible dependent elements data be returned in every call of autocomplete_dependent_population_test_one_callback() -- which could cause excessive database calls in many scenarios.

And why do you require widget type ('textfield', 'textarea', 'select') to be specified in returned data array, if you can easily determine it by widget name on the clientside?

Manual review comments:

Some minor review comments

1) Please add line break before return statments.
2) In autocomplete_dependent_population_process its better to add a isset every where it is accessing the element variable properties to avoid warnings
3) May be in theme function theme_autocomplete_dependee_field you can use drupal7 renderable array concept which way anyone using module can override the theme easily. And moreover output variable is unnecessary there, instead you return the element directly.
4) May it is better to implement hook_requirments and set the check for required library presence.
5) A small drupal standards formatting issue js doesnot follow 2 spaces per tab setting. Apart from that there are some formating issue like space after if etc.
For ex as below
jQuery.each(response['data'], function(index , value) {
cust_id[count] = index;
count ++;
});

6) And also could please try modularizing the js code since it is difficult to understand.

Please let me know if you need more info.

Regards,
Prajwal

Hello Debarya,

Thank for your contribution.

At the time of module installation i am getting following errors.
1 - Notice: Undefined offset: 0 in autocomplete_dependent_population_requirements() (line 16 of /opt/lampp/htdocs/drupal/sites/all/modules/autocomplete_dependent_population/autocomplete_dependent_population.install).

2- Jquery UI Autocomplete is not available.

If There is any kind of dependency on Jquery UI Autocomplete, Then you must describe it on requirement doc as well as info file.

Thanks!!

Please fix these two issue mentioned in previous comments.

Hello Debarya,

If Jquery UI Autocomplete is already available in Drupal core installation then there is no need to put this in info file. however i got error at the time of module installation, may be due to path which you wrote in your code, dont know exactly.

Today i will try again with your module.

Thanks!!

Assigning to myself for next review.

Automated Review

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

• 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

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes/No: Follows the licensing requirements

3rd party code

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

README.txt/README.md

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

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes/No. If "no", list security issues identified.

Coding style & Drupal API usage

Why are you using the global jQuery object instead of the $ paramater to the closure? Also, you need to use the context and settings that get passed to the behavior.

The formatting in the JS does not meet Drupal coding standards. See https://www.drupal.org/coding-standards

The JS needs more comments to describe what is going on. It is very hard to read.

If jquery.ui.autocomplete.min.js is part of core, why are you checking for it in a hook_requirements()?

You can use #attached for your settings, too, instead of drupal_add_js().

drupal_static is preferred over the native static in PHP.

(+) autocomplete_dependent_population_process will clash with hook_process. You need to rename this.

(+) Avoid linebreaks in translated strings.

(+) Your example menu paths should have access controls on them, 'access content' at the very least.

(+) You have a decent amount of bad formatting. I am not sure why the automated tests didn't pick them up. Read through the coding standards and try to use an IDE that is Drupal coding standard aware (like PhpStorm or Aptana).

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.

There are some big things that need to be addressed, but none are critical blockers, so I am setting RTBC. I want a second review by another admin on this because I have a feeling that I may have missed something.

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.

Forgot to unassign myself.

Assigning to myself for next review.

Automated Review

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

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

(+) autocomplete_dependent_population_generate_element(): could you document @return here so that is clear what comes out of this function? See @return: Function return values

(+) autocomplete_dependent_population.js: Some comments length exceeding 80 characters, all comments must be under 80 characters read https://www.drupal.org/node/1354 again.

(+) autocomplete_dependent_population.js: you are encoding this value twice.

var input = Drupal.encodePath(Drupal.checkPlain($.trim($(this).val())));, I think Drupal.encodePath should be enough.

Remove extra space from count ++;.

Remove extra comma & space from // Remove the last , from autocompleteDependeeFields.

Project Page: Extend this little bit more, better to go through Tips for a great project page again.

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.

Not seeing any blocking issues here, also issues mentioned by other admins have been addressed.

Inline comments and code formatting looks good now.

Thanks for your contribution, debaryadas!

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.