Delivery policy to prevent orders from "starving"

Daniel, thanks, sounds like a great improvement.

Regards,

Carlos Ruiz

I also want to second, Carlos, comments.
Description of the functionality on the wiki page looks very well.

I'm keen to see it and try it.

Thanks for the encouragement :-) I'm very close to a working solution now.
I'll post it here as a patch first.

Attaching patches and DB-scripts. The patches are created from revision 12428.
If anyone is interested in making the equivalent oracle script for XXX-B-FR 3004020 Delivery Policy.sql I'd be very happy.

Patch FR 3004020-rev 12428

First SQL-file

Second SQL file

First SQL-file Oracle

Second SQL-file Oracle

I managed to convert the PostgreSQL script to Oracle PL/SQL.
Uploaded new Oracle scripts.
Will commit tomorrow.

+ 1 vote since it does not break backward compatibility rule and is well explained and i also not aware of Libero version on this.

http://adempiere.svn.sourceforge.net/adempiere/?rev=12757&view=rev

Hi,

Here is my quick and partial code review.

AllocateSalesOrders.getOrderLinesToAllocate(Connection conn, int productId, String trxName)
* don't use the conn parameter. When u need to create a prepared statement you can use DB.prepareStatement
* please always close the PreparedStatement and ResultSet in a result set
Please apply ABP from http://www.adempiere.com/index.php/ADempiere_Best_Practices#How_to_use_PreparedStatement.2FResultSet_.3F

AllocateSalesOrders.doIt()
* you could use Java5 syntax in order to iterate a List or Vector like:
for (Iterator<StockInfo> it = products.iterator(); it.hasNext();)

MaterialReceiptModelValidator.docValidate()
* why are u using Connection here.
* why are u creating a new transaction here. Please use the transaction name that comes from PO object that u have as an argument. If u want to revert on a specific point, please use savepoints (see Trx.setSavepoint).

I_AD_ClientInfo
MClientInfo
* why did u add DELIVERY_POLICY_STRICT_ORDER and DELIVERY_POLICY_NO_HOLD to MCLientInfo. When u created the AD_ClientInfo.DeliveryPolicy field, you should made it as List and create a list for those constants as "Reference".
Same for AD_OrgInfo.

Ok, as a general rule from what i've seen here are:
* don't use Connection. Use instead DB helper class
* In most of the cases u don't need to pass trxName as argument to non static methods because the transaction should come from the PO itself or from other PO passed as parameter. Please review this in your code.

Best regards,
Teo Sarca

Thanks for the review :-) I'll adjust according to your suggestions.
If possible, could you give me a quick brief on creating constants for lists. I tried to figure it out but didn't get far enough on that.

> If possible, could you give me a quick brief on creating constants for
> lists. I tried to figure it out but didn't get far enough on that.

at short, go to table and columns, go and select "Delivery_Policy" column. Instead of String set the reference as List. On Reference Value you will need to set the list that you need previously to create.

Best regards,
Teo Sarca

One more thing,
> Thanks for the review :-) I'll adjust according to your suggestions.

Yes, the idea is that your code should be complient with ABP (http://www.adempiere.com/index.php/ADempiere_Best_Practices) to reach the trunk.

Best regards,
Teo Sarca

Hi Daniel !

My comment about your contribution are:

1.- we are using the Query class to create the instante any persistence Objects, and the use the JDBC should be deprecate.

List<MOrderLine> lines = new Query(getCtx(), I_C_OrderLine.Table_Name, whereClause, get_TrxName()).setParameters(,,,,,,)list(); .....

Please see :

2.- If you want use object collection please use List interface vs Vector

Vector<MOrderLine> result = new Vector<MOrderLine>();

http://www.adempiere.com/index.php/ADempiere_Best_Practices#How_to_use_Adempiere_Query_class.3F

3.-please you change not the sing method because this break the back compatibility

I saw that you change the sing method :

/**
* Update Storage Info add.
* Called from MProjectIssue
* @param ctx context
* @param M_Warehouse_ID warehouse
* @param M_Locator_ID locator
* @param M_Product_ID product
* @param M_AttributeSetInstance_ID AS Instance
* @param reservationAttributeSetInstance_ID reservation AS Instance
* @param diffQtyOnHand add on hand
* @param diffQtyReserved add reserved
* @param diffQtyOrdered add order
* @param trxName transaction
* @return true if updated
*/
public static boolean add (Properties ctx, int M_Warehouse_ID, int M_Locator_ID,
int M_Product_ID, int M_AttributeSetInstance_ID, int reservationAttributeSetInstance_ID,
BigDecimal diffQtyOnHand,
BigDecimal diffQtyReserved,
BigDecimal diffQtyOrdered,
BigDecimal diffQtyAllocated,
String trxName)
{

Now the libero MFG break for this reason.

please see

http://www.adempiere.com/index.php/ADempiere_Best_Practices#How_to_change_parameters_of_existing_method_from_ADempiere_core

4.- I saw that create a new transaction in MaterialReceiptModelValidator, and I think you should use the same the persisten object .

allocateProducts(conn, trxName, line);

and you should use

allocateProducts(conn, PO.get_TrxName(), line);

So is not necessary create a new transaction.

Daniel please if you need some help or support to solve this point you can contact me.

Now I offert as voluntary to validate each commit in trunk, and this commit break of back compatibility, So you should solve this issue.

I think give some days before of revert this revision.

kind regards
Victor Perez

@Victor

Sorry about the breaking of backwards compat for MStorage. Now it's corrected.

http://adempiere.svn.sourceforge.net/adempiere/?rev=12778&view=rev

It was necessary to add a column for allocated but of course I should have left a backwards compat method as well. I wasn't thinking MStorage was used outside the main project but I guess one should never assume.

In MaterialReceiptModelValidator; if I use the PO.get_TrxName, is that automatically committed when the validator is finished? That seems to be a better way, thanks!

I'll look through the rest of the suggestions now.

@Teo
Removed the constants in MClientInfo (I had already generated them without knowing :-) ).

http://adempiere.svn.sourceforge.net/adempiere/?rev=12785&view=rev

About the Trx and Connections. I'll look over that as I continue to test this feature with high volumes. I don't want to change it right now because what I committed now is tested and if I make a quick change now I'll be committing untested code. Hope that's ok with you that I do it in that order.

Best regards
/Daniel T

@Victor
About using the Query class. I'm using that as far as possible. With the particular cases in AllocateSalesOrder it was not possible nor efficient to use the Query for the specific queries. It would kill the performance. I did the best "middle way" I could.

Best regards
/Daniel

Hi Daniel,

Do you have any updates on this tracker?

Best regards,
Teo Sarca

Hi Teo,

I haven't had time to do much more testing. Is there anything in particular that you think is urgent? If so I can try to put some higher priority on that.

Best regards
/Daniel T

Changed transaction and connection handling according to ABP.

http://adempiere.svn.sourceforge.net/adempiere/?rev=12968&view=rev

Minor GUI adjustment.

http://adempiere.svn.sourceforge.net/adempiere/?rev=13190&view=rev

SF3004020 - Delivery policy. Show QtyAllocated in Warehouse Tab of InfoProduct window.