Vote #76849
完了When changing the tracker of an existing issue, new custom fields are not initialized with their default value
0%
説明
Given two configured trackers "Support" and "Task" with different enabled custom fields. There is a required boolean custom field named "Bool" which is only enabled on the Task tracker. The custom field defaults to "No".
Given an existing saved issue on the Support tracker, we navigate to the issue update form and select the Task tracker which has the additional custom field defined. When rendering the form, the value of the CustomValue for the "Bool" field is not properly initialized with the default value. This results in the form rendering with no explicitly selected value. The browser thus selects the first value of the list, i.e. "Yes" by default. On saving, the custom field is set to Yes because the browser sends this value, despite the default of the field being set to "No". This can be very surprising as the user typically has not updated the field manually and has the possibility to break implicit workflows.
The cause of this issue is that in @CustomValue#initialize@, the value is only set as the default if the customized object (the issue in this case) is still new and unsaved. Because of this logic, selecting a tracker works when creating a new issue; in this case, the custom values are properly initialized with the default value. In our case however, the issue is not new anymore but would still need the default value initialized.
Unfortunately, a fix it probably isn't straightforward to implement as it seems @nil@ is a value value today to denote "Not set" for fields which do not require a value. Thus, a possible fix could be to distinguish between an empty string and @nil@ here and set the default value for @nil@ values. This change would have to be checked against the API.
At the very least, the CustomField inititializer could check if the field is required (and thus doesn't allow @nil@ / empty string as a valid value anyway) and force the nil value to the field's default value here. At least this should be changed so that required fields have their default value set.
I'm asking for thoughts here first. If there is a consensus about how to proceed, I can provide a patch, courtesy of "Planio":https://plan.io/redmine-hosting. This issue affects every version of Redmine since 0.7 when defaults for custom fields were introduced.
journals
Couldn't we just do:
<pre>
Index: app/models/custom_value.rb
===================================================================
--- app/models/custom_value.rb (revision 14752)
+++ app/models/custom_value.rb (working copy)
@@ -22,7 +22,7 @@
def initialize(attributes=nil, *args)
super
- if new_record? && custom_field && (customized_type.blank? || (customized && customized.new_record?))
+ if new_record? && custom_field
self.value ||= custom_field.default_value
end
end
</pre>
That would fix this issue and set the default value as well for custom fields that are added after an issue is created.
Is there any problem with that?
--------------------------------------------------------------------------------
This was my first guess too. I'm slightly concerned however, that this could change the custom fields on save too. Right now, @nil@ and the empty string seems both to be valid values for the "blank" value of non-required custom fields.
While your proposed patch solves the immediate problem, it might have undesired side-effects when people send @nil@ as a custom field value, e.g. via the API or by omitting the field value in a request.
You could argue that this is the desired behavior, i.e. sending @nil@ results in the default value being set. But it would change existing behavior here and might break certain API use-cases where users send @nil@ instead of an empty string and expect the blank value to be set. After having thought about this for a bit longer, it is however probably worth the change, as it makes the final behavior more obvious and consistent.
Thus, finally, +1 from me for your proposed patch.
--------------------------------------------------------------------------------
Thanks.
--------------------------------------------------------------------------------
Actually I think there is a problem with this.
Steps to reproduce:
* Create a new issue.
* Create a new boolean issue custom field with a default of yes or no and _use as filter_ set.
* look at the previously created issue in issues list (have the custom field displayed) and detail view
* note the different values being shown for the custom field - issue details page shows the configured default, while the issue list shows the field as unset (empty).
* When filtering the issue list for the field's value (or simply looking at the database) it turns out that the field is indeed unset (neither filtering for the field being 'yes' or 'no' turns up the issue). So clearly the details view is lying about the field's true value by replacing _unset_ with the configured default.
I think this also applies to other field formats, not just booleans.
Imho the default value for unset custom fields (where no corresponding custom_value record exists) should only be preset in the edit form, but not when merely showing the issue details. I came up with a patch that checks if there is actually any custom value present in the database for the given field before calling @show_value@, but to be honest I'm not very happy with that.
--------------------------------------------------------------------------------
Jens, your comment made me re-read this whole issue and it reminded me of an issue I encountered and reported a long time ago (#9849). As of today I don't know whether or not that issue is still present and still reproducible (and I'm currently unable to test this vigorously), but I can't help thinking it might be related to this issue in a sense. So to be sure, I post this comment to bring that issue to the attention within the context of this current issue.
However, since this issue has been closed targeted against 3.2.0, it might be a better idea to create a new issue altogether...
--------------------------------------------------------------------------------
posted as new issue #25726
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
@Holger: as being the OP of this issue, do you maybe have have some ideas on #25726? What do you think about the solution of handling custom field default values as proposed by Jens?
--------------------------------------------------------------------------------
related_issues
relates,Closed,25726,Issue details page shows default values for custom fields that aren't actually set