Login  Register

Re: Dev - Bug in the updateCreditCard service

Posted by David E. Jones on Apr 18, 2006; 6:51am
URL: http://ofbiz.116.s1.nabble.com/Dev-Bug-in-the-updateCreditCard-service-tp167652p167657.html


Taking a step back from the specifics on this... Why are we updating payment methods that point to the address in the first place? The whole point of having immutable artifacts like a ContactMech is to avoid this. Do we really want to be doing this? Would it not be better to go through and change the payment methods manually as we really don't know if the user intended that address change to carry over into this area?

-David


Si Chen wrote:

> Jacopo,
>
> I think we should fix it in updateCreditCard, but by doing a check right
> at the beginning to see if the change is for the expiration date and, if
> not, whether the credit card is already expired.  Thus, we should let
> the credit card be brought back to life if the user is updating the
> expiration date.  Otherwise, if the card is already expired, no other
> updates should be allowed.
>
> Si
>
> Jacopo Cappellato wrote:
>> Hi,
>>
>> there is a problem in the current implementation of the
>> "updateCreditCard" service: the service expires the old cc (by setting
>> the PaymentMethod.thuDate field) and creates a new one (CreditCard and
>> PaymentMethod) with the updated values.
>> The problem is that, if the cc has been already expired, calling the
>> "updateCreditCard" service causes the cc to be re-enabled.
>>
>> I'm not sure if this is good or not however this is a problem when the
>> "updateCreditCard" service is called as a consequence of the billing
>> address update; this can happen when:
>>
>> 1) you call the "updatePostalAddress" service; this service expires
>> the original address and creates a new one
>> 2) a seca calls the "updatePaymentMethodAddress" service
>> 3) the "updatePaymentMethodAddress" service calls the
>> "updateCreditCard" service for all the cc that have the old address
>> associated to them
>>
>> #3 will make all the expired ccs, associated with that address,
>> re-enabled automatically.
>>
>> The attached patch for the "updateCreditCard" service fixes the bug
>> but I'm not sure that this is the best way to do this... should we
>> modify the "updatePaymentMethodAddress" service instead (to update
>> only the active ccs)?
>>
>> Jacopo
>> ------------------------------------------------------------------------
>>
>> Index: applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java
>> ===================================================================
>> --- applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (revision 7275)
>> +++ applications/accounting/src/org/ofbiz/accounting/payment/PaymentMethodServices.java (working copy)
>> @@ -324,7 +324,10 @@
>>  
>>          newPm.set("partyId", partyId);
>>          newPm.set("fromDate", context.get("fromDate"), false);
>> -        newPm.set("thruDate", context.get("thruDate"));
>> +        // The following check is needed to avoid to reactivate an expired pm
>> +        if (newPm.get("thruDate") == null) {
>> +            newPm.set("thruDate", context.get("thruDate"));
>> +        }
>>          newCc.set("companyNameOnCard", context.get("companyNameOnCard"));
>>          newCc.set("titleOnCard", context.get("titleOnCard"));
>>          newCc.set("firstNameOnCard", context.get("firstNameOnCard"));
>>  
>> ------------------------------------------------------------------------
>>
>>  
>> _______________________________________________
>> Dev mailing list
>> [hidden email]
>> http://lists.ofbiz.org/mailman/listinfo/dev
>
> ------------------------------------------------------------------------
>
>  
> _______________________________________________
> Dev mailing list
> [hidden email]
> http://lists.ofbiz.org/mailman/listinfo/dev
 
_______________________________________________
Dev mailing list
[hidden email]
http://lists.ofbiz.org/mailman/listinfo/dev