+1 for sit and wait some time Jacques
> I put rev. 544442 in because it was a simple check for null on > paymentPrefs. > > While my knowledge of the billing account code is pretty limited, I think > that billing accounts have been pretty throughly implemented in OFBiz to the > point that the refactor is really just a big bugfix for all the problems in > the previous implementation. > > The problem with a big refactor is that the new code can introduce new bugs > which we don't want in a release branch, but I think in this case the > refactor will probably fix a lot more problems than it could potentially > cause. > > Perhaps for now the best approach is to sit and wait (possibly create a jira > for it) until the code has been used for a while. > > Thanks > Scott > > On 06/06/07, Jacopo Cappellato <[EMAIL PROTECTED]> wrote: > > > > Scott, > > > > that is part of the refactoring of the processes related to billing > > accounts, that I have completed in the weekend (but needs more tests). > > That was a refactoring and a bug fix in the same time (that resolved > > several bugs reported in Jira); in fact the previous implementation of > > the billing account process was incomplete. > > If we commit this we should commit all the other revisions as well (I > > can provide a list, of course), but before we do this it would be great > > to have some feedback from others about this subject. > > However, I noticed that in rev. 544442 you already committed part of > > that work: I didn't look at the details of that commit but we should > > consider to revert it if we don't put in the other stuff of the billing > > accounts I did recently. > > > > Thanks for your great work, > > > > Jacopo > > > > > > Scott Gray wrote: > > > Hi Jacopo > > > > > > Do you know if this commit needs to be applied to the release branch? I > > > can't tell if it's an improvement or a bug fix, but when I merged it I > > > noticed it also relies on another commit which changed the > > > CheckOutHelper.availableAccountBalance method signature. > > > > > > Any info would be appreciated. > > > > > > Thanks > > > Scott > > > > > > On 25/05/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > > >> > > >> Author: jacopoc > > >> Date: Thu May 24 09:26:42 2007 > > >> New Revision: 541349 > > >> > > >> URL: http://svn.apache.org/viewvc?view=rev&rev=541349 > > >> Log: > > >> Added code to always set the maxAmount in the OrderPaymentPreference. > > >> > > >> Modified: > > >> > > >> > > >> > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/Shopping Cart.java > > >> > > >> > > >> Modified: > > >> > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/Shopping Cart.java > > >> > > >> URL: > > >> > > http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?view=diff&rev=541349&r1=541348&r2=541349 > > >> > > >> > > >> > > ======================================================================== ====== > > >> > > >> --- > > >> > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/Shopping Cart.java > > >> > > >> (original) > > >> +++ > > >> > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/Shopping Cart.java > > >> > > >> Thu May 24 09:26:42 2007 > > >> @@ -3385,14 +3385,17 @@ > > >> } > > >> > > >> /** make a list of all OrderPaymentPreferences and Billing info > > >> including all payment methods and types */ > > >> - public List makeAllOrderPaymentInfos() { > > >> + public List makeAllOrderPaymentInfos(LocalDispatcher dispatcher) { > > >> List allOpPrefs = new LinkedList(); > > >> - Iterator i = paymentInfo.iterator(); > > >> - while (i.hasNext()) { > > >> - CartPaymentInfo inf = (CartPaymentInfo) i.next(); > > >> - > > >> allOpPrefs.addAll(inf.makeOrderPaymentInfos(this.getDelegator > > >> ())); > > >> - } > > >> + double remainingAmount = this.getGrandTotal() - > > >> this.getPaymentTotal(); > > >> if (getBillingAccountId() != null) { > > >> + double billingAccountAvailableAmount = > > >> billingAccountAvailableAmount = > > >> CheckOutHelper.availableAccountBalance(getBillingAccountId(), > > >> dispatcher); > > >> + if (remainingAmount < getBillingAccountAmount()) { > > >> + this.billingAccountAmt = remainingAmount; > > >> + } > > >> + if (billingAccountAvailableAmount < > > >> getBillingAccountAmount()) { > > >> + this.billingAccountAmt = > > billingAccountAvailableAmount; > > >> + } > > >> GenericValue opp = > > >> delegator.makeValue("OrderPaymentPreference", > > >> new HashMap()); > > >> opp.set("paymentMethodTypeId", "EXT_BILLACT"); > > >> opp.set("presentFlag", "N"); > > >> @@ -3400,6 +3403,19 @@ > > >> opp.set("maxAmount", new > > Double(getBillingAccountAmount())); > > >> opp.set("statusId", "PAYMENT_NOT_RECEIVED"); > > >> allOpPrefs.add(opp); > > >> + remainingAmount = remainingAmount - > > >> getBillingAccountAmount(); > > >> + if (remainingAmount < 0) { > > >> + remainingAmount = 0; > > >> + } > > >> + } > > >> + Iterator i = paymentInfo.iterator(); > > >> + while (i.hasNext()) { > > >> + CartPaymentInfo inf = (CartPaymentInfo) i.next(); > > >> + if (inf.amount == null) { > > >> + inf.amount = new Double(remainingAmount); > > >> + remainingAmount = 0; > > >> + } > > >> + > > >> allOpPrefs.addAll(inf.makeOrderPaymentInfos(this.getDelegator > > >> ())); > > >> } > > >> return allOpPrefs; > > >> } > > >> @@ -3636,7 +3652,7 @@ > > >> result.put("orderItemAttributes", > > >> this.makeAllOrderItemAttributes > > >> ()); > > >> result.put("orderContactMechs", > > >> this.makeAllOrderContactMechs()); > > >> result.put("orderItemContactMechs", > > >> this.makeAllOrderItemContactMechs()); > > >> - result.put("orderPaymentInfo", this.makeAllOrderPaymentInfos > > ()); > > >> + result.put("orderPaymentInfo", this.makeAllOrderPaymentInfos > > >> (dispatcher)); > > >> result.put("orderItemShipGroupInfo", > > this.makeAllShipGroupInfos > > >> ()); > > >> result.put("orderItemSurveyResponses", > > >> this.makeAllOrderItemSurveyResponses()); > > >> result.put("orderAdditionalPartyRoleMap", > > >> this.getAdditionalPartyRoleMap()); > > >> > > >> > > >> > > > > > > > > > >
