Fix the default value condition in NumericFieldDisplayDriver

[MikeAlhayek]

Fix #12971

[sebastienros]

Please check the other settings/fields/parts.

[jtkech]

We already had discussions on default values so not sure I agree with this PR.

Normally when we import items we populate data and use handlers for validations, not drivers.

But if you want to provide an editor experience maybe a wrong usage of build and update editor methods where isNew = true means that the Default value should be used, these methods are intended to be used for creating new items or editing existing ones.

To fit your use case where parts/fields are pre-populated, I think you need a custom impletention where you may have to build an editor with isNew = false.

---

As a side note I saw your demo about Excel import, what you can use for a long running process is the
ExecuteAfterEndOfRequestAsync() method of our HttpBackgroundJob component. It allows to register a job to start like a background task in an isolated scope but just after the current HTTP request is completed, so that the controller/page action can return immediately and the job can update some progress states that can be queried on the client side.

[jtkech]

Yes I remember a little a discussion around NULL which can be considered as a value, for example when we edit and then submit an empty string that is mvc bound to a null value.

Then when we edit again we may see that field.Value.HasValue is false but we still need to init the model with this NULL value, not fully sure but I think that's one reason we introduce the isNew parameter to fully make the difference between a new or existing value.

So not sure it's good to change the current pattern, I'm quite sure that if we fix the use case related to this PR we will break other use cases.

[MikeAlhayek]

@jtkech this should not break anything. Because if you are using what we have today, then passing contentItem to the drivers with IsNew most likely field.Value.HasValue will always be false forcing the second check and applying the DefaultValue just like it does today. So the behavior won't change. The only time this behavior change ONLY if the contentItem that is sent to the driver using IsNew has initialized values. So it does not change the current usage or break anything else.

DefaultValue should be applied ONLY if there is no preset value. We should respect existing value.

[sebastienros]

this should not break anything

How do we insert LMAO gifs here?

[jtkech]

@MikeAlhayek

Okay, maybe because I'm used to see you fully sure too quickly, but here it seems that you're right, so just to say to be careful ;)

[MikeAlhayek]

How do we insert LMAO gifs here?

@sebastienros "this should not break anything" != "this will not break anything".... Glad to see you interacting out side of the scope of meetings...

From what I can see here is the only other field settings that uses default value is BooleanFieldSettings. But, since the property is not nullable, it's not possible to do similar check to ensure we only apply DefaultValue for boolean field.

The LinkFieldSettings has also default value which I also changed here to the same logic.

[sebastienros]

@jtkech I think default values in this case are fine because there is a check that the property is null or has no value. If it had a value it would not use the default one on imports.

The only caveat is that we assume that the way to import a field's property with an empty value is to use "" for instance and not null.

[jtkech]

@sebastienros Okay then