Readmore per node

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.

Automated Review

Pareview: The last commit message is just one word, you should provide a meaningful short summary what you changed.
Coder: No problems found.

Manual Review

Individual user account

Yes

No duplication

Yes: There is https://www.drupal.org/project/read_more for Drupal 7. But it only seems to allow for system wide customization.

Master Branch

Yes

Licensing

Yes

3rd party assets/code

Yes: No 3rd party items used.

README.txt/README.md

No: Does not follow the guidelines for the README Template.
The project page does not contain any documentation at all.
You could recommend the use of token module.

Code long/complex enough for review

Yes

Secure code

Yes

Coding style & Drupal API usage

No issues found.

Hi hmdnawaz,

1) No need to use quotes in *.info file.
Your *.info file should looks like

name = Read more per node
description = Adds functionality to change read more link text per node
core = 7.x
package = Other

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

3) Check this link http://pareview.sh/pareview/httpgitdrupalorgsandboxhmdnawaz2569551git and fix that error of README.txt file.

Your module needs to have 1 newline at end of file for Readme.txt
Please make that change as well.

Hi Ahmad, thanks for your contribution. Let me go through some things I noticed when testing your module.

1) Can you please explain why using this module is preferable to using a regular text field attached to a content type which would then be displayed as a read more link in views lists? Obviously in this case one would have to use views to create a custom teaser list.
If you find good reasons in favour of this module, views integration would be a nice to have for a stable version.

2) I appreciate the token support, but it seems not too usable as one would have to set it for every new node. A default token setting on the content type page would make this feature cool, as one could override the token setting on certain nodes.

3) The code is clean, the only thing I found is a $data() variable set and not used later (line 87 of the .module file).

The module works by the way. ;)

I will set the application to 'needs work', however if you disagree with point 1) feel free to change the status back.

I tested this module and it is working fine, also the code seems to be clean.

Tested. Worked as per expectations.

In a word, the module is good enough with some limitations.
Now the altered read more button only works well in teaser view mode.
I think it should be fix this small problem.

Thanks.

Overally looks good. Just a few tweaks and UI glitches:

> if (isset($form['#node']->readmore_text) && $form['#node']->readmore_text != '') {

Just a code readability thing: the two conditions here can be replaced with !empty().

> $enabled = variable_get('readmore_per_node_' . $type, 0);

Again, mostly readability, but it's better to use FALSE rather than 0 here.

      '#title' => t('Readmore text'),

You use 'read more' with a space in the module title. I'd say with a space it looks better.

    '#on_value' => 1,
    '#off_value' => 0,

That's surely redundant here? Checkboxes do that OOTB.

  $form['readmore_per_node']['title'] = array(
    '#type' => 'markup',
    '#markup' => '<h2>' . t('Readmore per node') . '</h2>',
  );

This looks wrong -- the other vertical tabs on the node type form don't have it.

> '#title' => t('Enable'),

What you DO need though is a better title than this for the check box! Eg:

#title: Read more text per node
#description: Allows the read more text to be set separately for each node.

> $form['#submit'][] = 'readmore_per_node_type_submit';

I don't think you need this -- IIRC, the node type form does this for you. For instance, look at comment_form_node_type_form_alter() -- it doesn't add a submit handler.

> $variables['content']['links']['node']['#links']['node-readmore']['title'] = t('@text', array('@text' => $text));

I'm not sure there's any point to having a translatable string that's entirely placeholder!

Which does bring me to two big limitations of the module:

1. The readmore text won't work with node revisions.

2. The readmore text won't work with Entity Translation.

I don't think either of those should block this application, but you should probably address them at some point -- translation in particular will affect your data storage. One idea would be to use a text field just for the storage, since that handles revisions and translation OOTB -- and output it yourself in the teaser, and control its creation and removal via the option in the node type form.

Change the name to ==> [D7] Readmore per node

@rakesh.gectcr: Anyone can change the issue title, this is surely not an application blocker. Please do a manual review of the code.

I echo the limitations of the module (Translate, views, tokens), but I guess the limitations are opportunities. Keep it up.
The code is very clean and easy to follow (some additional commenting may not hurt, particularly of the module will grow both in functionality and co-maintainers)

I also like Joachim's suggestion of referring to "Read More" rather than "Readmore" ln 23, 50, 89 of .module file

Some comments from reading your readme file

Why is Token a recommended module? How is it used?
"Override the ds readmore tex"t - what is ds

May I suggest some changes to the Read me Instructions
* After enabling the module, activate "Readmore per node" on the content type's edit tab.
* A vertical tab (bottom of form) "Readmore text" will appear on the individual node form (Edit or add node for given content type)
* You can add a custom text of your own which will be use instead of the default "readmore" text in teaser view of the node.

Manual Review

Individual user account: Follows
No duplication: See #4
Master Branch: Follows
Licensing: Follows
3rd party assets/code: N/A
README.txt/README.md: Follows
Code long/complex enough for review: Follows
Secure code: Meets the security requirements.
Coding style & Drupal API usage:
Automated review tossed an error for line 56
* '#description' => 'Allows the read more text to be set separately for each node.', according to the review it should likely be '#description' => t('Allows the read more text to be set separately for each node.')

http://pareview.sh/pareview/httpgitdrupalorgsandboxhmdnawaz2569551git

FILE: /var/www/drupal-7-pareview/pareview_temp/readmore_per_node.module
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
56 | WARNING | #description values usually have to run through t() for
| | translation
---------------------------------------------------------------------------

Hi hmdnawaz,

please provide the parreview link:http://git.drupal.org/sandbox/hmdnawaz/2569551.git

By Default a node will show the full mode but not the teaser mode of node. We have to use views to show the teaser view of the node. So please provide some clarification to how to show the readmore text after installing the module in readme.txt file

Manual Review:
Individual user account: Follows
No duplication: See #4
Master Branch: Follows
Licensing: Follows
3rd party assets/code: N/A
README.txt/README.md: Follows
Code long/complex enough for review: Follows
Secure code: Meets the security requirements.
Coding style & Drupal API usage:
Automated review: No errors

I reviewd this module and it is working fine. Please add check_plain() for variables you used. This validates strings as UTF-8 to prevent cross site scripting attacks on Internet Explorer 6.

Example:
$variables['nid'] and etc.

@Buvaneswaran Chandrasekaran: The node ID is not user provided text and therefore it is not necessary to run it through check_plain(). Make sure to read https://www.drupal.org/node/28984 again.

@klausi Thats really helps.

Thanks.

Project page at https://www.drupal.org/sandbox/hmdnawaz/2569551 is empty. Please provide a description according to https://www.drupal.org/node/997024

manual review:

1. I'm a bit worried about the token support and the security implications. Example: if there is an access protected field on the node then any node editor can get access to field by just using [node:field-secret] in the readmore link and anyone can now see the content of the access restricted field. On the other hand I think that is a general problem in Drupal's token system that there is no access control, so not really special in this module. Users just need to be aware that as soon as they enable this for a content type that any editor can read all fields on a node.
2. readmore_per_node_help(): all user facing text in there must run through t() for translation.

But otherwise looks good to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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

1. This could use a #type => 'link' instead of #markup.
   $variables['content']['node_link'][0]['#markup'] = l(t('@text', array('@text' => $text)), 'node/' . $nid);
2. In the hook_uninstall, instead of doing a db_delete('variable') and a LIKE query, you are much better off building a foreach loop and calling variable_del(). As a general rule, it is more specific and less prone to issue if others extend the module.

None of these are blockers. Thanks for your contribution, Ahmad!

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.