[jira] Created: (OFBIZ-2790) Payment totals in Accounting

[Nicolas Malin (Jira)]

Payment totals in Accounting
----------------------------

                 Key: OFBIZ-2790
                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
             Project: OFBiz
          Issue Type: Improvement
          Components: accounting
    Affects Versions: SVN trunk
            Reporter: Ahmed Dini
             Fix For: SVN trunk
         Attachments: paymentTotals.patch

Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.

Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.

NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ahmed Dini updated OFBIZ-2790:
------------------------------

    Attachment: paymentTotals.patch

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Anil K Patel reassigned OFBIZ-2790:
-----------------------------------

    Assignee: Anil K Patel

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12746849#action_12746849 ]

Sumit Pandit commented on OFBIZ-2790:
-------------------------------------

Hi Ahmad,

Thanks for your initiation and contribution, your idea is good and It should work. Though I am applying your patch after a long time, therefore there are some conflicts. To resolve this conflicts I would like you to upload the updated patch so that I can perform its functional testing.

BTW, here are some quick comments on your work -

1) FindPaymentTotals.groovy ->
a) Please have a look at other groovy files in OFBiz and follow its convention,
b) Avoid to use so many while spaces, unused lines in your code.
c) Write code in 4 spaces indentation of a block - for ex -
instead of -
+if(context.listIt != null) {
+totalAmount = 0.0;

use
+if(context.listIt != null) {
+    totalAmount = 0.0;
d) In calculation of Amount (money calculation) use BigDecimal

2)
a) Though "${groupName}${lastName},${firstName}" is used at some place in OFBiz - but I would recommend to use "${groupName}${lastName} ${firstName}" .
b) In implementation of a new code you have changed the formatting style of old code, Please keep the formatting of existing code untouched and change only that code which is required for your implementation. It helps us to find the difference in the code.

Please do above changes and upload the new patch. Your efforts are much appreciable.

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ahmed Dini updated OFBIZ-2790:
------------------------------

    Attachment: paymentTotals.patch

Hello Sumit,

Thanks for your comments and recommendations.  As recommended I have made the changes requested.  Sorry about my mistakes. In my future developments, I will make sure to check everything with the best practice conventions.
Attached is mended patch. Thanks again I appreciate your comments.
NB if there is more improvements needed please let me know, more than happy to change it again.

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch, paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ahmed Dini updated OFBIZ-2790:
------------------------------

    Attachment:     (was: paymentTotals.patch)

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ahmed Dini updated OFBIZ-2790:
------------------------------

    Attachment: paymentTotals.patch

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch, paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747256#action_12747256 ]

Sumit Pandit commented on OFBIZ-2790:
-------------------------------------

Please find more comments -
1) For performance reasons do not use the EntityListIterator.hasNext() method, just call next() until it returns null.
2) In your case - lastRowId != null never happen.
3) I have a doubt in following assignment -
listValue.paymentId ="lastRowId";
I think you want to code like - listValue.paymentId = lastRowId;
4) Please have a look at console - there are some errors are showing. They occurred in calculation of Outstanding amount. Please correct it.
5) No indentation require after - totalAmount = totalAmount + amount;
6) Remove last three lines from file  - /FindPaymentTotals.groovy
7) Use BigDecimal's method for operations.
8) Please have a look at your logic in groovy file, I have a doubt on it.
9) Pleas verify your values in iteration.

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch, paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748143#action_12748143 ]

Ahmed Dini commented on OFBIZ-2790:
-----------------------------------

Hello Sumit,

Thanks for your feedback.   I have made the changes you pointed out.  

You said you had a doubt about the logic of the file. What I was trying to do is to add a row to the  ListPayments form and display the total amount of the returned payments in that row.  I could not figure out a possible way of adding a row to the form because the rows of the form are being populated from a 'listIt'  (EntityListIterator).  The only way I could think of adding this row to the form is to manually add a new element to the 'listIt'.   But the 'listIt' (EntityListIterator)'s 'add' method was designed not to allow manual addition of new elements (this means it only gets the result from the database) and if you try to add it manually it throws this error message "CursorListIterator currently only supports read-only access".

So what I did was I copied the content of 'listIt' into java.util.List object and added the total row manually  and then put the content of the new list object in the context with the name of 'listIt'.  Below is the

1) For performance reasons do not use the EntityListIterator.hasNext() method, just call next() until it returns null.
  -   Done --
    while ((listValue = listIt.next()) != null)

2) In your case - lastRowId != null never happen.
     - You were right, I removed it from the ListPayments form.

3) I have a doubt in following assignment -
      listValue.paymentId ="lastRowId";
      I think you want to code like - listValue.paymentId = lastRowId;
    -  This is still there, I couldn't think of any other way of making the paymnet Id unique other than to manually hard code a string value which could be evaluated in the Forms file.

4) Please have a look at console - there are some errors are showing. They occurred in calculation of Outstanding amount. Please correct it.
   - Done, I added bsh:if(!lastRowId.equals(paymentId)) to stop the system for searching a payment Id with value of 'lastRowId".

5) No indentation require after - totalAmount = totalAmount + amount;
    - Done,

6) Remove last three lines from file - /FindPaymentTotals.groovy
     - Done

7) Use BigDecimal's method for operations.
    - Done, for totalAmount.

8) Please have a look at your logic in groovy file, I have a doubt on it.
    - See the explanation above please.

9) Pleas verify your values in iteration.
    - I didn't get this.

Everything is working fine for me, except one problem which I think I have introduced after playing with EntityListIterator.  I keep getting the following error message, everytime I click on the Search button to search for new payments.

2009-08-26 21:12:35,139 (Finalizer) [ EntityListIterator.java:521:ERROR] ---- runtime exception report -------------------------------------------------- Error closing the SQLProcessor in finalize EntityListIterator Exception: java.lang.NullPointerException Message: null ---- stack trace --------------------------------------------------------------- java.lang.NullPointerException org.apache.commons.dbcp.managed.ManagedConnection.close(ManagedConnection.java:147) org.ofbiz.entity.jdbc.SQLProcessor.close(SQLProcessor.java:234) org.ofbiz.entity.util.EntityListIterator.close(EntityListIterator.java:140) org.ofbiz.entity.util.EntityListIterator.finalize(EntityListIterator.java:517) java.lang.ref.Finalizer.invokeFinalizeMethod(Native Method) java.lang.ref.Finalizer.runFinalizer(Unknown Source) java.lang.ref.Finalizer.access$100(Unknown Source) java.lang.ref.Finalizer$FinalizerThread.run(Unknown Source) -------------------------------------------------------------------------------
 -

Please find the attached patch.

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch, paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2790?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ahmed Dini updated OFBIZ-2790:
------------------------------

    Attachment: paymentTotals.patch

> Payment totals in Accounting
> ----------------------------
>
>                 Key: OFBIZ-2790
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2790
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: SVN trunk
>            Reporter: Ahmed Dini
>            Assignee: Anil K Patel
>             Fix For: SVN trunk
>
>         Attachments: paymentTotals.patch, paymentTotals.patch, paymentTotals.patch
>
>
> Under Accounting Manager --> Payments, Find Payments.  I thought it would be a handy improvement if I display a total of the Amount fields of all payments under the search result screen no matter what their status is. This new improvement might not makes sense when payment status are mixed i.e. The total of cancelled payments plus the total of received payments, but I thinks it might make sense if you want to filter the query. i.e if you want the total of all payments in NOT-PAID status by a particular party or customer.
> Attached is patch which introduces a new groovy file namely  FindPaymentTotals.groovy and the modification of the existing PaymentForms.xml.
> NB. Since this is my first major improvement for Ofbiz project, I might have done things differently than most of the commiters or contributors expect. Please let me know if I break Ofbiz coding conventions, I am willing to improve the code. Otherwise the attached patch works fine and has passed all my tests

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