The ledger data can, in fact, sometimes have a missing entityId for
various reasons. For now, these donations should just be ignored.
Perhaps in future a warning of some sort should be generated.
This passes all tests:
ok 224 - donorTotalGaveInPeriod(): dies with undefined donorId
ok 225 - donorTotalGaveInPeriod(): dies with non-numeric donorId
ok 226 - donorTotalGaveInPeriod(): dies with non-existent id
ok 227 - donorTotalGaveInPeriod(): dies with non ISO-8601 string in startDate
ok 228 - donorTotalGaveInPeriod(): dies with non ISO-8601 string in endDate
ok 229 - donorTotalGaveInPeriod(): dies if given an argument that is not recognized
ok 230 - donorTotalGaveInPeriod(): total for a donor with no period named succeeds...
ok 231 - donorTotalGaveInPeriod(): ...and returned value is correct.
ok 232 - donorTotalGaveInPeriod(): check for total with both start and end date succeeds...
ok 233 - donorTotalGaveInPeriod(): ...and returned value is correct.
ok 234 - donorTotalGaveInPeriod(): check for total with just a start date succeeds...
ok 235 - donorTotalGaveInPeriod(): ...and returned value is correct.
ok 236 - donorTotalGaveInPeriod(): check for total with just a end date succeeds...
ok 237 - donorTotalGaveInPeriod(): ...and returned value is correct.
These two new tests:
ok 222 - supporterExpirationDate(): same donation amount in year...
ok 223 - supporterExpirationDate(): ...returns the latter date.
did not pass without this change. The list for annuals in
supporterExpirationDate() was sorted in the wrong order, producing
erroneous results.
ok 48 - setPublicAck: fails supporterId invalid
ok 49 - setPublicAck: fails supporterId is string
ok 50 - setPublicAck: fails supporterId is undef
ok 51 - setPublicAck: 1 failed calls changed nothing.
ok 52 - setPublicAck: 1 failed calls changed nothing.
ok 53 - setPublicAck: 1 failed calls changed nothing.
ok 54 - setPublicAck: lives when valid id is given for undefining...
ok 55 - setPublicAck: ...and suceeds in changing value.
ok 56 - setPublicAck: lives when valid id is given for off...
ok 57 - setPublicAck: ...and suceeds in changing value.
ok 58 - setPublicAck: lives when valid id is given for on...
ok 59 - setPublicAck: ...and suceeds in changing value.
All tests now pass:
ok 15 - getLedgerEntityId: fails when rows are not returned but _verifyId() somehow passed
ok 16 - getLedgerEntityId: fails when rows are not returned but _verifyId() somehow passed
ok 17 - getLedgerEntityId: lives when valid id is given...
ok 18 - getLedgerEntityId: ...and return value is correct.
This seems to work and all existing tests for it pass:
ok 134 - findDonor: no search criteria dies
ok 135 - findDonor: 1 lookup of known missing succeeds ...
ok 136 - findDonor: ... but finds nothing.
ok 137 - findDonor: 2 lookup of known missing succeeds ...
ok 138 - findDonor: ... but finds nothing.
ok 139 - findDonor: 1 and'ed criteria succeeds ...
ok 140 - findDonor: ... but finds nothing.
ok 141 - findDonor: 2 and'ed criteria succeeds ...
ok 142 - findDonor: ... but finds nothing.
ok 143 - findDonor: 1 valid multiple criteria succeeds ...
ok 144 - findDonor: ... and finds right entry.
ok 145 - findDonor: 2 valid multiple criteria succeeds ...
ok 146 - findDonor: ... and finds right entry.
ok 147 - findDonor: 3 valid multiple criteria succeeds ...
ok 148 - findDonor: ... and finds right entry.
ok 149 - findDonor: single criteria find expecting multiple records succeeds...
ok 150 - findDonor: ... and finds the right entires.
Since the emailAddress criterion could find more than one entry, this
change has findDonor returning a list rather than a scalar integer and
thus finding multiple items are ok.
Tests are more extensive now that this API change is in effect.
This should succeed as the previous tests show. They now pass:
ok 21 - addEmailAddress: fails adding existing email address with mismatched type.
ok 22 - addEmailAddress: succeeds when adding email that already exists...
ok 23 - addEmailAddress: ... and returns same id.
Instead of a bunch of serial arguments, reimplement getRequest() to take
a hash of parameters.
In the process, add support for requestTypeId to be included.
I found need to have _verifyRequestTypeId() actually return the
request_type in a reimplementation of getRequest() that I'm working on,
so I've made this change.
I found need to have _verifyRequestTypeId() actually return the
request_type in a reimplementation of getRequest() that I'm working on,
so I've made this change. Some tests are failing because of the use of
_verifyRequestTypeId(). Next commit will address that.
Since we converted to making this a more general donor database, change
the handle used for an individual in the database from supporterId to
donorId.
Note that I left addSupporter() method name untouched because it does
automatically assume you mean a supporter, not a mere donor. Later, an
addDonor() method should likely be added.
Up until now, this software has been focused on just Supporters, but
really there is no reason this should not be a general donor database.
Therefore, don't use the name supporter in the database, and add a
field.
public_ack is now allowed to be NULL, because the idea being we don't
have an answer from all who donate whether or not they want public
acknowledgment.
the is_supporter boolean is added to record whether or not they came
through the supporter program.
This is the location where there was unbalanced _beginWork()/_commit()
calls. In future, when writing tests, it's probably good to check this
often in the tests.
These tests now pass:
ok 106 - setPreferredEmailAddress: dies for undefined id
ok 107 - setPreferredEmailAddress: dies for non-numeric id
ok 108 - setPreferredEmailAddress: email address undefined fails
ok 109 - setPreferredEmailAddress: email address with extra @ fails to add.
ok 112 - setPreferredEmailAddress: email address not found in database does not die....
ok 113 - setPreferredEmailAddress: ....but returns undef
ok 116 - setPreferredEmailAddress: setting preferred email address succeeds....
ok 117 - setPreferredEmailAddress: ... and returns correct email_address_id on success
of course, getRequest() returns a hash so we must properly extract the
value we want.
This fix causes this test to now pass:
ok 73 - fulfillRequest: databse entry from successful return is correct
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
