[D7] Views Nested Accordion

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

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.

Manual review

Line 10, file views_nested_accordion_style_plugin.inc

Your class name should be views_nested_accordion_style_plugin to match with parent class name.

Line 11, file views_nested_accordion.module, function views_nested_accordion_views_api()

Your views api path should be drupal_get_path('module', 'views_nested_accordion') . '/views'

Line 32, file views_nested_accordion_style_plugin.inc, function options_form()

You shoud use single quote in function t() (like the previous one)

Line 39, file views_nested_accordion_style_plugin.inc, function pre_render()

You should remove this method because it does nothing.

Hi anil280988,

Line 50, file views_nested_accordion_style_plugin.inc, function render()

if ($this->options['nestedaccordion'] == '1'){
}

I think this is not related to the value saved in Drupal.settings. It's just compare value stored in database (style options) with a value.
For example: You can see form element checkbox in form create/edit Article and the code in file node.module, line 1108

if ($node->is_new || !empty($node->revision)) {
}

I debug (use Netbeans and Xdebug) and find that the value $this->options['nestedaccordion'] is integer.
So, i think the best way to make a comparison is use function empty() like revision in node.

if (!empty($this->options['nestedaccordion'])){
}

Line 10, file views_nested_accordion_style_plugin.inc

I think PA Review is not always true.
For example: i create new views plugin display and overwrite method options_form(). When i check with PA Review, i get this error:

25 | ERROR | Method name "views_plugin_display_table_rowspan::options_form" is
| | not in lowerCamel format, it must not contain underscores

This can not be fixed because method options_form() must have the same name with parent class.
I find that the class name of views plugin is alway in lowercase (in module Views and other module contrib).
https://api.drupal.org/api/views/plugins%21views_plugin_style.inc/group/...

Its working fine on my end. Solved my problem.
Thanks!!!

Hi @Anil280988,
First of all I wanted to tell you that your module looks very good. I like the cleanness of your solution and the functionality is one that I have needed in the past. Good job :)

Automated Review

Your automated review came back clean except for the class & method names that you have already explained need to stay that way due to inheritance.

Manual Review

Individual user account: Yes.
Master Branch: Follows guidelines.
Licensing: None included (Follows guidelines).
3rd party code: Doesn't appear to include any.
README.txt/README.md: Follows Guidelines content-wise although "Summary" should be "Introduction". Your formatting for headings is off though. See: https://www.drupal.org/node/2181737.
Code long/complex enough for review: Yes. Meets minimum requirement of 5 functions and >120 lines.
Secure code: Yes, as far as I can tell.
Coding style & Drupal API usage: As mentioned above, the outstanding coding style deviations are necessary to override an existing class. One small bug that I noticed was a PHP-style single-line comment (//) in your CSS file. You should always use /* */ comments in CSS. Your usage of the Views API looks spot on to me.

No Duplication

Although I like the functionality this module provides, I wonder if it wouldn't be more useful and accessible if it were added to the existing views accordion module. Have you contacted the module maintainers proposing this? That said, the project application process is supposed to focus on the applicants skills and I feel this module shows that you have a good understanding of Drupal hooks including advanced views programming.

Just an added suggestion, I think your views_nested_accordion_style_plugin.inc file should be nested in a views folder and your views-view-accordion.tpl.php file should be nested in a theme folder. Many modules organize their files this way and it just makes it easier to see what's available at a glance.

Since there are no major problems that I can see, I'm marking this as "Reviewed & Tested by the Community" :)

Hello @anil280988

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

Manual review:

1. Code in the JS file is structured improperly. Do not use (function ($) { ... })(jQuery); inside behavior's attach() method. Behaviors declaration should be inside it.
2. There are also no good comment explaining what does the behavior do in the JS.
3. Module description description = "Create Nested Accordion" is not very descriptive in the .info file.
4. Also in the .info file lines ; Information added by Drupal.org packaging script on 2014-07-09 and core = "7.x" should be removed. They are added by drupal.org packaging system automatically.
5. What does the options form in the plugin do? I see that there's only a checkbox resulting in adding JS & CSS files to the page when enabled. Otherwise the plugin renders a usual views_accordion, which can be rendered without this plugin.

6.   /**
      * Pre_render() necessary to prevent markup mayhem.
      */
     public function pre_render($result) {
       parent::pre_render($result);
     }
   

   Can you explain how exactly it prevents the mayhem. I don't see any reason for implementing this method too.

7. Since there is not really much Drupal API usage present I think this project on its own does not warrant giving you the git vetted user role. This single project can be promoted manually once it is ready.

Automated Review

$ pareview.sh http://git.drupal.org/sandbox/anil280988/2303527.git

Git default branch is not set, see the documentation on setting a default branch.

Review of the 7.x-1.x branch (commit 67e9037):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: /home/matt/PAR/pareview_temp/views_nested_accordion_style_plugin.inc
  --------------------------------------------------------------------------------
  FOUND 4 ERRORS AFFECTING 3 LINES
  --------------------------------------------------------------------------------
   10 | ERROR | Class name must begin with a capital letter
   10 | ERROR | Class name must use UpperCamel naming without underscores
   14 | ERROR | Public method name
      |       | "views_nested_accordion_style_plugin::option_definition" is not
      |       | in lowerCamel format, it must not contain underscores
   24 | ERROR | Public method name
      |       | "views_nested_accordion_style_plugin::options_form" is not in
      |       | lowerCamel format, it must not contain underscores
  --------------------------------------------------------------------------------
  
  Time: 232ms; Memory: 5.5Mb
  

• 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.

These are false positives b/c of Views conventions.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

No: Follows the guidelines for project length and complexity. WIll proceed with single project promition.

Secure code

Yes/No. If "no", list security issues identified.

Coding style & Drupal API usage

The spacing in the JS should be twp spaces per level.

(+) The behavior gets two parameters, contect and settings. You also should use these instead of the globals.

You should use .hasClass() instead of manually splittng .attr('class')

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.

Not much to this module; not seeing any blocking issues. Keeping single project tag and setting RTBC for another admin to have a look.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Removed one review link which did not contain a manual review.

mpdonadio's comment has not been addressed yet, but otherwise looks good to me.

Thanks for your contribution, anil280988!

I promoted this project for you: https://www.drupal.org/project/views_nested_accordion

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

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.