Invoice Item type copied into GL Account type?

[Paul Foxworthy]

Hi all,

As you might guess if you've been looking at Jira and the dev list lately,
I've been working on purchases and sales generating GL transactions.

Please have a look at the createAcctgTransForPurchaseInvoice method in
applications/accounting/minilang/ledger/GeneralLedgerServices.xml

line 2024 in trunk does this:

                <set field="debitEntry.glAccountTypeId"
from-field="invoiceItem.invoiceItemTypeId"/>

Wat?

There are two different sets of values defined in two different places for
invoice item types and GL account types. Surely you can't just copy a value
from one to the other? Shouldn't there be a service right here to find a
decent GL account type based on the invoice item type?

Later on, the getGlAccountFromAccountType service looks at all sorts of
things to decide the final GL account to use. But the GL Account *type* is
an input to that.

Am I missing something, or is this a very nasty bug?

Thanks

Paul Foxworthy

--
Coherent Software Australia Pty Ltd
PO Box 2773
Cheltenham Vic 3192
Australia

Phone: [phone]
Web: http://www.coherentsoftware.com.au/
Email: [hidden email]

[taher]

Looks very wrong to me. Two different type sets should not be just copied
over. While we're at, it might be better to rewrite this in Java or Groovy

On Feb 10, 2017 11:05 AM, "Paul Foxworthy" <[hidden email]> wrote:

> Hi all,
>
> As you might guess if you've been looking at Jira and the dev list lately,
> I've been working on purchases and sales generating GL transactions.
>
> Please have a look at the createAcctgTransForPurchaseInvoice method in
> applications/accounting/minilang/ledger/GeneralLedgerServices.xml
>
> line 2024 in trunk does this:
>
>                 <set field="debitEntry.glAccountTypeId"
> from-field="invoiceItem.invoiceItemTypeId"/>
>
> Wat?
>
> There are two different sets of values defined in two different places for
> invoice item types and GL account types. Surely you can't just copy a value
> from one to the other? Shouldn't there be a service right here to find a
> decent GL account type based on the invoice item type?
>
> Later on, the getGlAccountFromAccountType service looks at all sorts of
> things to decide the final GL account to use. But the GL Account *type* is
> an input to that.
>
> Am I missing something, or is this a very nasty bug?
>
> Thanks
>
> Paul Foxworthy
>
> --
> Coherent Software Australia Pty Ltd
> PO Box 2773
> Cheltenham Vic 3192
> Australia
>
> Phone: [phone]
> Web: http://www.coherentsoftware.com.au/
> Email: [hidden email]
>

[Christian Geisert]

Paul,

I too stumbled over this some time ago, changed it to a glAccountTypeId
and was surprised that the glAccountId wasn't found anymore.
Yes, it's irritating and should be changed..

And I agree with Taher to convert this to Groovy (all Minilang ;)

Let's start with tests.

Christian

Am 10.02.2017 09:04, schrieb Paul Foxworthy:

> Hi all,
>
> As you might guess if you've been looking at Jira and the dev list lately,
> I've been working on purchases and sales generating GL transactions.
>
> Please have a look at the createAcctgTransForPurchaseInvoice method in
> applications/accounting/minilang/ledger/GeneralLedgerServices.xml
>
> line 2024 in trunk does this:
>
>                 <set field="debitEntry.glAccountTypeId"
> from-field="invoiceItem.invoiceItemTypeId"/>
>
> Wat?
>
> There are two different sets of values defined in two different places for
> invoice item types and GL account types. Surely you can't just copy a value
> from one to the other? Shouldn't there be a service right here to find a
> decent GL account type based on the invoice item type?
>
> Later on, the getGlAccountFromAccountType service looks at all sorts of
> things to decide the final GL account to use. But the GL Account *type* is
> an input to that.
>
> Am I missing something, or is this a very nasty bug?
>
> Thanks
>
> Paul Foxworthy
>

[Jacopo Cappellato-5]

On Fri, Feb 10, 2017 at 9:04 AM, Paul Foxworthy <[hidden email]> wrote:

> Hi all,
>
> As you might guess if you've been looking at Jira and the dev list lately,
> I've been working on purchases and sales generating GL transactions.
>
> Please have a look at the createAcctgTransForPurchaseInvoice method in
> applications/accounting/minilang/ledger/GeneralLedgerServices.xml
>
> line 2024 in trunk does this:
>
>                 <set field="debitEntry.glAccountTypeId"
> from-field="invoiceItem.invoiceItemTypeId"/>
>
> Wat?
>
> There are two different sets of values defined in two different places for
> invoice item types and GL account types. Surely you can't just copy a value
> from one to the other? Shouldn't there be a service right here to find a
> decent GL account type based on the invoice item type?
>
> Later on, the getGlAccountFromAccountType service looks at all sorts of
> things to decide the final GL account to use. But the GL Account *type* is
> an input to that.
>
> Am I missing something, or is this a very nasty bug?
>

It is not a bug, because the system use that value to decode the account id.
However it is a mess that we should improve: the whole GL posting
configuration mechanism need to be refactored, improved and simplified.
We should start by deciding what should be the real purpose of a few
fields, like the GL Account Type (that we could consider as the event that
is triggering the transaction).
In short, we need to identify the following:
* how to represent the events (invoice, payment, payment application,
inventory receive, inventory issuance, inventory adjustment etc...)
* how to map events and its subelements (Cr and Dr components) to actual GL
Accounts

Best regards,

Jacopo

[Paul Foxworthy]

On 10 February 2017 at 21:50, Jacopo Cappellato <
[hidden email]> wrote:


> It is not a bug, because the system use that value to decode the account
> id.
>

Thanks Jacopo.

I still think we have a serious problem.

I see in getGlAccountFromAccountType at line 533

http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml?view=markup#l533

<field-map field-name="invoiceItemTypeId"
from-field="parameters.glAccountTypeId"/>

So yes, at that point the method is expecting an InvoiceItemType, and you
could say the parameter is misnamed. There is some method to the madness
:-).

However, before that, at line 438

http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml?view=markup#l438

it has called getPartyGlAccountInline, which wants a GL account type.

And if the getGlAccountFromAccountType has not found specific GL accounts
for a party or product, towards the end at line 556 it
calls getGlAccountTypeDefaultInline. getGlAccountTypeDefaultInline again
expects a GL account type, not an invoice item type.

http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/minilang/ledger/GeneralLedgerServices.xml?view=markup#l566

I think we need separate parameters invoiceItemTypeId and varianceReasonId,
instead of attempting to overload the meaning of the glAccountTypeId
parameter.

What do you think?

Thanks

Paul Foxworthy

--
Coherent Software Australia Pty Ltd
PO Box 2773
Cheltenham Vic 3192
Australia

Phone: [phone]
Web: http://www.coherentsoftware.com.au/
Email: [hidden email]

[Jacopo Cappellato-5]

On Mon, Feb 13, 2017 at 5:37 AM, Paul Foxworthy <[hidden email]> wrote:

> [...]
> I think we need separate parameters invoiceItemTypeId and varianceReasonId,
> instead of attempting to overload the meaning of the glAccountTypeId
> parameter.
>
> What do you think?


One way to go to preserve the existing functionalities would be that of
creating one GlAccountType for each InvoiceItemType and VarianceReason
records. Then we would need to define a mechanism to map
InvoiceItemType/VarianceReason to GlAccountType.
This would make the design a bit more "correct" but still it would be very
complex, without adding much value.
We could do this but, as I mentioned earlier, my preference would be to
spend some time thinking/redefining the meaning of GlAccountTypes and (that
could be that of identifying events that are relevant for the GL) and then
clean up the system accordingly.

Jacopo

[Paul Foxworthy]

On 14 February 2017 at 01:00, Jacopo Cappellato <jacopo.cappellato@
hotwaxsystems.com> wrote:

>
> One way to go to preserve the existing functionalities would be that of
> creating one GlAccountType for each InvoiceItemType and VarianceReason
> records. Then we would need to define a mechanism to map
> InvoiceItemType/VarianceReason to GlAccountType.
> This would make the design a bit more "correct" but still it would be very
> complex, without adding much value.
>

Thanks again Jacopo.

Variance I think is fine as-is. You can use VarianceReasonGlAccount to get
directly to a GL account, no need to think about
GL account types. Note that in getGlAccountFromAccountType, variance is
handled early and simply. So I think the only problem
is that we are overloading the glAccountTypeId with the varianceReasonId,
and a separate parameter is all that's needed.

I'd like to suggest a fairly simple way to get the GL account type for GL
entries for an invoice .

In many cases, when getGlAccountFromAccountType is called, the caller
already knows the right GL account type, and
we have the type in the glAccountTypeId parameter.

For invoices, it's more complex. In InvoiceItemType, there is a default GL
account in the defaultGlAccountId attribute. In getGlAccountFromAccountType,
if we've received an invoice item but no GL account type, how about we
infer the GlAccountType from the default GL account? In other words,
go InvoiceItem->InvoiceItemType->default GL account->GL account type. Get
the GlAccountType associated with the default GL account and
use that as we look up PartyGlAccount, ProductGlAccount or
InvoiceItemTypeGlAccount. If there is nothing relevant in the ...GlAccount
entities,
you'd end up with that default GL account and therefore that GL account
type anyway.

A possible alternative would be to explicitly add an attribute to
InvoiceItem for a default GL account type, in addition to the current
specific
GL account. But surely the specific account would be of that type? In other
words, we can infer it anyway, without any need for a new attribute.

What do people think?

We could do this but, as I mentioned earlier, my preference would be to
> spend some time thinking/redefining the meaning of GlAccountTypes and (that
> could be that of identifying events that are relevant for the GL) and then
> clean up the system accordingly.
>

I have been writing up some notes on categorising GL accounts, which I will
post here soon. It seems very messy. At the moment I agree redefining or
clarifying
the meaning of GL account types is a good idea.

When you talk about "identifying events relevant for GL", I'm not quite
with you. Are you suggesting a more data driven approach rather than the
specific services
createAcctgTransFor... ? If so, yes that would be a good idea and more
extensible.

Cheers

Paul

--
Coherent Software Australia Pty Ltd
PO Box 2773
Cheltenham Vic 3192
Australia

Phone: [phone] <+61%203%209585%206788>
Web: http://www.coherentsoftware.com.au/
Email: [hidden email]

[Jacopo Cappellato-5]

Hi Paul,

thanks, this is really a valuable conversation; please see inline:

On Tue, Feb 14, 2017 at 1:36 AM, Paul Foxworthy <[hidden email]> wrote:

> [...]
> I have been writing up some notes on categorising GL accounts, which I will
> post here soon.


Great!


> It seems very messy. At the moment I agree redefining or
> clarifying the meaning of GL account types is a good idea.
>
> When you talk about "identifying events relevant for GL", I'm not quite
> with you. Are you suggesting a more data driven approach rather than the
> specific services
> createAcctgTransFor... ? If so, yes that would be a good idea and more
> extensible.
>

Each of the "macro events" would map to one of the existing specific
services createAcctgTransFor<EVENT>

However, for each we could define more granular events.
For example, for the "macro event" represented by the "creation of a
Purchase Invoice" (which is associated to the service
"createAcctgTransForPurchaseInvoice" we could define the following granular
events and each of them would be mapped to a specific account id:

PURCHASE_INVOICE_CREDIT    this will be mapped to an account id for
"Accounts Payable"
PURCHASE_INVOICE_DEBIT_PINVOICE_ADJ    this will be mapped the account id
for purchase invoice adjustment
PURCHASE_INVOICE_DEBIT_SHIP_CHARGES    this will be mapped the account id
for purchase invoice shipping charges
etc... (one for each invoice item type).

With a mapping like the one above, the service
createAcctgTransForPurchaseInvoice would simply do the lookup to resolve
the gl accounts for the posting.

I am aware I am (on purpose) ignoring several details, but this is just the
general idea.

Jacopo

[Paul Foxworthy]

Once again I find Kongrath has been ahead of us. See
https://issues.apache.org/jira/browse/OFBIZ-6722



-----
--
Coherent Software Australia Pty Ltd
http://www.coherentsoftware.com.au/

Bonsai ERP, the all-inclusive ERP system
http://www.bonsaierp.com.au/

--
Sent from: http://ofbiz.135035.n4.nabble.com/OFBiz-Dev-f165671.html