This module allows admins to create a twig template filter, written in PHP, directly in the administration interface.

Installation
No special installation requirements

Sandbox project page link:
drupal.org/sandbox/freeandeasy/2545860

To clone the project:
git clone --branch 8.x-1.x http://git.drupal.org/sandbox/FreeAndEasy/2545860.git twigfilter

How to use the module

Step 1: Create Filter

  • go to /admin/config/system/twigfilter
  • create a new filter by clicking the "Add Twigfilter" button
  • name your filter and enter your filter code, save

Step 2: Use Filter
use your filter anywhere (template/views etc) like this:
{{ 'mytext'|twigfilter('my-filter-machinename') }}

Automated review results:
pareview.sh/pareview/httpgitdrupalorgsandboxfreeandeasy2545860git

Manual reviews of other projects
www.drupal.org/node/2422391
www.drupal.org/node/2450085
www.drupal.org/node/2427463

CommentFileSizeAuthor
#15 coder-results.txt1.51 KBklausi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FreeAndEasy created an issue. See original summary.

FreeAndEasy’s picture

Issue summary: View changes
FreeAndEasy’s picture

Issue summary: View changes
gaja_daran’s picture

Hi,

The clone project URL asking the password. The clone URL like as

git clone --branch 8.x-1.x http://git.drupal.org/sandbox/FreeAndEasy/2545860.git twigfilter

Kindly update it.

FreeAndEasy’s picture

Issue summary: View changes
PA robot’s picture

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.

FreeAndEasy’s picture

Issue summary: View changes
FreeAndEasy’s picture

Issue summary: View changes
FreeAndEasy’s picture

Issue summary: View changes
FreeAndEasy’s picture

Issue tags: +PAreview: review bonus

met requirements of review bonus system https://www.drupal.org/node/1975228

FreeAndEasy’s picture

Issue summary: View changes
jwilson3’s picture

Status: Needs review » Needs work

Automated Review

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    
    FILE: ...al-7-pareview/pareview_temp/src/TwigExtension/TwigfilterExtension.php
    ---------------------------------------------------------------------------
    FOUND 2 ERRORS AFFECTING 2 LINES
    ---------------------------------------------------------------------------
     50 | ERROR | [x] Whitespace found at end of line
     59 | ERROR | [x] Whitespace found at end of line
    ---------------------------------------------------------------------------
    PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ---------------------------------------------------------------------------
    
    Time: 128ms; Memory: 6Mb
    

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

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.
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
Code looks good and follows standard Drupal API
  1. (*) Enabling the module using the latest version of Drupal 8.0.x produces a fatal error when accessing any page that goes away when the module is disabled:
            <a href="<br />
            <b>Fatal error</b>:  Call to a member function generateFromRoute() on a non-object in <b>~/Projects/Drupal/8.x/core/lib/Drupal/Core/Template/TwigExtension.php</b> on line <b>224</b><br />
            
  2. No automated test cases were found. Due to the dangerous nature of this module and how easy it would be to shoot yourself in the foot, I recommend writing Simpletests or PHPUnit tests to simulate success of a properly formatted filter outputting the correct/expected output from the custom twig filter and to test triggering the PHP code error detection and handling capability of the module. This is not a requirement but encouraged for professional software development.

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.

This review uses the Project Application Review Template.

FreeAndEasy’s picture

Hi jwilson3,

thank you for the review. Can you download the latest changes from git? I fixed the error that you mentioned yesterday. It should work with rc1 and dev.

I also added additional help and warnings to the php input form.

FreeAndEasy’s picture

Status: Needs work » Needs review
klausi’s picture

Status: Needs review » Needs work
Issue tags: -PAreview: review bonus +PAreview: security
FileSize
1.51 KB

Review of the 8.x-1.x branch (commit 44fa9fa):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
  • DrupalPractice has found some issues with your code, but could be false positives.
    
    FILE: /home/klausi/pareview_temp/src/Form/TwigfilterForm.php
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 6 LINES
    ----------------------------------------------------------------------
     65 | WARNING | Variable $warnings is undefined.
    ----------------------------------------------------------------------
    
  • 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. what is the purpose of this module? To me it seems like a bad idea same as PHP module to let admins insert executable code through the user interface. What are the use cases? Please describe that on the project page.
  2. twigfilter.permissions.yml: the permission must be marked as "restrict access": TRUE due to the dangerous nature of this module (only trusted admins should be able to create twigfilters). This is currently a security blocker. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
  3. Twigfilter.php: why do you specify "administer site configuration" here as permission and not your own?
  4. TwigfilterForm: don't call the global t() function, use $this->t() with the injected string translation instead.
  5. TwigfilterForm: why do you call exec() here and not eval()? Please add a comment.
  6. I think you don't need the edit and delete routes in the routing.yml file, Drupal core will generate those from the links in the entity annotation, correct?

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

FreeAndEasy’s picture

Thank you klausi, great review!

I fixed the PAReview warnings.

1. I added a better use case describtion to the project page.
2., 3., 4. Fixed.
5. I removed the input validation for now. Admins should be aware of the implications of using this module. Warnings are displayed right above the input form.
6. Thouse routes are needed in Druapl rc1, maybe they are not needed in later release candidates, I will investigate that.

No further review necessary until then.

FreeAndEasy’s picture

Status: Needs work » Needs review

Tested in RC3, edit/delete routes are necessary.

klausi’s picture

nope, they are not, as you can see in node module: there are no routes in the node.routing.yml file to edit or delete nodes.

FreeAndEasy’s picture

The node module has its own route provider where the edit and delete routes are defined (look at NodeRouteProvider.php). Its not the same as configuration entities.

klausi’s picture

I see, thanks for pointing that out!

#2350509: Implement auto-route generation for all core entities and convert all of the core entities. shows that it is possible (the API to do it is there already), but unfortunately that never landed.

FreeAndEasy’s picture

FreeAndEasy’s picture

Issue tags: -PAReview: security PAReview: review bonus +PAreview: security, +PAreview: review bonus
klausi’s picture

Assigned: Unassigned » ELC
Status: Needs review » Reviewed & tested by the community

Review of the 8.x-1.x branch (commit 3336d25):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/src/Form/TwigfilterForm.php
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ----------------------------------------------------------------------
     9 | WARNING | [x] Unused use statement
    ----------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------
    
    FILE: /home/klausi/pareview_temp/twigfilter.info.yml
    ----------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------
     6 | ERROR | [x] Expected 1 newline at end of file; 3 found
    ----------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------
    
    FILE: /home/klausi/pareview_temp/twigfilter.routing.yml
    ----------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------
     31 | ERROR | [x] Expected 1 newline at end of file; 0 found
    ----------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------
    
    FILE: /home/klausi/pareview_temp/twigfilter.permissions.yml
    ----------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------
     4 | ERROR | [x] Expected 1 newline at end of file; 0 found
    ----------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------
    
  • 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:

  • twigfilter.schema.yml: indentation errors, always use 2 spaces for levels.

But otherwise looks good to me. Assigning to ELC as he might have time to take a final look at this.

klausi’s picture

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

No objections for more than a week, so ...

Thanks for your contribution, FreeAndEasy!

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:

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.

Status: Fixed » Closed (fixed)

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