Thanks Suraj, after reviewing that old commit I am inclined to think that the change you are suggesting makes sense. Before that old commit all the inventory items (regardless of their type and qty) were selected and there was logic to iterate thru the result set and exclude the ones with the wrong type and reserve only the ones with ATP. With that commit the type constraint was added to the query and also an additional constraint on QOH (rather than ATP): maybe at that time there was code requiring it or maybe it was done that way to be extra careful. I think we can now proceed as you suggest but before we do we should review the code that calls the following services: reserveProductInventoryByFacility reserveProductInventoryByContainer
and make sure that the change will not impact them negatively. Kind regards, Jacopo On Mon, Apr 9, 2018 at 3:27 PM, Suraj Khurana < suraj.khur...@hotwaxsystems.com> wrote: > Thanks Scott, > > I looked around and found some relevant commit. > IMO, it has been mistakenly committed as commit log also doesn't shows any > functional change in commit. > Here > <https://svn.apache.org/viewvc/ofbiz/trunk/applications/product/script/ > org/ofbiz/product/inventory/InventoryReserveServices.xml? > r1=650764&r2=650763&pathrev=650764> > is the link for reference. > > -- > Thanks and Regards, > *Suraj Khurana* | Omni-channel OMS Technical Expert > HotWax Commerce by HotWax Systems > Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010 > > > On Sat, Apr 7, 2018 at 3:24 AM, Scott Gray <scott.g...@hotwaxsystems.com> > wrote: > > > Hi Suraj, > > > > I haven't reviewed the code in question so I don't have any comment at > this > > stage. But one thing I want to point out is that OFBiz has many years of > > history available in commit logs, jira and mailing lists. It's often > quite > > a simple task to look back over that history and determine why a certain > > thing was done a certain way. > > > > As part of proposing a change to existing functionality it is extremely > > useful to anyone who might review the proposal to have some of that > context > > provided with the proposal. > > > > In this case it could be a simple matter of a past mistake being > overlooked > > until now, or it could be that using QOH was found to be beneficial for > > some reason that isn't immediately obvious. But without first > researching, > > we can't ever be sure of the answer. > > > > Regards > > Scott > > > > On Fri, 6 Apr 2018, 18:25 Suraj Khurana, <suraj.khurana@hotwaxsystems. > com> > > wrote: > > > > > Hello, > > > > > > While checking around code around inventory reservations, I was > surprised > > > to see that *reserveProductInventory *service only checks for QOH > > quantity > > > greater than one apart from that when *reserveFromInventoryItemInline > > *is > > > called, it checks for ATP confirming system to behave as required. > > > > > > Everything works fine but this is redundant code and we can have check > > for > > > ATP at top level so make reservations logic works faster. Is there any > > > other specific case I am missing or we can improve this flow by adding > > ATP > > > check at *reserveProductInventory* service as well. > > > > > > We can check QOH being on safer side, but ideally a system will always > > have > > > lesser ATP than QOH and logically we should only check for ATP while > > doing > > > reservations. > > > > > > -- > > > Thanks and Regards, > > > *Suraj Khurana* | Omni-channel OMS Technical Expert > > > HotWax Commerce by HotWax Systems > > > Plot no. 80, Scheme no. 78, Vijay Nagar, Indore, M.P. India 452010 > > > > > >
