It seems that date('now'), at least in sqlite, is in UTC. So, we want
the tests to match that.
There are still timing bugs possible here, but with this change, they
should only occur if you run the test right at 23:58 or 23:59 UTC. :)
We don't really need lookup based on requestTypeId for this anyway, so
this method really only needs two arguments (perhaps a third optional
argument to be added later).
Parameter check tests now pass:
ok 66 - fufillRequest: dies if supporterId not specified
ok 67 - fufillRequest: dies if supporterId not found in database
ok 68 - fufillRequest: dies if requestType not specified
ok 69 - fufillRequest: who not specified
Also added new test.
addRequest-specific unit tests now all pass:
ok 55 - addRequest: dies if supporterId not specified.
ok 56 - addRequest: dies if requestTypeId / requestType not specified.
ok 57 - addRequest: dies if supporterId invalid.
ok 58 - addRequest: dies if requestTypeId invalid.
ok 59 - addRequest: succeeds with a requestType but no configuration parameter.
ok 60 - addRequest: id returned on successful addRequest() is a number
ok 61 - addRequest: underlying call to addRequestType works properly, per getRequestType
ok 62 - addRequest: succeeds with a requestType and requestConfiguration and a note.
ok 63 - addRequest: succeeds with a requestTypeId and requestConfigurationId with no a note.
The older transaction counter was for the whole class, which was
problematic if you had more than one supporter database open at a time,
or were otherwise using different supporter databases.
I'm not completely sure this fix is fully functional, because perhaps we
should be carrying this counter along with the DBH (i.e., what if two
Supporter instances are created with the same dbh).
I suppose we could fix this problem by changing the new() interface to
require the instance itself be in control of the dbh.
There was some parameter confusion (using ids instead of the actual
types/descriptions) on some of the public facing methods.
Also, lookup of existing ids was buggy; just use public facing methods.
These tests now pass:
ok 110 - _getOrCreateRequestConfiguration: dies on empty hash
ok 111 - _getOrCreateRequestConfiguration: dies for string requestConfigurationId
ok 112 - _getOrCreateRequestConfiguration: dies for non-existant requestConfigurationId
ok 113 - _getOrCreateRequestConfiguration: dies for string request id
ok 114 - _getOrCreateRequestConfiguration: dies for non-existant requestTypeId
Basic implementation of addRequest, which causes these test to now pass:
ok 55 - addRequest: dies if supporterId not specified.
ok 56 - addRequest: dies if requestTypeId / requestType not specified.
ok 57 - addRequest: dies if supporterId invalid.
ok 58 - addRequest: dies if requestTypeId invalid.
ok 61 - addRequest: underlying call to addRequestType works properly, per getRequestType
Tests pass for this now:
ok 97 - _getOrCreateRequestType: dies on empty hash
ok 98 - _getOrCreateRequestType: dies for string request id
ok 99 - _getOrCreateRequestType: dies for non-existant requestTypeId
ok 100 - _getOrCreateRequestType: succeeds with just requestType
ok 101 - _getOrCreateRequestType: lookup of a request works after _getOrCreateRequestType
ok 102 - _getOrCreateRequestType: lookup of a request works after _getOrCreateRequestType
ok 103 - _getOrCreateRequestType: lookup of existing requestType suceeds.
ok 104 - _getOrCreateRequestType: deletes requestType if both are provided.
All tests related to addPostalAddress now pass.
ok 28 - addPostalAddress: dies for undefined id
ok 29 - addPostalAddress: dies for non-numeric id
ok 30 - addPostalAddress: postal address undefined fails
ok 31 - addPostalAddress: type is not added when other input paramaters are invalid
ok 32 - addPostalAddress: addPostalAddress of a valid formatted_address works.
ok 33 - addPostalAddress: id returned is sane.
This test now started failing after the other corrections made in the
last few commits.
As it turns out, this test was buggy. We wanted to test a request_type
that didn't exist, and verify that calls to addRequestConfigurations
with bad arguments didn't create the request_type if it didn't already
existed. That's what the test now does.
Upon fixing the reference counter thing in the previous commit, this bug
caused tests to fail. We have to _rollback() properly any time we've
called _begin_work(), particularly before die()'ing after a
_begin_work().
This code was clearly not correct as it was. The main problem is that I
was envisioning begin_work/commit pairs as always matched, but of course
if we die in the middle of a transaction, the counter has to be reset.
That's why we have to set the $dbh->{HandleError} in new() to a
subroutine that does that.
Also, we need to similarly wrap rollback() calls. When rolling back, we
have to reset the counter as we're not going to commit() (that's the
point).