Defect #21074
closedWhen changing the tracker of an existing issue, new custom fields are not initialized with their default value
0%
Description
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. This issue affects every version of Redmine since 0.7 when defaults for custom fields were introduced.
Files
Related issues
Updated by Jean-Philippe Lang almost 9 years ago
Couldn't we just do:
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
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?
Updated by Holger Just almost 9 years ago
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.
Updated by Jean-Philippe Lang almost 9 years ago
- Status changed from New to Closed
- Assignee set to Jean-Philippe Lang
- Target version set to 3.2.0
- Resolution set to Fixed
Thanks.
Updated by Jens Krämer over 7 years ago
- File 0001-do-not-display-default-values-that-aren-t-actually-s.patch 0001-do-not-display-default-values-that-aren-t-actually-s.patch added
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.
Updated by Mischa The Evil over 7 years ago
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...
Updated by Mischa The Evil about 7 years ago
- Related to Defect #25726: Issue details page shows default values for custom fields that aren't actually set added
Updated by Mischa The Evil about 7 years ago
holger mareck: 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?