[D7] Google Drive Docs Viewer

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

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.

Hi mrken201016,

Please see my comments below:

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

No: Does not follow the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. I installed and configured this module and it working as defined. Its a good attempt and nice feature providing. It would be better if it can integrate with Drupal views.

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.

Thanks,
ARUN AK

I have some suggestions.

1. Configure https://docs.google.com/viewer?srcid=' . $value . '&pid=explorer&efh=false&a=v&chrome=false&embedded=true parameters (globally or for even field display).
Maybe need configure iframe parameters also.

2. When I go to the https://drive.google.com/drive/my-drive and I see my document, I do "Get shareable link" and get URL like https://drive.google.com/open?id=fsdf5345fsdfsdf . Would be cool if I can input to my text field not only id (fsdf5345fsdfsdf), but all url and then module extract id from url.

Automated Review

By PAReview.sh:
http://pareview.sh/pareview/httpsgitdrupalorgsandboxmrken2010162777339git

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

No: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

No: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. (*) Not working for me. I am attaching the screenshot, I am not able to find field formatter , if i am missing something Please update the readme.
2. (+) Please add hook_help

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 walk through 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.

Hi mrken201016,

Thanks for clarification and adding the hook_help, i will review your code again...

As you are not altering any existing field formatters use hook_field_formatter_info(), instead of googledrive_doc_viewer_field_formatter_info_alter(&$info) to add new field formatter.

In your previous comments(#6, #9) you have mentioned you are working on implementing hook_help() and extract document id from google document url. But didn't see those updates in your code. If missed to commit please commit your code properly, will have a look on it again.

Add your manual review links in your own project application summary to get review bonus. Refer Review bonus to know how to get 'PAReview: review bonus' tag for your application.

Removing 'PAReview: review bonus' tag. You can add it again with your three manual review links.

Thanks,
ARUN AK

inside hook_field_formatter_info() use #attached property to include your js instead of drupal_add_js(), since that way your file is included in the render cache.

As per the current implementation users can only use file urls in the format of https://drive.google.com/open?id=1234567890. Please mention this in help page and README.txt file. Google drive public file urls also can be available in the format of https://drive.google.com/file/d/1234567890/view?usp=sharing.

In your code it seems like you are directly using user entered value in your code. As you are working with url and id, use check_url() and check_plain() functions to make sure url/id you are using in a safe manner.

Code too short

Module code is too short. Currently discussion in going on it. But everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed by admins. However, git admin can promote this single project manually to a full project for you.

Otherwise looks good to me. Moving in to RTBC.

Meanwhile you can go with review bonus program to put your application on the high priority list. Then admins will take a look at your project right away.

Thanks,
ARUN AK

Removing review bonus tag, you did not list any manual reviews in the issue summary? See https://www.drupal.org/node/1975228

Not sure why ARUN AK set this to RTBC when there is an XSS vulnerability in the module?

This is vulnerable to XSS exploits. If I enter 123"></iframe><script>alert('XSS');</script> as text field content then I will get a nasty javascript popup when the field is displayed. You need to sanitize user provided text before printing to HTML, make sure to read https://www.drupal.org/node/28984 again.

Upon installing and testing this module, I have following observations:

• The xss vulnerability seems to have been fixed.
• I tested using google ID for document as well as full url and both worked fine.
• The hook help also lists all information needed to install and use the module.
• pareview.sh also did not list any errors.

Since all outstanding issues have been fixed, I am marking this as RTBC.

as per #17 adding single project promote tag. Remove gitignore file from repo.

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

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• 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:

1. project page: what are the differences to existing projects such as https://www.drupal.org/project/droogle ?
2. "$value = check_plain(strip_tags($item['value']));": why the extra strip_tags() call here? Check_plain() should sanitize this just fine? Please add a comment.

Otherwise looks good to me.

Thanks for your contribution, Nhan!

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

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.