[
http://issues.apache.org/jira/browse/OFBIZ-289?page=comments#action_12435956 ]
Jacques Le Roux commented on OFBIZ-289:
---------------------------------------
A very interesting comment from Iain Fogg in the dev ML
__________________________________________________________________
I've been investigating the reasons behind order and item percentage
discounts not working in POS...
I've focused on order-level (Sale Disc) to start, and it seem pretty
clear why it doesn't work. The POS application adds the "percent" order
adjustment to the cart, and when the grand total is calculated
"calcOrderAdjustmentBd(GenericValue, BigDecimal)" (in
OrderReadHelper.java) quite happily ignores the adjustment because the
orderAdjustment.get("amount") returns null - quite right too since our
adjustment is a "percent". IWO, calcOrderAdjustmentBd only takes notice
of "amount" adjustments.
Compare this with the handling of something like an order level discount
applied via Order Entry. For example, if you apply a 10% promotional
discount to the order, ProductPromoWorker.performAction has code for
handling the percentage discount.
The question is, can we mod OrderReadHelper.calcOrderAdjustmentBd with
impunity to include handling of "percent" discounts?
First up, we need to change the order adjustment type in POS to
"sourcePercentage" or we just fail with an exception (others have
spotted this in the past). In my testing, I added the following block of
code to calcOrderAdjustmentBd:
else if (orderAdjustment.get("sourcePercentage") != null) {
// round amount to best precision (taxCalcScale) because db
value of 0.825 is pulled as 0.8249999...
BigDecimal percent =
orderAdjustment.getBigDecimal("sourcePercentage").setScale(taxCalcScale,
taxRounding);
BigDecimal amount =
orderSubTotal.multiply(percent).setScale(taxCalcScale, taxRounding);
adjustment = adjustment.add(amount);
}
Run POS, create an order, and then apply a percent discount to the order
and POS updates to reflect the adjustment. Looks good so far, but....
Is this mod likely to break other apps that might have
"sourcePercentage" adjustments?
The accounting needs to be tweaked. I checked out the transactions
entered into the GL, and it records a SALES_ACCTG_TRANS for the
non-discounted amount and a PAYMENT_ACCTG_TRANS for the discounted
amount. (If you do an "amount" discount in POS, this is correctly
recorded in the GL).
This needs someone with a deeper understanding of the core code (inc.
Accounting) to fix properly.
Cheers, Iain
> Percentage item and sale discounts not working
> ----------------------------------------------
>
> Key: OFBIZ-289
> URL: http://issues.apache.org/jira/browse/OFBIZ-289
> Project: OFBiz (The Open for Business Project)
> Issue Type: Bug
> Components: pos
> Affects Versions: SVN trunk
> Environment: N/A
> Reporter: Marco Risaliti
> Assigned To: Jacques Le Roux
>
> Copy of http://jira.undersunconsulting.com/browse/OFBIZ-670 from Ray Barlow
> ===========================================================
> In the pos application it is throwing exceptions and failing to apply
> percentage discounts either at the sale level:
> 3526968 (Thread-39) [ PosButton.java:136:ERROR]
> ---- exception report
> ----------------------------------------------------------
> Button invocation exception - menuSaleDisc
> Exception: org.ofbiz.pos.config.ButtonEventConfig$ButtonEventException
> Message: null
> ---- stack trace
> ---------------------------------------------------------------
> org.ofbiz.pos.config.ButtonEventConfig$ButtonEventException
> org.ofbiz.pos.config.ButtonEventConfig.invoke(ButtonEventConfig.java:228)
> org.ofbiz.pos.config.ButtonEventConfig.invokeButtonEvent(ButtonEventConfig.java:117)
>
> org.ofbiz.pos.component.PosButton$1.construct(PosButton.java:132)
> net.xoetrope.xui.helper.SwingWorker$2.run(Unknown Source)
> java.lang.Thread.run(Thread.java:534)
> java.lang.reflect.InvocationTargetException
> sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>
> java.lang.reflect.Method.invoke(Method.java:324)
> org.ofbiz.pos.config.ButtonEventConfig.invoke(ButtonEventConfig.java:220)
> org.ofbiz.pos.config.ButtonEventConfig.invokeButtonEvent(ButtonEventConfig.java:117)
>
> org.ofbiz.pos.component.PosButton$1.construct(PosButton.java:132)
> net.xoetrope.xui.helper.SwingWorker$2.run(Unknown Source)
> java.lang.Thread.run(Thread.java:534)
> Caused by: java.lang.IllegalArgumentException: [GenericEntity.set]
> "percentage" is not a field of OrderAdjustment, must be one of:
> orderAdjustmentId, orderAdjustmentTypeId, orderId, orderItemSeqId,
> shipGroupSeqId, comments, description, amount, productPromoId,
> productPromoRuleId, productPromoActionSeqId, productFeatureId,
> correspondingProductId, sourceReferenceId, sourcePercentage,
> customerReferenceId, primaryGeoId, secondaryGeoId, exemptAmount,
> taxAuthGeoId, taxAuthPartyId, overrideGlAccountId, includeInTax,
> includeInShipping, createdDate, createdByUserLogin, oldAmountPerQuantity,
> oldPercentage, lastUpdatedStamp, lastUpdatedTxStamp, createdStamp,
> createdTxStamp
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:365)
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:345)
> org.ofbiz.pos.PosTransaction.addDiscount(PosTransaction.java:400)
> org.ofbiz.pos.event.MenuEvents.saleDiscount(MenuEvents.java:274)
> ... 9 more
> or the item level:
> 3561402 (Thread-41) [ PosButton.java:136:ERROR]
> ---- exception report
> ----------------------------------------------------------
> Button invocation exception - menuItemDisc
> Exception: org.ofbiz.pos.config.ButtonEventConfig$ButtonEventException
> Message: null
> ---- stack trace
> ---------------------------------------------------------------
> org.ofbiz.pos.config.ButtonEventConfig$ButtonEventException
> org.ofbiz.pos.config.ButtonEventConfig.invoke(ButtonEventConfig.java:228)
> org.ofbiz.pos.config.ButtonEventConfig.invokeButtonEvent(ButtonEventConfig.java:117)
>
> org.ofbiz.pos.component.PosButton$1.construct(PosButton.java:132)
> net.xoetrope.xui.helper.SwingWorker$2.run(Unknown Source)
> java.lang.Thread.run(Thread.java:534)
> java.lang.reflect.InvocationTargetException
> sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>
> java.lang.reflect.Method.invoke(Method.java:324)
> org.ofbiz.pos.config.ButtonEventConfig.invoke(ButtonEventConfig.java:220)
> org.ofbiz.pos.config.ButtonEventConfig.invokeButtonEvent(ButtonEventConfig.java:117)
>
> org.ofbiz.pos.component.PosButton$1.construct(PosButton.java:132)
> net.xoetrope.xui.helper.SwingWorker$2.run(Unknown Source)
> java.lang.Thread.run(Thread.java:534)
> Caused by: java.lang.IllegalArgumentException: [GenericEntity.set]
> "percentage" is not a field of OrderAdjustment, must be one of:
> orderAdjustmentId, orderAdjustmentTypeId, orderId, orderItemSeqId,
> shipGroupSeqId, comments, description, amount, productPromoId,
> productPromoRuleId, productPromoActionSeqId, productFeatureId,
> correspondingProductId, sourceReferenceId, sourcePercentage,
> customerReferenceId, primaryGeoId, secondaryGeoId, exemptAmount,
> taxAuthGeoId, taxAuthPartyId, overrideGlAccountId, includeInTax,
> includeInShipping, createdDate, createdByUserLogin, oldAmountPerQuantity,
> oldPercentage, lastUpdatedStamp, lastUpdatedTxStamp, createdStamp,
> createdTxStamp
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:365)
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:345)
> org.ofbiz.pos.PosTransaction.addDiscount(PosTransaction.java:400)
> org.ofbiz.pos.event.MenuEvents.itemDiscount(MenuEvents.java:313)
> ... 9 more
>
>
> All Comments Work Log Change History Sort Order:
> Comment by Jacques Le Roux [19/Jan/06 02:27 PM] [ Permlink ]
> I get the same problem. I'm sorry to have no time to look at it yet.
> Jacques
> Comment by Si Chen [19/Jan/06 02:49 PM] [ Permlink ]
> Hey there - Just a wild guess here, but how about you update
> src/org/ofbiz/pos/PosTransaction.java at line 400 and change
> .set("percentage" ) to .set("sourcePercentage" ...)
> Would that work for you?
> Sorry - I'm just guessing from looking at your error message:
> Caused by: java.lang.IllegalArgumentException: [GenericEntity.set]
> "percentage" is not a field of OrderAdjustment, must be one of:
> orderAdjustmentId, orderAdjustmentTypeId, orderId, orderItemSeqId,
> shipGroupSeqId, comments, description, amount, productPromoId,
> productPromoRuleId, productPromoActionSeqId, productFeatureId,
> correspondingProductId, sourceReferenceId, sourcePercentage,
> customerReferenceId, primaryGeoId, secondaryGeoId, exemptAmount,
> taxAuthGeoId, taxAuthPartyId, overrideGlAccountId, includeInTax,
> includeInShipping, createdDate, createdByUserLogin, oldAmountPerQuantity,
> oldPercentage, lastUpdatedStamp, lastUpdatedTxStamp, createdStamp,
> createdTxStamp
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:365)
> org.ofbiz.entity.GenericEntity.set(GenericEntity.java:345)
> org.ofbiz.pos.PosTransaction.addDiscount(PosTransaction.java:400)
> This actually tells you what the error is (percentage not being a field of
> OrderAdjustment) and where it happens:
> org.ofbiz.pos.PosTransaction.addDiscount(PosTransaction.java:400)
> Unfortunately I don't know about your particular use case or how the POS
> model works to know if that's the answer, but want to try it?
> Comment by Jacques Le Roux [19/Jan/06 03:44 PM] [ Permlink ]
> Good eyes Si.
> Thanks it's work
> Jacques
> Comment by Si Chen [19/Jan/06 04:08 PM] [ Permlink ]
> Jacques,
> Can you confirm that the orders created are good? If so, do you want to send
> me a patch file, or is it exactly the change I described? I'll put it in the
> SVN.
> Si
> Comment by Jacques Le Roux [19/Jan/06 04:13 PM] [ Permlink ]
> It works, but after I had a problem with the default tax autorithies values
> which looks like this :
> 77797 (Thread-43) [TaxAuthorityServices.java:283:WARN ] In TaxAuthority
> Product Rate no records were found for condition:(productStoreId = '9100' AND
> ((taxAut
> hPartyId = '_NA_' AND taxAuthGeoId = '_NA_') OR (taxAuthPartyId = 'USA_IRS'
> AND taxAuthGeoId = 'USA') OR (taxAuthPartyId = 'NY_DTF' AND taxAuthGeoId =
> 'NY'))
> AND (productCategoryId IS NULL OR productCategoryId IN ('TSTCSL',
> 'CATALOG1_SEARCH', '101', '100')) AND (minItemPrice IS NULL OR minItemPrice
> <= '25.989999999
> 999998436805981327779591083526611328125') AND (minPurchase IS NULL OR
> minPurchase <= '25.989999999999998436805981327779591083526611328125'))
> Is it possible to set somenthing by default to _NA_ which is used in POS I
> guess ?
> Comment by Jacques Le Roux [19/Jan/06 04:16 PM] [ Permlink ]
> Ho, sorry it's too late and I'm to tired and in a hurry. I confused 2
> problems (TVA and %) so no it's not working but the error message is not
> there anymore.
> Going to bed :(
> Jacques
> Comment by Jacques Le Roux [19/Jan/06 04:18 PM] [ Permlink ]
> Also it seems that something have to be done in POS code regarding double and
> BigDecimal
> 1091594 (Thread-102) [ GenericEntity.java:393:ERROR] In entity field set the
> value passed in [java.math.BigDecimal] is not compatible with the Java type
> of the field [Double]
> I make a new issu for it
> Jacques
> Comment by Jacques Le Roux [19/Jan/06 04:21 PM] [ Permlink ]
> Something to be said also about the message disappearing : I have set the log
> to only warn and error messages. Si perhaps there is something more, too
> tired...
> Jacques
> Comment by Jacques Le Roux [20/Jan/06 02:36 AM] [ Permlink ]
> It seems that the modifications Si suggested
> //adjustment.set("percentage", new Double(discount));
> adjustment.set("sourcePercentage", new Double(discount));
> is not sufficient. We get
> 172860 (Thread-39) [ Log4jLoggerWriter.java:74 :INFO ] [POS @ pos-1 TX:10090]
> - transaction created
> 173860 (Thread-40) [ Log4jLoggerWriter.java:74 :INFO ] [POS @ pos-1 TX:10090]
> - add item(GZ-1001/1.0)
> 174188 (Thread-40) [ UtilXml.java:242:DEBUG] XML Read 0.235s:
> file:/D:/workspace/ofbiz/applications/product/script/org/ofbiz/product/store/ProductS
>
> toreServices.xml
> 174360 (Thread-40) [ ServiceDispatcher.java:447:DEBUG] [[Sync service
> finished- total:0.453,since last(Begin):0.453]] - 'POSDispatcher /
> isStoreInventoryAvai
> lableOrNotRequired'
> 174703 (Thread-40) [ ServiceDispatcher.java:447:DEBUG] [[Sync service
> finished- total:0.343,since last(Begin):0.343]] - 'POSDispatcher /
> calculateProductPric
> e'
> 175250 (Thread-40) [ GenericEntity.java:393:ERROR] In entity field set the
> value passed in [java.math.BigDecimal] is not compatible with the Java type
> of
> the field [Double]
> 175250 (Thread-40) [ GenericEntity.java:393:ERROR] In entity field set the
> value passed in [java.math.BigDecimal] is not compatible with the Java type
> of
> the field [Double]
> 175250 (Thread-40) [TaxAuthorityServices.java:364:INFO ] NOTE: A tax
> calculation was done without a billToPartyId or taxAuthGeoId, so no tax
> exemptions or tax
> IDs considered; billToPartyId=[null] taxAuthGeoId=[_NA_]
> 175266 (Thread-40) [ ServiceDispatcher.java:447:DEBUG] [[Sync service
> finished- total:0.281,since last(Begin):0.281]] - 'POSDispatcher / calcTax'
> 175266 (Thread-40) [ CheckOutHelper.java:675:INFO ] Added item adjustments to
> ship group [0 / 0] - [[GenericEntity:OrderAdjustment][amount,5.095(java.math
> .BigDecimal)][comments,TVA
> 19,6%(java.lang.String)][orderAdjustmentTypeId,SALES_TAX(java.lang.String)][overrideGlAccountId,224000(java.lang.String)][primaryGe
>
> oId,_NA_(java.lang.String)][sourcePercentage,19.60000000000000142108547152020037174224853515625(java.math.BigDecimal)][taxAuthGeoId,_NA_(java.lang.String)][ta
>
> xAuthPartyId,_NA_(java.lang.String)]]
> 181547 (Thread-43) [ Log4jLoggerWriter.java:74 :INFO ] [POS @ pos-1 TX:10090]
> - add item adjustment
> 181563 (Thread-43) [ GenericEntity.java:393:ERROR] In entity field set the
> value passed in [java.math.BigDecimal] is not compatible with the Java type
> of
> the field [Double]
> 181563 (Thread-43) [ GenericEntity.java:393:ERROR] In entity field set the
> value passed in [java.math.BigDecimal] is not compatible with the Java type
> of
> the field [Double]
> 181578 (Thread-43) [TaxAuthorityServices.java:364:INFO ] NOTE: A tax
> calculation was done without a billToPartyId or taxAuthGeoId, so no tax
> exemptions or tax
> IDs considered; billToPartyId=[null] taxAuthGeoId=[_NA_]
> 181578 (Thread-43) [ ServiceDispatcher.java:447:DEBUG] [[Sync service
> finished- total:0.031,since last(Begin):0.031]] - 'POSDispatcher / calcTax'
> 181578 (Thread-43) [ CheckOutHelper.java:675:INFO ] Added item adjustments to
> ship group [0 / 0] - [[GenericEntity:OrderAdjustment][amount,5.095(java.math
> .BigDecimal)][comments,TVA
> 19,6%(java.lang.String)][orderAdjustmentTypeId,SALES_TAX(java.lang.String)][overrideGlAccountId,224000(java.lang.String)][primaryGe
>
> oId,_NA_(java.lang.String)][sourcePercentage,19.60000000000000142108547152020037174224853515625(java.math.BigDecimal)][taxAuthGeoId,_NA_(java.lang.String)][ta
>
> xAuthPartyId,_NA_(java.lang.String)]]
> I tried to modify addDiscount method in PosTransaction.java using BigDecimal
> public void addDiscount(String productId, double discount, boolean
> percent) {
> GenericValue adjustment =
> session.getDelegator().makeValue("OrderAdjustment", null);
> adjustment.set("orderAdjustmentTypeId", "DISCOUNT_ADJUSTMENT");
> if (percent) {
> //adjustment.set("percentage", new Double(discount));
> adjustment.set("sourcePercentage", new Double(discount));
> } else {
> adjustment.set("amount", new Double(discount));
> //adjustment.set("amount", new BigDecimal(discount));
> }
> But it's not working. I think something has to be done in field amount of
> OrderAdjustment Entity but I'm not sure.
> Is it related to calcAndAddTax in CheckOutHelper.java ?
> // pass in BigDecimal values instead of Double
> List taxReturn = this.getTaxAdjustments(dispatcher, "calcTax",
> serviceContext);
> HTH
> Jacques
> Comment by Ray Barlow [20/Jan/06 09:46 AM] [ Permlink ]
> I don't see anything about a BigDecimal error message on the test system I
> just tried but I will try it on a cleaner version shortly as my DB may need a
> fresh install.
> As regards your suggestion Si it has stopped the visible error message in the
> logs but it still won't apply the % discount to the item/order. I will double
> check on clean system though.
> Thanks.
> Comment by Si Chen [20/Jan/06 06:03 PM] [ Permlink ]
> Ray/Jacques -
> Ok, so much for lucky guesses. Unfortunately I'm not familiar enough with
> what you're talking about to know how to fix it right off, but here are some
> questions that could help:
> 1. Is % discount just for POS or is it in the catalog manager/ecommerce as
> well?
> 2. If it's just for POS, then is it trying to create an OrderAdjustment in
> the PosTransaction.java? In which case, how is it different than what
> ShoppingCartHelper or OrderServices is doing? By comparing the two, you
> should be able to tell what is needed.
> 3. If it's in catalog manager/ecommerce, then are those broken as well? If
> so, open an issue for them and we'll take a peek.
> Comment by Tony Harris [12/Feb/06 10:50 AM] [ Permlink ]
> For information I am looking at this issue over the coming week.
> Comment by Jacques Le Roux [11/Mar/06 02:26 AM] [ Permlink ]
> I will work on this soon (beginning of april at least)
> Tony had you time to work on it ?
> Jacques
> Comment by Marco Risaliti [12/Sep/06 08:35 AM] [ Permlink ]
> Sorry Jacques,
> can you look at this issue and see if it can be closed or move to the new
> jira server.
> Thanks in advance
> Marco
> Comment by Jacques Le Roux [12/Sep/06 08:51 AM] [ Permlink ]
> Yes Marco,
> I was just willing to do it. But have some tasks to complete before. Will see
> that ASAP.
> Comment by Marco Risaliti [14/Sep/06 04:37 AM] [ Permlink ]
> Hi Jacques,
> If you want I can move it for you ?
> Thanks
> Marco
> Comment by Jacques Le Roux [14/Sep/06 09:48 AM] [ Permlink ]
> I wanted to review this before moving to Apache, but actually I got no time
> for that.
> So yes thanks, Marco. I will review it later...
> Comment by Marco Risaliti [14/Sep/06 02:02 PM] [ Permlink ]
> May attention Jacques I'm starting to move it to avoid duplication of issue.
> Thanks
> Marco
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
