Merge lp:~csmith-thecsl/mvhub/abtesting into lp:mvhub

381 + MVHub::Utils::DB::insert_one_record(

Good, most people would have created their own sub routine to do this. It is nice you took the time to read the existing docs/code

17 + my $mytest = new MVHub::UserTest(

Indirect object syntax is deprecated. See Perl Best Practices, chapter 15, objects pages 349-351

changes to app-mvhub/DocumentRoot/cgi-bin/mvhub/contact_form.pl should be reverted as temp code shouldn't go in production. Trunk should always be ready for production.

However, it is really, really nice to have sample code.

**optionally***, create a completely artificial example section in POD

Recently recorded standard

http://wiki.thecsl.org/mediawiki/index.php/Standards:Code_style_guide#Database_.2F_SQL_.2F_Schema

130 +sub generate {
131 + my $self = shift or die q/missing param '$self'/;

139 +sub begin_test {
140 + my $self = shift or die q/missing param '$self'/;

http://wiki.thecsl.org/mediawiki/index.php/Standards:Code_style_guide#Database_.2F_SQL_.2F_Schema

"assert" means:

 "we really don't expect to have a problem"

not:

   "routine data validation"

You might rename some _assert* routines to _validate*

176 + # don't go crazy handling an invalid test id here.
177 + # It could just be that a session expired while the
178 + # test was running (This test data is lost, wave goodbye).
179 + if ( !$usertest_id ) {
180 + return 0;
181 + }

Helpful comment, also points to issue with longer term conversions

220 + my $cookie_name = 'UTSSID';

 possibly better:

220 + my $cookie_name = 'mvhub-UTSSID';

How do we handle multiple simultanious A/B tests ?

  "We don't", might be a fine answer.

462 + cases - Gives an array of names of *well scoped*

What does *Well scoped* mean ?

Is there a simpler, more self-explanatory term ?

476 + casenames - An array of semantically meaningful names to
477 + use when saving conversions for individual
478 + cases. Defaults to ["A", "B"].

fewer words/same meaning better, specific examples better. Perhaps:

  + casenames - names used saving conversions for individual
  + cases. For example: ["current submit button",'trial big red submit button' ]
  + Defaults to ["A", "B"].

86 + if ( exists( $args{casenames} ) ) {
87 + @casenames = delete $args{casenames};
88 + }

You are assigning a scalar or an array reference to an array

89 + else {
90 + @casenames = [ "A", "B" ];
91 + }

You are assigning an array reference to an array.

109 + TESTNAME => $testname,
110 + CASE => -1,
111 + CASENAMES => @casenames,

Assigning an array to a scalar, you probably want \@casenames

112 + MODE => $mode,
113 + CASES => @cases,

Assigning an array to a scalar, you probably want \@casenames

114 + CASECOUNT => $casecount,
115 + PARAMS => {%params}

you should probably create test code that accesses all these things so you know that they are being stored correctly.

589 +# EDIT

Should remove this line

590 +use Test::More tests => 20;
591 +use Test::MockModule;
592 +use Test::MockObject;
593 +use Test::Exception;
594 +use CGI;
595 +use MVHub::UserTest;

http://wiki.thecsl.org/mediawiki/index.php/Standards:Code_style_guide#use_.3Clibrary.3E_statements_ordered_by_type_.283rd_party_vs_ours.29_and_then_alphabetical

165 +# ***ALWAYS REMEMBER TO CALL begin_test BEFORE THE GENERATED
166 +# CASE TEST IS OUTPUTTED***

Not done in SYNOPSIS

# Maybe some different names for sub routines.

#!/usr/bin/perl

# Example
use strict;
use warnings;
use CGI;
use MVHub::UserTest;

my $TESTNAME = 'red vs blue vs green submit button 2011-12-01';

if ( !MVHub::UserTest->test_exists($TESTNAME) ) {
    MVHub::UserTest->create(
        name => $TESTNAME,
        cases => [ 'red', 'blue', 'green' ]
    );
}

my $q = CGI->new();

if ( $q->param('Submit') || ) {
    # record_conversion sucks cookie / hidden field / url param
    # for user
    MVHub::UserTest->record_conversion($q);
}
else { # starting test, store coo

    # store start of test run in database
    my $tr = MVHub::UserTest->new_test_run($TESTNAME);
    my $color = $tr->get_casename();
    my $cookie = $tr->get_case_cookie();
    print $q->header( cookie => [$cookie] );
    print generate_form($color);
}

sub generate_form {
    my $color = shift;
    return <<"FORM";
<form>
      <input type="submit" name="Submit" value="Submit" style='background-color: $color;' />
  <input
</form>
FORM
}

Existing tables break 2nd Normal form

random, untested , partly formed SQL

BEGIN;

INSERT INTO version_log ( version,note )
 VALUES (12,'tables for in AB testing');

CREATE TABLE usertest_tests (
    id integer PRIMARY KEY,
    name text
 );

-- each test can have several cases / choices /outcomes
CREATE TABLE usertest_cases (
   id integer PRIMARY KEY,
   test_id integer NOT NULL,
   FOREIGN KEY (test_id) REFERENCES usertest_tests (id),
   name text
);

-- each test can be run many times with many results
CREATE TABLE usertest_runs (
   id integer PRIMARY KEY,
   test_id integer NOT NULL,
    FOREIGN KEY (test_id) REFERENCES usertest_tests (id),
    "timestamp" timestamp without time zone DEFAULT now() NOT NULL
);

CREATE TABLE usertest_conversions (
    run_id integer,
    case_id integer,
    FOREIGN KEY (case_id) REFERENCES usertest_cases (id),
    FOREIGN KEY (run_id) REFERENCES usertest_runs (id),
   "timestamp" timestamp without time zone DEFAULT now() NOT NULL
);

COMMIT;

SELECT
  usertest_conversions.case_id,
  count(*) AS cnt
FROM
   usertest_conversions
GROUP BY usertest_conversions.case_id;