[D8] Views Complex Grouping

Thank you for applying! I added the Git instructions for non-maintainer users and the PAReview checklist link.

Remember to change status, when the project is ready for review, as in this queue Active means Don't review yet the project I am using for this application.

Review of the 8.x-1.x branch (commit 572c128):

• The views_complex_grouping.module does not implement hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .
• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: ...dor/drupal/pareviewsh/pareview_temp/views_complex_grouping.theme.inc
  --------------------------------------------------------------------------
  FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
  --------------------------------------------------------------------------
   12 | WARNING | Unused variable $sdfs.
  --------------------------------------------------------------------------
  
  Time: 468ms; Memory: 4Mb
  

• No automated test cases were found, did you consider writing 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.

Please check this.

Line views_complex_grouping.theme.inc
------ -----------------------------------------------------------------------------------------------------------------------------------
11 Function template_preprocess_views_view_complex_grouping_level not found while trying to analyse it - autoloading is probably not
configured properly.

Given that the hook_theme() implementation is the following one, I take the tool used to analyze the code isn't able to understand what file to load to find that preprocess function.

function views_complex_grouping_theme($existing, $type, $theme, $path) {
  \Drupal::moduleHandler()->loadInclude('views_complex_grouping', 'inc', 'views_complex_grouping.theme');

  return [
    'views_view_complex_grouping_level' => [
      'variables' => [
        'view' => NULL,
        'grouping' => NULL,
        'grouping_level' => NULL,
        'rows' => NULL,
        'title' => NULL,
        'fields' => NULL,
        'grouping_branch' => NULL,
      ],
      'file' => 'views_complex_grouping.theme.inc',
    ],
    'views_view_complex_grouping_leave' => [
      'variables' => [
        'view' => NULL,
        'grouping' => NULL,
        'grouping_level' => NULL,
        'rows' => NULL,
        'title' => NULL,
        'fields' => NULL,
        'grouping_branch' => NULL,
      ],
    ],
  ];
}

Eventually, what should be changed is removing the \Drupal::moduleHandler()->loadInclude('views_complex_grouping', 'inc', 'views_complex_grouping.theme'); line since Drupal uses the value given for the file key to load the file with the preprocess function.

That example is wrong. Drupal uses the file key returned from hook_theme(), which must be added for every theme function that isn't in the module file.

1. class ComplexGrouping: please don't just repeat the class name in the doc comment. Describe what the class is used for and why.
2. ComplexGrouping::renderGrouping(): why do you need the strip_tags() call here? Please add a code comment. I think this is not an XSS problem because strip_tags() should be safe and Twig will auto-escape later again anyway.

Otherwise looks good to me.

Thanks for your contribution, Pratik!

I updated your account so you can opt into security advisory coverage now.

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 Slack or 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.