[D7] Dynamic Menu Links

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

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.

no $form = array(); it does not matter.Сan be called decor.

Automated Review

No problems found in Coder or Pareview

Manual Review

Individual user account

Yes, follows the guideline.

No duplication

I do understand the difference of this and path module, I'm just not sure why you would need to put something in menu_router instead of url_alias? Could you please elaborate on a potential use case? I did not really understand the one in README.txt.

Master Branch

Yes, follows the Master Branch.

Licensing

Yes, follows

3rd party assets/code

Does not use third party assets

README.txt/README.md

Yes, follows the guide lines

Code long/complex enough for review

Yes, more then 5 functions and 120 lines of code.

Secure code

Yesy, could not find any security problems

Coding style & Drupal API usage

1. (*) drupal_add_css(drupal_get_path('module', 'dynamic_menu_links') . '/css/custom_menu_links.css'); <- the file is called dynamic_menu_links.css. Do you need a CSS file here at all?
2. (+) You create a field called Menu Path but when you render it you use the node title instead in hook_menu(). Do you need that extra field?
3. You should have admin/config in the configuration sites url so it's added automatically to the admin area, admin/config/search/dynamic-menu-links would be good for instance since that's where you can find path module
4. You should probable add the form id in the css since all those selectors has pretty generic names or add that it should not preprocess since that css is only needed on that page.
5. You should probably make a fieldset or something around the adding of the menus so you understand what that form is for and how it differs from the listing.
6. It would be good to have a possibility to edit the link.
7. Since you saving language specific nodes, should you not load language specific nodes? So if I want to have two different destinations for two different languages for the same path. Just an idea since you already save the language.
8. In dynamic_menu_links_form_validate(), should you not check so the target path exists? Since you say in README.txt "The target path must be an existing site path or alias.", it would be good to validate this.
9. The admin area should maybe be in the install file so it's directly linked from the modules list. See https://www.drupal.org/node/542202#configure
10. Why do you use db_query instead of db_select in the dynamic_menu_links_uninstall()?
11. Minor misspelling in README.txt - sharingyour to sharing your
12. Not necessary an issue, just a question - why do you require path module?

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.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2456981

Project 2: https://www.drupal.org/node/2411721

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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