Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0033814Openbravo ERPA. Platformpublic2016-08-25 15:442016-09-06 11:44
Reportercaristu 
Assigned Tocaristu 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target Version3.0PR16Q4Fixed in Versionpi 
Merge Request Status
Review Assigned Toshuehner
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression levelProduction - QA Approved
Regression date2016-06-10
Regression introduced in release
Regression introduced by commithttps://code.openbravo.com/erp/devel/pi/rev/ec3e4492a91c88e84d09e34b9e3e487f13ad3cb8 [^]
Triggers an Emergency PackNo
Summary0033814: Initialization in OBCriteria should not be prevented under some circumstances
DescriptionInitialization in OBCriteria should not be prevented under some circumstances, for example if we use list() with a new order by clause to an existing criteria after calling count(), i.e.:

  if (plVersionCrit.count() > 0) {
    plVersionCrit.addOrderBy(PriceListVersion.PROPERTY_VALIDFROMDATE, false);
    return plVersionCrit.list().get(0);
   }

The call to list does not initialize the query so it will skip the order by clause.
Steps To ReproduceThe PricelistVersionFilterExpression stops working as expected:

1 - Log in livebuilds 16Q3
2 - Go to Price List window and open "Tarifa de ventas"
3 - Create a new "Price list version", and use as base version "Tarifa de ventas"
4 - Go to "Product Price" tab
5 - Change the unit price of "Agua sin Gas 1L"
6 - Go to "Sales Order" and create a new sales order, select the price list "Tarifa de ventas"
7 - Create a new line
8 - Realize that when select the "Agua sin Gas 1L" product, the prices are the older prices.
Proposed Solution
Additional Information
TagsNo tags attached.
Relationships
related to defect 0033797 closed Triage Omni OMS Product selector in sales order lines, don't take the last price list version 
depends on backport 00338153.0PR16Q3.1 closed platform Initialization in OBCriteria should not be prevented under some circumstances 
caused by defect 0033138 closed alostale OBCriteria.initialize() is not protected against multiple calls per instance 
related to defect 0033816 closed caristu Find a complete solution to protect OBCriteria.initialize() against multiple calls per instance 
Attached Filestxt queries.txt (3,209) 2016-08-25 15:58
https://issues.openbravo.com/file_download.php?file_id=9754&type=bug
Issue History
Date ModifiedUsernameFieldChange
2016-08-25 15:44caristuNew Issue
2016-08-25 15:44caristuAssigned To => platform
2016-08-25 15:44caristuModules => Core
2016-08-25 15:44caristuRegression level => Production - QA Approved
2016-08-25 15:44caristuRegression date => 2016-06-10
2016-08-25 15:44caristuRegression introduced by commit => https://code.openbravo.com/erp/devel/pi/rev/ec3e4492a91c88e84d09e34b9e3e487f13ad3cb8 [^]
2016-08-25 15:44caristuTriggers an Emergency Pack => No
2016-08-25 15:44caristuStatusnew => acknowledged
2016-08-25 15:44caristuStatusacknowledged => scheduled
2016-08-25 15:44caristuTarget Version => 3.0PR16Q4
2016-08-25 15:45caristuRelationship addedrelated to 0033797
2016-08-25 15:56shuehnerRelationship addedcaused by 0033138
2016-08-25 15:58caristuFile Added: queries.txt
2016-08-25 15:59caristuNote Added: 0089408
2016-08-25 16:00shuehnerNote Added: 0089409
2016-08-25 16:07hgbotCheckin
2016-08-25 16:07hgbotNote Added: 0089410
2016-08-25 16:07hgbotStatusscheduled => resolved
2016-08-25 16:07hgbotResolutionopen => fixed
2016-08-25 16:07hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/e309320cd579c59080b4bb04a060b2862840ab2e [^]
2016-08-25 16:16caristuReview Assigned To => shuehner
2016-08-25 16:16caristuIssue Monitored: shuehner
2016-08-25 16:18caristuRelationship addedrelated to 0033816
2016-08-25 16:48shuehnerNote Added: 0089412
2016-08-25 16:48shuehnerStatusresolved => closed
2016-08-25 16:48shuehnerFixed in Version => pi
2016-08-25 22:53hudsonbotCheckin
2016-08-25 22:53hudsonbotNote Added: 0089419
2016-09-06 11:44caristuAssigned Toplatform => caristu

Notes
(0089408)
caristu   
2016-08-25 15:59   
Attached queries executed by the PricelistVersionFilterExpression for the count() and list() operations. Note that the order by clause is missing.
(0089409)
shuehner   
2016-08-25 16:00   
Bug is that the change did prevent calling 'initialize()' multiple times on the same instance to prevent double-adding some filters to the generated query (i.e. client or org filters).
However the skipped method is also in charge of 'adding orderBy'

So given following calling code (as used in 33797 before fixing 0033705)
    if (plVersionCrit.count() > 0) {
      plVersionCrit.addOrderBy(PriceListVersion.PROPERTY_VALIDFROMDATE, false);
      return plVersionCrit.list().get(0);
    }

The SQL generated for the list call is completely missing the order by.

So the change must be revisited to only skip part which are known to be needed really just once, and cannot change.
Note: even client+org filter may change if after a first i.e. count call setFilterOnReadable ... methods or similar are called.

So maybe a different fix could be to not skip 'initialize()' call at all.
But instead make it 'start from 0' fresh to add all current filters to a completely clean hql.
(0089410)
hgbot   
2016-08-25 16:07   
Repository: erp/devel/pi
Changeset: e309320cd579c59080b4bb04a060b2862840ab2e
Author: Carlos Aristu <carlos.aristu <at> openbravo.com>
Date: Thu Aug 25 16:06:43 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/e309320cd579c59080b4bb04a060b2862840ab2e [^]

fixes issue 33814: backed out initialization prevention changes

---
M src/org/openbravo/dal/service/OBCriteria.java
---
(0089412)
shuehner   
2016-08-25 16:48   
Verified that change if correctly backing out initial change.

For testing, undo of 33705
And tested that for the use-case described in this issue the order is no longer missing for the 2nd .list() query.
(0089419)
hudsonbot   
2016-08-25 22:53   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/5e50832c9b35 [^]
Maturity status: Test