GatherContent

Hello,

We found several errors on your module, you may like to look at the following URl for list of errors

http://ventral.org/pareview/httpgitdrupalorgsandboxbrunodbo1883798git

Do take time to rectify the issues.

Custom classes should be moved out of the module file into it's own and registered in your .info file. http://drupal.org/node/542202#files

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Hi,

I've taken a look at your class GatherContentPages and notice that in most cases each method is used on its own with out implementing any other method in the class apart from sortRecursive, could I suggest that you implement theses as static classes as this seems to be how they are being used, also the method sortRecursive is marked as Protected is there an intention to extend this class in the future and to implement this method in children? If not it should be set as Private in order to enforce encapsulation.

Looking more at the methods in your class I would look to breaking them down into smaller methods that do different work (this is if you intend not to make them static classes), getPages for example does not just get pages it checks if a page is approved, it does some sorting, and it does some variable initialising, have a think about this and see if you can break it into further methods and it will make it easier to understand, remembering to comment each method.

Hope this is of help :)

manual review:

1. gathercontent_permission(): Since importing stuff into full HTML fields has security implications this permission should use 'restrict access' => TRUE.
2. gathercontent_get_command(): why can't you use drupal_http_request() and have to use cURL? Please add a comment.
3. "$cls->error = t($msg);": do not pass variables to t() where possible, always use string literals. Otherwise translation extraction tools cannot find the translatable strings.
4. "t('%node_title created successfully.', array('%node_title' => check_plain($node->title)))": the "%" placeholder will run check_plain() for you already, so no need for that. Make sure to read the documentation of t() again. Also elsewhere.
5. public function pageImportForm(): "'#title' => $page->name,": this is vulnerable to XSS exploits. The page comes from an external service and is therefore considered untrusted user provided input. You need to sanitize that before printing, see http://drupal.org/node/28984
6. public function getPages(): what is going on in that method? Please add some inline comments.
7. public function createNodes(): why do you call node_submit()? Please add a comment.

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

Tagging.

Hi there,

I've just given a shot today to test this out as it seems interesting. I got an issue with the API key not recognized, even though it the same that the one provided by GatherContent:

The GatherContent API could not be reached. Please verify your API credentials (Error message: There was a problem contacting the API. Please check your API credentials.)

I'm using a trial version of GatherContent, it might be a reason.

Config:
- Drupal 7.19
- Only Ctools and GatherContent are installed

Not sure if you've ever seen this issue before.

Great thanks for that. I will check it.

Hi @brunodbo,
I just found GatherContent and I love it. I'm also intrigued by the future integration of a beta gathercontent module for Drupal. By reviewing the notes and postings, it looks like your getting awful close to a beta release. ...and I'm sure you don't want to hear this question, but I have to ask... Do you have a timeframe on the beta release?

thanks and keep up the great work.
~e

This looks good to me. Should definitely be a full project.

manual review:

• if you are requiring cURL you should add a hook_requirements() like simpletest does: http://api.drupal.org/api/drupal/modules!simpletest!simpletest.install/f...

Surely no blocker, so ...

Thanks for your contribution, brunodbo!

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.