A simple module to collect Anonymous feedback from users and provide a formatted email to a settable email address or saved to database. Module intentionally strips away identifiable information about user.

uses drupal hook_mail for email submission
admin/config/system/anonymous_suggestion_box allows for configuration of: to email, from email, if emails are sent, if suggestions are saved to database, and intro/explanatory text.
Originally designed for use within company Staff Portal site for submissions to Human Resources Manager for compliance with worker's comp policy requirements. Site being closed to anonymous users, required a different approach to submitting suggestions anonymously.

Project homepage
https://www.drupal.org/sandbox/benjaminarthurt/2312631
Git
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/benjaminarthurt/2312631.git
Automated Review
http://pareview.sh/pareview/httpgitdrupalorgsandboxbenjaminarthurt231263...

Reviews:

Comments

benjaminarthurt’s picture

benjaminarthurt’s picture

Issue summary: View changes
benjaminarthurt’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.

gaurav.pahuja’s picture

Automated Review

No issue reported.
http://pareview.sh/pareview/httpgitdrupalorgsandboxbenjaminarthurt231263...

Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a

lot, then post a link to the automated review and mention that problems should be addressed.

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

href="https://www.drupal.org/node/2181737">README Template

.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
No. If "no", list security issues identified.
Coding style & Drupal API usage
  1. (*) anonymous_suggestion_box_admin: This is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> in the admin

    settings form, tested for anonymous_suggestion_box_form_intro element, every time If I visit this Suggestion form, get a

    nasty javascript popup. You need to sanitize this before rendering, make sure to read

    href="https://www.drupal.org/node/28984">https://www.drupal.org/node/28984

    again.

    In given code anonymous_suggestion_box_form_intro variable coming form user provided input and you are directly rendering without sanitize that's why these popup coming.

    Security Issue

    Security issue
    Also check the same for other places too in your code.

  2. (*) Email need to be validated irrespective of usage.
    /**
     * Implements hook_validate().
     */
    function anonymous_suggestion_box_admin_validate($form, &$form_state) {
      if ($form_state['values']['anonymous_suggestion_box_method_email'] == 1) {
        if (!valid_email_address($form_state['values']['anonymous_suggestion_box_to_email'])) {
          form_set_error('anonymous_suggestion_box_to_email', t('The To e-mail address is not valid.'));
        }
        if (!valid_email_address($form_state['values']['anonymous_suggestion_box_from_email'])) {
          form_set_error('anonymous_suggestion_box_from_email', t('The From e-mail address is not valid.'));
        }
      }
    }

    Validation Issue

  3. (*)
    Any specific reason that you limited teatarea content to 21845?
     if (drupal_strlen($form_state['values']['suggested']) < 2) {
        form_set_error('suggested', t('You must enter more text for the suggested improvements.'));
      }
      if (drupal_strlen($form_state['values']['location']) > 21845) {
        form_set_error('suggested', t('You must enter less text for the location.'));
      }

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.

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.

gaurav.pahuja’s picture

Status: Needs review » Needs work
Issue tags: +PAReview: security

Adding PAReview: security tag to this thread. please don't remove the security tag, we keep that for statistics and to show examples of security problems.

benjaminarthurt’s picture

Status: Needs work » Needs review
FileSize
55.39 KB

1 (*) anonymous_suggestion_box_admin: This is vulnerable to XSS exploits.
Corrected - Checked code for properly sanitized output of all input. I incorrectly assumed that the trusted user roles that could access the config page could be trusted, but you are completely correct these should be sanitized as well.

2 (*) Email need to be validated irrespective of usage.
Email is now being validated regardless of user selection for method. I did allow a blank input to be accepted if the user only selects DB (email addresses are not used in this case).

3 (*) Any specific reason that you limited teatarea content to 21845?
Yes, 21845 is an approximation of the amount of text can can be held by the database ('type' => 'text', 'size' => 'normal'). Without those checks user input exceeding 21845 characters would cause an error while being saved to the database.

benjaminarthurt’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
benjaminarthurt’s picture

Issue summary: View changes
naveenvalecha’s picture

Status: Needs review » Postponed

@benjaminarthurt
Thanks for your contribution.
After reviewing the code & functionality it seems to me that this form can be created by the webform module and does not provide some additional functionality.if it provide additional features, would you please add the differences of this module with the webform module in the summary above.
I am changing its status to postponed. Please change it to 'Needs Review' so that we can review the differences after update.

Thanks again!

benjaminarthurt’s picture

Status: Postponed » Needs review
FileSize
19.97 KB

The Webform module is not Anonymous Example from Webform Module. The anonymity is the core purpose of this module. I'll update the project page to better explain this.

naveenvalecha’s picture

Status: Needs review » Needs work
FileSize
160.98 KB
147.54 KB

@benjaminarthurt Thanks for your quick feedback.

The anonymity is the core purpose of this module. I'll update the project page to better explain this.

Regarding the anonymity there is also a contibuted module Webform Anonymous that does the same job and it adds a simple Yes/No question in the webform form settings to "Anonymize results." It also adds a checkbox to permanently lock the results in an anonymous state, so that it would be impossible for anyone to see who used a webform.
For more confirmation I have attached the webform in which I have used the webform anonymous module and submitted 4 applications.
Webform Anonymous Results
Webform Anonymous module single result page
I have changed the status to 'Needs work'. Please change its status to 'Needs review' with the differences of the your module with the above modules I mentioned so that your module Does not cause module duplication and fragmentation

Thanks Again!

benjaminarthurt’s picture

Status: Needs work » Needs review
FileSize
51.93 KB

The webform and webform_anonymous modules are already addressed on the Project Page. Please take a look there.

Even with the Webform Anonymous Module it is possible to tell that a logged in user submitted the webform. This issue is exactly what prompted the creation of this module. This is possible using only webform w/ webform_anonymous. It is easily defeated from an anonimity standpoint, by using only Druapl core modules. This was unacceptable from a compliance standpoint which is the reason this module was created.
screen shot of webform submission and identifiable information that is logged even when using webform_anonymous

naveenvalecha’s picture

Even with the Webform Anonymous Module it is possible to tell that a logged in user submitted the webform. This issue is exactly what prompted the creation of this module.

This looks like the cool point that makes me to agree at a point regarding it's uniqueness you mentioned but I am not confirmed about it.Will see what other reviewers will think on this point ? Please add it to your issue summary with the difference between this module VS webform + webform anonymous module.Also add other differences if have. I will comeup with my reviews & comments soon.
Thanks Again !

naveenvalecha’s picture

Hi @benjaminarthurt,
Thanks for your contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.

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
Yes: Follows the guidelines for project length and complexity.
Secure code
No. If "no", list security issues identified.
  1. if (isset($_REQUEST['asb_view_start'])) {
        $record_start = $_REQUEST['asb_view_start'];
      }

    uses 'asb_view_start' directly in a URL. This is user input, so it is possible for a user with the 'view anonymous suggestion box' to inject something
    naughty on the page. You need to check_plain() this.

Coding style & Drupal API usage
  1. if (drupal_strlen($form_state['values']['location']) < 2) {
        form_set_error('location', t('You must enter more text for the location.'));
      }

    Please keep the minimum characters in a constant and used at other places too where you have used.So that it can be easily manageable.Similar is for the maximum characters limit check.

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.

Thanks Again!

This review uses the Project Application Review Template.

naveenvalecha’s picture

Status: Needs review » Needs work
benjaminarthurt’s picture

Status: Needs work » Needs review

Good catch with the missing sanitization on the $_REQUEST variable. I've corrected that. As well as moved the min and max values for the text fields into a variable. Perhaps with future development this could be opened up to the user to specify/override these values.

Thanks for your review of my project @naveenvalecha

naveenvalecha’s picture

Issue summary: View changes
Status: Needs review » Needs work

Hi benjaminarthurt,
Thanks for the updates. All looking good.Few More changes below.

Automated Review

All best practices identified by the pareview.sh http://pareview.sh/pareview/httpgitdrupalorgsandboxbenjaminarthurt231263...

Few more code changes

  1. No need to set variable value on hook_enable.Use the default value while use it as default value
    function anonymous_suggestion_box_enable(){  variable_set("anonymous_suggestion_box_min_textarea", "2");
    
      variable_set("anonymous_suggestion_box_max_textarea", "21845");}

    Please remove hook_enable completely including the configuration link message because there is already configure set in the module info file.

  2. Use variable_get with default value in function anonymous_suggestion_box_form()Change this if (drupal_strlen($form_state['values']['location']) < variable_get("anonymous_suggestion_box_min_textarea")) { to if (drupal_strlen($form_state['values']['location']) < variable_get("anonymous_suggestion_box_min_textarea" , "2")) {

Thanks Again !

benjaminarthurt’s picture

Status: Needs work » Needs review

I'm going to keep the hook_enable section, as this helps lay the ground work for some future development that I had planned. I plan to allow users with administrative access to be able to reduce the max or increase the minimum. There isn't a functional issue with the use of hook_enable that I am aware of. I am not planning on working on any future development while the module is in the application review process, but it doesn't make sense to back that out and then add it again later.

benjaminarthurt’s picture

Issue summary: View changes
benjaminarthurt’s picture

Scratch that, ignore my comment at#19, I did dump the hook_enable as @naveenvalecha suggested. I realized even if the values would be used elsewhere they could still be set in the code instead of at enable, and also if set on enable it would re-set if the module was disabled and re-enabled at anytime which would be less than desireable. I commited the changes to the project and reran the PAReview.sh checks which found no new coding standards issues.

madhusudanmca’s picture

Status: Needs review » Needs work

Hi benjaminarthurt,

I have gone through you module, and found few things:

Automated Review

No issue reported.

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
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.
Secure code
No. If "no", list security issues identified.
  • (+)In “.module” at line #263 and #264 $_REQUEST is used there without any sanitization. Please correct it as you are directly using that value in DB query, which can cause SQL injection
  • function anonymous_suggestion_box_entry_load($entry = array()) {
      if (isset($_REQUEST['asb_view_start'])) {
        $record_start = $_REQUEST['asb_view_start'];
      }
      else {
        $record_start = 0;
      }
      $record_end = $record_start + 10;
      $select = db_select('anonymous_suggestion_box', 's');
      $select->fields('s');
      $select->orderBy('sid', 'DESC');
      $select->range($record_start, $record_end);
      foreach ($entry as $field => $value) {
        $select->condition($field, $value);
      }
      return $select->execute()->fetchAll();
    }
    
    
    Coding style & Drupal API usage
    1. You have set min value (chars) to be entered as suggestion. It doesn’t seems to be good idea I would suggest you to validate it against blank field instead of min chars.
    2. You said in comment #21 that you have scratched the “hook_enable” but still I can see it there.
    3. I can see the unnecessary validations checks for email fields are there.

    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.

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

    I think either you have not committed your changes or have committed to new tag. Please update!!

    Commit the changes again :)

    Thanks Again!

    benjaminarthurt’s picture

    Status: Needs work » Needs review

    My mistake, I negelcted to push the changes back to the repository. I've done this now and you should be able to see the most recent version.

    Thanks for your review @madhusudanmca.

    mpdonadio’s picture

    Status: Needs review » Postponed (maintainer needs more info)
    Duplication
    This sounds like a feature that should live in the existing Webform Anonymous project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the Webform Anonymous issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process. Based on your descriptions, total anonymity could be a feature that could be added to Webform Anonymous as a patch.

    If that fails for whatever reason, or if you think a patch is totally impossible, please get back to us and set this back to "needs review", and we will continue this application

    benjaminarthurt’s picture

    Status: Postponed (maintainer needs more info) » Needs review

    @mpdonadio, thanks for taking a look at this. I'm not sure this really duplicates the functionality of Webform_Anonymous. The lack of full anonymity in that module is not a result of an issue with the module, but more of how the webform module handles these submissions. This module was created for the sole purpose of not relying on webform for this reason.

    Webform_Anonymous works by stripping out submitted data after it has been submitted. Leaving the original data intact if it has been caught by the core Tracker module, or other similar user tracking modules.

    $submission->uid = 0;
    $submission->name = 'Anonymous';
    $submission->remote_addr = '0.0.0.0';

    In the environment this module was originally created for, All authenticated users have the "Access all webform results" permission. As webforms are used in multiple work-flows outside of the anonymous suggestion box. User of the Permissions per Webform module got us closer to being able to prevent the leaking of submissions from the suggestion box, but added an additional layer of complexity to ALL other webforms created once that was enabled, not a great solution.

    The user (director of HR) who reviews the submissions to this specific box also holds the "view site reports" permission, which when using the webform module, would allow for her to see the tracking information related to the individual webform submissions. This module was developed as a custom module to allow for separate permissions from the widely used webform module, and also prevent any correlation of date, time, user ID, IP address from being logged, tracked or recorded. This wasn't optional as the company this was originally created for was required to implement this type of solution on their (Drupal based) intranet site. Being a regulatory compliance requirement, the 3 separate module solution that didn't quite anonymize the submissions described above wasn't sufficient. I submitted this module in hopes that others who struggle to meet these types of regulatory requirements can do so in a truly anonymous and easy to maintain way.

    I have submitted my findings about the issues with webform_anonymous module's level of anonymity. However still do not see how there could be a patch made to this module that could fix the issues described above.

    Thanks for your consideration and time.

    mpdonadio’s picture

    Thanks for the details. I don't have any objections, and think this can proceed.

    mpdonadio’s picture

    Assigned: Unassigned » mpdonadio

    Assigning to myself for next review.

    mpdonadio’s picture

    Status: Needs review » Needs work

    Automated Review

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

      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 fragmentation; addressed sufficiently.
      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
      Yes: Follows the guidelines for project length and complexity.
      Secure code
      Yes. If "no", list security issues identified.
      Coding style & Drupal API usage
      You may want to eventually move the page callbacks to include files. This can help reduce memory footprint at runtime.

      (+) A hook_help() would be nice.

      (+) The mail body is anonymous_suggestion_box_mail() is untranslated. It is also best to make the body an array of lines, instead of adding the newlines yourself. See https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func...

      (*) Your usage of check_plain on the email addresses in anonymous_suggestion_box_mail_send() is incorrect. These are not going to output. See https://api.drupal.org/api/drupal/modules%21contact%21contact.pages.inc/... for similar handling of email addresses from user input.

      In anonymous_suggestion_box_db_insert(), when you write a full record to your own schema, the normal convention is to use drupal_write_record()/

      anonymous_suggestion_box_db_insert() has some weird indentations / spacings.

      You have a fail number of strings that are using double quotes. Drupal preference is to use single quoted strings whenever possible. See https://www.drupal.org/coding-standards#quotes

      anonymous_suggestion_box_admin_validate() isn't a hook_validate. It is a validation handler for the anonymous_suggestion_box_admin_form form.

      (*) In anonymous_suggestion_box_view(), you have a few problems. The all boil down to now using the Drupal pager. Your db_select() should use the PagerDefault extension (https://www.drupal.org/node/508796) and then the output should include the pager. See pager_example in the Example module for a concrete example.

      (+) anonymous_suggestion_box_view() should return a render array and not themed output directly.

      Technically, you also don't need to plain your Submission ID since the database layer is generating this, and it isn't user input.

      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.

      I also want to note that the security problem in #22 is incorrect as noted in the comment (I am keeping the security tag as I did not got back to to look at this in detail). You are outputting $_REQUEST['asb_view_start'] so it needs to be plained, but user input going to db_select() uses placeholder queries which prevent SQL injection. See the D7 database examples in https://www.drupal.org/writing-secure-code for more details

      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.

      mpdonadio’s picture

      Assigned: mpdonadio » Unassigned

      Forgot to unassign myself.

      benjaminarthurt’s picture

      Status: Needs work » Needs review

      Thanks for the indepth review @mpdonadio

      I've corrected the issues you pointed out and am changing this back to needs review.

      • added a hook_help() for the module, and to the admin page
      • body of anonymous_suggestion_box_mail is now being translated. (user input is not translated.)
      • body is now made from an array of lines
      • I had a hard time with this one, but I removed check_plain from anonymous_suggestion_box_mail_send(), input to that field is being validated at input from anonymous_suggestion_box_admin_validate. This seems to conform with the handling of email addresses in the example you gave.
      • anonymous_suggestion_box_db_insert() now uses drupal_write_record instead of db_insert.
      • Corrected a number of minor coding standards issues, including replacing double quotes with single quote wherever possible.
      • anonymous_suggestion_box_view() now uses the PagerDefault extension for a much more elegant solution, thanks for the link.
      • anonymous_suggestion_box_view() now returns an array to be themed.
      • corrected function doc comment for anonymous_suggestion_box_admin_validate()
      derek.deraps’s picture

      Status: Needs review » Needs work

      Major issues:

      • Default values of variable_get()s in anonymous_suggestion_box_form_submit() do not match defaults in anonymous_suggestion_box_admin() form. Therefore, until the admin form gets saved the first time, the default actions (send email but do not save to DB) do not match the admin form (http://bit.ly/1wszkGX). Instead of having to maintain parity between default values on the admin settings form and default values in the form submit, set default values for the Drupal variables in your hook_install().
      • hook_form_submit() is not real. Change the docblock comment to something like "Form submit callback for anonymous_suggestion_box_form".
      • It's possible for the "site_mail" Drupal variable to be empty, in which case function anonymous_suggestion_box_mail_send() would pass empty $to and $from values to drupal_mail(). If both "site_mail" and "anonymous_suggestion_box_xx_email" are empty, then display an appropriate error message instead of attempting to send.
      • anonymous_suggestion_box_db_insert(): $caught_error is always FALSE.
      • Comments needed. The only function with inline comments is anonymous_suggestion_box_mail_send(). And even the function docblock comments are pretty short. If the docblock comment only says "Implements hook_x()", then the docblock additional description usually needs details about the implementation and purpose.

      Minor issues:

      • Fix the capitalization error "Here" to "here": http://bit.ly/1sViU9r
      • README.txt says "See below for captcha configuration" but no captcha configuration instructions provided.
      • Move $options definition inside the switch statement. No need to call it on every hook_mail() invocation: http://bit.ly/1whsCXw Also, why only pass langcode to the first t() call and not the others?
      • On configuration page, change "Do you want to records saved to the Database when a suggestion is submitted?" to "Do you want to save records to the Database when a suggestion is submitted?"
      • In the email, add spaces between field name and submitted content: http://bit.ly/1zn209L

      Suggestion:

      Consider using a custom entity to store the the submissions. In this way you can customize the entity properties and be sure not to store any sensitive data, but still have the power to leverage FieldAPI. The main benefit here is that you could then allow users to customize the form questions.

      derek.deraps’s picture

      Issue tags: -PAReview: review bonus

      Removing review bonus. Need one more review (6 total) to get it back after failing a review.

      benjaminarthurt’s picture

      Status: Needs work » Needs review

      Thanks for the indepth review @derek.deraps.
      I was able to correct all the major and minor issues you identified. No other significant changes were made to the code over the past several commits.

      I did consider your suggestion of using entities already, however that means a significant rewrite. And am considering approaching this as a 7.x-2.x possibility down the road.

      I'm changing this back to "Needs review", and will look for a few more modules to do manual reviews of to get the review bonus back.

      Thank you!

      benjaminarthurt’s picture

      Issue summary: View changes
      Issue tags: +PAReview: review bonus

      Re-adding the review bonus tag after completeing a 6th project application review.

      keeganhr’s picture

      Status: Needs review » Needs work
      FileSize
      27.99 KB
      21.71 KB

      Hi benjaminarthurt,

      First of all great work on this module! I really enjoy how simple and to the point it is. Overall, everything seemed to work as expected for me but I did find a couple smaller items that could use a little polish.

      Issues

      • Submit button running into description text
      • Permission title's not capitalized.

      Submit button layout issue

      Capitalization issue

      mpdonadio’s picture

      Status: Needs work » Needs review

      Those two aren't what we consider blocking issues (they are small bugs). Is there anything major that your found?

      er.pushpinderrana’s picture

      Assigned: Unassigned » er.pushpinderrana

      Assigning to myself for next review, which will hopefully be tonight.

      keeganhr’s picture

      Hi mpdonadio,

      I didn't find any major bugs in my testing. Just the small ones I noted above.

      er.pushpinderrana’s picture

      Assigned: er.pushpinderrana » Unassigned
      Status: Needs review » Needs work
      FileSize
      97.34 KB

      Automated Review

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

        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. (*) anonymous_suggestion_box_view(): This is vulnerable to XSS exploits. If I enter <script>alert('XSS Form');</script> in the Anonymous Suggestion Box form, tested for Location, Observation and Suggested Improvement elements, every time If I visit View All Submissions page, get a nasty javascript popup. You need to sanitize this before rendering, make sure to read https://www.drupal.org/node/28984 again.
          In given code, Location, Observation and Suggested Improvement variables coming from user provided input and you are directly rendering without sanitize that's why these popup coming.
          foreach ($results as $row) {
              $rows[$row->sid] = array($row->location, $row->observation, $row->suggested);
            }
            $output['content'] = array(
              '#theme' => 'table',
              '#header' => array('location','observation','suggestion'),
              '#rows' => $rows,
              '#sticky' => 'false',
              '#empty' => t('No results found.'),
            );



          XSS

        2. (+) IMHO, you should move your setting form to a separate admin.inc file that is useful to keep all administration stuff like menu callbacks and forms etc. It's also useful for performance as well because all the functions from .module files get loaded at every initialization.

        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.

        benjaminarthurt’s picture

        Status: Needs work » Needs review

        @er.pushpinderrana I corrected the XSS issue that was introduced after redoing the anonymous_suggestion_box_view() recently. Thank you for catching that.

        I've also taken your suggestion and moved the admin functions/page callbacks out of the .module file and into a anonymous_suggestion_box.admin.inc

        @keeganhr I've addressed your issues as well. Permission titles are now using proper capitilization, and I've added a line break so the submit buttion shows below the last line of markup.

        Thank you both.

        guelzow’s picture

        Hi benjaminarthurt

        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: Follows 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
        Yes: Meets the security requirements.

        This review uses the Project Application Review Template.

        I'm not really sure, if this counts as a review, because I was not able to find anything . . .

        Tobi

        er.pushpinderrana’s picture

        Status: Needs review » Reviewed & tested by the community

        @guelzow,

        I'm not really sure, if this counts as a review, because I was not able to find anything . . .

        If you do manual code review then it would be consider as a review, if not found any application blockers simply move to RTBC. See the workflow: https://www.drupal.org/node/532400.

        Anyways I am going to further review this.

        Automated Review

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

          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

          anonymous_suggestion_box_mail(): the switch() statement does not make sense here since you only have one case? Use if() instead?

          Blocking issues from #39 have been addressed and above point is just a suggestion. Been sitting at RTBC for a while now, so...

          er.pushpinderrana’s picture

          Status: Reviewed & tested by the community » Fixed

          Thanks for your contribution, benjaminarthurt!

          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.

          benjaminarthurt’s picture

          @er.pushpinderrana Thanks to you and all the other reviewers. This is a great process and helped me learn and improve my code along the way. Thank you!

          Status: Fixed » Closed (fixed)

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