You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "Si Chen (JIRA)" <ji...@apache.org> on 2007/02/01 19:25:05 UTC

[jira] Created: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Possibly dangerous code in createPaymentApplication for billingAccount
----------------------------------------------------------------------

                 Key: OFBIZ-682
                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
             Project: OFBiz (The Open for Business Project)
          Issue Type: Bug
          Components: accounting
            Reporter: Si Chen


This code is back in createPaymentApplication and could be potentially dangerous:
            <if-not-empty field-name="invoice.billingAccountId">
                <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
            </if-not-empty> 

there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Si Chen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469588 ] 

Si Chen commented on OFBIZ-682:
-------------------------------

I think there's a good case to be made for leaving the flexibility of more than one billing account per order or invoice.  It would be nice at some point to allow multiple billing accounts or store credits on the same account.  I don't see a reason why it should be one and only one billing account.

Also, the problem I'm having is with one billing account and a credit card.

In the meantime, the highlighted code is *definitely* causing a bug, and I'd like to remove it until you have something else.

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Jacopo Cappellato (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato closed OFBIZ-682.
-----------------------------------

    Resolution: Fixed

this is no more an issue after rev. 543923

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>            Assignee: Jacopo Cappellato
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Jacopo Cappellato (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469589 ] 

Jacopo Cappellato commented on OFBIZ-682:
-----------------------------------------

Yes,

I definitely think that it makes sense to comment the code for now.

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Si Chen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469577 ] 

Si Chen commented on OFBIZ-682:
-------------------------------

I think the problem would arise here:

Let's say you have an order for $50, $20 to be paid by billing account, $30 by credit card.

When you create an invoice for $50, you wil have these records created:
- Invoice 10000 for $50
- Payment 10000, paymentMethodTypeId=BillingAccount, 
- PaymentApplication for paymentId 10000, billingAccountId 10000, invoiceId 10000, amount $20

This is all good, but when you create a Payment to capture to the credit card, you may end up with this:
- Payment 10001, paymentMethodTypeId=Credit Card
- PaymentApplication for paymentId 10001, invoiceId 10000, amount $30

This would be right, but if the PaymentApplication also had billingAccountId 10000, then that PaymentApplication would be counted as a use of the billing account.  Take a look at BillingAccountWorker methods for billing account balances and you'll see what I mean.

If the code I mentioned is called then this would happen and it would be a bug.  I just verified a use case and this is indeed happening and causing a bug.

I also think the pattern of billingAccountId on OrderHeader and Invoice is probably old and may be better to be replaced by a billingAccountId in OrderPaymentPreference.  It is not necessarily the case that an order and an ivnoice must be associated with one and only one billing account.  It would be nice if a customer can use multiple store credits on an order, for example.  We do not have to re-factor this right now, but we should not be too strictly held to it.

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Jacopo Cappellato (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacopo Cappellato reassigned OFBIZ-682:
---------------------------------------

    Assignee: Jacopo Cappellato

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>            Assignee: Jacopo Cappellato
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Jacopo Cappellato (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469557 ] 

Jacopo Cappellato commented on OFBIZ-682:
-----------------------------------------

Si,

I see what you mean, I still think that the code should stay there as is; however, since I think that the existing code related to billing accounts still needs some fixes to work properly (I manage to fix it for the customer I'm working on, and as soon as I'll have some free time I'll try to prepare a patch for them), if you want to comment it now, go for it.
The reason I still think that the code should stay as is, is this:

* if a billing account is selected during order entry, the entire order is associated to the billing account (that is why the billing account id is store in the order header), even if multiple payment methods are selected
* when an invoice for an order associated to the billing account is created, the entire invoice is associated to the billing account (that is why the billing account id is store in the invoice header)

I'm pretty sure that the existing data model and processes were created according to this design.
This means that all the payments for the order/invoice must be associated to the billing account (even the ones from the credit card).

In my local copy I have a modified version of the methods that computes the billing account balances accordingly to these design, and everything seems to work fine.

Does it make sense?


> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Si Chen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469795 ] 

Si Chen commented on OFBIZ-682:
-------------------------------

It's commented out as of SVN r 502651.

I'm actually very interested in seeing your code--I can't imagine how to make it work with this code in place myself...

> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-682) Possibly dangerous code in createPaymentApplication for billingAccount

Posted by "Jacopo Cappellato (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12469587 ] 

Jacopo Cappellato commented on OFBIZ-682:
-----------------------------------------

Si,

yes, I'm aware of this problem; in fact I have modify the code of the custom project I'm working on to correctly handle this.
I mean that the billing accounts balance routines were assuming that an order could be associated to more than one billing account but - in my opinion - (and I could be wrong) there are no big advantages of handling things in this way.
I'd prefer, as soon as a decision is taken to modify the existing design of how billingaccounts/orders/invoices interact, that we make things working for one billing account per order per invoice.
But, again, I could be wrong.
Jacopo


> Possibly dangerous code in createPaymentApplication for billingAccount
> ----------------------------------------------------------------------
>
>                 Key: OFBIZ-682
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-682
>             Project: OFBiz (The Open for Business Project)
>          Issue Type: Bug
>          Components: accounting
>            Reporter: Si Chen
>
> This code is back in createPaymentApplication and could be potentially dangerous:
>             <if-not-empty field-name="invoice.billingAccountId">
>                 <set field="paymentAppl.billingAccountId" from-field="invoice.billingAccountId"/>
>             </if-not-empty> 
> there could be problems with balance calculations when billing accounts and other methods are used to pay for orders.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.