Bug #51507 for Business-OnlinePayment-PayflowPro: Add tax, tax exempt, freight, duty, and ponum fields

Fri Nov 13 11:48:56 2009 josh [...] infogears.com - Ticket created

Subject:

Add tax, tax exempt, freight, duty, and ponum fields

This patch: *) adds the TAXAMT, FREIGHTAMT, DUTYAMT, TAXEXEMPT, and PONUM fields. I did not find a document on what fields BOP wants to use for these, so I used the fields found in the following BOP::AuthorizeNet pending patch, which seemed to make sense to me. (tax, freight, duty, tax_exempt, and po_number.) http://rt.cpan.org/Public/Bug/Display.html?id=42046 *) allows the user to explicitly set CUSTREF. I did not know a good BOP field to use for this, so it needs to be used directly if the user needs it. *) adds clarification in documentation that login/password need to be used in content call. -- Josh

Subject:

PayflowPro_tax_freight_field_1_01_diff.patch

--- /usr/local/lib/perl5/site_perl/5.10.0/Business/OnlinePayment/PayflowPro.pm 2009-09-22 15:11:53.000000000 -0600 +++ PayflowPro.pm 2009-11-13 09:31:39.000000000 -0700 @@ -8,7 +8,7 @@ use base qw(Business::OnlinePayment::HTTPS); -$VERSION = '1.01'; +$VERSION = '1.02'; $VERSION = eval $VERSION; $DEBUG = 0; @@ -216,6 +216,13 @@ SHIPTOSTATE => 'ship_state', SHIPTOZIP => 'ship_zip', SHIPTOCOUNTRY => 'ship_country', + + TAXAMT => 'tax', + FREIGHTAMT => 'freight', + DUTYAMT => 'duty', + TAXEXEMPT => 'tax_exempt', + PONUM => 'po_number', + ); # Reload %content as _revmap_fields makes our copy old/invalid! @@ -244,6 +251,7 @@ SHIPTOFIRSTNAME SHIPTOLASTNAME SHIPTOSTREET SHIPTOCITY SHIPTOSTATE SHIPTOZIP SHIPTOCOUNTRY CUSTCODE + TAXAMT FREIGHTAMT DUTYAMT TAXEXEMPT PONUM CUSTREF ) ); @@ -397,13 +405,15 @@ my $tx = new Business::OnlinePayment( 'PayflowPro', - 'vendor' => 'your_vendor', + 'vendor' => 'your_vendor_aka_merchant_login', 'partner' => 'your_partner', 'client_certification_id' => 'GuidUpTo32Chars', ); # See the module documentation for details of content() $tx->content( + login => 'your_username', + password => 'your_password', type => 'VISA', action => 'Normal Authorization', description => 'Business::OnlinePayment::PayflowPro test', @@ -647,6 +657,13 @@ SHIPTOZIP => 'ship_zip', SHIPTOCOUNTRY => 'ship_country', + TAXAMT => 'tax', + FREIGHTAMT => 'freight', + DUTYAMT => 'duty', + TAXEXEMPT => 'tax_exempt', + PONUM => 'po_number', + + The required Payflow Pro parameters for credit card transactions are: TRXTYPE TENDER PARTNER VENDOR USER PWD ORIGID

Sat Nov 14 21:08:05 2009 ivan-pause [...] 420.am - Correspondence added

RT-Send-CC:

phil [...] perkpartners.com

On Fri Nov 13 11:48:56 2009, josh@infogears.com wrote: Show quoted text

> This patch: > > *) adds the TAXAMT, FREIGHTAMT, DUTYAMT, TAXEXEMPT, and PONUM fields. > I did not find a document on what fields BOP wants to use for these, > so I used the fields found in the following BOP::AuthorizeNet pending > patch, which seemed to make sense to me. (tax, freight, duty, > tax_exempt, and po_number.) > http://rt.cpan.org/Public/Bug/Display.html?id=42046

I applied the patch in 42046 to the AuthorizeNet module, and also updated B:OP and included those fields as standard content fields. The changes will appear in the next releases of those modules. Show quoted text

> *) allows the user to explicitly set CUSTREF. I did not know a good > BOP field to use for this, so it needs to be used directly if the > user needs it.

Looking at the Payflow Pro documentation, they suggest using the invoice number, or a unique transaction identifier. I could see doing one of two things: 1) Use invoice_number. We don't map this to anything else except COMMENT2 right now, and it is what they suggest. Setting this to our own unique transaction identifier probably isn't necessary to find transactions, since we return the PNREF as order_number. 2) Define a new B:OP content field for unique identifiers and pass this as CUSTREF (generate one if not passed?). We could use request_id? (the current docs don't seem to mention anything about requiring it for X-VPS-Request-ID or any X-VPS-* headers, actually. I wonder if they're still necessary). Leaning toward the first option.

Sat Nov 14 21:08:06 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Nov 16 21:10:46 2009 josh [...] infogears.com - Correspondence added

On Sat Nov 14 21:08:05 2009, IVAN wrote: Show quoted text

> On Fri Nov 13 11:48:56 2009, josh@infogears.com wrote:

> > *) allows the user to explicitly set CUSTREF. I did not know a good > > BOP field to use for this, so it needs to be used directly if the > > user needs it.

> > Looking at the Payflow Pro documentation, they suggest using the invoice > number, or a unique transaction identifier. > > I could see doing one of two things: > > 1) Use invoice_number. We don't map this to anything else except > COMMENT2 right now, and it is what they suggest. Setting this to our > own unique transaction identifier probably isn't necessary to find > transactions, since we return the PNREF as order_number. > > 2) Define a new B:OP content field for unique identifiers and pass this > as CUSTREF (generate one if not passed?). We could use request_id? > (the current docs don't seem to mention anything about requiring it for > X-VPS-Request-ID or any X-VPS-* headers, actually. I wonder if they're > still necessary). > > Leaning toward the first option.

Hmm, this is sort of a difficult decision here, because it seems if we make either of these changes, we may end up breaking something for someone. For example with 1, the CUSTREF field is limited to 12 characters according to the docs, while COMMENT2 is limited to 128. So making that change could start truncating someone's method of reporting if they are using the invoice_number field for something longer. I guess we could store to both to keep backward compatibility. In #2, switching to use request_id seems kind of scary too. Best not to mess with code that has already been time-tested and works. I like the idea of adding the unique identifiers, because there are fields that just aren't mappable sometimes. Perhaps we can take a two pronged approach here: *) map invoice_number to CUSTREF and COMMENT2. This will maintain backward compatibility and keep the BOP public interface correct. *) give the user the option to add/override specific payment fields. This override would be from the custom BOP content unique identifier field you mentioned. (The keys/values there would get added to the submitted content data after the regular mapping is done.) That should give 0 breakage, but provide flexibility to the user in the cases they need it. (ie. For times when there is no field mapping for a field they need or if they need to override a mapping done in one of the payment modules.) Thoughts?