Thor Gallery

Some drupal standards missing. can be seen here:
http://ventral.org/pareview/httpgitdrupalorgsandboxain1875240git

manual review:

1. thorgallery_install(): why do you need to change the module weight? Please add a comment.
2. thorgallery_install(): you are using get_t() incorrectly, it returns the actual translation function and not the text.
3. thorgallery_create_automated_alias(): why is that function needed, it just sets a variable? Also, no need to set variables upon installation as you can make use of default values with variable_get() anyway.
4. thorgallery_node_info(): shouldn't the description also run through t()?
5. thorgallery_insert(): use drupal_write_record() instead of db_query() and you get schema validation for free.
6. "theme('default', $images, $params, $node->body),": 'default' is too generic, please use you module prefix to avoid name collisions with other modules and their theme functions.
7. thorgallery_set_message(): not good for the watchdog() calls, since you insert already translated strings to the log. You should always insert them untranslated, because they are sent through t() when the log entry is displayed.
8. thorgallery.js: indentation errors, always use 2 spaces per indentation level.
9. notice: Undefined index: type in /home/klausi/workspace/drupal-6/sites/all/modules/thor_gallery/thorgallery.module on line 240. Please enable PHP notices in your development environment.
10. README.txt: what does the album URL look like? Where can I find it? What is the format? Simply copying the album URL from the facebook account does not work?

Don't forget to add the "PAReview: review bonus" tag as indicated in #1410826: [META] Review bonus for your next review bonus, otherwise you won't show up on my high priority list.

Further output from the Coder module:
1. form_set_error()/drupal_set_message() lines 385, 388, 527 in thorgallery.module and line 59 thorgallery.install: When using drupal_set_message, use t() to filter the text. In all cases, when using t(), use placeholders for your variables. The Drupal documentation for t() explains how this is done.

Also, although it is obvious once you think about it, you might want to add something in the README.txt troubleshooting area about making sure that the Facebook album you are using is set to be Public.

Thanks atozstudio for your feedback and review.

I just allowed myself to quickly change the status to needs work since I assume a few changes could still be carried to follow up with #6.

Thanks to all for your testing, comments, reviews and feedbacks.

Please add all your reviews to the issue summary so that we can track them better.

manual review:

1. node-thorgallery.tpl.php: you are printing $image->getCaption() and others without sanitization. Unfortunateley I was not able to exploit this since I could not get my public photo album to work. Can you point me to the code in the Facebook SDK or to a Facebook documentation page where they say that the contents has already been sanitized against XSS attacks? Otherwise you must use check_url() and check_plain() to protect against that. See also http://drupal.org/node/28984 . And please add that as comment in the template file, I guess a lot of other people might wonder if it is safe to just print those variables.
2. thorgallery_init(): only add you javascript to pages where it is actually needed, now you are adding it to every single page request. Actually you should add your JS in thorgallery_view() and remove the init hook, no?
3. "'access arguments' => array('access thorgallery content'),": that permission is not defined?

The first point could be a security issue, so I have to set this back to "needs work". Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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

no objections for more than a week, so ...

Thanks for your contribution, ain!

I updated your account to let you 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 get 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.