Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0009757Openbravo ERPZ. Otherspublic2009-07-03 11:152009-07-22 00:00
Reportershuehner 
Assigned Toharikrishnan 
PriorityimmediateSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Versionpi 
Target VersionFixed in Version 
Merge Request Status
Review Assigned To
OBNetwork customerNo
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0009757: API change:api check: build 62 fails
DescriptionThe following API check build failed:

http://builds.openbravo.com/job/erp_devel_pi-module-integrity-test/62/console [^]


+++++++++++++++++++++++++++++++++++++++++++++++++++
  Errors in API model validation
+++++++++++++++++++++++++++++++++++++++++++++++++++
  Column type change from date to NVARCHAR
  Column size decreased from 10 to 7: column: MA_INCIDENCE.STRTIME


Test did not validate API


java test:
Errors:

org.openbravo.model.manufacturing.transaction:
Bad
method org.openbravo.model.manufacturing.transaction.Incidence.getStartingTime(): type java.lang.String in api-checks/java/reference/250, but type java.sql.Timestamp in /var/www/localhost/htdocs/japi/62
Missing
method org.openbravo.model.manufacturing.transaction.Incidence.setStartingTime(java.lang.String): missing in /var/www/localhost/htdocs/japi/62
Steps To Reproduce
Proposed SolutionPlease add explanation why this change is needed and cannot be done without an API change and reassign issue to pjuvara.
Additional Information
TagsNo tags attached.
Relationships
related to defect 0007111 closed dmiguelez On Work Incidence, Starting Time field has no Time format 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2009-07-03 11:15shuehnerNew Issue
2009-07-03 11:15shuehnerAssigned To => harikrishnan
2009-07-03 11:15shuehnerOBNetwork customer => No
2009-07-03 11:15shuehnerRelationship addedrelated to 0007111
2009-07-03 11:16shuehnerNote Added: 0017776
2009-07-03 12:04shuehnerNote Added: 0017784
2009-07-03 12:41harikrishnanNote Added: 0017787
2009-07-03 12:41harikrishnanAssigned Toharikrishnan => pjuvara
2009-07-03 12:45harikrishnanNote Added: 0017788
2009-07-03 19:21pjuvaraNote Added: 0017901
2009-07-03 19:21pjuvaraStatusnew => scheduled
2009-07-03 19:21pjuvaraAssigned Topjuvara => harikrishnan
2009-07-03 19:21pjuvarafix_in_branch => pi
2009-07-03 19:24shuehnerNote Added: 0017903
2009-07-03 19:30pjuvaraNote Added: 0017904
2009-07-06 09:30rafarodaIssue Monitored: rafaroda
2009-07-10 16:20rafarodaNote Added: 0018114
2009-07-10 16:20rafarodaStatusscheduled => resolved
2009-07-10 16:20rafarodaResolutionopen => fixed
2009-07-10 16:20rafarodaFixed in SCM revision => https://code.openbravo.com/erp/devel/pi/rev/e2aa45438b62f2f32aca9eac409e6f8a7b2700bb [^]
2009-07-10 16:20rafarodafix_in_branchpi =>
2009-07-21 11:09plujanStatusresolved => closed
2009-07-22 00:00anonymoussf_bug_id0 => 2825055

Notes
(0017776)
shuehner   
2009-07-03 11:16   
The two errors, are coming from the same source. The change of a single column datatype.
(0017784)
shuehner   
2009-07-03 12:04   
@pjuvara:
If we allow this change we need extra effort to make a change in dbsourcemanager.

Right now this change does break update.database and (most likely) the content of the affected table is lost.
(0017787)
harikrishnan   
2009-07-03 12:41   
Since this field deals with the time we cant keep it as character in the database.Hence i made the change to timestamp.PJurvara whether we have to change in the API?.Hence im reassigning this issue to pjuvara.
(0017788)
harikrishnan   
2009-07-03 12:45   
Reference issue is https://issues.openbravo.com/view.php?id=7111 [^]
(0017901)
pjuvara   
2009-07-03 19:21   
I cannot accept this change for many reasons and it needs to be backed out and corrected.

Looking at the fix for issue 7111 it looks like in order to add proper UI level formatting, we changed the data type of the column. This is not needed in the first place.

Second, it looks like we change the type from DATE to NVARCHAR. DATE is a more appropriate data type to store date and time.

The correct fix for issue 7111 was to simply change the reference of the field to be time, without the need to change the data storage.

Last but not least, issue 7111 is low priority. We should never break a public API to fix a low priority issue.

In this case the correct course of action is to:
a) Re-open issue 7111
b) Backout the incorrect fix
c) Fix 7111 in the correct way
(0017903)
shuehner   
2009-07-03 19:24   
@pjuvara: The type was changed from NVARCHAR -> date. The output of the model test is incorrect and has the two relevant fields swapped (i created issue 9765 to track/fix this problem)
(0017904)
pjuvara   
2009-07-03 19:30   
After the comment by shuehner, I now understand that the fix was technically correct.
Yet, I believe it is not appropriate to break a public API in order to fix a minor issue that was not reported by a customer (7111 was reported internally by QA).

So I confirm my decision to reject this exception and I request the fix to be backed out.

Harikrishnan should investigate an alternative fix for 7111 that does not involve breaking the API. If that is not possible, we should postpone the fix for 7111 to after we have created the maintenance repository for 2.50 so that we can fix the issue for the next release without breaking the existing API.
(0018114)
rafaroda   
2009-07-10 16:20   
API change reverted https://issues.openbravo.com/view.php?id=7111#bugnotes [^]