Thread: [WebERP-developers] Fwd: Re: TableRows()

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Hi Ricard,

Only items with mbflag=='M' should have any QOO for WOs? Unless I am 
missing something?

I don't think we need to send $db in the function calls as this is now a 
global

Phil

Hi all:

I'm coding this change, and found that when calculating the QOO the
following scripts only check into PO, not WO.

Seems wrong to me as QOO should always include PO and WO, but before
making this change (use always PO and WO)would like your confirmation:

InventoryPlanningPrefSupplier
InventoryPlanning
StockLocStatus

Should we use also QOO due to WO on these scripts?

Regards,
Ricard

2014-10-03 20:54 GMT+08:00 Tim Schofield <tim...@gm...>:

> In fact a quick grep f the code shows the following all have a
> quantity on order calculation in them:
> 
> CounterReturns.php
> CounterSales.php
> InventoryPlanning.php
> InventoryPlanningPrefSupplier.php
> SelectOrderItems.php
> SelectProduct.php
> StockLocStatus.php
> StockStatus.php
> TopItems.php
> 
> Thanks
> Tim
> 
> On 3 October 2014 05:31, Pak Ricard <pak...@gm...> wrote:
> 
>> Hi all:
>> 
>> returning a moment to this...
>> 
>> I just got a mistake somewhere in the indexes (and got wrong QOO
> results).
>> Looking for the solution to this issue, I had to check 3 scripts
> and
>> discovered that:
>> 
>> We calculate the Quantity on Order for 1 item at least in 3 places,
> and all
>> 3 SQL are different,
>> 
>> At StockStatus.php:
>> 
>> SELECT
> SUM(purchorderdetails.quantityord-purchorderdetails.quantityrecd)
>> FROM purchorders LEFT JOIN purchorderdetails
>> ON purchorders.orderno=purchorderdetails.orderno
>> WHERE purchorderdetails.itemcode='" . $StockID . "'
>> AND purchorders.intostocklocation='" . $myrow['loccode'] . "'
>> AND (purchorders.status<>'Cancelled'
>> AND purchorders.status<>'Pending'
>> AND purchorders.status<>'Rejected'
>> AND purchorders.status<>'Completed')
>> and
>> 
>> SELECT SUM(woitems.qtyreqd-woitems.qtyrecd) AS qtywo
>> FROM woitems INNER JOIN workorders
>> ON woitems.wo=workorders.wo
>> WHERE workorders.closed=0
>> AND workorders.loccode='" . $myrow['loccode'] . "'
>> AND woitems.stockid='" . $StockID . "'"
>> 
>> At StockLocStatus.php
>> 
>> $sql = "SELECT SUM(purchorderdetails.quantityord -
>> purchorderdetails.quantityrecd) AS qoo
>> FROM purchorderdetails
>> INNER JOIN purchorders
>> ON purchorderdetails.orderno=purchorders.orderno
>> WHERE purchorders.intostocklocation='" . $myrow['loccode'] . "'
>> AND purchorderdetails.itemcode='" . $StockID . "'
>> AND (purchorders.status = 'Authorised' OR
> purchorders.status='Printed')
>> 
>> So, conditions are different. Some might say are equivalent, but
> this is a
>> weak point, as if we need to add, modify the PO status, then it
> won't work.
>> Also, there is no SQL for QOO due to Work Orders.
>> 
>> At SelectProduct.php
>> 
>> SELECT SUM(purchorderdetails.quantityord
> -purchorderdetails.quantityrecd) AS
>> QtyOnOrder
>> FROM purchorders INNER JOIN purchorderdetails
>> ON purchorders.orderno=purchorderdetails.orderno
>> INNER JOIN locationusers ON
>> locationusers.loccode=purchorders.intostocklocation AND
>> locationusers.userid='" . $_SESSION['UserID'] . "' AND
>> locationusers.canview=1
>> WHERE purchorderdetails.itemcode='" . $StockID . "'
>> AND purchorderdetails.completed =0
>> AND purchorders.status<>'Cancelled'
>> AND purchorders.status<>'Pending'
>> AND purchorders.status<>'Rejected'
>> 
>> and for the WO
>> 
>> SELECT SUM(woitems.qtyreqd-woitems.qtyrecd) AS qtywo
>> FROM woitems INNER JOIN workorders
>> ON woitems.wo=workorders.wo
>> INNER JOIN locationusers ON
> locationusers.loccode=workorders.loccode AND
>> locationusers.userid='" . $_SESSION['UserID'] . "' AND
>> locationusers.canview=1
>> WHERE workorders.closed=0
>> AND woitems.stockid='" . $StockID . "'
>> 
>> Besides the clear fact that SelectProduct is for All locations, the
> SQL for
>> PO and the user rights to that location... SQL is still missing
> the
>> condition "AND purchorders.status<>'Completed'", so it can lead to
> problems.
>> 
>> Woudn't it be better to have 2 functions:
>> Get_Quantity_On_Order_Due_To_Purchase_Orders(StockId, Location)
>> Get_Quantity_On_Order_Due_To_Work_Orders(StockId, Location)
>> if Location is "", then ALL locations are considered :-)
>> 
>> returning the value, so we would be sure that all webERP the data
> is fully
>> consistent?
>> Would be the script less readable?
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Regards,
>> Ricard
>> 
>> 2014-09-13 4:42 GMT+08:00 Tim Schofield
> <tim...@gm...>:
>>> 
>>> Hi Exson, actually Phil's suggestion
>>> http://sourceforge.net/p/web-erp/mailman/message/28434400/ [1] was
> to move
>>> the stock search into the SqlCommonFunctions.inc file rather than
> the
>>> MiscFunctions.php one in order to save keep repeating the same
> code
>>> across scripts.
>>> 
>>> Tim
>>> 
>>> On 12 September 2014 08:22, Tim Schofield
> <tim...@gm...>
>>> wrote:
>>> > Hi Exson,
>>> >
>>> > Well the point I was making was that the person reading the code
>>> > wouldn't know when they met the function call whether it was
>>> > abstracted to the bottom of that script, or to
> MiscFunctions.php. In
>>> > fact if it was me I would more likely expect it to be in
>>> > MiscFunctions.php.
>>> >
>>> > I would personally like it to be in a separate function in a
> separate
>>> > file. Phil did give the go ahead for stock search to be
> abstracted to
>>> > a separate file a few years ago, it was just nobody actually did
> the
>>> > code. This would allow us to use smarter search algorithms
> (soundex
>>> > for instance) without having to repeat it everywhere that stock
>>> > searching is done.
>>> >
>>> > Thanks
>>> > Tim
>>> >
>>> > On 12 September 2014 02:32, Exson Qu <hex...@gm...>
> wrote:
>>> >> Hi, Tim,
>>> >>
>>> >> Thank you for your mail.
>>> >> My initial intention for this modification is that
> we can
>>> >> reduce
>>> >> some lines in the script which maybe speed things up, and at
> the same
>>> >> time, the search items function has been used in many places,
> but
>>> >> unfortunately, it's not powerful enough, (or in other words,
> not
>>> >> flexible
>>> >> enough, since we have to add some constraint to the search
> result), so
>>> >> if
>>> >> only abstract to a function for existing codes, there is no big
>>> >> difference.
>>> >> As Phil said, there is slightly different sometime.That's why
> I did
>>> >> not
>>> >> transfer those functions to MiscFunctions.php.
>>> >> Best regards!
>>> >> Exson.
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> 2014-09-11 15:38 GMT+08:00 Tim Schofield
> <tim...@gm...>:
>>> >>>
>>> >>> Hi Exson, In SelectSalesOrders.php you abstracted the code to
> search
>>> >>> for items into its own function called GetSearchItems() and
> put the
>>> >>> function at the end of the script. Would it not make more
> sense to
>>> >>> have this in the MiscFunctions.php and use it wherever we
> search for
>>> >>> items? The abstraction has already been approved so we might
> as well
>>> >>> make it available everywhere. Then if we fix a bug, or do an
>>> >>> improvement to the searching algorithm we only do the change
> once
>>> >>> lessening the chance of bugs. The same in SelectOrderItems.php
> where
>>> >>> you abstracted the code to get the customer branch details to
> a
>>> >>> function GetCustBranchDetails(). This is code used in many
> scripts so
>>> >>> as the abstraction has been approved it makes sense to use it
>>> >>> everywhere.
>>> >>>
>>> >>> Often decisions on readability are taken that slow the code
> down not
>>> >>> speed it up. For instance if you remember from a discussion
> you and I
>>> >>> had with Phil the following SQL was considered more readable
>>> >>>
>>> >>> AND effectiveafter<'".date('Y-m-d')."'
>>> >>>
>>> >>> than using the ANSI standard constant:
>>> >>>
>>> >>> AND effectiveafter<CURRENT_DATE
>>> >>>
>>> >>> However the first method is clearly slower as it uses two
> string
>>> >>> concatenations which the PHP engine is very slow at.
>>> >>>
>>> >>> As I have said privately to a number of people on this
> discussion I
>>> >>> agree with the basic aims of the design of webERP but it is on
> their
>>> >>> implementation where we get a bit mixed up. We do abstract
> code into
>>> >>> functions but the when and where is not always consistent and
>>> >>> often as I demonstrated in my email about the table rows we
> end up
>>> >>> with less readable code.
>>> >>>
>>> >>> Thanks
>>> >>> Tim
>>> >>>
>>> >>> On 11 September 2014 03:39, ExsonQu <hex...@gm...>
> wrote:
>>> >>> > *Dear all,*
>>> >>> >
>>> >>> > I agree with Phil that we should pay more
> attention to
>>> >>> > those
>>> >>> > old
>>> >>> > codes' (or called redundant?). The abstraction to avoid have
> at
>>> >>> > least
>>> >>> > some
>>> >>> > merit that:
>>> >>> > 1. If there is a bug in the code, it's very
> clear.
>>> >>> > 2. I believe in most of time, it's fast.
>>> >>> > 3. If we change these tested stable codes, it
> becomes
>>> >>> > green
>>> >>> > code.
>>> >>> > It introduce new risk to our users.
>>> >>> > 4. And most of the time, it sacrifice the
> readability.
>>> >>> > Because we
>>> >>> > have to understand a new definition which seems intuitive to
> the
>>> >>> > author
>>> >>> > but
>>> >>> > others have to study it.
>>> >>> >
>>> >>> > I think most of the time, I don't like the code
> style of
>>> >>> > webERP
>>> >>> > because I cannot code fast. Its a hand-make style and not
> economic.
>>> >>> > But
>>> >>> > the
>>> >>> > style make those codes reliable and robust and fast. And
> it's very
>>> >>> > easy
>>> >>> > to
>>> >>> > modify what you need to do is just to search those strings
> you need.
>>> >>> >
>>> >>> > So lets focus on adding new features instead of
> those
>>> >>> > existed
>>> >>> > codes or the coding convention, IMHO.
>>> >>> >
>>> >>> > Best regards!
>>> >>> >
>>> >>> > Exson
>>> >>> >

Thread: [WebERP-developers] Fwd: Re: TableRows()

Accounting & Best Practice Business Administration System

web-erp-developers