[MGNLUI-4546] ConfiguredBinder assumes the null representation of text fields is always a string Created: 08/Aug/18  Updated: 24/Sep/18  Resolved: 20/Sep/18

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: None
Fix Version/s: 6.0

Type: Bug Priority: Neutral
Reporter: Roman Kovařík Assignee: Unassigned
Resolution: Obsolete Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File workaround.patch    
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Epic Link: UI framework implementation
Story Points: 5

 Description   

Easily reproducible with MGNLUI-4538. The progress columns fails to be edited if the property doesn't exists beforehand, falling back to null representation:

Caused by: java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Number
	at com.vaadin.data.converter.AbstractStringToNumberConverter.convertToPresentation(AbstractStringToNumberConverter.java:40) ~[vaadin-server-8.4.2.jar:8.4.2]
	at com.vaadin.data.Converter$2.convertToPresentation(Converter.java:173) ~[vaadin-server-8.4.2.jar:8.4.2]

This is caused be the fact that the null representation is always an empty string:

//see https://github.com/vaadin/framework/issues/9619
            if (TextFieldDefinition.class.isAssignableFrom(definition.getClass())) {
                bindingBuilder = (BindingBuilder<T, PT>) ((BindingBuilder<T, String>) bindingBuilder).withNullRepresentation("");
            }


 Comments   
Comment by Roman Kovařík [ 08/Aug/18 ]

I was able to workaround the problem by removing the current workaround from the binder and allowing null value for text fields instead but I'm not sure about the consequences.

Comment by Aleksandr Pchelintcev [ 08/Aug/18 ]

rkovarik I've added a link to the related vaadin issue, which supposedly has been fixed a while ago now and shouldn't bite us.

However, I see this a fairly logical issue - we are by default assigning a text field to edit a property of any type without supplying a proper conversion/validation setting. I see two ways out of this:
1) be a bit smarter re: components that we supply by default (i.e. we could create a simple mapping for at least primitive types)
2) provide a generic converter (e.g. based on BeanUtils) which would autoconvert between the type from the definition (configured propertty type) and the type of the configured editor component. This would require the knowledge of those type in runtime, but I presume we have that.

Comment by Roman Kovařík [ 08/Aug/18 ]

However, I see this a fairly logical issue - we are by default assigning a text field to edit a bq. property of any type without supplying a proper conversion/validation setting.

I see the point but this seems like an another problem/improvement since even with proper converter, the null representation is assigned and the converter can do nothing about it.

Comment by Antti Hietala [ 20/Sep/18 ]

Patch was applied. Closing as obsolete.

Generated at Mon Feb 12 09:17:42 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.