Lombiq/Helpful-Libraries

The existing field's DisplayName gets overwritten for no reason. (OSOE-474)

dministro opened this issue · 7 comments

The ContentPartDefinitionBuilder<TPart>.WithField<TField>(...) extension method overwrites the DisplayName of an existing field.

public ContentPartDefinitionBuilder<TPart> WithField<TField>(
Expression<Func<TPart, TField>> fieldPropertySelector,
Action<ContentPartFieldDefinitionBuilder> configuration = null)
{
var property = ((MemberExpression)fieldPropertySelector.Body).Member;
var name = property.Name;
Builder = Builder.WithField(
name,
field =>
{
field = field.WithDisplayName(name).OfType(typeof(TField).Name);
configuration?.Invoke(field);
});
return this;
}

Expected behavior:
Keep the original DisplayName value if present.

Jira issue

Isn't the point of this method that you use it to configure the field instead of doing it otherwise, even partially?

This is a good shortcut to creating and configuring a field in the Create() method of migration and it can be useful if we could use it in a UpdateFromX to partially change the field settings. But the current behavior overwrites the field DisplayName, so if I would use this to partially change the field settings, I should set the DisplayName too.

Is there any alternative shortcut like this to partially configure the field?

I see, yes, we should only override the display name with the technical name if the display name is not provided there yet (and then if you want to deliberately overwrite it, you can do that form the configuration parameter.

Implementation note: Instead of using WithDisplayName() we'll need to use MergeSettings() like WithDisplayName() internal also does, to do a conditional overwrite, since that's where the current display name can be retrieved.

I know of no alternatives.

the conditional overwrite requires current display name retrieval in public ContentPartDefinitionBuilder<TPart> WithField<TField>( Expression<Func<TPart, TField>> fieldPropertySelector, Action<ContentPartFieldDefinitionBuilder> configuration = null) which is not possible. Is it possible to modify the MergeSettings() by adding another boolean parameter that specifies if the overwrite is allowed? because the only way to retrieve the current display name is from MergeSettings()(If there is another way I am missing, please let me know). So if DisplayName is in the configuration parameter, the value of the boolean is True, otherwise, it is False.

I suggest you ask a question in our General Developers channel in Teams, where you lay out the problem, how you've tried to solve it, and where you face obstacles to high to overcome by yourself. There are more people available to help than just the two you've pinged.

Yep!