[D7] Node Help Text

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

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.

1. Add the drupal version to your issue name ("[D7] Node Help Text")
2. Add the git clone command to the issue summary

Manual Review

general: Prefix your variable name with the module name to avoid duplication issues with other modules (in all your variable_set and variable_get function calls)
general: It appears that any user with 'access content' permission can add or save help text for node types, which is then displayed to every user. Consider implementing a custom permission or letting module users know that this is the case.
node_help_text.module: fix the documentation block for node_help_text_form_submit(), it currently states that it implements hook_permission()
node_help_text.module: fix the documentation block for node_help_text_viewb()
node_help_text.module: Consider renaming node_help_text_viewb() to something more descriptive related to its function

Hi ashish.verma85,
Please correct your git link, as this requires your password for cloning.

Hi ashish.verma85,
Now the git command is correct.
I would like to point out few things,
1. In your .info file, remove the last line. Its not required.
2. In your .install file, the function should be hook_uninstall() rather than hook_install().

Please correct them.

Manual Review:
Individual User Accounts
Yes: Follows the guidelines for individual user accounts.

Licensing
Yes: Follows the licensing requirements.

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

Secure code
Yes: Meets the security requirements.

Hi ashish.verma85,

1) In README.txt add the config details.

2) On config page, after saving help text it is not getting pre-populated in text box.It shows empty.
I guess $value_field = variable_get('FIELD_VALUES'); must be 'NODE_HELP_TEXT_FIELD_VALUES'

Hi ashish.verma85,

1) No need to use quote in info file
It should be
description = Provides Interface to add help text for each Content type.

2) There is already a menu name 'help' available in Drupal.
You need to change menu name.

3) configure link is missing in your .info file.

4) In line no. 90 (*.module),
You have written <?php $ntype = $node->type; ?>

I have assigned help block on a page which have no any content type like as view page. it's showing below error, reducing the reliability of the module.

Notice: Trying to get property of non-object in node_help_text_view_content() (line 90 of D:\xampp\htdocs\sitename\sites\all\modules\node_help_text\node_help_text.module).
    Notice: Undefined index: in node_help_text_view_content() (line 92 of D:\xampp\htdocs\sitename\sites\all\modules\node_help_text\node_help_text.module).

5) You should provide the hook_help to allow site builders to find information about your module using Drupal UI.

Above point 3.
configure = admin/content/help
is still missing in your *.info file.

Module works well and code is well written. I have only a few suggestions for improvement:

1) At the end of README.txt, you mention setting permissions. Which permissions? This module does not define any, so I would just rephrase that. Blocks are by default visible for all users, even anonymous, but that can be configured in block settings, per block. So I really think there is no need for other permissions.

2) Block name is help, but there is already block system-help...consider changing it to Content help, or something like that.

3) It is recommended to always implement hook_install(). Here you can find an example.

4) Coder module reported issues in .module file on lines 32 and 35:

Line 32: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
      '#title' => t('Help text for:') . ' ' . $nname,

Line 35: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
      '#description' => t('Add help text here for') . ' ' . $nname,

Here you can find an example for better (the right) way of writing t() functions and passing parameters.

5) Improve documetation for form-generating functions
There is no hook_form_submit() in Drupal 7, that is just your submission handler.

6) In .module file on line 109, sentence is a bit confusing; maybe change to "all the content type help texts"?

Hi,
1.it is recommended to add PHP version and package name in your .info file.

Your first review is not a manual review, you just posted git URL correction. I'm going to remove the bonus tag, as well as links on reviews from the issue summary. Make sure to read through the source code of the other projects, as requested on the review bonus page. You can add it again if you have done 1 more review of other projects.

Hi ashish.verma85,
I think you have not taken into consideration this comment https://www.drupal.org/node/2525758#comment-10101418 . Please make sure, you have gone through all the comments and recommendations of the reviewers as they help you to improve your module.

An advice from git administrators.
The project might prove useful, but doesnot satisfy the code length requirements. So should it be given Single project promotion?
This is only a problem according to me, else it can be approved to RTBC.

It is working fine and ready for RTBC for a full release

Hi ashish.verma85,
Sorry for providing wrong link. the correct link is https://www.drupal.org/node/2525758#comment-10100364 .
Please change the block name from "Help" to something like "Help Text" so that it reflects your project's properties. Drupal already has "Help" with it. So please change it as per the comments of babusaheb.vikas

I'll look at this tonight or tomorrow.

Automated Review

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

• 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

No: Causes module duplication and/or fragmentation. Your project page and README should elaborate on how this is different than custom blocks provided by Drupal core, and by the help text already provided on the node forms, which is configurable. Right now, I don't see a difference. Not a blocking issue.

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

Not seeing any problems.

Coding style & Drupal API usage

Why do you need PHP 5.3 for this? I didn't see any 5.3 specific things?

node_help_text_install(), that isn't the best wayto handle translated links. See the comments on https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7

NODE_HELP_TEXT_FIELD_VALUES, normally variables are all lower case.

The config page is protected by 'administer site configuration', and node_help_text_form() uses 'full_html'. You may need to rethink this if you
use your own permission in the future, as you will need to take into account whether the user has access to this text format.

The check_plain() on the #title and #description in node_help_text_form() are superflous. t() w/ @arg placeholders will generate a safe string. See https://www.drupal.org/node/28984

You need to handle content type creation and deletion.

node_help_text_view_content(), the variable_get() should have a default value. The function should also return a render array (TBH, I don't 100% recall what will be in the value for that form element, though).

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. This appears to be the first thorough review (based on the use of the Review Template), so I am assigning to @klasui for a second look, if he has time.

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.

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

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

  
  FILE: /home/klausi/pareview_temp/node_help_text.info
  ----------------------------------------------------------------------
  FOUND 1 ERROR AFFECTING 1 LINE
  ----------------------------------------------------------------------
   6 | ERROR | Drupal 7 core already requires PHP 5.2
  ----------------------------------------------------------------------
  

• 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: can you describe what the difference is to just putting help text with the fields of a node for example?
2. I don't think it is a good idea to store your help text in the variable system, since this will be loaded on every single page request, while it is only needed when the block is displayed.

But otherwise looks good to me, so ...

Thanks for your contribution, ashish.verma85!

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.