dbeaver/pro#7769 fix properties mapping for enums#4052
dbeaver/pro#7769 fix properties mapping for enums#4052sergeyteleshev merged 11 commits intodevelfrom
Conversation
devnaumov
requested review from
Nexus6v2,
SychevAndrey,
Wroud,
sergeyteleshev and
yagudin10
and removed request for
Wroud
| } | ||
|
|
||
| return value.displayName || value.value || JSON.stringify(value); | ||
| return value.value || value.displayName || JSON.stringify(value); |
There was a problem hiding this comment.
this change will affect behaviour of the getObjectPropertyDisplayValue probably in unexpected way
There was a problem hiding this comment.
Surely will but its right now. We just need to run auto-tests and fix the cases appeared in-place
There was a problem hiding this comment.
I think this is kind of expected. getObjectPropertyDisplayValue is intended to get property.value, always as a string, not property.displayName. Falling back to displayName is more of a last resort here and probably should be removed, since it’s not related to the property value.
The good news is that we use getObjectPropertyDisplayValue only in the metadata viewer, so it will be easy to check.
| } | ||
|
|
||
| return value.displayName || value.value || JSON.stringify(value); | ||
| return value.value || value.displayName || JSON.stringify(value); |
There was a problem hiding this comment.
Surely will but its right now. We just need to run auto-tests and fix the cases appeared in-place
sergeyteleshev
deleted the
dbeaver/pro#7769-fix-properties-mapping-for-enums
branch
7 participants
Closes https://github.com/dbeaver/pro/issues/7769
What changed
Now, values inside ObjectPropertyInfo (validValues and defaultValue) can be objects with { displayName, value }.
I created two additional functions to use when iterating over validValues or when getting a property value without considering the data type as getObjectPropertyValue fn relies on property type (for example, in the properties table).
Also there is a custom interface for object property so we can change type there and get type errors in our codebase. Previously there were not errors as types were always any.
However, it’s becoming clear that ObjectPropertyInfo is getting more complex to work with. Maybe we should consider implementing an ObjectViewerPropertyMapper class and use it like ObjectViewerPropertyMapper(property).defaultValue. If we always map properties through this object, the behavior would be more predictable in theory.