[D7] Stop Broken Link In Body

Could you take a look at https://www.drupal.org/project/linkchecker and explain how this module differs from that please?

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.

Working Fine for me...

@joachim: Well! This module provides a add-on feature, editors/publishers would get list of broken links before publishing the content. I have used/and been using link checker module, which provides a facility, once the contents are published by running a "check". On the other hand, this module provides just one step before!

Hope you get my point, currently we are working to extend scope of this module, I would appreciate you, if you can share more cases, if you have any to extend the scope.

Working Fine for me...
useful module
so this module is perfect for me

thank!
Sudishth

Its working on my local.
Its working fine.

Module limits to body field only ? As per current implementation we can't extentd it to other fields. Why not create a filter instead and it will allow site admins to use on any input format as they like ?
Is there any scope to extend it to other fields ?

if you are planing to change as suggested above. please ignore review below

Move the admin settings form to a helper .admin.inc file it will give little performance hit

Use the default values with the variables http://cgit.drupalcode.org/sandbox-shaktik-2612690/tree/stop_broken_link...

WHy there is LANGUAGE_NONE constant ? Use field_get_items instead. http://cgit.drupalcode.org/sandbox-shaktik-2612690/tree/stop_broken_link... Handle multiple values for the body field.

Why double santization here http://cgit.drupalcode.org/sandbox-shaktik-2612690/tree/stop_broken_link... t() will take care for you.

Add a hook_help and use the readme template for the readme.txt

For long term, define a new permission and use that for the menu path http://cgit.drupalcode.org/sandbox-shaktik-2612690/tree/stop_broken_link...

prefix the variable 'select_node_types' with module name

Work fine

Removing review bonus tag, you have not done all manual reviews, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

Removing review bonus tag, you have not completed 3 manual reviews?

Working fine for me. Following are the steps followed :

1. Take a git clone of the module in /sites/all/module/contrib
2. Go to Modules page in your site and enable the module.
3. Go to admin/config/system/stop-broken-link-in-body
4. Select content type for restricting the broken links in the body field and Save.
5. Add some content to the content type and try some incorrect urls.
6. The content will not be saved and an error message will be generated.

manual review:

1. project page: how is this different from https://www.drupal.org/project/linkchecker ? Please add that to the project page, see also https://www.drupal.org/node/997024
2. README.txt: don't break words like that, start them as a whole on a new line.
3. "variable_del('select_node_types');": all variables defined by your module need to prefixed with your module's name to avoid name clashes with others.
4. stop_broken_link_in_body_form_alter(): so this will only work for node entities, right? Could you document that on the project page that this is hardcoded to nodes and the body field?
5. stop_broken_link_in_body_form_alter(): this should only target node forms, right? Then you should use hook_form_node_form_alter(). See also https://api.drupal.org/api/drupal/modules!system!system.api.php/function...
6. stop_broken_link_in_body_validate(): this will run into a timeout if there are many URLs to check. How are you going to prevent that?
7. "check_plain(t('Link check of ...": check_plain() is wrong here since the result of the t() call is already sanitized. You are using the "@" placeholder which will already sanitize the user provided variables for you. Make sure to read https://www.drupal.org/node/28984 again.
8. This module is somewhat vulnerable to SSRF attacks, same as the core aggregator module. See also #2496157: SSRF present in aggregator module. It is good that you are restricting to http/https URLs, so at least all the cURL tricks are not possible. I think this should still be mentioned on the project page, so that people are aware when using Drupal an a sensitive intranet.

The wrong check_plain() usage is a blocker right now, you need to know when to use it and when not.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Manual review:

1. Project page: Difference from https://www.drupal.org/project/linkchecker found in project description.
2. "variable_del('select_node_types');": Corrected with prefix of module name.
3. stop_broken_link_in_body_form_alter(): Note for this found on project page.
4. stop_broken_link_in_body_form_alter(): has been changed to hook_form_node_form_alter().
5. stop_broken_link_in_body_validate(): Check for potential issue added with foreach statements to handle multiple urls.
6. "check_plain(t('Link check of ...": has been removed.
7. vulnerable to SSRF attacks: Note for this has been added to project page.
8. "variable_get('stop_broken_link_in_body_restrict_the_number')": in stop_broken_link_in_body_validate() being used more then 2 times, it is good if you can cache this into variable.

All issues mentioned by @klausi, seems to be resolved. However found one small potential issue see #8, Please fix that also then I'll be happy to move this into RTBC :)

Thanks for your contribution! ;)

Manual review:

Fixed above mentioned issues! Moving this to RTBC.

Only thing which is left is to get review bonus ;) then you are good to go :)
Thanks for the hard work!

Nice Module!
Manual Review (91ed95) :

1. Remove the used variable stop_broken_link_in_body_restrict_the_number
2. where are you using the permission administer stop_broken_link_in_body ? For long term restrict the admin/config/system/stop-broken-link-in-body admin configuration path,with your own defined permission
3. Will it only works for the body field on only node entities not other entities like field collection ? if yes then specify about it on project page
4. stop_broken_link_in_body_validate : $form_state['values']['body'][LANGUAGE_NONE][0]['value'] Use the field_get_items function to extracting the values. Will this work for only single valued field not multiple value ?
5. Document the regressions in stop_broken_link_in_body_add_base_url, stop_broken_link_in_body_hrefget_urls and stop_broken_link_in_body_get_urls
6. Use proper git commit messages https://www.drupal.org/node/2612690/commits see more https://www.drupal.org/node/52287

Moving to N/W due to open questions, Rest looks good to me to go.

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

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

Looks good to me :)

Manual Review:

1. 'access arguments' => array('access administration menu'),: that permission is not defined by your module in hook_permission()?
   Even this permission is not part of Drupal core too. You should define some permission to access this say "administer stop broken link body" or something similar?

   FYI... This permission is part of https://www.drupal.org/project/admin_menu module.

2. You should also really have a hook_help() with some basic info about the module.

The misleading and undefined permission is a blocker right now, otherwise this would seem good to go.

Looks good to me now.

Since this was already RTBC and the only confusing thing was the used permission I think we can approve you.

Thanks for your contribution, Shakti!

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.

I reused it on my new development project it working fine