Properly extract Options from PaymentItemInfo

The Options fields found in
/PaymentTransactionDetails/PaymentItemInfo/PaymentItem/Options have
never been extracted in the proper way.  In fact, I'd be surprised if
anyone found the previous method of extraction useful at all.

I discovered this bug when I needed to lookup the Options selected
during payment, and found they weren't returned by the API.

Specifically, the value inside the SOAP tags, which is what the API
previously returned, are simply not useful: they are typically empty.
The interesting data is stored in the attributes of 'name' and 'value'.

Note, BTW, that each PaymentItem can have its own set of Options.  This
code added herein properly extracts the Options data for each
PaymentItem, and places it both in the Options field for that
PaymentItem, and in the array in the PII_Options "convenience" field.

As can be seen in the accompanying changes to t/OptionsFields.t, the
return values of the Options data is now a hash rather than a list.  I
believe this API-user-visible change is appropriate since a hash is
ultimately the natural representation of this data, and since having the
Options as an array of empty strings wasn't useful, anyway.

Nevertheless, if existing user code of this API relies on Options being
an array of empty strings (which I suppose could have been usefully used
in scalar context to get the count of options in the record), this
change theoretically creates a user-visible change in the API.  I can't
imagine anyone found the previous return value useful in the past
anyway, so I'd be surprised if anyone relies on this mis-feature.  If
they did, they can simply change their code to:
       scalar keys ...{Options}
instead of:
       scalar @...{Options}

However, since this is technically an API change, I've updated the
Changes file and the module documentation to mention it.
This commit is contained in:
Bradley M. Kuhn 2015-01-02 13:09:50 -05:00
parent dd0ff8e2f4
commit 5113ced7e3
3 changed files with 46 additions and 5 deletions

View file

@ -9,6 +9,8 @@ Revision history for Perl module Business::PayPal::API
- Extract out more payer details from XML. - Extract out more payer details from XML.
(PayerName, NameSuffix, PayerCountry). (PayerName, NameSuffix, PayerCountry).
- Fix https://rt.cpan.org/Public/Bug/Display.html?id=67386 - Fix https://rt.cpan.org/Public/Bug/Display.html?id=67386
- Options fields of GetTransactionDetails are now returned as a hash,
containing the actual options data, rather than array of empty strings.
0.70 2012-11-13 0.70 2012-11-13

View file

@ -127,12 +127,43 @@ sub GetTransactionDetails {
Number => 'Number', Number => 'Number',
Quantity => 'Quantity', Quantity => 'Quantity',
Amount => 'Amount', Amount => 'Amount',
Options => 'Options',
} }
); );
if ( scalar( @$paymentitems ) > 0 ) { if ( scalar( @$paymentitems ) > 0 ) {
# Options data must be extracted differently. Specifically, the
# "interesting" parts of the Options data is not in the values inside
# the $som structure (which means $som->valueof(), which
# $self->getFieldsList() above calls underneath, won't extract the
# useful data, because the values are empty.
# The convoluted loop below finds any Options values and matches them
# up properly with the correct PaymentItem. Note that the loop is
# written this way to account for the fact that there may be multiple
# PaymentItems.
# Finally, I contemplated placing this loop below in getFieldsList()
# with a special-case for Options, but I am not sure it belongs
# there. Ultimately, I think this is unique to the
# GetTransactionsDetails call in the API, and thus it's more
# appropriately placed here.
my $ii = 0;
my @fulloptions;
foreach my $rec ( $som->dataof($path . '/PaymentItemInfo/PaymentItem' ) ) {
my %options;
foreach my $subrec ($rec->value()) {
foreach my $fieldrec ($$subrec->value()) {
$options{$fieldrec->attr()->{name}} = $fieldrec->attr()->{value}
if ($fieldrec->name() eq "Options");
}
}
$paymentitems->[$ii]{Options} = \%options;
push(@fulloptions, \%options);
}
# Now, we can save the payment items properly
$response{PaymentItems} = $paymentitems; $response{PaymentItems} = $paymentitems;
# And set the PII_Options properly too.
$response{PII_Options} = \@fulloptions;
} }
return %response; return %response;
@ -260,6 +291,7 @@ records:
Number => '...', Number => '...',
Quantity => '...', Quantity => '...',
Amount => '...', Amount => '...',
Options => { 'key' => 'value', ... },
}, },
{ SalesTax => ..., etc. { SalesTax => ..., etc.
} ] } ]
@ -272,6 +304,9 @@ Example:
for my $item ( @{ $resp{PaymentItems} } ) { for my $item ( @{ $resp{PaymentItems} } ) {
print "Name: " . $item->{Name} . "\n"; print "Name: " . $item->{Name} . "\n";
print "Amt: " . $item->{Amount} . "\n"; print "Amt: " . $item->{Amount} . "\n";
for my $optionname (keys %$item->{Options}) {
print "Option: $optionname is " . $item->{Options}{$optionname} . "\n";
}
} }
=head2 ERROR HANDLING =head2 ERROR HANDLING

View file

@ -10,7 +10,7 @@ if ( !$ENV{WPP_TEST} || !-f $ENV{WPP_TEST} ) {
'No WPP_TEST env var set. Please see README to run tests'; 'No WPP_TEST env var set. Please see README to run tests';
} }
else { else {
plan tests => 6; plan tests => 14;
} }
use_ok( 'Business::PayPal::API::TransactionSearch' ); use_ok( 'Business::PayPal::API::TransactionSearch' );
@ -75,10 +75,14 @@ foreach my $record (@{$resp}) {
like($detail{PaymentItems}[0]{Name}, qr/Field\s+Options/i, 'Found field options test transaction'); like($detail{PaymentItems}[0]{Name}, qr/Field\s+Options/i, 'Found field options test transaction');
like($detail{PII_Name}, qr/Field\s+Options/i, 'Found field options test transaction'); like($detail{PII_Name}, qr/Field\s+Options/i, 'Found field options test transaction');
foreach my $options ($detail{PaymentItems}[0]{Options}, $detail{PII_Options}) { foreach my $options ($detail{PaymentItems}[0]{Options}, $detail{PII_Options}[0]) {
ok((scalar(@$options) == 2 and $options->[0] eq '' and $options->[0] eq ''), ok(scalar(keys %$options) == 2, "The PaymentItems Options has 2 elements");
"PaymentItems's Options has 2 elements with empty strings"); ok(defined $options->{firstOption}, "'firstOption' is present");
ok($options->{firstOption} eq 'Yes', "'firstOption' is selected as 'Yes'");
ok(defined $options->{size}, "'size' option is present");
ok($options->{size} eq "Large", "'size' option is selected as 'Large'");
} }
# Local Variables: # Local Variables:
# Mode: CPerl # Mode: CPerl
# indent-tabs-mode: nil # indent-tabs-mode: nil