Skip to content

Conversation

@cportele
Copy link
Contributor

@cportele cportele commented Aug 15, 2024

This change fixes two issues:

  • The label template was only applied, if there was an explicit label that differs from the name. It is now always applied, using the name as the default label.
  • The label template was not applied on properties in fragments. This is addressed by moving the schema transformation after resolving the schemas. So far the labelTemplate transformer was the only transformer applied in FeatureProviderSqlData while all the other transformers are applied in FeatureProviderSqlFactory.

In addition, after the schema resolver transformation has been applied, the fragments are no longer used and can be removed from the data.

This is an undocumented option. Shouldn't the option be documented?

Closes ldproxy/ldproxy#1259

This change fixes two issues:

- The label template was only applied, if there was an explicit label that differs from the name. It is now always applied, using the name as the default label.
- The label template was not applied on properties in `fragments`. This is addressed by moving the schema transformation after resolving the schemas. So far the labelTemplate transformer was the only transformer applied in `FeatureProviderSqlData` while all the other transformers are applied in `FeatureProviderSqlFactory`.

In addition, after the schema resolver transformation has been applied, the fragments are no longer used and can be removed from the data.
@cportele cportele requested a review from azahnen as a code owner August 15, 2024 15:43
@cportele cportele added the bug label Aug 15, 2024
@cportele cportele self-assigned this Aug 15, 2024
azahnen
azahnen previously approved these changes Aug 16, 2024
Copy link
Collaborator

@azahnen azahnen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, moving the resolving to the factory makes sense.
I think we should document it, since it should be stable now. Maybe we should we also add a default value?

@cportele
Copy link
Contributor Author

I will document it and then also add an ldproxy issue that we can reference in release notes.

The default would be {{value}} to be backwards compatible.

document `labelTemplate`
@azahnen
Copy link
Collaborator

azahnen commented Aug 16, 2024

@cportele I actually meant a default value containing the unit. Since most people don't use units and it only affects the labels, I would not deem this a breaking change. And it would make using units simpler and more intuitive.

@cportele
Copy link
Contributor Author

@azahnen - Since we and also others have sometimes included the unit manually in the label, this would then result in labels like Länge der Bohrung (m) [m] without changing the labels in the provider. In that sense, I think this is a breaking change for those APIs.

@azahnen
Copy link
Collaborator

azahnen commented Aug 16, 2024

@cportele Ok.

@cportele cportele merged commit 23b36ec into master Aug 16, 2024
@cportele cportele deleted the fix-label-template branch August 16, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix 'labelTemplate' option in the SQL feature provider

3 participants