1. we have to distinguish between (a) completely new functionality or major refactorings and (b) the enhancement of functionality which is already in the code base
2. for (a), we should first have consenus that we want the proposed solution and we should look for a complete, well designed and carefully tested solution before the first commit will be done. This is to prevent the introduction of new problematic code.
3. for (b), I think every improvement of existing code/functionality helps and should be committed if there are no flaws in the design or technical solution and it does not break existing funtionality. of course, it should be complete within the *scope* of the improvement.
4. if the solution for (b) does not cover other wishes or things which could be enhanced also, this would be no reason to not commit the improvement. If people have further requirements, they can provide concepts and solutions/patches anytime to make things better.
In this case, for me it is important if Suraj's commit a. breaks anything? b. is vetoed by other committers in view of code quality or design flaws? If none of these questions get a "yes", then I see no reason to revert.If you have additional requirements, you are encouraged to provide solutions or concepts for them.
Thanks, Michael Brohl ecomify GmbH www.ecomify.de Am 19.08.18 um 09:25 schrieb Pierre Smits:
Please read the comment in the related ticket [1] https://issues.apache.org/jira/browse/OFBIZ-7482?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16584721#comment-16584721 Best regards, Pierre Smits Apache Trafodion <https://trafodion.apache.org>, Vice President Apache Directory <https://directory.apache.org>, PMC Member Apache Incubator <https://incubator.apache.org>, committer *Apache OFBiz <https://ofbiz.apache.org>, contributor (without privileges) since 2008* Apache Steve <https://steve.apache.org>, committer On Sat, Aug 18, 2018 at 9:01 PM, Michael Brohl <michael.br...@ecomify.de> wrote:How do these citations relate to Suraj‘s work? Please provide arguments in your own words if you think someone‘s work has flaws and should be reverted. Thanks, MichaelAm 18.08.2018 um 13:54 schrieb Pierre Smits <pierresm...@apache.org>: As Michael recently pointed out in another thread: {quote} *If it does break anything or introduces functionality which is notworkingcompletely, we should revert.* {quote} And: {quote} *We are struggling with half baked, incomplete or buggy code in several areas which often shows up a long time after it was committed. In other cases, even if problems are raised soon after the commit, the code isleftuntouched for a long time until it is work on again or simply isforgotten.*{quote} Best regards, Pierre Smits On Sat, Aug 18, 2018 at 10:52 AM, Suraj Khurana < suraj.khur...@hotwaxsystems.com> wrote:Hi Pierre, This is not a new patch, this is updated version of two years old patch which has been already reviewed if you follow comments on ticket. We need to add updated patch as well since many file path have beenchangedand we have data files refactoring as well. HTH. -- Best Regards, Suraj Khurana | Omni-channel OMS Technical Expert HotWax Commerce by HotWax Systems On Sat, Aug 18, 2018 at 2:13 PM, Pierre Smits <pierresm...@apache.org> wrote:Hi Suraj, Please revert! Within 10 minutes you posted a new patch and committedittotrunk and closed the issue. It is customary to follow the 72 hr delayruleto allow the community to review the changes and assess the impact. Best regards, Pierre Smits Apache Trafodion <https://trafodion.apache.org>, Vice President Apache Directory <https://directory.apache.org>, PMC Member Apache Incubator <https://incubator.apache.org>, committer Apache OFBiz <https://ofbiz.apache.org>, contributor since 2008 Apache Steve <https://steve.apache.org>, committerOn Sat, Aug 18, 2018 at 10:34 AM, <sur...@apache.org> wrote: Author: surajk Date: Sat Aug 18 08:34:18 2018 New Revision: 1838320 URL: http://svn.apache.org/viewvc?rev=1838320&view=rev Log: Improved: Added support to calculate deposit price as well whilecreatingshopping cart item. (OFBIZ-7482) Modified: ofbiz/ofbiz-framework/trunk/applications/datamodel/data/ seed/OrderSeedData.xml ofbiz/ofbiz-framework/trunk/applications/datamodel/data/ seed/ProductSeedData.xml ofbiz/ofbiz-framework/trunk/applications/order/src/main/ java/org/apache/ofbiz/order/shoppingcart/ShoppingCartItem.java Modified: ofbiz/ofbiz-framework/trunk/applications/datamodel/data/ seed/OrderSeedData.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/ applications/datamodel/data/seed/OrderSeedData.xml?rev= 1838320&r1=1838319&r2=1838320&view=diff ============================================================ ================== --- ofbiz/ofbiz-framework/trunk/applications/datamodel/data/seed/OrderSeedData.xml(original) +++ ofbiz/ofbiz-framework/trunk/applications/datamodel/data/seed/OrderSeedData.xmlSat Aug 18 08:34:18 2018 @@ -49,6 +49,7 @@ under the License. <OrderAdjustmentType description="Additional Feature" hasTable="N" orderAdjustmentTypeId="ADDITIONAL_FEATURE"/> <OrderAdjustmentType description="Warranty" hasTable="N" orderAdjustmentTypeId="WARRANTY_ADJUSTMENT"/> <OrderAdjustmentType description="Marketing Package Adjustment" hasTable="N" orderAdjustmentTypeId="MKTG_PKG_AUTO_ADJUST"/> + <OrderAdjustmentType description="Deposit" hasTable="N" orderAdjustmentTypeId="DEPOSIT_ADJUSTMENT"/> <OrderBlacklistType orderBlacklistTypeId="BLACKLIST_ADDRESS" description="Addresss"/> <OrderBlacklistType orderBlacklistTypeId="BLACKLIST_CREDITCARD" description="Credit Card"/> Modified: ofbiz/ofbiz-framework/trunk/applications/datamodel/data/ seed/ProductSeedData.xml URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/ applications/datamodel/data/seed/ProductSeedData.xml?rev= 1838320&r1=1838319&r2=1838320&view=diff ============================================================ ================== --- ofbiz/ofbiz-framework/trunk/applications/datamodel/data/seed/ProductSeedData.xml(original) +++ ofbiz/ofbiz-framework/trunk/applications/datamodel/data/seed/ProductSeedData.xmlSat Aug 18 08:34:18 2018 @@ -281,6 +281,7 @@ under the License. <ProductPriceType description="Minimum Order Price" productPriceTypeId="MINIMUM_ORDER_PRICE"/> <ProductPriceType description="Shipping Allowance Price" productPriceTypeId="SHIPPING_ALLOWANCE"/> + <ProductPricePurpose description="Deposit price" productPricePurposeId="DEPOSIT"/> <ProductPricePurpose description="Purchase/Initial" productPricePurposeId="PURCHASE"/> <ProductPricePurpose description="Recurring Charge" productPricePurposeId="RECURRING_CHARGE"/> <ProductPricePurpose description="Usage Charge" productPricePurposeId="USAGE_CHARGE"/> Modified: ofbiz/ofbiz-framework/trunk/applications/order/src/main/ java/org/apache/ofbiz/order/shoppingcart/ShoppingCartItem.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/ applications/order/src/main/java/org/apache/ofbiz/order/ shoppingcart/ShoppingCartItem.java?rev=1838320&r1=1838319& r2=1838320&view=diff ============================================================ ================== --- ofbiz/ofbiz-framework/trunk/applications/order/src/main/ java/org/apache/ofbiz/order/shoppingcart/ShoppingCartItem.java(original)+++ ofbiz/ofbiz-framework/trunk/applications/order/src/main/ java/org/apache/ofbiz/order/shoppingcart/ShoppingCartItem.java SatAug1808:34:18 2018 @@ -1038,6 +1038,7 @@ public class ShoppingCartItem implements ProductPromoWorker.doPromotions(cart, dispatcher); } + calcDepositAdjustments(); if (!"PURCHASE_ORDER".equals(cart.getOrderType())) { // store the auto-save cart if (triggerExternalOps && ProductStoreWorker.autoSaveCart(delegator,productStoreId)) { @@ -1061,6 +1062,33 @@ public class ShoppingCartItem implements } } + public void calcDepositAdjustments() { + List<GenericValue>itemAdjustments = this.getAdjustments(); + try { + GenericValue depositAmount = EntityQuery.use(delegator). from("ProductPrice").where("productId", this.getProductId(), "productPricePurposeId", "DEPOSIT", "productPriceTypeId", "DEFAULT_PRICE").filterByDate().queryFirst(); + if (UtilValidate.isNotEmpty(depositAmount)) { + Boolean updatedDepositAmount = false; + BigDecimal adjustmentAmount = depositAmount.getBigDecimal("price").multiply(this.getQuantity(), generalRounding); + // itemAdjustments is a reference so directly setting updated amount to the same. + for(GenericValue itemAdjustment :itemAdjustments) {+ if("DEPOSIT_ADJUSTMENT".equals(itemAdjustment. getString("orderAdjustmentTypeId"))) { + itemAdjustment.set("amount", adjustmentAmount); + updatedDepositAmount = true; + } + } + if (!updatedDepositAmount) { + GenericValue orderAdjustment =delegator.makeValue("OrderAdjustment"); + orderAdjustment.set("orderAdjustmentTypeId", "DEPOSIT_ADJUSTMENT"); + orderAdjustment.set("description", "Surcharge Adjustment"); + orderAdjustment.set("amount", adjustmentAmount); + this.addAdjustment(orderAdjustment); + } + } + } catch (GenericEntityException e){ + Debug.logError("Error in fetching deposite pricedetails!!",module); + } + } + public void updatePrice(LocalDispatcher dispatcher, ShoppingCart cart) throws CartItemModifyException { // set basePrice using the calculateProductPrice service if (_product != null && isModifiedPrice == false) {
smime.p7s
Description: S/MIME Cryptographic Signature
