Commerce Moodle Integration

Please add a git link.

Manual review:
commerce_moodle.install: please remove unnesessary endings on line 12 and 21.
commerce_moodle.module: line 18, there is no need to specify MENU_NORMAL_ITEM, because it uses by default.
line 66, It should be like this $time = REQUEST_TIME. Don’t use a function time().

Removed links to drupal.org issues

The theming you are applying is quite strange...
You are using 'commerce_moodle_make_links' function in order to convert the array into a string with some lines with '
'. After that, you use theme_commerce_moodle function for just add a DIV tag with an id. There are many things here to change:

1. You can still use commerce_moodle_make_links to sanitize stuff. But it should be better to return the array, and join all in theme_commerce_moodle function. Also, don't use a fixed id property, try to generate an auto-incremental one, or just a class... if anyone call this block more than once, then you can avoid duplicated ids.
2. Please, use theme('commerce_moodle', ... instead of theme_commerce_moodle in line 296, so anyone can override it into his own theme template.
3. Please, try to let people customize as many as possible :) when no courses, use another theming function (i.e. theme_commerce_moodle_empty) to let the developers change it, or use a variable_set/variable_get (and sanitize HTML to avoid XSS, and translate) to let both developers and admins to customize that text.

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

manual review:

1. you project page could be improved: http://drupal.org/node/997024
2. you could add a helper function that returns you a database connection object for your moodle database, so you don't have to duplicate that code all the time. And you should then run the queries on that object instead of using db_set_active().
3. commerce_moodle_commerce_checkout_complete(): does the watchdog() call even work while you changed the database?
4. "watchdog('Commerce Moodle', "Error inserting enrolment or role assignement for user id '$moodle_user_id' and course id '$moodle_course_id'");": do not concatenate variables into translatable strings, use placeholders with watchdog() instead. This could be considered a small security vulnerability as soon as there is malicious data in $moodle_user_id.
5. "$courses_array[] = l(check_plain($name), $url);": no need to use check_plain() in l(), double escaping is bad. Please read http://drupal.org/node/28984 again.
6. "if (!$user->uid || !user_access('view commerce moodle')) {": why do you check the user uid? The permission could be assigned to anonymous users, then they would not have access?

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

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

9th review...

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

Thanks for your contribution, netol!

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.

Just leave it at "fixed", it will close automatically after 2 weeks.