Intro

This module provides a filter that converts predefined keywords into links. The keywords and links are defined via a custom administration panel.

Project page

https:///www.drupal.org/project/dynamic_links_filter
Contains much more detail about the project.

Similar projects

Dynamic Internal Linking. Ideal if you only need to crosslink internal pages. However, although the motivation is similar, this module allows you to cross link to external sites as well as a more seamless conversion using filters.

Git Clone

git clone --branch 7.x-1.x https://git.drupal.org/project/dynamic_links_filter.git

Manual reviews of other projects

This is my first project application, but I am co-maintainer on several projects including FlexSlider, View Unpublished and Entity connect all of which I have ported to D8. I have been an active member for 10 years. This particular module has been in use for 4 years and I finally received the permission to contribute it back to the Drupal community.

Comments

amaria created an issue. See original summary.

PA robot’s picture

Status: Needs review » Needs work

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

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.

amaria’s picture

Status: Needs work » Needs review

Git error that was reported about default branch is now fixed. Plus fixed the warnings for good measure.

khurram_awan’s picture

Status: Needs review » Needs work

Hi There,
This module does not work properly. if you just add "blah" in "Enter substring of URLs which will not be converted to lower case in new lines." it does not redirects to home page. it is because you are using $currentUrl = $protocol . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
please use current_path()

Thanks,
Khurram

amaria’s picture

Status: Needs work » Reviewed & tested by the community

@khurram_awan I think maybe you applied this review to the wrong module. None of that code is in the Dynamic Links Filter module.

amaria’s picture

Status: Reviewed & tested by the community » Needs review
khurram_awan’s picture

Oh I am sorry it was "seo 301 redirect" module.

jefuri’s picture

Status: Needs review » Reviewed & tested by the community

Automated Review

  • 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/or fragmentation.
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
Yes: Meets the security requirements.
Coding style & Drupal API usage
  1. (*) In the help info hook you use dirname(__FILE__). I would recommend using drupal_get_path() to get the module path.
  2. (*) Also I found the dynamic_links_filter_ui_list_dynamic_links() in the dynamic_links_filter_ui submodule missing inline comments to explain what is happening. Same might go for the rest of the functions defined there.

This review uses the Project Application Review Template.

jefuri’s picture

Status: Reviewed & tested by the community » Needs work

Should be needs work in hindsight. Because the 2 recommendations are kind of important from drupal coding standards perspective and usage of drupal API.

amaria’s picture

Status: Needs work » Needs review

Ok, added comments in the ui module admin include file and replaced __FILE__ with drupal_get_path.

qzmenko’s picture

Status: Needs review » Needs work

Automated Review

FILE: ...eview_temp/dynamic_links_filter_ui/dynamic_links_filter_ui.admin.inc
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
108 | ERROR | [x] Expected 1 space after "=>"; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/dynamic_links_filter.module
--------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------
269 | ERROR | [x] Parameter comment indentation must be 3 spaces, found
| | 4 spaces
272 | ERROR | [x] Parameter comment indentation must be 3 spaces, found
| | 4 spaces
274 | ERROR | [x] Parameter comment indentation must be 3 spaces, found
| | 4 spaces
276 | ERROR | [x] Parameter comment indentation must be 3 spaces, found
| | 4 spaces
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
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
Yes: Meets the security requirements.
Coding style & Drupal API usage
  1. (+) You hardcoded disabling the cache for the filter. Please fix it.
  2. (+) I'm trying to add keyword "@test". And filter does not replace this keyword. If I remove "@" from keyword - all works. I think you need to fix the regular expression.
  3. Replace all the function calls strtolower() to drupal_strtolower()
  4. I think that the filter should check the user's access to the path before replacement. Not sure about this.

This review uses the Project Application Review Template.

amaria’s picture

Status: Needs work » Needs review

Ok, fixed the new issues that were raised.

No more pareview errors.

Coding style & Drupal API usage

  1. (+) You hardcoded disabling the cache for the filter. Please fix it.
  2. (+) I'm trying to add keyword "@test". And filter does not replace this keyword. If I remove "@" from keyword - all works. I think you need to fix the regular expression.
  3. Replace all the function calls strtolower() to drupal_strtolower()
  4. I think that the filter should check the user's access to the path before replacement. Not sure about this.

Coding style & Drupal API usage

  1. Done
  2. Improved the regex expression for keywords.
  3. Done.
  4. Yes. Done. Good catch!
sumit.prajapati’s picture

Hi amaria,

Module is not working There is no setting available in content type.
Replace with $_GET['q'] to drupal_get_query_parameters().

amaria’s picture

@Sumit, please look over the README. The setting is not in the content type. It's in the filter.

amaria’s picture

Also, drupal_get_query_parameters() is not what needs to replace $_GET['q'] but current_path(). I have done that as well as a couple of other modifications to avoid function call nesting within the filter process callback.

qzmenko’s picture

Status: Needs review » Reviewed & tested by the community

Looks good for me after fixes in #12

amaria’s picture

Issue summary: View changes
amaria’s picture

Issue summary: View changes
Issue tags: +PAreview: review bonus
amaria’s picture

Status: Reviewed & tested by the community » Fixed

Module promoted and available at https://www.drupal.org/project/dynamic_links_filter.

Thanks for the reviews!

amaria’s picture

Status: Fixed » Reviewed & tested by the community

Go back to reviewed and tested, for security check access.

amaria’s picture

Issue summary: View changes
amaria’s picture

Priority: Normal » Major

Any way to get some eyes on this for the security advisory status? The module has a release candidate version. Thanks.

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » Fixed

Thank you for your contribution!

I am going to update your account so you can opt into security advisory coverage now.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.

Thank you, also, for your patience with the review process. (I apologize it took a year to reach this point.)
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.

I thank all the dedicated reviewer(s) as well.

avpaderno’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

avpaderno’s picture

Priority: Major » Normal