Requests on hold can never be fulfilled.
If you want to fulfill a request that is currently on hold, the right
semantic is that you should remove the hold, then fulfill the request.
Note that this test now passes and it didn't before:
fulfillRequest: .... but undef is returned when attempting to fulfill a held request.
This change, which is not properly documented nor tests present, adds
support for making sure those who don't want emails from us do not
receive them.
I believe I've caught most of the places we want this change supported.
if we are unable to fulfill a request, this method turns it into an
indefinite hold on the request.
This design model for handling failure in fulfillment may not be the
best one, but it seemed to roughly fit the behavior and data model we're
looking for.
A little information is lost, but is at least saved in the 'why' field
of the request_hold table.
Requests can now be placed "on hold", and getRequest() can ignore held
requests.
This required addition of a table, and another API call holdRequest().
Tests were not written here, which was a mistake. Unit tests and docs
are needed. A FIXME was added, at least.
Also, minor imporvements to reporting on fulfilled requests.
I debated whether to create a getRequestTypes() instead, but this seemed
reasonable. I am too far out of Perl5 programming culture to know if
this sort of interface is recommended practice.
First of all, I originally thought about releaseDate all wrong. We want
to store the date that the hold was released, which is the indicator
that the request is no longer on hold and can be fulfilled.
We also need tests that assure a request is not fulfilled while on
hold. Those are added here.
I shook out a few other changes to the test ordering that are necessary
for the change to testing holdRequest().
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.
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.
