[D7] Views Show More

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

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.

Automated Review

As I checked on pareview.sh there are still some issues, that need to be fixed.

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

No: Currently this is empty, specify about your module.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

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

Removing review bonus tag, you have not done any manual review, you just copied the empty review template in two reviews and in one review just reported about pareview.sh. Make sure to read through the source code of the other projects, as requested on the review bonus page.

Automated Review

pareview.sh still showing issues.

If those issues can not be corrected as they are extending another function // @codingStandardsIgnoreFile can be added to a file to make it pass the Coding Standards checks. This should only ever be used as a last resort in cases where the code is correct or can not be corrected to pass the automated coding review checks.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Can you explain how this is different from projects like: Views Load more

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: meets minumums for readme files.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

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

I'd like to see a bit more info on the Project Page.

Side note: I'm looking forward to a full release version of this as it will be perfect for a site I've been working on.

Overall this module seems to be well written and is easy to configure. There are some items that Need Work and some that require further documentation and discussion.

Automated Review

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

Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
FILE: ...ar/www/drupal-7-pareview/pareview_temp/views_plugin_pager_show_more.inc

--------------------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 5 LINES
--------------------------------------------------------------------------------
11 | ERROR | Method name "ViewsShowMorePlugin::summary_title" is not in
| | lowerCamel format, it must not contain underscores
11 | ERROR | Visibility must be declared on method "summary_title"
27 | ERROR | Method name "ViewsShowMorePlugin::option_definition" is not in
| | lowerCamel format, it must not contain underscores
27 | ERROR | Visibility must be declared on method "option_definition"
55 | ERROR | Method name "ViewsShowMorePlugin::options_form" is not in
| | lowerCamel format, it must not contain underscores
55 | ERROR | Visibility must be declared on method "options_form"
154 | ERROR | Visibility must be declared on method "query"
173 | ERROR | Visibility must be declared on method "render"
--------------------------------------------------------------------------------

Time: 146ms; Memory: 7Mb

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.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Unsure, more detail requested below: Does not cause module duplication and fragmentation.

Discuss in the Project Application Issue description why this module should exist as a separate module rather than a patch to one of the many existing views pager modules.

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.

• It's a good idea to add ctools as a requirement too as stated in README template you must "List all requirements here, including those that follows indirectly from another module, etc."
• List differences between this module and other related modules in README and/or project page, e.g. - Views Load More,
  Views Infinite Scroll
• Minor spelling corrections to "Configuration" section: Goto > Go to, secton > section, chosser > chooser

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

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

Coding style & Drupal API usage

1. (*) When AJAX is turned on, I would expect the original page of entries to be replaced by the next page of entries as it does in no-ajax mode.
   At the moment we can see in the screenshots attached my settings. When I click "Show More" the list expands rather than being replaced. When I click "Show More" again, this happens until no more entries and the "Show More" link disappears when there is no more.
   If this works as designed, the differences between AJAX and no-AJAX mode should be described on the project page / README.txt.
2. Minor spelling mistake in .module on line 34: "Theme function for pager link generatrion." > "Theme function for pager link generation."

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.

Changing to Needs Work.

This is what I get all the time when I choose this plugin or try to configure it in Views UI:

Notice: Undefined index: speed_type in options_form() (line 142 of W:\home\dru\7\sites\all\modules\test\views_show_more\views_plugin_pager_show_more.inc).

You should provide all defaults in option_definition() method.

Automated Review

Review of the 7.x-1.x branch (commit 47ea7aa):

• 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. Addressed, and different enough, but please put the module comparisons on the project page, too.

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/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. / No: List of security issues identified.]

Coding style & Drupal API usage

There is a spelling error, ViewsShowMorePlugin, line 82.

ViewsShowMorePlugin::summary_title(), why the string concatenation? Just use a single format_plural.

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 seeing anything major. Assigning to @er.pushpinderrana for a second look if he has time.

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.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. None

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

• 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: Follow 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

(*)No: List of security issues identified.
theme_pager_show_more(): This is vulnerable to XSS exploits. If I enter <script>alert("XSS Show more pager link text when empty")</script> in Show more pager link text when empty field under pager option form, every time on last page of pagination, get a nasty javascript popup. You need to sanitize this before rendering, make sure to read https://www.drupal.org/node/28984 again. User provided input should always be sanitize before printing on screen.

Coding style & Drupal API usage

1. views_show_more_views_pre_render(): Not sure whether it is applicable in your scenario or not but using #attached with render arrays is preferred over drupal_add_js().
2. As @mpdonadio recommended you please put the module comparisons on the project page too. Better to go through Tips for a great project page again.

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.

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

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

Automated Review

I reviewed the Project on Pareview.sh, It’s still showing some issues.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation as mentioned in README in reference to ‘Views Infinite Scroll’ and ‘Views Load More Modules’.

Master Branch

Yes: Follows the guidelines for master branch. The guidelines are taken under consideration.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

n/a.

README.txt/README.md

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

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity. Code is Complex enough for a review, though some of previously mentioned issues have been fixed.

Coder ReviewI reviewed views_show_more (active) with Coder Module, There is  one normal warning.

The module will be good addition to Drupal Community, Looking forward for full released with fixed issues mentioned above.

Automated Review

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

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

Source: http://pareview.sh/ - PAReview.sh online service

Manual Review

views_show_more_views_ajax_data_alter(): is_a is deprecated, better to use the instanceof operator. See https://www.drupal.org/node/1793894

But that are not critical application blockers, otherwise I think this is RTBC.

Also blocking issues from #23 have been addressed. Been sitting at RTBC for a while now, so...

Thanks for your contribution, saniyat!

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.