Accounting transaction for commission invoice

[Bob Morley]

We are making us of the generation of commission invoices from sales order.  The invoice looks great, but when we try to make payment to the invoice and look at the resulting accounting transactions, they appear to be incorrect ...

When other invoices are created (sales / purchase) there is an SECA that gets involved with the invoice moves into "INVOICE_READY" state.  These are defined in secas_ledger.xml with service invocations for "createAcctgTransForPurchaseInvoice" and "createAcctgTransForSalesInvoice".  Each of these implementations only work on invoice types that are explicitly "PURCHASE_INVOICE" and "SALES_INVOICE" and they typically create the transaction that works with the A/R or A/P and specifics from the invoice (determined by configured mapping using invoice type).

Now, when a commission invoice is created and moved to ready it does not get included in the "createAcctgTransForPurchaseInvoice" service (even though its invoice type has purchase order as its parent type).  I have made a minor change in our solution to allow this service to execute against invoices that are either "PURCHASE_INVOICE" or "COMMISSION_INVOICE".  The logic in this service properly uses type mapping to determine the GL accounts to hit, so doing this resulted in the following:

AcctgTrans of type "PURCHASE_INVOICE" that is related to our commissionable sales rep (partyId) and the commision invoice (invoiceId) (although the roleType BILL_FROM_VENDOR) looks incorrect.

AcctgTransEntry - C A/P (210000) and D Sales Commissions (601300) and these look right.


The back end GL transactions when you actually pay this invoice have always looked correct (to me).  They are:

C Cash (112000) & D Accrued Commissions Due (221100)
C Accured Commissions Due (221100) & D A/P (210000)

So after a commission invoice is created and paid to their employee, you are left with ...

Cash goes down and Sales Commission expense goes up.  (as the A/P to the employee and accrued commission due both net to $0).  (This is assuming we paid the employee cash / cash equivalent that mapped to 112000).

Does anyone have any comments on this?  After doing a lot of reading and working on these things, it appears that this minor change produced the desired results.  In fact, I believe what we should be doing is having this initial accounting transaction check the invoice type hierarchy when determining if it should act on the invoice.  This would result in invoices of type commission, payroll, purchase, and customer return being handled as purchase invoices (money going out) and invoices of type sales, interest, purchase return being handled as sales invoices (money going in).  (this is based on the invoice type hierarchy from the InvoiceType entity seeding).

I am hoping someone has worked with the accounting transactions on invoices that were not the base sales/purchase types and can let me know if this seems right or if I have strayed off track.

[Sharan-F]

Hi Bob

I think you're right and I totally agree with everything you've said here. Although I'm not an accountant, I've written a lot of the OFBiz online accounting documentation.

I think another possible Purchase Invoice type to go along with the ones you've mentioned (eg payroll etc) will be for employee expenses.

Thanks
Sharan

[Jacopo Cappellato-4]

I would suggest to replace static "equals" condition for "PURCHASE_INVOICE" with the utility method:

if (CommonWorkers.hasParentType(delegator, "InvoiceType", "invoiceTypeId", invoiceTypeId, "parentTypeId", "PURCHASE_INVOICE")) {
    // ....
}

(where invoiceTypeId could contain "COMMISSION_INVOICE").
Of course it requires a proper definition of parent/child relationships for invoice types.
A similar approach is used for "ProductType" for marketing packages.

Jacopo

On Feb 22, 2010, at 5:59 PM, Bob Morley wrote:

>
> We are making us of the generation of commission invoices from sales order.
> The invoice looks great, but when we try to make payment to the invoice and
> look at the resulting accounting transactions, they appear to be incorrect
> ...
>
> When other invoices are created (sales / purchase) there is an SECA that
> gets involved with the invoice moves into "INVOICE_READY" state.  These are
> defined in secas_ledger.xml with service invocations for
> "createAcctgTransForPurchaseInvoice" and "createAcctgTransForSalesInvoice".
> Each of these implementations only work on invoice types that are explicitly
> "PURCHASE_INVOICE" and "SALES_INVOICE" and they typically create the
> transaction that works with the A/R or A/P and specifics from the invoice
> (determined by configured mapping using invoice type).
>
> Now, when a commission invoice is created and moved to ready it does not get
> included in the "createAcctgTransForPurchaseInvoice" service (even though
> its invoice type has purchase order as its parent type).  I have made a
> minor change in our solution to allow this service to execute against
> invoices that are either "PURCHASE_INVOICE" or "COMMISSION_INVOICE".  The
> logic in this service properly uses type mapping to determine the GL
> accounts to hit, so doing this resulted in the following:
>
> AcctgTrans of type "PURCHASE_INVOICE" that is related to our commissionable
> sales rep (partyId) and the commision invoice (invoiceId) (although the
> roleType BILL_FROM_VENDOR) looks incorrect.
>
> AcctgTransEntry - C A/P (210000) and D Sales Commissions (601300) and these
> look right.
>
>
> The back end GL transactions when you actually pay this invoice have always
> looked correct (to me).  They are:
>
> C Cash (112000) & D Accrued Commissions Due (221100)
> C Accured Commissions Due (221100) & D A/P (210000)
>
> So after a commission invoice is created and paid to their employee, you are
> left with ...
>
> Cash goes down and Sales Commission expense goes up.  (as the A/P to the
> employee and accrued commission due both net to $0).  (This is assuming we
> paid the employee cash / cash equivalent that mapped to 112000).
>
> Does anyone have any comments on this?  After doing a lot of reading and
> working on these things, it appears that this minor change produced the
> desired results.  In fact, I believe what we should be doing is having this
> initial accounting transaction check the invoice type hierarchy when
> determining if it should act on the invoice.  This would result in invoices
> of type commission, payroll, purchase, and customer return being handled as
> purchase invoices (money going out) and invoices of type sales, interest,
> purchase return being handled as sales invoices (money going in).  (this is
> based on the invoice type hierarchy from the InvoiceType entity seeding).
>
> I am hoping someone has worked with the accounting transactions on invoices
> that were not the base sales/purchase types and can let me know if this
> seems right or if I have strayed off track.
> --
> View this message in context: http://n4.nabble.com/Accounting-transaction-for-commission-invoice-tp1564778p1564778.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Bob Morley]

I like the idea of employee expenses, but we have not considered them to date for our solution.  Right now I think they would be entered by the bookkeeper directly as an acctgTrans with associated entries.  Not think I think having that type of invoice is a bad idea (as a more robust model of these expenses).

Jacopo --

I like the utility method that you provided.  However, the implementation of these services is in minilang so calling it is a little bit of a pain.  As a result, I have done the following (which is consistent with other recursive checks in accounting related objects like payment types and account types).

Updated UtilAccountiing.java with the following:

    public static boolean isPurchaseInvoice(GenericValue invoice) throws GenericEntityException {
        if (invoice == null) {
            return false;
        }
        return CommonWorkers.hasParentType(invoice.getDelegator(), "InvoiceType", "invoiceTypeId", invoice.getString("invoiceTypeId"), "parentTypeId", "PURCHASE_INVOICE");
    }

Then in my simple method I am doing the following ...

  <entity-one entity-name="Invoice" value-field="invoice"/>
    <set type="Boolean" field="isPurchaseInvoice" value="${bsh:org.ofbiz.accounting.util.UtilAccounting.isPurchaseInvoice(invoice)}" />
    <if-compare field="isPurchaseInvoice" operator="equals" value="true" type="Boolean">
      [logic ...]


I have not executed this; but if we are in agreement I will go ahead and test it out and then create a patch for trunk.

[Jacopo Cappellato-4]

On Feb 23, 2010, at 5:29 PM, Bob Morley wrote:

> Jacopo --
>
> I like the utility method that you provided.  However, the implementation of
> these services is in minilang so calling it is a little bit of a pain.  

Yes, there are some examples of its usage in Minilang, like:

 <set field="isMarketingPkgAuto" value="${groovy: org.ofbiz.common.CommonWorkers.hasParentType(delegator, 'ProductType', 'productTypeId', product.productTypeId, 'parentTypeId', 'MARKETING_PKG_AUTO')}" type="Boolean"/>

but I agree it is not very good; the other option is to use the <call-class-method element (that I like more).
But the utility method you have created is fine, especially if you have to use it a lot.
Or maybe its variant:

  public static boolean hasInvoiceParentType(GenericValue invoice, String parentTypeId)

Kind regards,

Jacopo

> As a
> result, I have done the following (which is consistent with other recursive
> checks in accounting related objects like payment types and account types).
>
> Updated UtilAccountiing.java with the following:
>
>    public static boolean isPurchaseInvoice(GenericValue invoice) throws
> GenericEntityException {
>        if (invoice == null) {
>            return false;
>        }
>        return CommonWorkers.hasParentType(invoice.getDelegator(),
> "InvoiceType", "invoiceTypeId", invoice.getString("invoiceTypeId"),
> "parentTypeId", "PURCHASE_INVOICE");
>    }
>
> Then in my simple method I am doing the following ...
>
>  <entity-one entity-name="Invoice" value-field="invoice"/>
>    <set type="Boolean" field="isPurchaseInvoice"
> value="${bsh:org.ofbiz.accounting.util.UtilAccounting.isPurchaseInvoice(invoice)}"
> />

Just a minor detail but I'd recommend using ${groovy: instead of ${bsh:



>    <if-compare field="isPurchaseInvoice" operator="equals" value="true"
> type="Boolean">
>      [logic ...]
>
>
> I have not executed this; but if we are in agreement I will go ahead and
> test it out and then create a patch for trunk.
> --

[Bob Morley]

Makes sense.  I have gone with the individual methods to be consistent with other methods in the UtilAccounting class -- they split out a bunch of them so that they can embed the "known" type under the covers.

As a side note, when I went to run this my JVM would actually die.  It turns out that the new method on CommonWorker does not have loop protection, so if the seeded values are incorrect it will infinite loop.  I was actually getting a "Invalid access of stack red zone" message to the console before the JVM would die.  I have changed CommonWorker as well to (internally) keep a Set of childTypeIds and if it encounters one it has already checked in its recursion, it short circuits and returns false.

We would personally fix the bad seeding in the InvoiceType entity (you can see PURCHASE_INVOICE actually has a parent of itself).  To do this we would use a notion we had come up with where you could specify an explicit null in the seed data (by default Ofbiz uses unspecified or empty values in the xml as "not specified" and ignores them).  There was a JIRA provided for this fix, but it had some push back and not sure if it got in or not.  Either way, to correct the seed data (on an existing install) you would have to somehow get the parentId set to null ...