Cannot export windows via 2pack if any field has a Field Group
Description
Environment
Attachments
- 22 Dec 2020, 06:23 PM
is caused by
Activity
Heng Sin Low December 24, 2020 at 1:32 AMEdited
The isText is an obvious bug that should be addressed.
The replacement of DisplayType.List comparision with isList is a very low risk refactoring change, I don’t think you have to test them all (very very unlikely to have any issue with replacing if (DisplayType.List == displayType) with if (DisplayType.isList(displayType)).
Actually, the isText fix has a higher chance of producing unexpected side effect than isList - that has potential to change existing flow as you can have isText comparison that comes before DisplayType.List == displayType check.
Diego Ruiz December 23, 2020 at 1:48 PM
Hi @Carlos Ruiz / @Heng Sin Low,
I think both comments are exclusive, after checking, the 2pack problem, adding List to DisplayType.isText would be enough.
Changing
else if (DisplayType.List == displayType) to
else if (DisplayType.isText(displayType))would produce the same outcome as the current patch, the Strings and List objects are handled by the same method in PoExporter. A similar validation is in the iDempiere-rest and so on.
However, being honest, changing the code in the other parts of the code that you mention is something big with many potential side-effects and I’m not sure how to test them all.
If you guys agree with my proposed solution, adding List to the isText method and changing it in the PoExporter as a first instance, I can proceed and modify the pull request.
Best regards,
Diego Ruiz
Carlos Ruiz December 23, 2020 at 11:43 AM
Another strange thing found:
DisplayType.isText includes all references that are saved as Text (including RadioGroupList and the ChosenMultiple, but it doesn't include List - that sounds like an missing reference there.
Carlos Ruiz December 22, 2020 at 7:35 PM
Hi @Diego Ruiz,
I searched for DisplayType.List in my eclipse and I found many occurrences of the same issue.
For example found 4 times the same problem in idempiere-rest, 2 times in omnisearch, 1 time in kanbanboard, 4 in fitnesse.
And a lot of occurrences in core (MColumn, MLookupFactory, MProcessPara, PO, DataEngine, RColumn, PackRollProcess, WWFActivity, InfoWindow, InfoGeneralPanel, DB_Oracle, DB_PostgreSQL, ModelADServiceImpl, Process)
I think maybe would be better to create a method DisplayType.isList - similar to the other is* methods, and replace the comparisons against DisplayType.List by calling to that method.
Regards,
Carlos Ruiz
When trying to export a window via 2pack with a newly created Field Group the Export Package process fails.
I reproduced the test case in https://demo.globalqss.com/webui/ (version 8.2) (Window Testing)
Steps to reproduce:
Login as System
Create a new Window in Window, Tab & Field
Run the copy Window Tabs process
Select any window
Navigate to a Field
Create a new Field Group record with any field group type or null
Set the newly created Field Group to a Field from the above-created window
Open the Pack out Window
Create a new record
Create a new package detail
Type = Window
Window = the new window
Run the Export Package process
Expected result
The window is exported
Actual result
The process fails with the following message: "Failed to export window tab"
The logs show this error:
19:01:12.425===========> TabElementHandler.create: java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap') [265] org.xml.sax.SAXException: java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap') java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer (java.lang.String and java.lang.Integer are in module java.base of loader 'bootstrap') at org.adempiere.pipo2.handler.FieldElementHandler.create(FieldElementHandler.java:133)After debugging the issue I found that the problem is that the FieldGroupType in AD_FieldGroup has the new type Radio button.