-
Notifications
You must be signed in to change notification settings - Fork 7
Quantity support for volume and mass units and conversion between them #6918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
(just working here, not for 24.11)
…s conversion more consistent in a bunch of places. ColumnRenderProperties.getConvertFn()
|
WARNING: This PR appears to have the default title generated by GitHub. Please use something more descriptive. |
|
|
||
| if (null != expr && expr.canRender(ctx.getFieldMap().keySet())) | ||
| { | ||
| // TODO handle Quantity values here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@labkey-matthewb ???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there is a format we need to convert to defaultDisplayUnit() before calling .format()
| { | ||
| try | ||
| { | ||
| if (PropertyType.FILE_LINK.equals(col.getPropertyType()) && value instanceof String strVal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this if FileLine else block repeated a few times. Now that we have the col.getConvertFn() mechanism to handle conversion, could the FileLink check be handled as part of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe??? I like the idea, if the FILE_LINK case is handled consistently in the various code paths. I don't know if that is true.
Rationale
This pull request introduces support for handling columns with quantity units by recognizing a special unit suffix in column names, updating formatting and conversion logic, and enhancing how units are displayed in the UI. The changes are focused on enabling a feature flag for quantity column suffixes, updating value formatting to handle units, and ensuring proper data conversion and display.
Related Pull Requests
Changes
__<unit>suffix when theQUANTITY_COLUMN_SUFFIX_TESTINGfeature is enabled