holdRequest():testing hold on already held request

It's clear from the Supporters.pm code that when an hold is attempted on
an already held request, it simply returns the id of the existing hold.

I don't actually remember what behavior I really wanted here.  There are
certainly possibility for semantic confusion in the API with the current
functionality, since the API caller must actually check to verify if the
hold they got is a new one or just an existing one.

Perhaps this should be rethought.  I left a comment in the test code for
that reason.
This commit is contained in:
Bradley M. Kuhn 2017-01-14 19:41:57 -08:00
parent 68133e8cee
commit 836a70c0ab

View file

@ -591,12 +591,22 @@ is($sp->getRequestType("does-not-exist"), undef,
my $reHoldId;
dies_ok { $reHoldId = $sp->holdRequest( { donorId => $drapperId, holdReleaseDate => '2112-05-15',
# FIXME: Do the following two tests really exhibit the behavior we actually
# want? The API caller might be receiving unexpected results here,
# because as the two tests below show, it's possible, when attempting
# to hold a request that is already held, that you're returned an id
# for a hold that has different details (other than the requestType,
# of course).
lives_ok { $reHoldId = $sp->holdRequest( { donorId => $drapperId, holdReleaseDate => '2112-05-15',
requestType => "t-shirt-0", who => 'peggy',
heldBecause => "will leave in his office." }); }
"holdRequest: attempt to hold an already-hold request dies ...";
"holdRequest: attempt to hold an already-held request lives ...";
is_deeply($reHoldId, $drapperTShirt0HoldId, "holdRequest: ... but returns the id of the old hold request.");
my $holdRequest;
lives_ok { $newHoldId = $sp->holdRequest( { donorId => $olsonId, holdReleaseDate => '2048-05-15',
requestTypeId => $tShirt0RequestTypeId, who => 'john',
heldBecause => "will delivery at conference" }); }