[D7] Tin Can API

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxdevanchase2435069git

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,

There are some errors found in PAReview.sh

Please address those issues before submitting for review...

http://pareview.sh/pareview/httpgitdrupalorgsandboxdevanchase2435069git

The above is not application blocker.Do you find anything else that stopped you to set this to RTBC

Hello,
I have installed the module .On navigating to the Tin Can API configuration page, the fields in the page accepts invalid data.

So include the validation function, 'tincanapi_settings_validate()' inside 'tincanapi_admin_form()' in 'tincanapi.admin.inc' file.

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. However there is no default branch set. (also stated in pareview)

Licensing

Yes: Followsthe licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template. Missing Requirements section (Libraries module). Installation section covers both installation and configuration. Template suggests splitting into separate configuration section. Also missing the recommended modules section, which could include Token, since it can be used in tincanapi_get_actor(), for instance. The submodules don't have README files that describe the libraries they need. tincanapi_vimeo, for instance, uses froogaloop but that isn't specified anywhere.
Here is a summary of the preferred format:

• Headings in all caps.
• Headings underlined with ===/--- to the length of the heading, followed by a newline.
• Two lines prior to headings (except the first one).
• Bullets denoted by asterisks (*) with hanging indents.
• Numbered lists indented 4 spaces.
• Bulleted lists indented 1 space.
• Text manually word-wrapped within around 80 cols.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No: Does not meet the security requirements. Use check_plain() for the _POST values in tincanapi_ajax_track(). Risk here may be low, but it is taking input straight out of the post.

Coding style & Drupal API usage

List of identified issues in no particular order.

1. No major findings or release blockers
2. tincanapi_track_data() - Recommend using parse_url to replace the scheme rather than string replacement. This will also help verify that the data you're working with is an actual URI instead of some gobbledygook.
3. tincanapi_send() - One of your watchdog() calls checks for the tincanapi_watchdog variable, but one does not (line 377). Wrap all of the watchdog calls.
4. tincanapi_quiz - Consider using the Quiz module's API to get results instead of the db_select. quiz_result_load() gets an entity that you can use. You can use the stuff Quiz gives you to get result and item data for each of its item types to avoid querying the database directly.

This review uses the Project Application Review Template.

This is a really exciting project. I love to see Drupal/SCORM integration. Very ambitious and exciting! I have a project that needs to collect SCORM completion data from learning objects, rather than going the other way around - otherwise I would use this module myself right now. Great contribution.