[ 
https://issues.apache.org/jira/browse/OFBIZ-5761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14226104#comment-14226104
 ] 

Jacques Le Roux edited comment on OFBIZ-5761 at 11/26/14 12:08 PM:
-------------------------------------------------------------------

Hi Nicolas,

I reviewed (by patches comparison, boring but sure) and tested your last patch. 
It's quite good. I like how you cleaned the code and I like the UI also.

I found only one issue where deleteOrderItemShipGroupAssoc needs a userLogin.
{code}
-+                    Map<String, String> localCtx = UtilMisc.toMap("orderId", 
orderItem.getString("orderId"),
++                    Map<String, Object> localCtx = 
UtilMisc.toMap("userLogin", userLogin,
++                            "orderId", orderItem.getString("orderId"),

I found some other trivial points, you can see them in the attached "OFBIZ-5761 
- OISG Management.patch.diff" file. I notably removed the duplicate "create new 
ship group" buttons. I kept the original one, but it now uses the new 
AddOrderItemShipGroup service.
{code}
I attach also the last "OFBIZ-5761 - OISG Management.patch". Before I commit, I 
will add a Junit test and will change the delegator.* calls to EntityQuery.* 
calls.

That should be it :)


was (Author: jacques.le.roux):
Hi Nicolas,

I reviewed (by patches comparison, boring but sure) and tested your last patch. 
It's quite good. I like how you cleaned the code and I like the UI also.

I found only one issue where deleteOrderItemShipGroupAssoc needs a userLogin.
{code}
-+                    Map<String, String> localCtx = UtilMisc.toMap("orderId", 
orderItem.getString("orderId"),
++                    Map<String, Object> localCtx = 
UtilMisc.toMap("userLogin", userLogin,
++                            "orderId", orderItem.getString("orderId"),

I found some other trivial points, you can see them in the attached "OFBIZ-5761 
- OISG Management.patch.diff" file. I notably removed the duplicate "create new 
ship group" buttons. I kept the original one, but it now uses the new 
AddOrderItemShipGroup service.

I attach also the last "OFBIZ-5761 - OISG Management.patch". Before I commit, I 
will add a Junit test and will change the delegator.* calls to EntityQuery.* 
calls.

That should be it :)

> Allow to edit ship groups contents after and order has been created
> -------------------------------------------------------------------
>
>                 Key: OFBIZ-5761
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-5761
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: order
>    Affects Versions: Trunk
>            Reporter: Jacques Le Roux
>            Assignee: Jacques Le Roux
>             Fix For: Upcoming Branch
>
>         Attachments: OFBIZ-5761 - OISG Management.patch, OFBIZ-5761 - OISG 
> Management.patch, OFBIZ-5761 - OISG Management.patch, OFBIZ-5761 - OISG 
> Management.patch, OFBIZ-5761 - OISG Management.patch, OFBIZ-5761 - OISG 
> Management.patch, OFBIZ-5761 - OISG Management.patch.change, OFBIZ-5761 - 
> OISG Management.patch.diff, OFBIZ-5761.patch.diff
>
>
> Currently you can only move order items between ship groups while you create 
> an order. I needed to do it after order creation. When I met Olivier (Heintz) 
> at the RMLL 2014 in July, I found the Neogia team has developed a such 
> feature and had it as an addon (named oisg-management) for R12.04. Then 
> exchanging with Nicolas (Malin), and Pierre (Gaudin) I decided to give it a 
> go. I will quickly explain the following history, for the Neogia team to know 
> the current situation and what has changed. 
> After updating the code to work with current trunk (instead of R12.04) I 
> found it was working well but some minor issues. I then exchanged with Leila 
> (Mekika) from the Neogia team and we could quickly fix the minor issues:
> * text harcoded, no labels. I began to fix them, thanks to Leila who 
> completed the major part and explained me some tricks about the 
> oisg-management addon.
> * A redundant button associated with the new addOrderItemShipGroup service.  
> I removed it because the current button calls createOrderItemShipGroup which 
> is enough. We could BTW consider using addOrderItemShipGroup instead. It's 
> more complete (see below for instance) but that"s rather a matter of taste.
> There was a mechanism to merge sales taxes to get them grouped by ship groups 
> in order adjustments. I removed it because this can be done dynamically (see 
> invoice.pdf) and it was removing the shipGroupSeqId from the order 
> adjustments.
> I sorted (DESC) the OrderItemShipGroup in addOrderItemShipGroup in order to 
> use the 1st ship group when copying shipmentMethodTypeId, carrierPartyId, 
> carrierRoleTypeId, contactMechId when shipmentMethodTypeId and carrierPartyId 
> are not passed to the service.
> I later fixed a bug I found in loadCartForUpdate service when removing the 
> adjustments. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to