Possible bug in CheckoutHelper.processPayment

[Bilgin Ibryam]

Hi all,

I want to ask is "EntityUtil.filterByAnd" a bug or done intentionally in the following code snippet:
CheckoutHelper.processPayment:1026
Actaully it is not a big deal, this code will only aprove the order, but this will happen only if we have "CASH", "EXT_COD", adnd "EXT_BILLACT" payment method types in the order. And i donot know haw can i add these 3 types at once.

I think "EntityUtil.filterByAnd" in 1031 should be chaged to filterByOr
WDYT ?


        } else {
            // Get the paymentMethodTypeIds - this will need to change when ecom supports multiple payments
            List cashCodBaExpr = UtilMisc.toList(new EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "CASH"),
                                           new EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "EXT_COD"),
                                           new EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "EXT_BILLACT"));
            List cashCodBaPaymentPreferences = EntityUtil.filterByAnd(allPaymentPreferences, cashCodBaExpr);

            if (UtilValidate.isNotEmpty(cashCodBaPaymentPreferences) &&
                    UtilValidate.isNotEmpty(allPaymentPreferences) &&
                    cashCodBaPaymentPreferences.size() == allPaymentPreferences.size()) {
                // approve this as long as there are only CASH, COD and Billing Account types
                boolean ok = OrderChangeHelper.approveOrder(dispatcher, userLogin, orderId, manualHold);
                if (!ok) {
                    throw new GeneralException("Problem with order change; see above error");
                }
            } else {
                // There is nothing to do, we just treat this as a success
            }
        }

[Scott Gray]

Sounds like a bug to me, is this a recent change because I'm used to COD etc
orders being approved straight away?

Regards
Scott

On 22/10/2007, Bilgin Ibryam <[hidden email]> wrote:

>
>
> Hi all,
>
> I want to ask is "EntityUtil.filterByAnd" a bug or done intentionally in
> the
> following code snippet:
> CheckoutHelper.processPayment:1026
> Actaully it is not a big deal, this code will only aprove the order, but
> this will happen only if we have "CASH", "EXT_COD", adnd "EXT_BILLACT"
> payment method types in the order. And i donot know haw can i add these 3
> types at once.
>
> I think "EntityUtil.filterByAnd" in 1031 should be chaged to filterByOr
> WDYT ?
>
>
>         } else {
>             // Get the paymentMethodTypeIds - this will need to change
> when
> ecom supports multiple payments
>             List cashCodBaExpr = UtilMisc.toList(new
> EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "CASH"),
>                                            new
> EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "EXT_COD"),
>                                            new
> EntityExpr("paymentMethodTypeId", EntityOperator.EQUALS, "EXT_BILLACT"));
>             List cashCodBaPaymentPreferences =
> EntityUtil.filterByAnd(allPaymentPreferences, cashCodBaExpr);
>
>             if (UtilValidate.isNotEmpty(cashCodBaPaymentPreferences) &&
>                     UtilValidate.isNotEmpty(allPaymentPreferences) &&
>                     cashCodBaPaymentPreferences.size() ==
> allPaymentPreferences.size()) {
>                 // approve this as long as there are only CASH, COD and
> Billing Account types
>                 boolean ok = OrderChangeHelper.approveOrder(dispatcher,
> userLogin, orderId, manualHold);
>                 if (!ok) {
>                     throw new GeneralException("Problem with order change;
> see above error");
>                 }
>             } else {
>                 // There is nothing to do, we just treat this as a success
>             }
>         }
> --
> View this message in context:
> http://www.nabble.com/Possible-bug-in-CheckoutHelper.processPayment-tf4669654.html#a13339382
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>
>

[Bilgin Ibryam]

In latest trunk, COD are not aproved automatically.
I checked the file history and in r538970 is done some refactoring of processpayment, including the mentioned lines. Here is the commit comment from Jacopo:
" Pretty important refactoring of the processOrderPayments service (and related methods): the service (triggered by ecas on order edit) doesn't load a cart from the order anymore; this is possible because there is now a (static) CheckOutHelper.processPayment method that doesn't rely on the cart object.
There is still huge room for improvements in this area, but this is a first step. "
 
Jacopo, may be you can take a look at tell me should i create a jira issue or you did it intentionally ?

[Jacopo Cappellato]

Hi Bilgin,

you are right and I've applied your fix to rev. 587073.
Thanks for catching this!

Jacopo


Bilgin Ibryam wrote:

> In latest trunk, COD are not aproved automatically.
> I checked the file history and in r538970 is done some refactoring of
> processpayment, including the mentioned lines. Here is the commit comment
> from Jacopo:
> " Pretty important refactoring of the processOrderPayments service (and
> related methods): the service (triggered by ecas on order edit) doesn't load
> a cart from the order anymore; this is possible because there is now a
> (static) CheckOutHelper.processPayment method that doesn't rely on the cart
> object.
> There is still huge room for improvements in this area, but this is a first
> step. "
>  
> Jacopo, may be you can take a look at tell me should i create a jira issue
> or you did it intentionally ?