Menu
▾
▴
Re: [WebERP-developers] Fwd: Re: TableRows()
|
From: Phil D. <ph...@lo...> - 2014-10-06 04:58:15
|
Yes makes sense. The $db call really needs to be taken out of all DB_query calls.... I will modify one to illustrate. Thanks for this nice contribution :-) Phil Phil Daintree Logic Works Ltd - +64 (0)275 567890 http://www.logicworks.co.nz On 06/10/14 12:12, Pak Ricard wrote: > Hi Phil: > > Sorry, I don't understand. What is the issue with the mbflag=='M' ? I > think I did not change the logic where the QOO was, just the > abstraction (at least that was my goal). > > If you refer to the issue that InventoryPlanningPrefSupplier, > InventoryPlanning and StockLocStatus did not used the WO to get the > QOO, I think it's still used as some items can be bought or > manufactured at the same time, so it's important to know how many you > already have ordered (via PO or via WO). > > Regarding the $db: I did it as many other functions in the same > include file use $db as a parameter. Would be good if we do not need > it (KISS principle). Could you please point me to an example on how to > use it properly as a global? > > > > Regards, > Ricard > > 2014-10-06 4:54 GMT+08:00 <ph...@lo... > <mailto:ph...@lo...>>: > > 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... <mailto: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... > <mailto: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... <mailto: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... <mailto: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... > <mailto: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... <mailto: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... > <mailto: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 > >>> >>> > > > ------------------------------------------------------------------------------ > Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer > Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS > Reports > Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper > Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer > http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk > _______________________________________________ > Web-erp-developers mailing list > Web...@li... > <mailto:Web...@li...> > https://lists.sourceforge.net/lists/listinfo/web-erp-developers > > > > > ------------------------------------------------------------------------------ > Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer > Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports > Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper > Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer > http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk > > > _______________________________________________ > Web-erp-developers mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/web-erp-developers |
