[D7] Views Exposed Filters Summary

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2476823

Project 2: https://www.drupal.org/node/2280911

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

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

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.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

pareview.sh has found a number of issues for your project. Those should be fixed.

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

No: Does not follow 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

No: Does not follow the guidelines for in-project documentation and/or the README Template.

Please work on your project page. As of now, there are only two screenshots. Please follow the project page template and read the tips for a great project page.

Please work on your README file and follow the README template linked above.

Improve DocBlocks of your template file and non-hook functions so that they more closely follow the API documentation and comment standards.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No: Doesn't meet the security requirements. See below for a list of the issues identified.

Coding style & Drupal API usage

1. (*) XSS: You're working with the raw values entered by the user without filtering them for XSS. This is a security issue! You need to sanitize all insecure values using either check_plain(), filter_xss() or filter_xss_admin() before output. To test, try the following:
   • XSS for field labels:
     1. Add an exposed filter for e.g. node title and change the label to <IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img>.
     2. Add your area to the header, select the node title filter and check Display even if view has no result.
     3. Save your view.
     4. Open the page with the view and add something to the exposed filter for node tilte.
     5. You should see a popup displaying XSS.
   • XSS for filter values:
     1. Add an exposed filter for e.g. node title.
     2. Add your area to the header, select the node title filter and check Display even if view has no result.
     3. Save your view.
     4. Open the page with the view and add <IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img> to the exposed filter for node tilte.
     5. You should see a popup displaying XSS.
2. (*) Please fix the issues found by pareview.sh (see above). While perfect adherance to Drupal Coding Standards isn't a requiremnet, modules should follow them as closely as possible.
3. (+) List Views module as dependency in your info file.
4. (+) Since your js file is empty, remove it from the info file and the repository for clarity and better performance.
5. (+) Your hook_views_data() implementation should be placed in a MODULENAME.views.inc file.
6. (+) Since your handler isn't tied to any specific database table or module, instead of using hook_views_data() and adding a dummy table for your handler, you should probably implement hook_views_data_alter() in your MODULENAME.views.inc and alter the data provided by views_views_data(). Add your new area handler to the views table, like it is done with the other area handlers by views.
7. (+) Either implement transform_summary() method of your handler or remove it.
8. (+) Fix the example in your README file or remove it.
9. Your template file contains a lot of PHP code. Move that into a preprocess function and provide only the variables needed for rendering the actual HTML.
10. In your info file, remove the commented out configure line for clarity.
11. In your css file, remove the commented out css. If you need it later, check the history of your repository.
12. Your module doesn't seem to work with node:status, user:status, user:name or user:email filters. No value is shown. If this is expected, document the filters your module will work with on the project page and the README file.
13. For select fields, your module displays the raw value of the field, not the human readable value, which would be better.
14. On the page containing the exposed filters, I get a warning Warning: array_pop() expects parameter 1 to be array, string given in include() (line 35 of views-exposed-filters-summary-results.tpl.php).

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.

Revision reviewed: 4770293

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.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi irinaz,

please set the right git clone command. You currently point to the master branch, you should point to 7.x-1.x.
git clone --branch 7.x-1.x https://git.drupal.org/sandbox/irinaz/2279929.git views_exposed_filters_summary

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Fixed the git clone URL in the issue summary for non-maintainer users.

I'm a robot and this is an automated message from Project Applications Scraper.

This application is not fixed? See the workflow https://www.drupal.org/node/532400

Hi irinaz.

I've just ran the code sniffer for your code, here is output:

FILE: ...exposed_filters_summary_handler_area_exposed_filters_summary.inc
----------------------------------------------------------------------
FOUND 7 ERRORS AND 6 WARNINGS AFFECTING 11 LINES
----------------------------------------------------------------------
   8 | ERROR   | [ ] Missing short description in doc comment
  16 | ERROR   | [ ] Method name
     |         |     "ViewsExposedFiltersSummaryHandlerAreaExposedFiltersSummary::option_definition"
     |         |     is not in lowerCamel format
  16 | ERROR   | [ ] Visibility must be declared on method
     |         |     "option_definition"
  26 | ERROR   | [ ] Method name
     |         |     "ViewsExposedFiltersSummaryHandlerAreaExposedFiltersSummary::options_form"
     |         |     is not in lowerCamel format
  26 | ERROR   | [ ] Visibility must be declared on method
     |         |     "options_form"
  57 | ERROR   | [ ] Visibility must be declared on method "render"
  62 | ERROR   | [ ] Missing short description in doc comment
  88 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: )
 132 | WARNING | [x] There must be no blank line following an inline
     |         |     comment
 142 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: $name
 154 | WARNING | [ ] Line exceeds 80 characters; contains 102
     |         |     characters
 172 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: $name
 238 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: 'regular_expression'
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ews_exposed_filters_summary/views_exposed_filters_summary.module
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 52 | WARNING | Line exceeds 80 characters; contains 91 characters
----------------------------------------------------------------------

You have to fix all of these issues(probably except the methods naming issues as it's related to Views module).

Thanks.

Valentine.

Minor coding standard issues are surely not application blockers, please do a real manual review.

CS issues fixed. Looks good so far. RTBC.

Thanks for your contribution, irinaz!

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.