[D7] Google A/B Testing

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

I'm a robot and this is an automated message from Project Applications Scraper.

It seems that the git branch is not named as per Git Branch Naming Conventions. Please follow the drupal git branch naming conventions for it.

Code looks fine to me.

Removing the PAReview: review bonus, as you have not done any manual reviews of the modules rather than copied the result from the pareview.sh automated result.

You can do another 3 manual reviews of the modules and add the PAReview: review bonus tag.

Please update the proper git repository URL in the issue summary.

You haven't done any manual reviews of the other projects, so please do another 3 manual reviews of the modules and add the PAReview: review bonus tag.

First of all the idea behind the module is good, but do we have any such custom modules already in D7? I haven't come across any though.

Before I jump into the code for manual review I tried to install the module. Here are some of suggested improvements:

If the module is intended to be a D7 module then change this issue name to something like "[D7] Google A/B Testing", this is the nomenclature that is followed while creating issues to propose a module.

Add module name with the git clone command provided above, something like below:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/sandykadam/2189611.git google_abtesting
So that it will clone to correct module directory with a meaningful name.

If the module is intended to be a D7 module then the module info file has a fault, please correct it
core = 7.x-1.x-dev should be core = 7.x

The Readme file should have some information about how to get the experiment key from Google.

With the above comments setting it to needs work.

How is this module different from https://drupal.org/project/multivariate ?

You should add the differences to existing projects to your project page, so that users can make an educated choice.

There is also
https://drupal.org/project/google_website_optimizer
https://drupal.org/project/split_test

I think this should be merged into the google_website_optimizer project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the google_website_optimizer issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

I see, then please add the differences to the existing projects to your project page. See https://drupal.org/node/997024

manual review:

1. Your module does not include any test file, did you consider writing automated tests with simpletest? See https://drupal.org/simpletest
2. google_abtesting_get_experiment_code(): Do not use db_select() for simple static queries, use db_query() instead. See http://drupal.org/node/310075
3. google_abtesting_admin_settings_form(): Doc block is wrong, this is not a hook. See https://drupal.org/node/1354#forms . Also elsewhere.
4. google_abtesting_admin_add_experiments_form_submit(): do not use drupal_goto() here since then other possible submit handlers of the form will be skipped. Use $form_state['redirect'] instead.
5. google_abtesting_preprocess_page(): the inline script is quite ugly and cannot be altered - why can't you use drupal_add_js()? Please add a comment.
6. Notice: Undefined index: values in google_abtesting_admin_add_experiments_form() (line 29 of includes/google_abtesting.admin.inc). Please enable PHP notices in your development environment.
7. admin/config/system/gab-experiments/list: this is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> as experiment path I will get a nasty JavaScript popup on the overview table. You need to sanitize user provided text before printing. Make sure to read https://drupal.org/node/28984 again. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.
8. google_abtesting_preprocess_page(): this is vulnerable to XSS exploits, too. If I enter 5'; alert('XSS'); a=' as Domain name I will get a nasty javascript popup on pages that have an experiment configured. You must either make sure that the domain is sanitized properly before it is used in the script tag or you mark the administrative permission used to set this value as "restrict access" => TRUE, which means that only high level admins can use your module who can take over the site anyway.

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.