Fixed issue 39617: Code review improvements
authorVíctor Martínez Romanos <victor.martinez@openbravo.com>
Mon, 12 Nov 2018 19:38:38 +0100
changeset 34967 af52c7d01edc
parent 34966 37ad45032da1
child 34968 c24eed29cc3d
Fixed issue 39617: Code review improvements

Fixed db consistency.
Avoid useless usage of flush() and commitAndClose(). Refreshing needed objects instead.
TestUtils class declared at package level.
Refactor InvoiceGeneratorFromGoodsShipment class to improve code cleanliness.
- Pass Process Invoice as parameter
- Declare enum with the list of supported Invoice Terms
- Avoid to store hibernate objects as class variables
- Properly manage memory consumption by evicting useless objects
- Simplify exception management
- Validate order line is returned by C_Invoice_Candidate_V
- Take into account only completed invoices
- Properly calculate pending qty to invoice
- Reduce complexity in several methods
- Get the invoice document type using the standard AD_GET_DOCTYPE
HTML/XML/Java related to ProcessGoods:
- It now follows Openbravo standards (parse dates and checkbox, remove useless catch)
- Added default display logic hidden to invoice related fields
src-db/database/sourcedata/AD_ELEMENT.xml
src-db/database/sourcedata/OBUIAPP_PARAMETER.xml
src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/InvoiceFromShipmentTest.java
src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/TestUtils.java
src/org/openbravo/common/actionhandler/InvoiceFromShipmentActionHandler.java
src/org/openbravo/erpCommon/ad_actionButton/DocAction.html
src/org/openbravo/erpCommon/ad_actionButton/ProcessGoods.java
src/org/openbravo/materialmgmt/InvoiceGeneratorFromGoodsShipment.java
--- a/src-db/database/sourcedata/AD_ELEMENT.xml	Thu Oct 25 17:59:27 2018 -0400
+++ b/src-db/database/sourcedata/AD_ELEMENT.xml	Mon Nov 12 19:38:38 2018 +0100
@@ -26152,6 +26152,18 @@
 <!--5B04EA553995A5A8E040007F01007F3D-->  <ISGLOSSARY><![CDATA[N]]></ISGLOSSARY>
 <!--5B04EA553995A5A8E040007F01007F3D--></AD_ELEMENT>
 
+<!--5B12F3FCB67D46568CED2DE9BCA8080E--><AD_ELEMENT>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <AD_ELEMENT_ID><![CDATA[5B12F3FCB67D46568CED2DE9BCA8080E]]></AD_ELEMENT_ID>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <AD_ORG_ID><![CDATA[0]]></AD_ORG_ID>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <COLUMNNAME><![CDATA[processInvoice]]></COLUMNNAME>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <NAME><![CDATA[Process Invoice]]></NAME>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <PRINTNAME><![CDATA[Process Invoice]]></PRINTNAME>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E-->  <ISGLOSSARY><![CDATA[N]]></ISGLOSSARY>
+<!--5B12F3FCB67D46568CED2DE9BCA8080E--></AD_ELEMENT>
+
 <!--5BA951974876BDB2E040007F01011671--><AD_ELEMENT>
 <!--5BA951974876BDB2E040007F01011671-->  <AD_ELEMENT_ID><![CDATA[5BA951974876BDB2E040007F01011671]]></AD_ELEMENT_ID>
 <!--5BA951974876BDB2E040007F01011671-->  <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID>
--- a/src-db/database/sourcedata/OBUIAPP_PARAMETER.xml	Thu Oct 25 17:59:27 2018 -0400
+++ b/src-db/database/sourcedata/OBUIAPP_PARAMETER.xml	Mon Nov 12 19:38:38 2018 +0100
@@ -891,6 +891,9 @@
 <!--5738F8786CA04941A68AD66EF04927CF-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <NAME><![CDATA[Price List]]></NAME>
+<!--5738F8786CA04941A68AD66EF04927CF-->  <DESCRIPTION><![CDATA[A catalog of selected items with prices defined generally or for a specific partner.]]></DESCRIPTION>
+<!--5738F8786CA04941A68AD66EF04927CF-->  <HELP><![CDATA[
+Price Lists are used to determine the pricing, margin and cost of items purchased or sold.]]></HELP>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <SEQNO><![CDATA[20]]></SEQNO>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <AD_REFERENCE_ID><![CDATA[95E2A8B50A254B2AAE6774B8C2F28120]]></AD_REFERENCE_ID>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <AD_REFERENCE_VALUE_ID><![CDATA[133F26E7C401427997D41E132940A78B]]></AD_REFERENCE_VALUE_ID>
@@ -899,6 +902,7 @@
 <!--5738F8786CA04941A68AD66EF04927CF-->  <FIELDLENGTH><![CDATA[0]]></FIELDLENGTH>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <ISMANDATORY><![CDATA[Y]]></ISMANDATORY>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <DEFAULTVALUE><![CDATA[OB.getFilterExpression("org.openbravo.materialmgmt.InvoiceFromGoodsShipmentDefaultValueFilterExpression")]]></DEFAULTVALUE>
+<!--5738F8786CA04941A68AD66EF04927CF-->  <AD_ELEMENT_ID><![CDATA[449]]></AD_ELEMENT_ID>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <ISFIXED><![CDATA[N]]></ISFIXED>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <EVALUATEFIXEDVALUE><![CDATA[N]]></EVALUATEFIXEDVALUE>
 <!--5738F8786CA04941A68AD66EF04927CF-->  <OBUIAPP_PROCESS_ID><![CDATA[62250E8866EA4D96A66C309878DC039E]]></OBUIAPP_PROCESS_ID>
@@ -2317,6 +2321,7 @@
 <!--EF0F86614DD8439DA948B956E21B2778-->  <FIELDLENGTH><![CDATA[0]]></FIELDLENGTH>
 <!--EF0F86614DD8439DA948B956E21B2778-->  <ISMANDATORY><![CDATA[Y]]></ISMANDATORY>
 <!--EF0F86614DD8439DA948B956E21B2778-->  <DEFAULTVALUE><![CDATA['Y']]></DEFAULTVALUE>
+<!--EF0F86614DD8439DA948B956E21B2778-->  <AD_ELEMENT_ID><![CDATA[5B12F3FCB67D46568CED2DE9BCA8080E]]></AD_ELEMENT_ID>
 <!--EF0F86614DD8439DA948B956E21B2778-->  <ISFIXED><![CDATA[N]]></ISFIXED>
 <!--EF0F86614DD8439DA948B956E21B2778-->  <EVALUATEFIXEDVALUE><![CDATA[N]]></EVALUATEFIXEDVALUE>
 <!--EF0F86614DD8439DA948B956E21B2778-->  <OBUIAPP_PROCESS_ID><![CDATA[62250E8866EA4D96A66C309878DC039E]]></OBUIAPP_PROCESS_ID>
@@ -2383,13 +2388,16 @@
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <NAME><![CDATA[Invoice Date]]></NAME>
+<!--FA8CA39EA0374C31A426878F80A1E851-->  <DESCRIPTION><![CDATA[The time listed on the invoice.]]></DESCRIPTION>
+<!--FA8CA39EA0374C31A426878F80A1E851-->  <HELP><![CDATA[The Date Invoice indicates the date printed on the invoice.]]></HELP>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <SEQNO><![CDATA[10]]></SEQNO>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <AD_REFERENCE_ID><![CDATA[15]]></AD_REFERENCE_ID>
-<!--FA8CA39EA0374C31A426878F80A1E851-->  <COLUMNNAME><![CDATA[MovementDate]]></COLUMNNAME>
+<!--FA8CA39EA0374C31A426878F80A1E851-->  <COLUMNNAME><![CDATA[DateInvoiced]]></COLUMNNAME>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <ISCENTRALLYMAINTAINED><![CDATA[Y]]></ISCENTRALLYMAINTAINED>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <FIELDLENGTH><![CDATA[0]]></FIELDLENGTH>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <ISMANDATORY><![CDATA[Y]]></ISMANDATORY>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <DEFAULTVALUE><![CDATA[@MovementDate@]]></DEFAULTVALUE>
+<!--FA8CA39EA0374C31A426878F80A1E851-->  <AD_ELEMENT_ID><![CDATA[267]]></AD_ELEMENT_ID>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <ISFIXED><![CDATA[N]]></ISFIXED>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <EVALUATEFIXEDVALUE><![CDATA[N]]></EVALUATEFIXEDVALUE>
 <!--FA8CA39EA0374C31A426878F80A1E851-->  <OBUIAPP_PROCESS_ID><![CDATA[62250E8866EA4D96A66C309878DC039E]]></OBUIAPP_PROCESS_ID>
--- a/src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/InvoiceFromShipmentTest.java	Thu Oct 25 17:59:27 2018 -0400
+++ b/src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/InvoiceFromShipmentTest.java	Mon Nov 12 19:38:38 2018 +0100
@@ -21,11 +21,13 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 import java.math.BigDecimal;
 import java.util.Calendar;
 import java.util.Date;
 
+import org.apache.commons.lang.time.DateUtils;
 import org.codehaus.jettison.json.JSONArray;
 import org.codehaus.jettison.json.JSONException;
 import org.codehaus.jettison.json.JSONObject;
@@ -117,7 +119,6 @@
           "InvoiceFromShipment_001", AFTER_DELIVERY);
       final OrderLine orderLine = getOrderLineByLineNo(salesOrder, 10L);
       setProductInOrderLine(orderLine, product);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut shipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -125,13 +126,13 @@
       final ShipmentInOutLine shipmentLine = getShipmentLineByLineNo(shipment, 10L);
       setProductInShipmentLine(shipmentLine, product);
       setOrderLineInShipmentLine(shipmentLine, orderLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertGeneratedInvoiceLine(invoice, product, new BigDecimal(12));
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          salesOrder.getPriceList(), "CO");
 
     } catch (Exception e) {
       log.error(e.getMessage(), e);
@@ -174,7 +175,6 @@
       setProductInOrderLine(firstLine, productOne);
       final OrderLine secondLine = getOrderLineByLineNo(salesOrder, 20L);
       setProductInOrderLine(secondLine, productTwo);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut shipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -182,13 +182,11 @@
       final ShipmentInOutLine shipmentLine = getShipmentLineByLineNo(shipment, 10L);
       setProductInShipmentLine(shipmentLine, productOne);
       setOrderLineInShipmentLine(shipmentLine, firstLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().flush();
-      OBDal.getInstance().commitAndClose();
+      OBDal.getInstance().refresh(salesOrder); // Necessary to update the delivery status
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertNoInvoiceWasGenerated(invoice);
 
       final ShipmentInOut secondShipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -196,14 +194,15 @@
       final ShipmentInOutLine secondShipmentLine = getShipmentLineByLineNo(secondShipment, 10L);
       setProductInShipmentLine(secondShipmentLine, productTwo);
       setOrderLineInShipmentLine(secondShipmentLine, secondLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(secondShipment);
-      OBDal.getInstance().commitAndClose();
+      OBDal.getInstance().getSession().clear(); // Necessary to force DAL to refresh objects status
 
       final Invoice secondInvoice = new InvoiceGeneratorFromGoodsShipment(secondShipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertGeneratedInvoiceLine(secondInvoice, productOne, new BigDecimal(12));
       assertGeneratedInvoiceLine(secondInvoice, productTwo, new BigDecimal(12));
+      assertInvoiceDatePriceListAndStatus(secondInvoice, secondShipment.getMovementDate(),
+          salesOrder.getPriceList(), "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -240,15 +239,14 @@
       ShipmentInOutLine secondShipmentLine = getShipmentLineByLineNo(shipment, 20L);
       setProductInShipmentLine(secondShipmentLine, productTwo);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
+      final PriceList priceList = shipment.getBusinessPartner().getPriceList();
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertGeneratedInvoiceLine(invoice, productOne, new BigDecimal(12));
       assertGeneratedInvoiceLine(invoice, productTwo, new BigDecimal(12));
-
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(), priceList, "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -297,7 +295,6 @@
           "InvoiceFromShipment_004", AFTER_DELIVERY);
       final OrderLine firstLine = getOrderLineByLineNo(firstSalesOrder, 10L);
       setProductInOrderLine(firstLine, productOne);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(firstSalesOrder);
 
       final Order secondSalesOrder = createAfterOrderDeliveredOrder(SALES_ORDER,
@@ -306,7 +303,6 @@
       setProductInOrderLine(secondLine, productTwo);
       final OrderLine thirdLine = getOrderLineByLineNo(secondSalesOrder, 20L);
       setProductInOrderLine(thirdLine, productThree);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(secondSalesOrder);
 
       final ShipmentInOut shipment = createShipmentReceiptWithTwoLines(GOODS_SHIPMENT_ID,
@@ -321,15 +317,17 @@
       setProductInShipmentLine(thirdShipmentLine, productFour);
       setOrderLineInShipmentLine(thirdShipmentLine, null);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
+      OBDal.getInstance().refresh(firstSalesOrder); // Necessary to update the delivery status
+      OBDal.getInstance().refresh(secondSalesOrder); // Necessary to update the delivery status
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(invoice, 2);
       assertGeneratedInvoiceLine(invoice, productOne, new BigDecimal(12));
       assertGeneratedInvoiceLine(invoice, productFour, new BigDecimal(12));
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          firstSalesOrder.getPriceList(), "CO");
 
       final ShipmentInOut secondShipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
           "InvoiceFromShipment_004");
@@ -337,16 +335,16 @@
       setProductInShipmentLine(fourthShipmentLine, productThree);
       setOrderLineInShipmentLine(fourthShipmentLine, thirdLine);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(secondShipment);
-      OBDal.getInstance().commitAndClose();
+      OBDal.getInstance().getSession().clear(); // Necessary to force DAL to refresh objects status
 
       final Invoice secondInvoice = new InvoiceGeneratorFromGoodsShipment(secondShipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(secondInvoice, 2);
       assertGeneratedInvoiceLine(secondInvoice, productTwo, new BigDecimal(12));
       assertGeneratedInvoiceLine(secondInvoice, productThree, new BigDecimal(12));
-
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          firstSalesOrder.getPriceList(), "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -384,14 +382,12 @@
           "InvoiceFromShipment_005", AFTER_DELIVERY);
       final OrderLine firstLine = getOrderLineByLineNo(firstSalesOrder, 10L);
       setProductInOrderLine(firstLine, productOne);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(firstSalesOrder);
 
       final Order secondSalesOrder = createSalesOrderWithInvoiceTerm(SALES_ORDER,
           "InvoiceFromShipment_005", AFTER_ORDER_DELIVERY);
       final OrderLine secondLine = getOrderLineByLineNo(secondSalesOrder, 10L);
       setProductInOrderLine(secondLine, productTwo);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(secondSalesOrder);
 
       final ShipmentInOut shipment = createShipmentReceiptWithTwoLines(GOODS_SHIPMENT_ID,
@@ -405,15 +401,16 @@
       setOrderLineInShipmentLine(secondShipmentLine, secondLine);
       updateMovementQuantity(secondShipmentLine, BigDecimal.TEN);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
+      OBDal.getInstance().refresh(firstSalesOrder); // Necessary to update the delivery status
+      OBDal.getInstance().refresh(secondSalesOrder); // Necessary to update the delivery status
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(invoice, 1);
       assertGeneratedInvoiceLine(invoice, productOne, BigDecimal.TEN);
-
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          firstSalesOrder.getPriceList(), "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -451,14 +448,12 @@
           "InvoiceFromShipment_006", IMMEDIATE);
       final OrderLine firstLine = getOrderLineByLineNo(firstSalesOrder, 10L);
       setProductInOrderLine(firstLine, productOne);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(firstSalesOrder);
 
       final Order secondSalesOrder = createSalesOrderWithInvoiceTerm(SALES_ORDER,
           "InvoiceFromShipment_006", DO_NOT_INVOICE);
       final OrderLine secondLine = getOrderLineByLineNo(secondSalesOrder, 10L);
       setProductInOrderLine(secondLine, productTwo);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(secondSalesOrder);
 
       final ShipmentInOut shipment = createShipmentReceiptWithTwoLines(GOODS_SHIPMENT_ID,
@@ -470,15 +465,14 @@
       setProductInShipmentLine(secondShipmentLine, productTwo);
       setOrderLineInShipmentLine(secondShipmentLine, secondLine);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(invoice, 1);
       assertGeneratedInvoiceLine(invoice, productOne, new BigDecimal(12));
-
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          firstSalesOrder.getPriceList(), "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -530,7 +524,6 @@
       setProductInOrderLine(firstLine, productOne);
       final OrderLine secondLine = getOrderLineByLineNo(salesOrder, 20L);
       setProductInOrderLine(secondLine, productTwo);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut firstShipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -539,9 +532,7 @@
       setProductInShipmentLine(firstShipmentLine, productOne);
       setOrderLineInShipmentLine(firstShipmentLine, firstLine);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(firstShipment);
-      OBDal.getInstance().commitAndClose();
 
       final Invoice invoice = createInvoiceFromShipment(firstShipment, SALES_INVOICE);
       TestUtils.processInvoice(invoice);
@@ -552,16 +543,15 @@
       setProductInShipmentLine(secondShipmentLine, productTwo);
       setOrderLineInShipmentLine(secondShipmentLine, secondLine);
 
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(secondShipment);
-      OBDal.getInstance().commitAndClose();
 
       final Invoice secondInvoice = new InvoiceGeneratorFromGoodsShipment(secondShipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(secondInvoice, 2);
       assertGeneratedInvoiceLine(secondInvoice, productOne, new BigDecimal(2));
       assertGeneratedInvoiceLine(secondInvoice, productTwo, new BigDecimal(12));
-
+      assertInvoiceDatePriceListAndStatus(secondInvoice, secondShipment.getMovementDate(),
+          salesOrder.getPriceList(), "CO");
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       throw new OBException(e);
@@ -589,14 +579,12 @@
   public void invoiceFromShipment_008() {
     OBContext.setAdminMode();
     try {
-
       final Product product = TestUtils
           .cloneProduct(T_SHIRTS_PRODUCT_ID, "InvoiceFromShipment_008");
       final Order salesOrder = createSalesOrderWithInvoiceTerm(SALES_ORDER,
           "InvoiceFromShipment_008", AFTER_DELIVERY);
       final OrderLine orderLine = getOrderLineByLineNo(salesOrder, 10L);
       setProductInOrderLine(orderLine, product);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut shipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -604,18 +592,19 @@
       final ShipmentInOutLine shipmentLine = getShipmentLineByLineNo(shipment, 10L);
       setProductInShipmentLine(shipmentLine, product);
       setOrderLineInShipmentLine(shipmentLine, orderLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
+          .createInvoiceConsideringInvoiceTerms(true);
       assertInvoiceLineNumber(invoice, 1);
       assertGeneratedInvoiceLine(invoice, product, new BigDecimal(12));
+      assertInvoiceDatePriceListAndStatus(invoice, shipment.getMovementDate(),
+          salesOrder.getPriceList(), "CO");
+      OBDal.getInstance().refresh(orderLine); // Necessary to force DAL to refresh invoiced qty
 
-      final Invoice secondIinvoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
-          .createAndProcessInvoiceConsideringInvoiceTerms();
-      assertNoInvoiceWasGenerated(secondIinvoice);
+      final Invoice secondInvoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId())
+          .createInvoiceConsideringInvoiceTerms(false);
+      assertNoInvoiceWasGenerated(secondInvoice);
 
     } catch (Exception e) {
       log.error(e.getMessage(), e);
@@ -650,7 +639,6 @@
           "InvoiceFromShipment_009", AFTER_DELIVERY);
       final OrderLine orderLine = getOrderLineByLineNo(salesOrder, 10L);
       setProductInOrderLine(orderLine, product);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut shipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -658,9 +646,7 @@
       final ShipmentInOutLine shipmentLine = getShipmentLineByLineNo(shipment, 10L);
       setProductInShipmentLine(shipmentLine, product);
       setOrderLineInShipmentLine(shipmentLine, orderLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
 
       Calendar calendar = Calendar.getInstance();
       calendar.add(Calendar.DAY_OF_MONTH, 1);
@@ -669,7 +655,7 @@
       PriceList priceList = OBDal.getInstance().get(PriceList.class, PRICE_LIST_SALES_ID);
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId(), invoiceDate,
-          priceList).createAndProcessInvoiceConsideringInvoiceTerms();
+          priceList).createInvoiceConsideringInvoiceTerms(true);
       assertGeneratedInvoiceLine(invoice, product, new BigDecimal(12));
       assertInvoiceDatePriceListAndStatus(invoice, invoiceDate, priceList, "CO");
 
@@ -706,7 +692,6 @@
           "InvoiceFromShipment_010", AFTER_DELIVERY);
       final OrderLine orderLine = getOrderLineByLineNo(salesOrder, 10L);
       setProductInOrderLine(orderLine, product);
-      OBDal.getInstance().flush();
       TestUtils.processOrder(salesOrder);
 
       final ShipmentInOut shipment = TestUtils.cloneReceiptShipment(GOODS_SHIPMENT_ID,
@@ -714,9 +699,7 @@
       final ShipmentInOutLine shipmentLine = getShipmentLineByLineNo(shipment, 10L);
       setProductInShipmentLine(shipmentLine, product);
       setOrderLineInShipmentLine(shipmentLine, orderLine);
-      OBDal.getInstance().flush();
       TestUtils.processShipmentReceipt(shipment);
-      OBDal.getInstance().commitAndClose();
 
       Calendar calendar = Calendar.getInstance();
       calendar.add(Calendar.DAY_OF_MONTH, 1);
@@ -725,7 +708,7 @@
       PriceList priceList = OBDal.getInstance().get(PriceList.class, PRICE_LIST_SALES_ID);
 
       final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipment.getId(), invoiceDate,
-          priceList).createInvoiceConsideringInvoiceTerms();
+          priceList).createInvoiceConsideringInvoiceTerms(false);
       assertGeneratedInvoiceLine(invoice, product, new BigDecimal(12));
       assertInvoiceDatePriceListAndStatus(invoice, invoiceDate, priceList, "DR");
 
@@ -913,10 +896,10 @@
       PriceList priceList, String status) {
     assertThat("Invoice status should have been " + status, invoice.getDocumentStatus(),
         equalTo(status));
-    assertThat("Invoice date should have been " + invoiceDate, invoice.getInvoiceDate().getTime(),
-        equalTo(invoiceDate.getTime()));
-    assertThat("Price List should have been " + priceList, invoice.getPriceList().getName(),
-        equalTo(priceList.getName()));
+    assertTrue("Invoice date should have been " + invoiceDate,
+        DateUtils.truncatedEquals(invoice.getInvoiceDate(), invoiceDate, Calendar.DATE));
+    assertThat("Price List should have been " + priceList.getId(), invoice.getPriceList().getId(),
+        equalTo(priceList.getId()));
   }
 
 }
--- a/src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/TestUtils.java	Thu Oct 25 17:59:27 2018 -0400
+++ b/src-test/src/org/openbravo/test/materialMgmt/invoiceFromShipment/TestUtils.java	Mon Nov 12 19:38:38 2018 +0100
@@ -39,7 +39,7 @@
 import org.openbravo.model.pricing.pricelist.ProductPrice;
 import org.openbravo.service.db.CallStoredProcedure;
 
-public class TestUtils {
+class TestUtils {
 
   private static final String COMPLETE_ACTION = "CO";
   private static final String DRAFT_STATUS = "DR";
@@ -54,7 +54,7 @@
    *          Name of the original Product
    * @return A new Product clone based on the original one
    */
-  public static Product cloneProduct(final String productId, final String name) {
+  static Product cloneProduct(final String productId, final String name) {
     final Product oldProduct = OBDal.getInstance().get(Product.class, productId);
     final Product newProduct = (Product) DalUtil.copy(oldProduct, false);
     int numberOfProductsWithSameName = getNumberOfProducts(name) + 1;
@@ -86,7 +86,7 @@
   /**
    * Returns the number of products with same Product name
    */
-  public static int getNumberOfProducts(final String name) {
+  static int getNumberOfProducts(final String name) {
     try {
       final OBCriteria<Product> criteria = OBDal.getInstance().createCriteria(Product.class);
       criteria.add(Restrictions.like(Product.PROPERTY_NAME, name + "-%"));
@@ -106,7 +106,7 @@
    *          new Order (a clone of the original one)
    * @return A new Order Line clone based on the original one
    */
-  public static Order cloneOrder(final String orderId, final String docNo) {
+  static Order cloneOrder(final String orderId, final String docNo) {
     final Order oldOrder = OBDal.getInstance().get(Order.class, orderId);
     final Order newOrder = (Order) DalUtil.copy(oldOrder, false);
     int numberOfOrdersWithSameDocNo = getNumberOfOrders(docNo) + 1;
@@ -135,7 +135,7 @@
   /**
    * Returns the number of orders with same Document Number
    */
-  public static int getNumberOfOrders(final String docNo) {
+  static int getNumberOfOrders(final String docNo) {
     try {
       final OBCriteria<Order> criteria = OBDal.getInstance().createCriteria(Order.class);
       criteria.add(Restrictions.like(Order.PROPERTY_DOCUMENTNO, docNo + "-%"));
@@ -155,7 +155,7 @@
    *          new Order (a clone of the original one)
    * @return A new Order Line clone based on the original one
    */
-  public static OrderLine cloneOrderLine(final OrderLine oldLine, final Order newOrder) {
+  static OrderLine cloneOrderLine(final OrderLine oldLine, final Order newOrder) {
 
     // Skip discount lines
     if (oldLine.getOrderDiscount() != null) {
@@ -184,7 +184,7 @@
    *          Order to be completed
    * @throws OBException
    */
-  public static void processOrder(final Order order) throws OBException {
+  static void processOrder(final Order order) throws OBException {
     final List<Object> parameters = new ArrayList<Object>();
     parameters.add(null);
     parameters.add(order.getId());
@@ -202,7 +202,7 @@
    *          docNo to set to the new Goods Receipt/Shipment
    * @return a Goods Receipt/Shipment not completed
    */
-  public static ShipmentInOut cloneReceiptShipment(final String mInoutId, final String docNo) {
+  static ShipmentInOut cloneReceiptShipment(final String mInoutId, final String docNo) {
     final ShipmentInOut oldInOut = OBDal.getInstance().get(ShipmentInOut.class, mInoutId);
     final ShipmentInOut newInOut = (ShipmentInOut) DalUtil.copy(oldInOut, false);
     int numberOfShipmentsWithSameDocNo = getNumberOfShipments(docNo) + 1;
@@ -230,7 +230,7 @@
   /**
    * Returns the number of Goods Receipts/Shipments with same Document Number
    */
-  public static int getNumberOfShipments(final String docNo) {
+  static int getNumberOfShipments(final String docNo) {
     try {
       final OBCriteria<ShipmentInOut> criteria = OBDal.getInstance().createCriteria(
           ShipmentInOut.class);
@@ -251,7 +251,7 @@
    *          new Goods Receipt/Shipment (a clone of the original one)
    * @return A new Goods Receipt/Shipment Line clone based on the original one
    */
-  public static ShipmentInOutLine cloneReceiptShipmentLine(final ShipmentInOutLine oldLine,
+  static ShipmentInOutLine cloneReceiptShipmentLine(final ShipmentInOutLine oldLine,
       final ShipmentInOut newInOut) {
     final ShipmentInOutLine newLine = (ShipmentInOutLine) DalUtil.copy(oldLine, false);
 
@@ -273,7 +273,7 @@
    *          Shipment or Receipt to be completed
    * @throws OBException
    */
-  public static void processShipmentReceipt(final ShipmentInOut shipmentReceipt) throws OBException {
+  static void processShipmentReceipt(final ShipmentInOut shipmentReceipt) throws OBException {
     final List<Object> parameters = new ArrayList<Object>();
     parameters.add(null);
     parameters.add(shipmentReceipt.getId());
@@ -288,7 +288,7 @@
    *          The Invoice.
    * @throws Exception
    */
-  public static void processInvoice(final Invoice invoice) throws Exception {
+  static void processInvoice(final Invoice invoice) throws Exception {
     if (invoice != null) {
       final List<Object> parameters = new ArrayList<>();
       parameters.add(null); // Process Instance parameter
--- a/src/org/openbravo/common/actionhandler/InvoiceFromShipmentActionHandler.java	Thu Oct 25 17:59:27 2018 -0400
+++ b/src/org/openbravo/common/actionhandler/InvoiceFromShipmentActionHandler.java	Mon Nov 12 19:38:38 2018 +0100
@@ -18,9 +18,6 @@
  */
 package org.openbravo.common.actionhandler;
 
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.Calendar;
 import java.util.Date;
 import java.util.Map;
 
@@ -33,6 +30,7 @@
 import org.openbravo.materialmgmt.InvoiceGeneratorFromGoodsShipment;
 import org.openbravo.model.common.invoice.Invoice;
 import org.openbravo.model.pricing.pricelist.PriceList;
+import org.openbravo.service.json.JsonUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -50,30 +48,21 @@
 
   @Override
   protected JSONObject doExecute(Map<String, Object> parameters, String content) {
-
-    Invoice invoice;
     final JSONObject message = new JSONObject();
     try {
       final JSONObject request = new JSONObject(content);
       final JSONObject params = request.getJSONObject("_params");
       final String shipmentId = request.getString("M_InOut_ID");
-      final String invoiceDateStr = params.getString("MovementDate");
+      final String invoiceDateStr = params.getString("DateInvoiced");
       final String priceListStr = params.getString("priceList");
       boolean processInvoice = params.getBoolean("processInvoice");
 
-      final Date invoiceDate = getInvoiceDate(invoiceDateStr);
+      final Date invoiceDate = JsonUtils.createDateFormat().parse(invoiceDateStr);
       final PriceList priceList = OBDal.getInstance().getProxy(PriceList.class, priceListStr);
 
-      if (processInvoice) {
-        invoice = new InvoiceGeneratorFromGoodsShipment(shipmentId, invoiceDate, priceList)
-            .createAndProcessInvoiceConsideringInvoiceTerms();
-      } else {
-        invoice = new InvoiceGeneratorFromGoodsShipment(shipmentId, invoiceDate, priceList)
-            .createInvoiceConsideringInvoiceTerms();
-      }
-
+      final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(shipmentId, invoiceDate,
+          priceList).createInvoiceConsideringInvoiceTerms(processInvoice);
       message.put(MESSAGE, getSuccessMessage(invoice));
-
     } catch (Exception e) {
       try {
         message.put(MESSAGE, getErrorMessage(e));
@@ -85,17 +74,6 @@
     return message;
   }
 
-  private Date getInvoiceDate(final String invoiceDateStr) {
-    Date invoiceDate = Calendar.getInstance().getTime();
-    try {
-      final SimpleDateFormat dateFormatter = new SimpleDateFormat("yyyy-MM-dd");
-      invoiceDate = dateFormatter.parse(invoiceDateStr);
-    } catch (ParseException e) {
-      log.error("Not possible to parse the following date: " + invoiceDateStr, e);
-    }
-    return invoiceDate;
-  }
-
   protected JSONObject getSuccessMessage(final Invoice invoice) {
     final JSONObject successMessage = new JSONObject();
     try {
--- a/src/org/openbravo/erpCommon/ad_actionButton/DocAction.html	Thu Oct 25 17:59:27 2018 -0400
+++ b/src/org/openbravo/erpCommon/ad_actionButton/DocAction.html	Mon Nov 12 19:38:38 2018 +0100
@@ -182,12 +182,13 @@
       var frm = document.frmMain,
           goodsShipmentWindow = '169',
           currentWindowId = frm.inpwindowId.value,
-          documentIsInDraftStatus = (frm.inpdocstatus.value === 'DR');
+          documentIsInDraftStatus = (frm.inpdocstatus.value === 'DR'),
+          isInvoiceIfPossibleChecked = frm.inpInvoiceIfPossible.checked;
       
       displayLogicElement('invoiceIfPossible', documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
-      displayLogicElement('processInvoice', documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
-      displayLogicElement('invoiceDateRow', documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
-      displayLogicElement('priceListRow', documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
+      displayLogicElement('processInvoice', isInvoiceIfPossibleChecked && documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
+      displayLogicElement('invoiceDateRow', isInvoiceIfPossibleChecked && documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
+      displayLogicElement('priceListRow', isInvoiceIfPossibleChecked && documentIsInDraftStatus && currentWindowId === goodsShipmentWindow);
     }
     </script>
 </head>
@@ -469,7 +470,7 @@
               	<td class="TitleCell"><span id="lblInvoiceIfPossible" class="LabelText">Invoice if possible</span></td>
               	<td class="Radio_Check_ContentCell">
               	  <span class="Checkbox_container_NOT_Focused">
-              	    <input type="checkbox" id="paramInvoiceIfPossible" name="inpInvoiceIfPossible">
+              	    <input type="checkbox" id="paramInvoiceIfPossible" name="inpInvoiceIfPossible" onchange="applyInvoiceIfPossibleDisplayLogic();" value="Y">
               	  </span>
               	</td>
               </tr>
@@ -531,7 +532,7 @@
               	<td class="TitleCell"><span id="lblProcessInvoice" class="LabelText">Process Invoice</span></td>
               	<td class="Radio_Check_ContentCell">
               	  <span class="Checkbox_container_NOT_Focused">
-              	    <input type="checkbox" id="paramProcessInvoice" name="inpProcessInvoice" checked>
+              	    <input type="checkbox" id="paramProcessInvoice" name="inpProcessInvoice" value="Y" checked>
               	  </span>
               	</td>
               </tr>
--- a/src/org/openbravo/erpCommon/ad_actionButton/ProcessGoods.java	Thu Oct 25 17:59:27 2018 -0400
+++ b/src/org/openbravo/erpCommon/ad_actionButton/ProcessGoods.java	Mon Nov 12 19:38:38 2018 +0100
@@ -23,7 +23,6 @@
 import java.io.PrintWriter;
 import java.math.BigDecimal;
 import java.text.ParseException;
-import java.util.Calendar;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -35,7 +34,6 @@
 
 import org.apache.commons.lang.StringUtils;
 import org.hibernate.query.Query;
-import org.openbravo.base.exception.OBException;
 import org.openbravo.base.filter.IsIDFilter;
 import org.openbravo.base.provider.OBProvider;
 import org.openbravo.base.secureApp.HttpSecureAppServlet;
@@ -166,8 +164,8 @@
     final String strTabId = vars.getGlobalVariable("inpTabId", "ProcessGoods|Tab_ID",
         IsIDFilter.instance);
     final String strM_Inout_ID = vars.getGlobalVariable("inpKey", strWindowId + "|M_Inout_ID", "");
-    final boolean invoiceIfPossible = StringUtils.equalsIgnoreCase(
-        vars.getStringParameter("inpInvoiceIfPossible"), "on");
+    final boolean invoiceIfPossible = StringUtils.equals(
+        vars.getStringParameter("inpInvoiceIfPossible"), "Y");
 
     OBError myMessage = null;
     try {
@@ -218,26 +216,10 @@
       log4j.debug(myMessage.getMessage());
       vars.setMessage(strTabId, myMessage);
 
-      if (GOODS_SHIPMENT_WINDOW.equals(strWindowId) && invoiceIfPossible
+      if (invoiceIfPossible && GOODS_SHIPMENT_WINDOW.equals(strWindowId)
           && !"Error".equalsIgnoreCase(myMessage.getType())) {
-
-        final boolean processInvoice = StringUtils.equalsIgnoreCase(
-            vars.getStringParameter("inpProcessInvoice"), "on");
-        final String invoiceDateStr = vars.getStringParameter("inpInvoiceDate");
-        final String priceListStr = vars.getStringParameter("inpPriceList");
-
-        Date invoiceDate = getInvoiceDate(invoiceDateStr);
-        final PriceList priceList = OBDal.getInstance().getProxy(PriceList.class, priceListStr);
-
-        Invoice invoice;
-        if (processInvoice) {
-          invoice = new InvoiceGeneratorFromGoodsShipment(goods.getId(), invoiceDate, priceList)
-              .createAndProcessInvoiceConsideringInvoiceTerms();
-        } else {
-          invoice = new InvoiceGeneratorFromGoodsShipment(goods.getId(), invoiceDate, priceList)
-              .createInvoiceConsideringInvoiceTerms();
-        }
-        myMessage = getResultMessage(invoice);
+        myMessage = createInvoice(vars, goods);
+        log4j.debug(myMessage.getMessage());
         vars.setMessage(strTabId, myMessage);
       }
 
@@ -247,7 +229,7 @@
       }
       printPageClosePopUp(response, vars, strWindowPath);
 
-    } catch (ServletException ex) {
+    } catch (ServletException | ParseException ex) {
       myMessage = Utility.translateError(this, vars, vars.getLanguage(), ex.getMessage());
       if (!myMessage.isConnectionAvailable()) {
         bdErrorConnection(response);
@@ -255,24 +237,24 @@
       } else {
         vars.setMessage(strTabId, myMessage);
       }
-    } catch (final OBException e) {
-      if (e.getCause() != null) {
-        throw new OBException(Utility
-            .translateError(this, vars, vars.getLanguage(), e.getMessage()).getMessage());
-      } else {
-        throw e;
-      }
     }
   }
 
-  private Date getInvoiceDate(final String invoiceDateStr) {
-    Date invoiceDate = Calendar.getInstance().getTime();
-    try {
-      invoiceDate = OBDateUtils.getDate(invoiceDateStr);
-    } catch (ParseException e) {
-      log4j.error("Not possible to parse the following date: " + invoiceDateStr, e);
-    }
-    return invoiceDate;
+  private OBError createInvoice(final VariablesSecureApp vars, final ShipmentInOut goodShipment)
+      throws ParseException {
+    OBError myMessage;
+    final boolean processInvoice = StringUtils.equals(vars.getStringParameter("inpProcessInvoice"),
+        "Y");
+    final String invoiceDateStr = vars.getStringParameter("inpInvoiceDate");
+    final String priceListStr = vars.getStringParameter("inpPriceList");
+
+    final Date invoiceDate = OBDateUtils.getDate(invoiceDateStr);
+    final PriceList priceList = OBDal.getInstance().getProxy(PriceList.class, priceListStr);
+
+    final Invoice invoice = new InvoiceGeneratorFromGoodsShipment(goodShipment.getId(),
+        invoiceDate, priceList).createInvoiceConsideringInvoiceTerms(processInvoice);
+    myMessage = getResultMessage(invoice);
+    return myMessage;
   }
 
   private OBError getResultMessage(Invoice invoice) {
--- a/src/org/openbravo/materialmgmt/InvoiceGeneratorFromGoodsShipment.java	Thu Oct 25 17:59:27 2018 -0400
+++ b/src/org/openbravo/materialmgmt/InvoiceGeneratorFromGoodsShipment.java	Mon Nov 12 19:38:38 2018 +0100
@@ -20,9 +20,11 @@
 
 import java.math.BigDecimal;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.codehaus.jettison.json.JSONArray;
 import org.codehaus.jettison.json.JSONException;
@@ -30,20 +32,26 @@
 import org.hibernate.ScrollMode;
 import org.hibernate.ScrollableResults;
 import org.hibernate.Session;
+import org.hibernate.criterion.Restrictions;
 import org.hibernate.query.Query;
 import org.openbravo.base.exception.OBException;
 import org.openbravo.base.model.Entity;
 import org.openbravo.base.model.ModelProvider;
 import org.openbravo.base.provider.OBProvider;
+import org.openbravo.base.util.Check;
 import org.openbravo.base.weld.WeldUtils;
 import org.openbravo.client.kernel.RequestContext;
 import org.openbravo.common.actionhandler.createlinesfromprocess.CreateInvoiceLinesFromProcess;
 import org.openbravo.dal.service.OBDal;
+import org.openbravo.dal.service.OBDao;
+import org.openbravo.erpCommon.utility.OBMessageUtils;
 import org.openbravo.erpCommon.utility.Utility;
 import org.openbravo.model.common.currency.Currency;
 import org.openbravo.model.common.enterprise.DocumentType;
+import org.openbravo.model.common.enterprise.Organization;
 import org.openbravo.model.common.invoice.Invoice;
 import org.openbravo.model.common.invoice.InvoiceLine;
+import org.openbravo.model.common.order.InvoiceCandidateV;
 import org.openbravo.model.common.order.Order;
 import org.openbravo.model.common.order.OrderLine;
 import org.openbravo.model.materialmgmt.transaction.ShipmentInOut;
@@ -62,37 +70,36 @@
  *
  */
 public class InvoiceGeneratorFromGoodsShipment {
-
-  private static final String C_INVOICE_TABLE_ID = "318";
   private static final Logger log = LoggerFactory
       .getLogger(InvoiceGeneratorFromGoodsShipment.class);
-  private ShipmentInOut shipment;
+
+  private String shipmentId;
   private Invoice invoice;
   private CreateInvoiceLinesFromProcess createInvoiceLineProcess;
   private Date invoiceDate;
-  private PriceList priceList;
+  private String priceListId;
+  private final Set<String> ordersWithAfterOrderDeliveryAlreadyInvoiced = new HashSet<>();
 
-  private static final String AFTER_ORDER_DELIVERY = "O";
-  private static final String AFTER_DELIVERY = "D";
-  private static final String IMMEDIATE = "I";
+  private enum InvoiceTerm {
+    IMMEDIATE("I"), AFTER_DELIVERY("D"), CUSTOMERSCHEDULE("S"), AFTER_ORDER_DELIVERY("O");
 
-  /**
-   * Creates an {@link InvoiceGeneratorFromGoodsShipment} based shipment Id
-   * 
-   * @param shipmentId
-   *          The shipment Id
-   * @param invoiceDate
-   *          The invoice date.
-   * @param priceList
-   *          The invoice price list
-   */
-  public InvoiceGeneratorFromGoodsShipment(final String shipmentId, final Date invoiceDate,
-      final PriceList priceList) {
-    this.shipment = OBDal.getInstance().get(ShipmentInOut.class, shipmentId);
-    this.createInvoiceLineProcess = WeldUtils
-        .getInstanceFromStaticBeanManager(CreateInvoiceLinesFromProcess.class);
-    this.invoiceDate = invoiceDate;
-    this.priceList = priceList;
+    private static final List<String> CAN_INVOICE_ORDERLINE_INDIVIDUALLY = Arrays.asList(
+        InvoiceTerm.IMMEDIATE.getInvoiceTerm(), InvoiceTerm.AFTER_DELIVERY.getInvoiceTerm(),
+        CUSTOMERSCHEDULE.getInvoiceTerm());
+
+    private String invoiceTermId;
+
+    private InvoiceTerm(final String invoiceTermId) {
+      this.invoiceTermId = invoiceTermId;
+    }
+
+    private static boolean canInvoiceOrderLineIndividually(final String invoiceTerm) {
+      return CAN_INVOICE_ORDERLINE_INDIVIDUALLY.contains(invoiceTerm);
+    }
+
+    private String getInvoiceTerm() {
+      return invoiceTermId;
+    }
   }
 
   /**
@@ -108,73 +115,91 @@
   }
 
   /**
-   * Creates and process an Invoice from Goods Shipment, considering the invoice terms of orders
-   * linked to shipment lines.
+   * Creates an {@link InvoiceGeneratorFromGoodsShipment} based on shipment Id
+   * 
+   * @param shipmentId
+   *          The shipment Id
+   * @param invoiceDate
+   *          The invoice date. If null it takes the from the shipment movement date.
+   * @param priceList
+   *          The invoice price list. If null it takes the business partner default
+   */
+  public InvoiceGeneratorFromGoodsShipment(final String shipmentId, final Date invoiceDate,
+      final PriceList priceList) {
+    Check.isNotNull(shipmentId, "Parameter shipmentId can't be null");
+    this.shipmentId = shipmentId;
+    setInvoiceDate(invoiceDate);
+    setPriceListId(priceList);
+    this.createInvoiceLineProcess = WeldUtils
+        .getInstanceFromStaticBeanManager(CreateInvoiceLinesFromProcess.class);
+  }
+
+  private void setInvoiceDate(final Date date) {
+    this.invoiceDate = date == null ? getShipment().getMovementDate() : date;
+  }
+
+  private void setPriceListId(final PriceList priceList) {
+    try {
+      this.priceListId = priceList.getId();
+    } catch (NullPointerException noPriceListProvided) {
+      try {
+        this.priceListId = getShipment().getBusinessPartner().getPriceList().getId();
+      } catch (NullPointerException bpWithoutPriceList) {
+        throw new OBException(OBMessageUtils.messageBD("notnullpricelist"));
+      }
+    }
+  }
+
+  private ShipmentInOut getShipment() {
+    return OBDal.getInstance().getProxy(ShipmentInOut.class, shipmentId);
+  }
+
+  private Date getInvoiceDate() {
+    return this.invoiceDate;
+  }
+
+  private PriceList getPriceList() {
+    return OBDal.getInstance().getProxy(PriceList.class, priceListId);
+  }
+
+  /**
+   * Creates a Sales Invoice from Goods Shipment, considering the invoice terms of available orders
+   * linked to the shipment lines.
+   * 
+   * @param doProcessInvoice
+   *          if true the invoice will be automatically processed, otherwise it will remain in draft
+   *          status
    * 
    * @return The invoice created
    */
-  public Invoice createAndProcessInvoiceConsideringInvoiceTerms() {
+  public Invoice createInvoiceConsideringInvoiceTerms(boolean doProcessInvoice) {
     try {
-
-      createInvoiceConsideringInvoiceTerms();
-      if (invoice != null) {
+      createInvoiceIfPossible();
+      if (doProcessInvoice && invoice != null) {
         processInvoice();
         OBDal.getInstance().refresh(invoice);
       }
-    } catch (OBException e) {
-      executeRollBack();
-      throw new OBException(e.getMessage());
-    } catch (Exception e1) {
-      executeRollBack();
-      Throwable e3 = DbUtility.getUnderlyingSQLException(e1);
-      throw new OBException(e3);
+    } catch (Exception e) {
+      OBDal.getInstance().rollbackAndClose();
+      Throwable ex = DbUtility.getUnderlyingSQLException(e);
+      throw new OBException(OBMessageUtils.translateError(ex.getMessage()).getMessage());
     }
 
     return invoice;
   }
 
-  /**
-   * Creates an Invoice from Goods Shipment, considering the invoice terms of orders linked to
-   * shipment lines. The invoice is in status 'DR'
-   * 
-   * @return The invoice created
-   */
-  public Invoice createInvoiceConsideringInvoiceTerms() {
-    try {
-
-      createInvoice();
-
-    } catch (OBException e) {
-      executeRollBack();
-      throw new OBException(e.getMessage());
-    }
-
-    return invoice;
-  }
-
-  private Invoice createInvoice() {
-    HashSet<String> ordersAlreadyInvoiced = new HashSet<>();
+  private Invoice createInvoiceIfPossible() {
     try (ScrollableResults scrollShipmentLines = getShipmentLines()) {
       while (scrollShipmentLines.next()) {
-        final ShipmentInOutLine shipmentLine = OBDal.getInstance().get(ShipmentInOutLine.class,
-            scrollShipmentLines.get()[0]);
-
-        final OrderLine orderLine = shipmentLine.getSalesOrderLine();
-        boolean shipmentLineIsLinkedToSalesOrderLine = orderLine != null;
-        final Order order = shipmentLineIsLinkedToSalesOrderLine ? orderLine.getSalesOrder() : null;
-        final String invoiceTerms = order != null ? order.getInvoiceTerms() : null;
-        final Long deliveryStatus = order != null ? order.getDeliveryStatus() : null;
-
-        if (AFTER_DELIVERY.equals(invoiceTerms) || IMMEDIATE.equals(invoiceTerms)
-            || !shipmentLineIsLinkedToSalesOrderLine) {
-          invoiceShimpentLineIfNotYetInvoiced(shipmentLine);
-
-        } else if (AFTER_ORDER_DELIVERY.equals(invoiceTerms) && deliveryStatus == 100
-            && !ordersAlreadyInvoiced.contains(order.getId())) {
-
-          inovoiceAllShipmentLinesNotFullyInvoicedLinkedToOrder(order);
-          ordersAlreadyInvoiced.add(order.getId());
+        final ShipmentInOutLine shipmentLine = (ShipmentInOutLine) scrollShipmentLines.get(0);
+        final OrderLine orderLine = (OrderLine) scrollShipmentLines.get(1);
+        final Order order = orderLine == null ? null : orderLine.getSalesOrder();
+        if (orderLine == null) {
+          invoiceShipmentLineWithoutRelatedOrderLine(shipmentLine);
+        } else if (isOrderCandidateToBeInvoiced(order)) {
+          invoiceShipmentLineWithRelatedOrderLine(shipmentLine, orderLine, order);
         }
+        evictObjects(shipmentLine, orderLine, order);
       }
       OBDal.getInstance().flush();
     }
@@ -182,29 +207,28 @@
   }
 
   private ScrollableResults getShipmentLines() {
-    final String shipmentLinesHQLQuery = "select iol.id " //
+    final String shipmentLinesHQLQuery = "select iol, ol " //
         + "from " + ShipmentInOutLine.ENTITY_NAME + " iol " //
-        + "where " + ShipmentInOutLine.PROPERTY_SHIPMENTRECEIPT + ".id = :shipmentId ";
-
+        + "left join iol." + ShipmentInOutLine.PROPERTY_SALESORDERLINE + " ol " //
+        + "where iol." + ShipmentInOutLine.PROPERTY_SHIPMENTRECEIPT + ".id = :shipmentId ";
     final Session session = OBDal.getInstance().getSession();
-    final Query<String> query = session.createQuery(shipmentLinesHQLQuery, String.class);
-    query.setParameter("shipmentId", shipment.getId());
-
+    final Query<Object[]> query = session.createQuery(shipmentLinesHQLQuery, Object[].class);
+    query.setParameter("shipmentId", shipmentId);
     return query.scroll(ScrollMode.FORWARD_ONLY);
   }
 
-  protected void invoiceShimpentLineIfNotYetInvoiced(final ShipmentInOutLine shipmentLine) {
-    final BigDecimal totalInvoiced = getTotalInvoicedForShipmentLine(shipmentLine);
-    if (totalInvoiced.compareTo(shipmentLine.getMovementQuantity()) != 0) {
-      invoiceShipmentLine(shipmentLine);
-    }
+  private void invoiceShipmentLineWithoutRelatedOrderLine(final ShipmentInOutLine shipmentLine) {
+    final BigDecimal qtyToInvoice = shipmentLine.getMovementQuantity().subtract(
+        getTotalInvoicedForShipmentLine(shipmentLine));
+    invoicePendingQtyForShipmentLine(shipmentLine, qtyToInvoice);
   }
 
   private BigDecimal getTotalInvoicedForShipmentLine(final ShipmentInOutLine iol) {
     final String invoiceLinesHqlQuery = "select coalesce(sum(il. "
         + InvoiceLine.PROPERTY_INVOICEDQUANTITY + "), 0) " //
         + "from " + InvoiceLine.ENTITY_NAME + " il " //
-        + "where il." + InvoiceLine.PROPERTY_GOODSSHIPMENTLINE + ".id = :shipmentLineId ";
+        + "where il." + InvoiceLine.PROPERTY_GOODSSHIPMENTLINE + ".id = :shipmentLineId " //
+        + "and il.invoice." + Invoice.PROPERTY_DOCUMENTSTATUS + "= 'CO' ";
 
     final Session sessionInvoiceLines = OBDal.getInstance().getSession();
     final Query<BigDecimal> queryInvoiceLines = sessionInvoiceLines.createQuery(
@@ -214,21 +238,76 @@
     return queryInvoiceLines.uniqueResult();
   }
 
-  private void invoiceShipmentLine(final ShipmentInOutLine shipmentLine) {
-    createInvoiceShipmentLine(shipmentLine, shipmentLine.getMovementQuantity());
-
+  protected void invoicePendingQtyForShipmentLine(final ShipmentInOutLine shipmentLine,
+      final BigDecimal qtyToInvoice) {
+    if (BigDecimal.ZERO.compareTo(qtyToInvoice) != 0) {
+      createInvoiceLine(shipmentLine, qtyToInvoice);
+    }
   }
 
-  private void createInvoiceShipmentLine(final ShipmentInOutLine shipmentLine,
+  private boolean isOrderCandidateToBeInvoiced(final Order order) {
+    return !OBDao
+        .getFilteredCriteria(InvoiceCandidateV.class,
+            Restrictions.eq(InvoiceCandidateV.PROPERTY_ID, order.getId())).setMaxResults(1).list()
+        .isEmpty();
+  }
+
+  private void invoiceShipmentLineWithRelatedOrderLine(final ShipmentInOutLine shipmentLine,
+      final OrderLine orderLine, final Order order) {
+    final String invoiceTerm = order.getInvoiceTerms();
+    if (InvoiceTerm.canInvoiceOrderLineIndividually(invoiceTerm)) {
+      final BigDecimal qtyToInvoice = orderLine.getOrderedQuantity()
+          .subtract(orderLine.getInvoicedQuantity()).min(shipmentLine.getMovementQuantity());
+      invoicePendingQtyForShipmentLine(shipmentLine, qtyToInvoice);
+    } else {
+      if (InvoiceTerm.AFTER_ORDER_DELIVERY.getInvoiceTerm().equals(invoiceTerm)) {
+        if (order.getDeliveryStatus() == 100
+            && !ordersWithAfterOrderDeliveryAlreadyInvoiced.contains(order.getId())) {
+          invoiceAllOrderLines(order);
+          ordersWithAfterOrderDeliveryAlreadyInvoiced.add(order.getId());
+        }
+      } else {
+        throw new OBException("Not supported Invoice Term: " + invoiceTerm);
+      }
+    }
+  }
+
+  private void invoiceAllOrderLines(final Order order) {
+    try (ScrollableResults scrollOrderShipmentLines = getAllShipmentLinesLinkedToOrder(order)) {
+      while (scrollOrderShipmentLines.next()) {
+        final ShipmentInOutLine iol = (ShipmentInOutLine) scrollOrderShipmentLines.get()[0];
+        final BigDecimal invoicedQuantity = getTotalInvoicedForShipmentLine(iol);
+        if (invoicedQuantity.compareTo(iol.getMovementQuantity()) != 0) {
+          createInvoiceLine(iol, iol.getMovementQuantity().subtract(invoicedQuantity));
+        }
+        OBDal.getInstance().getSession().evict(iol);
+      }
+    }
+  }
+
+  private ScrollableResults getAllShipmentLinesLinkedToOrder(final Order order) {
+    final String orderLinesHqlQuery = "select iol " //
+        + "from " + ShipmentInOutLine.ENTITY_NAME + " iol " //
+        + "join iol." + ShipmentInOutLine.PROPERTY_SALESORDERLINE + " ol " //
+        + "where ol." + OrderLine.PROPERTY_SALESORDER + ".id = :orderId ";
+
+    final Session sessionOrderLines = OBDal.getInstance().getSession();
+    final Query<ShipmentInOutLine> queryOrderLines = sessionOrderLines.createQuery(
+        orderLinesHqlQuery, ShipmentInOutLine.class);
+    queryOrderLines.setParameter("orderId", order.getId());
+
+    return queryOrderLines.scroll(ScrollMode.FORWARD_ONLY);
+  }
+
+  private void createInvoiceLine(final ShipmentInOutLine shipmentLine,
       final BigDecimal invoicedQuantity) {
     createInvoiceLineProcess.createInvoiceLinesFromDocumentLines(
-        getShipmentLineToBeInvoiced(shipmentLine, invoicedQuantity), getInvoiceHeader(),
+        formatShipmentLineToBeInvoiced(shipmentLine, invoicedQuantity), getInvoiceHeader(),
         ShipmentInOutLine.class);
   }
 
-  private JSONArray getShipmentLineToBeInvoiced(final ShipmentInOutLine shipmentInOutLine,
+  private JSONArray formatShipmentLineToBeInvoiced(final ShipmentInOutLine shipmentInOutLine,
       final BigDecimal invoicedQuantity) {
-
     final JSONArray lines = new JSONArray();
     try {
       final JSONObject line = new JSONObject();
@@ -256,34 +335,14 @@
     return lines;
   }
 
-  private void inovoiceAllShipmentLinesNotFullyInvoicedLinkedToOrder(final Order order) {
-    try (ScrollableResults scrollOrderShipmentLines = getShipmentLinesLinkedToASalesOrder(order)) {
-      while (scrollOrderShipmentLines.next()) {
-
-        final ShipmentInOutLine iol = OBDal.getInstance().get(ShipmentInOutLine.class,
-            (String) scrollOrderShipmentLines.get()[0]);
-        final BigDecimal invoicedQuantity = getTotalInvoicedForShipmentLine(iol);
-        if (invoicedQuantity.compareTo(iol.getMovementQuantity()) != 0) {
-          createInvoiceShipmentLine(iol, iol.getMovementQuantity().subtract(invoicedQuantity));
-        }
+  private void evictObjects(final Object... objects) {
+    for (final Object object : objects) {
+      if (object != null) {
+        OBDal.getInstance().getSession().evict(object);
       }
     }
   }
 
-  private ScrollableResults getShipmentLinesLinkedToASalesOrder(final Order order) {
-    final String orderLinesHqlQuery = "select iol.id " //
-        + "from " + ShipmentInOutLine.ENTITY_NAME + " iol " //
-        + "join iol." + ShipmentInOutLine.PROPERTY_SALESORDERLINE + " ol " //
-        + "where ol." + OrderLine.PROPERTY_SALESORDER + ".id = :orderId ";
-
-    final Session sessionOrderLines = OBDal.getInstance().getSession();
-    final Query<String> queryOrderLines = sessionOrderLines.createQuery(orderLinesHqlQuery,
-        String.class);
-    queryOrderLines.setParameter("orderId", order.getId());
-
-    return queryOrderLines.scroll(ScrollMode.FORWARD_ONLY);
-  }
-
   private Invoice getInvoiceHeader() {
     if (invoice == null) {
       invoice = createInvoiceHeader();
@@ -294,10 +353,11 @@
   private Invoice createInvoiceHeader() {
     final Entity invoiceEntity = ModelProvider.getInstance().getEntity(Invoice.class);
     final Invoice newInvoice = OBProvider.getInstance().get(Invoice.class);
+    final ShipmentInOut shipment = getShipment();
 
     newInvoice.setClient(shipment.getClient());
     newInvoice.setOrganization(shipment.getOrganization());
-    final DocumentType invoiceDocumentType = getInvoiceDocumentType();
+    final DocumentType invoiceDocumentType = getDocumentTypeForARI(getShipment().getOrganization());
     newInvoice.setDocumentType(invoiceDocumentType);
     newInvoice.setTransactionDocument(invoiceDocumentType);
     String documentNo = Utility.getDocumentNo(OBDal.getInstance().getConnection(false),
@@ -321,63 +381,48 @@
     newInvoice.setWithholdingamount(BigDecimal.ZERO);
     newInvoice.setPaymentMethod(shipment.getBusinessPartner().getPaymentMethod());
     newInvoice.setPaymentTerms(shipment.getBusinessPartner().getPaymentTerms());
+
+    checkInvoiceHasAllMandatoryFields(newInvoice);
+
     OBDal.getInstance().save(newInvoice);
     return newInvoice;
   }
 
-  private DocumentType getInvoiceDocumentType() {
-    String hql = "from DocumentType dt where dt.salesTransaction = true" //
-        + " and dt.default = true and dt.table.id = :cInvoiceTableId" //
-        + " and Ad_Isorgincluded(:invoiceOrgId, dt.organization.id, :clientId) <> -1";
-
-    final Query<DocumentType> query = OBDal.getInstance().getSession()
-        .createQuery(hql.toString(), DocumentType.class);
-    query.setParameter("cInvoiceTableId", C_INVOICE_TABLE_ID);
-    query.setParameter("invoiceOrgId", this.shipment.getOrganization().getId());
-    query.setParameter("clientId", this.shipment.getClient().getId());
-    query.setMaxResults(1);
-
-    final DocumentType invoiceDocumentType = query.uniqueResult();
-
-    if (invoiceDocumentType == null) {
+  private DocumentType getDocumentTypeForARI(final Organization org) {
+    final List<Object> parameters = new ArrayList<>(3);
+    parameters.add(org.getClient().getId());
+    parameters.add(org.getId());
+    parameters.add("ARI");
+    final String documentTypeId = (String) CallStoredProcedure.getInstance().call("AD_GET_DOCTYPE",
+        parameters, null, false);
+    try {
+      return OBDal.getInstance().get(DocumentType.class, documentTypeId);
+    } catch (Exception e) {
       throw new OBException("There is no Document type for Sales Invoice defined");
     }
-    return invoiceDocumentType;
-  }
-
-  private Date getInvoiceDate() {
-    if (this.invoiceDate != null) {
-      return this.invoiceDate;
-    }
-    return this.shipment.getMovementDate();
-  }
-
-  private PriceList getPriceList() {
-    if (this.priceList != null) {
-      return this.priceList;
-    }
-    return this.shipment.getBusinessPartner().getPriceList();
   }
 
   private Currency getCurrency() {
     return (getPriceList() == null) ? null : getPriceList().getCurrency();
   }
 
-  private void processInvoice() throws Exception {
+  private void checkInvoiceHasAllMandatoryFields(final Invoice newInvoice) {
+    Check.isNotNull(newInvoice.getInvoiceDate(), OBMessageUtils.messageBD("ParameterMissing") + " "
+        + OBMessageUtils.messageBD(Invoice.PROPERTY_INVOICEDATE));
+    Check.isNotNull(newInvoice.getPriceList(), OBMessageUtils.messageBD("notnullpricelist"));
+    Check.isNotNull(newInvoice.getCurrency(), OBMessageUtils.messageBD("ParameterMissing") + " "
+        + OBMessageUtils.messageBD(Invoice.PROPERTY_CURRENCY));
+    Check.isNotNull(newInvoice.getPaymentMethod(), newInvoice.getBusinessPartner().getIdentifier()
+        + " " + OBMessageUtils.messageBD("PayementMethodNotdefined"));
+    Check.isNotNull(newInvoice.getPaymentTerms(), OBMessageUtils.messageBD("notnullpaymentterm"));
+  }
+
+  private void processInvoice() {
     if (invoice != null) {
-      final List<Object> parameters = new ArrayList<>();
+      final List<Object> parameters = new ArrayList<>(2);
       parameters.add(null); // Process Instance parameter
       parameters.add(invoice.getId());
       CallStoredProcedure.getInstance().call("C_INVOICE_POST", parameters, null, false, false);
     }
   }
-
-  protected void executeRollBack() {
-    try {
-      log.error("Error executing creating Invoice");
-      OBDal.getInstance().rollbackAndClose();
-    } catch (Exception e2) {
-      log.error("An error happened when rollback was executed.", e2);
-    }
-  }
 }