Hi,
I have introduced a feature called Verify Pick in Facility component few months back based on a requirement. I was reviewing the code more closely and found some issues. The pre-condition to run this process is that order is in approved status. Steps to do Verify Pick are: 1) Create a Sales Order without split shipment option. Order should be in approved status. 2) Go to Facility and select the WebStoreWarehouse facility. 3) Lets first check whether the order is ready to pick or need stock move. To do this go to Picking tab. 4) If the order is ready to pick then click on Verify Pick tab. 5) Enter the order Id and click Verify Order. This will show the list of the items that need to be verified, by default all of the items are selected. 6) Click on Verify Items. 7) This will display the summary in another list at the bottom. User have the option to either cancel or complete the verify order process. 8) Click on Complete button to verify the order. If step 8 is a success then it will create a shipment in Picked status and as well as generate an invoice with In-Process status. The existing code was working fine but not implemented correctly as it end up creating just an Invoice and Invoice Items at max but there are certain other processes (entities where data is stored as well) when an invoice is created as implemented in createInvoiceForOrder service in InvoiceService class. There is also an eca on updateShipment service which look for shipment in PACKED status and then create the invoice. The service triggered on eca is createInvoicesFromShipment. This service then finally call createInvoiceForOrder and which create Invoice with In-Process status, Invoice Items as well few other things as well. Depending on the ProductStore.autoApproveInvoice flag, the status of invoice is changed to INVOICE_READY and finally payment is captured by capturePaymentsByInvoice triggered on eca rule defined on createInvoiceForOrder. Finally, the invoice goes to PAID status, Payments are captured and applied. Coming back to my question a little bit, in the Verify Order step 8, I do not want to capture payment and which is logical because Invoice status is In-Process status. My question is why payment is captured when the invoice status is In- Process? Shouldn't the system look for Invoice Ready status to capture the payments. If what I said above is true, I could use the existing artifacts and do some changes so that system validate against the invoice status before doing a capture. Any suggestions are most welcome and appreciated. Thanks, Vikas |
I did some changes for what I described earlier and I think this would
be the correct way to generate invoice for shipment. Changes are copied below for reference. Vikas Index: applications/product/src/org/ofbiz/shipment/verify/ VerifyPickSession.java =================================================================== --- applications/product/src/org/ofbiz/shipment/verify/ VerifyPickSession.java (revision 793812) +++ applications/product/src/org/ofbiz/shipment/verify/ VerifyPickSession.java (working copy) @@ -303,12 +303,10 @@ // check reserved quantity, it should be equal to verified quantity this.checkReservedQty(orderId, locale); String shipmentId = this.createShipment((this.getPickRows(orderId)).get(0)); + this.issueItemsToShipment(shipmentId, locale); - invoiceId = this.createInvoice(orderId); - for (VerifyPickSessionRow line: this.getPickRows(orderId)) { - invoiceItemSeqId = this.createInvoiceItem(line, invoiceId, shipmentId); - line.setInvoiceItemSeqId(invoiceItemSeqId); - } + Map context = UtilMisc.toMap("shipmentId", shipmentId, "userLogin", this.getUserLogin()); + this.getDispatcher().runSync("createInvoicesFromShipment", context); return shipmentId; } @@ -419,45 +417,4 @@ String shipmentId = (String) newShipResp.get("shipmentId"); return shipmentId; } - - protected String createInvoice(String orderId) throws GeneralException { - GenericDelegator delegator = this.getDelegator(); - Map createInvoiceContext = FastMap.newInstance(); - GenericValue orderHeader = delegator.findOne("OrderHeader", UtilMisc.toMap("orderId", orderId), false); - GenericValue billingAccount = orderHeader.getRelatedOne("BillingAccount"); - String billingAccountId = billingAccount != null ? billingAccount.getString("billingAccountId") : null; - createInvoiceContext.put("partyId", (EntityUtil.getFirst(delegator.findByAnd("OrderRole", UtilMisc.toMap("orderId", orderId, "roleTypeId", "BILL_TO_CUSTOMER")))).getString("partyId")); - createInvoiceContext.put("partyIdFrom", (EntityUtil.getFirst(delegator.findByAnd("OrderRole", UtilMisc.toMap("orderId", orderId, "roleTypeId", "BILL_FROM_VENDOR")))).getString("partyId")); - createInvoiceContext.put("billingAccountId", billingAccountId); - createInvoiceContext.put("invoiceTypeId", "SALES_INVOICE"); - createInvoiceContext.put("statusId", "INVOICE_IN_PROCESS"); - createInvoiceContext.put("currencyUomId", orderHeader.getString("currencyUom")); - createInvoiceContext.put("userLogin", this.getUserLogin()); - Map createInvoiceResult = this.getDispatcher().runSync("createInvoice", createInvoiceContext); - if (ServiceUtil.isError(createInvoiceResult)) { - throw new GeneralException(ServiceUtil.getErrorMessage(createInvoiceResult)); - } - String invoiceId = (String) createInvoiceResult.get("invoiceId"); - return invoiceId; - } - - protected String createInvoiceItem(VerifyPickSessionRow line, String invoiceId, String shipmentId) throws GeneralException { - Map createInvoiceItemContext = FastMap.newInstance(); - createInvoiceItemContext.put("invoiceId", invoiceId); - createInvoiceItemContext.put("orderId", line.getOrderId()); - createInvoiceItemContext.put("invoiceItemTypeId", "INV_FPROD_ITEM"); - createInvoiceItemContext.put("productId", line.getProductId()); - createInvoiceItemContext.put("quantity", line.getReadyToVerifyQty()); - createInvoiceItemContext.put("userLogin", this.getUserLogin()); - Map createInvoiceItemResult = this.getDispatcher().runSync("createInvoiceItem", createInvoiceItemContext); - if (ServiceUtil.isError(createInvoiceItemResult)) { - throw new GeneralException(ServiceUtil.getErrorMessage(createInvoiceItemResult)); - } - String invoiceItemSeqId = (String) createInvoiceItemResult.get("invoiceItemSeqId"); - GenericValue shipmentItemBilling = this.getDelegator().makeValue("ShipmentItemBilling", UtilMisc.toMap("invoiceId", invoiceId, "invoiceItemSeqId", invoiceItemSeqId)); - shipmentItemBilling.put("shipmentId", shipmentId); - shipmentItemBilling.put("shipmentItemSeqId", line.getShipmentItemSeqId()); - shipmentItemBilling.create(); - return invoiceItemSeqId; - } } \ No newline at end of file On Jul 15, 2009, at 4:05 PM, Vikas Mayur wrote: > Hi, > > I have introduced a feature called Verify Pick in Facility component > few months back based on a requirement. I was reviewing the code > more closely and found some issues. > > The pre-condition to run this process is that order is in approved > status. > > Steps to do Verify Pick are: > > 1) Create a Sales Order without split shipment option. Order should > be in approved status. > 2) Go to Facility and select the WebStoreWarehouse facility. > 3) Lets first check whether the order is ready to pick or need stock > move. To do this go to Picking tab. > 4) If the order is ready to pick then click on Verify Pick tab. > 5) Enter the order Id and click Verify Order. This will show the > list of the items that need to be verified, by default all of the > items are selected. > 6) Click on Verify Items. > 7) This will display the summary in another list at the bottom. User > have the option to either cancel or complete the verify order process. > 8) Click on Complete button to verify the order. > > If step 8 is a success then it will create a shipment in Picked > status and as well as generate an invoice with In-Process status. > The existing code was working fine but not implemented correctly as > it end up creating just an Invoice and Invoice Items at max but > there are certain other processes (entities where data is stored as > well) when an invoice is created as implemented in > createInvoiceForOrder service in InvoiceService class. > > There is also an eca on updateShipment service which look for > shipment in PACKED status and then create the invoice. The service > triggered on eca is createInvoicesFromShipment. This service then > finally call createInvoiceForOrder and which create Invoice with In- > Process status, Invoice Items as well few other things as well. > > Depending on the ProductStore.autoApproveInvoice flag, the status of > invoice is changed to INVOICE_READY and finally payment is captured > by capturePaymentsByInvoice triggered on eca rule defined on > createInvoiceForOrder. Finally, the invoice goes to PAID status, > Payments are captured and applied. > > Coming back to my question a little bit, in the Verify Order step 8, > I do not want to capture payment and which is logical because > Invoice status is In-Process status. > > My question is why payment is captured when the invoice status is In- > Process? Shouldn't the system look for Invoice Ready status to > capture the payments. > > If what I said above is true, I could use the existing artifacts and > do some changes so that system validate against the invoice status > before doing a capture. > > Any suggestions are most welcome and appreciated. > > Thanks, > Vikas > > > |
In reply to this post by Vikas Mayur-3
Hi Vikas,
On Jul 15, 2009, at 12:35 PM, Vikas Mayur wrote: > Hi, > > I have introduced a feature called Verify Pick in Facility component > few months back based on a requirement. I was reviewing the code > more closely and found some issues. > > The pre-condition to run this process is that order is in approved > status. > > Steps to do Verify Pick are: > > 1) Create a Sales Order without split shipment option. Order should > be in approved status. > 2) Go to Facility and select the WebStoreWarehouse facility. > 3) Lets first check whether the order is ready to pick or need stock > move. To do this go to Picking tab. > 4) If the order is ready to pick then click on Verify Pick tab. > 5) Enter the order Id and click Verify Order. This will show the > list of the items that need to be verified, by default all of the > items are selected. > 6) Click on Verify Items. > 7) This will display the summary in another list at the bottom. User > have the option to either cancel or complete the verify order process. > 8) Click on Complete button to verify the order. > > If step 8 is a success then it will create a shipment in Picked > status and as well as generate an invoice with In-Process status. > The existing code was working fine but not implemented correctly as > it end up creating just an Invoice and Invoice Items at max but > there are certain other processes (entities where data is stored as > well) when an invoice is created as implemented in > createInvoiceForOrder service in InvoiceService class. > > There is also an eca on updateShipment service which look for > shipment in PACKED status and then create the invoice. The service > triggered on eca is createInvoicesFromShipment. This service then > finally call createInvoiceForOrder and which create Invoice with In- > Process status, Invoice Items as well few other things as well. > > Depending on the ProductStore.autoApproveInvoice flag, the status of > invoice is changed to INVOICE_READY and finally payment is captured > by capturePaymentsByInvoice triggered on eca rule defined on > createInvoiceForOrder. Finally, the invoice goes to PAID status, > Payments are captured and applied. > > Coming back to my question a little bit, in the Verify Order step 8, > I do not want to capture payment and which is logical because > Invoice status is In-Process status. > > My question is why payment is captured when the invoice status is In- > Process? Shouldn't the system look for Invoice Ready status to > capture the payments. > A simple solution: we could add a flag in the product store like this: ProductStore.autoCapturePayments (default is Y). But then the problem is that we need an easy way to capture payments at a later moment (and not risk to forget them); we could do this based on the invoice status or on another shipment status or manually (but providing a good report/screen); what is your use case? when do you want to capture payments? Jacopo > If what I said above is true, I could use the existing artifacts and > do some changes so that system validate against the invoice status > before doing a capture. > > Any suggestions are most welcome and appreciated. > > Thanks, > Vikas > > > |
In reply to this post by Vikas Mayur-3
On Jul 15, 2009, at 4:35 AM, Vikas Mayur wrote: > Hi, > > I have introduced a feature called Verify Pick in Facility component > few months back based on a requirement. I was reviewing the code > more closely and found some issues. > > The pre-condition to run this process is that order is in approved > status. > > Steps to do Verify Pick are: > > 1) Create a Sales Order without split shipment option. Order should > be in approved status. > 2) Go to Facility and select the WebStoreWarehouse facility. > 3) Lets first check whether the order is ready to pick or need stock > move. To do this go to Picking tab. > 4) If the order is ready to pick then click on Verify Pick tab. > 5) Enter the order Id and click Verify Order. This will show the > list of the items that need to be verified, by default all of the > items are selected. > 6) Click on Verify Items. > 7) This will display the summary in another list at the bottom. User > have the option to either cancel or complete the verify order process. > 8) Click on Complete button to verify the order. > > If step 8 is a success then it will create a shipment in Picked > status and as well as generate an invoice with In-Process status. > The existing code was working fine but not implemented correctly as > it end up creating just an Invoice and Invoice Items at max but > there are certain other processes (entities where data is stored as > well) when an invoice is created as implemented in > createInvoiceForOrder service in InvoiceService class. > > There is also an eca on updateShipment service which look for > shipment in PACKED status and then create the invoice. The service > triggered on eca is createInvoicesFromShipment. This service then > finally call createInvoiceForOrder and which create Invoice with In- > Process status, Invoice Items as well few other things as well. > > Depending on the ProductStore.autoApproveInvoice flag, the status of > invoice is changed to INVOICE_READY and finally payment is captured > by capturePaymentsByInvoice triggered on eca rule defined on > createInvoiceForOrder. Finally, the invoice goes to PAID status, > Payments are captured and applied. > > Coming back to my question a little bit, in the Verify Order step 8, > I do not want to capture payment and which is logical because > Invoice status is In-Process status. > > My question is why payment is captured when the invoice status is In- > Process? Shouldn't the system look for Invoice Ready status to > capture the payments. > > If what I said above is true, I could use the existing artifacts and > do some changes so that system validate against the invoice status > before doing a capture. Yes, that sounds like the correct way to go. The payment capture trigger should not do anything if the invoice that triggered it is not in the Ready status (and perhaps just in that status, ie it shouldn't do anything in the In-Process status or the received status or anything. I haven't dug into the code to see if it is needed to add the option that Jacopo mentioned. My opinion right now is that the current behavior is wrong and should be corrected, ie the trigger shouldn't cause payment capture for an invoice that isn't Ready. -David |
In reply to this post by Jacopo Cappellato-4
On Jul 15, 2009, at 7:21 PM, Jacopo Cappellato wrote: >> Coming back to my question a little bit, in the Verify Order step >> 8, I do not want to capture payment and which is logical because >> Invoice status is In-Process status. >> >> My question is why payment is captured when the invoice status is >> In-Process? Shouldn't the system look for Invoice Ready status to >> capture the payments. >> > > A simple solution: we could add a flag in the product store like > this: ProductStore.autoCapturePayments (default is Y). > > But then the problem is that we need an easy way to capture payments > at a later moment (and not risk to forget them); we could do this > based on the invoice status or on another shipment status or > manually (but providing a good report/screen); what is your use > case? when do you want to capture payments? thanks for your comments, Jacopo! The use case is 1. Picker picks the items and collect them in a bin and pass this bin along with Pick Sheet to Pick Verifier. 2. Pick Verifier verify the order items in bin against the Pick Sheet and if all the items does match with items listed on Pick Sheet, the Pick Verifier completes the verification. 3. Once the verification is complete, a shipment is created in PICKED status. Items are also issued at this moment and an invoice is created with In-Process status. We are not doing payment capture at this moment. Payment capture only happens when packing is complete. Vikas |
In reply to this post by David E. Jones-2
On Jul 16, 2009, at 1:08 AM, David E Jones wrote: >> My question is why payment is captured when the invoice status is >> In-Process? Shouldn't the system look for Invoice Ready status to >> capture the payments. >> >> If what I said above is true, I could use the existing artifacts >> and do some changes so that system validate against the invoice >> status before doing a capture. > > Yes, that sounds like the correct way to go. The payment capture > trigger should not do anything if the invoice that triggered it is > not in the Ready status (and perhaps just in that status, ie it > shouldn't do anything in the In-Process status or the received > status or anything. > > I haven't dug into the code to see if it is needed to add the option > that Jacopo mentioned. My opinion right now is that the current > behavior is wrong and should be corrected, ie the trigger shouldn't > cause payment capture for an invoice that isn't Ready. thanks David for looking into it. I do have the same thought to do the correction to current behavior. I will start looking into it. Jacopo, Do you have any thoughts with the new field on ProductStore you mentioned earlier? |
On Jul 16, 2009, at 6:33 AM, Vikas Mayur wrote: > > On Jul 16, 2009, at 1:08 AM, David E Jones wrote: > >>> My question is why payment is captured when the invoice status is >>> In-Process? Shouldn't the system look for Invoice Ready status to >>> capture the payments. >>> >>> If what I said above is true, I could use the existing artifacts >>> and do some changes so that system validate against the invoice >>> status before doing a capture. >> >> Yes, that sounds like the correct way to go. The payment capture >> trigger should not do anything if the invoice that triggered it is >> not in the Ready status (and perhaps just in that status, ie it >> shouldn't do anything in the In-Process status or the received >> status or anything. >> >> I haven't dug into the code to see if it is needed to add the >> option that Jacopo mentioned. My opinion right now is that the >> current behavior is wrong and should be corrected, ie the trigger >> shouldn't cause payment capture for an invoice that isn't Ready. > > thanks David for looking into it. I do have the same thought to do > the correction to current behavior. I will start looking into it. > > Jacopo, Do you have any thoughts with the new field on ProductStore > you mentioned earlier? > If, as David and you are saying, we will attach the "capturePaymentsByInvoice" to the status change of the invoice (to Invoice ready), and detach it from the "createInvoiceForOrder" service, then we don't probably need the field in the store. In fact: a) if ProductStore.autoApproveInvoice = Y then the invoice will be automatically moved to the ready status, this will trigger the capturePaymentsByInvoice sservice and everything will work as now b) if ProductStore.autoApproveInvoice = N then the invoice will stay in the in-process status and payments will not be captured We may have to redesign the way ECA rules are defined; here is what I would suggest (it should work and be enough flexible): updateShipment --> PICKED: trigger createInvoiceForOrder (invoice in status IN PROCESS) ProductStore.autoApproveInvoice = Y: calls updateInvoice to status READY updateShipment --> PACKED: trigger updateInvoice to status READY (it is important to pass thru the status PICKED or make sure that invoices are created before the call to updateInvoice) updateInvoice --> READY: trigger capturePaymentsByInvoice Note: the above are just very generic notes and we will probably have to create small services for the above ECAs: for examplewhen a shipment is moved to PACKED then we will have to attach a service that find out all the invoices associated to the shipment and then for each of them it calls the updateInvoice service (in my notes I have simplified it saying that the event will call the updateInvoice Service) Jacopo |
On Jul 16, 2009, at 12:35 PM, Jacopo Cappellato wrote: > > On Jul 16, 2009, at 6:33 AM, Vikas Mayur wrote: > >> >> On Jul 16, 2009, at 1:08 AM, David E Jones wrote: >> >>>> My question is why payment is captured when the invoice status is >>>> In-Process? Shouldn't the system look for Invoice Ready status to >>>> capture the payments. >>>> >>>> If what I said above is true, I could use the existing artifacts >>>> and do some changes so that system validate against the invoice >>>> status before doing a capture. >>> >>> Yes, that sounds like the correct way to go. The payment capture >>> trigger should not do anything if the invoice that triggered it is >>> not in the Ready status (and perhaps just in that status, ie it >>> shouldn't do anything in the In-Process status or the received >>> status or anything. >>> >>> I haven't dug into the code to see if it is needed to add the >>> option that Jacopo mentioned. My opinion right now is that the >>> current behavior is wrong and should be corrected, ie the trigger >>> shouldn't cause payment capture for an invoice that isn't Ready. >> >> thanks David for looking into it. I do have the same thought to do >> the correction to current behavior. I will start looking into it. >> >> Jacopo, Do you have any thoughts with the new field on ProductStore >> you mentioned earlier? >> > > If, as David and you are saying, we will attach the > "capturePaymentsByInvoice" to the status change of the invoice (to > Invoice ready), and detach it from the "createInvoiceForOrder" > service, then we don't probably need the field in the store. > > In fact: > > a) if ProductStore.autoApproveInvoice = Y then the invoice will be > automatically moved to the ready status, this will trigger the > capturePaymentsByInvoice sservice and everything will work as now > b) if ProductStore.autoApproveInvoice = N then the invoice will stay > in the in-process status and payments will not be captured > > We may have to redesign the way ECA rules are defined; here is what > I would suggest (it should work and be enough flexible): > > updateShipment --> PICKED: trigger createInvoiceForOrder (invoice in > status IN PROCESS) > ProductStore.autoApproveInvoice = Y: calls updateInvoice to status > READY > updateShipment --> PACKED: trigger updateInvoice to status READY (it > is important to pass thru the status PICKED or make sure that > invoices are created before the call to updateInvoice) > updateInvoice --> READY: trigger capturePaymentsByInvoice > > Note: the above are just very generic notes and we will probably > have to create small services for the above ECAs: for examplewhen a > shipment is moved to PACKED then we will have to attach a service > that find out all the invoices associated to the shipment and then > for each of them it calls the updateInvoice service (in my notes I > have simplified it saying that the event will call the updateInvoice > Service) > Thanks Jacopo, Your notes helped a lot. I have attached a patch at https://issues.apache.org/jira/browse/OFBIZ-2740 I have tested the patch with various scenarios and it is working fine as desired. I will be doing some more testing and later on make the necessary changes to the Verify Pick process once this patch is in trunk. Vikas > Jacopo |
This is fixed in rev. 795809 and 795814.
Now do we want to have ProductStore.autoApproveInvoice == N as default for OFBiz E-Commerce Store? Vikas On Jul 20, 2009, at 2:49 PM, Vikas Mayur wrote: > > On Jul 16, 2009, at 12:35 PM, Jacopo Cappellato wrote: > >> >> On Jul 16, 2009, at 6:33 AM, Vikas Mayur wrote: >> >>> >>> On Jul 16, 2009, at 1:08 AM, David E Jones wrote: >>> >>>>> My question is why payment is captured when the invoice status >>>>> is In-Process? Shouldn't the system look for Invoice Ready >>>>> status to capture the payments. >>>>> >>>>> If what I said above is true, I could use the existing artifacts >>>>> and do some changes so that system validate against the invoice >>>>> status before doing a capture. >>>> >>>> Yes, that sounds like the correct way to go. The payment capture >>>> trigger should not do anything if the invoice that triggered it >>>> is not in the Ready status (and perhaps just in that status, ie >>>> it shouldn't do anything in the In-Process status or the received >>>> status or anything. >>>> >>>> I haven't dug into the code to see if it is needed to add the >>>> option that Jacopo mentioned. My opinion right now is that the >>>> current behavior is wrong and should be corrected, ie the trigger >>>> shouldn't cause payment capture for an invoice that isn't Ready. >>> >>> thanks David for looking into it. I do have the same thought to do >>> the correction to current behavior. I will start looking into it. >>> >>> Jacopo, Do you have any thoughts with the new field on >>> ProductStore you mentioned earlier? >>> >> >> If, as David and you are saying, we will attach the >> "capturePaymentsByInvoice" to the status change of the invoice (to >> Invoice ready), and detach it from the "createInvoiceForOrder" >> service, then we don't probably need the field in the store. >> >> In fact: >> >> a) if ProductStore.autoApproveInvoice = Y then the invoice will be >> automatically moved to the ready status, this will trigger the >> capturePaymentsByInvoice sservice and everything will work as now >> b) if ProductStore.autoApproveInvoice = N then the invoice will >> stay in the in-process status and payments will not be captured >> >> We may have to redesign the way ECA rules are defined; here is what >> I would suggest (it should work and be enough flexible): >> >> updateShipment --> PICKED: trigger createInvoiceForOrder (invoice >> in status IN PROCESS) >> ProductStore.autoApproveInvoice = Y: calls updateInvoice to status >> READY >> updateShipment --> PACKED: trigger updateInvoice to status READY >> (it is important to pass thru the status PICKED or make sure that >> invoices are created before the call to updateInvoice) >> updateInvoice --> READY: trigger capturePaymentsByInvoice >> >> Note: the above are just very generic notes and we will probably >> have to create small services for the above ECAs: for examplewhen a >> shipment is moved to PACKED then we will have to attach a service >> that find out all the invoices associated to the shipment and then >> for each of them it calls the updateInvoice service (in my notes I >> have simplified it saying that the event will call the >> updateInvoice Service) >> > > Thanks Jacopo, Your notes helped a lot. I have attached a patch at https://issues.apache.org/jira/browse/OFBIZ-2740 > > I have tested the patch with various scenarios and it is working > fine as desired. I will be doing some more testing and later on make > the necessary changes to the Verify Pick process once this patch is > in trunk. > > Vikas > >> Jacopo > |
Free forum by Nabble | Edit this page |