Redmine

I just noticed that the impact of this might be worse and more pervasive than earlier expected. I'll elaborate based on TS's STR:

• Create a new issues tree like:
  

  issue1
  issue2
   -> issue3
   -> issue4
   -> issue5
       -> issue6
  

• Create a new issue custom field (I tried with boolean type) with a default value
• ...several steps that are the same as in the initial description and which are not relevant here...
• Edit issue6, setting the custom field's value for the first time and to something other than the default

Observed behaviour

The custom value is saved correctly for issue6 (and the wrong journal is created as discussed earlier). But besides this, also issue5 and issue2 (the propagated parents of issue6) get the default value of the custom field set (without creating respective journals and without updating their 'updated' timestamps).

Desired Behavior

The default value for unset custom fields (where no corresponding custom_value exists) should not be set — silently — for parent issues (or any other issue) when updating a child issue (with an initial custom field value other than the default value).

A correction of this specific behavior is obviously not included in the proposed patch. This whole specific issue reminds me of issue #25806. I'll dig into that one to see if it's related in a way (see edit note).

Edit by Mischa The Evil: after triaging #25806, I've come to the conclusion that, while it is a valid defect, it does not seem to be related to this issue. For more details see #25806#note-1.

Imho the default value for unset custom fields (where no corresponding custom_value exists) should only be preset in the edit form, but not when merely showing the issue details. Under all circumstances the details view should be consistent with the issues list in this regard.

I agree that the details view should be consistent with the issue list. On the other hand, keeping the default value in the edit form for existing issues is more debatable as it raises the same issue (form content not consistent with the details view) and would cause an unexpected behaviour: a user who just enters a note will also generate a journal saying "custom field set to X".

Here is a simple fix for this that should also address the problem reported by Mischa for the parent issues (not tested).

Index: app/models/custom_value.rb
===================================================================
--- app/models/custom_value.rb    (revision 16595)
+++ app/models/custom_value.rb    (working copy)
@@ -24,7 +24,7 @@

   def initialize(attributes=nil, *args)
     super
-    if new_record? && custom_field && !attributes.key?(:value)
+    if new_record? && (customized.nil? || customized.new_record?) && custom_field && !attributes.key?(:value)
       self.value ||= custom_field.default_value
     end
   end

We should probably apply this fix and give the option of applying the default to all issues as proposed by Jens.

Looks like the best way to solve this.

Jean-Philippe Lang wrote:

Imho the default value for unset custom fields (where no corresponding custom_value exists) should only be preset in the edit form, but not when merely showing the issue details. Under all circumstances the details view should be consistent with the issues list in this regard.

I agree that the details view should be consistent with the issue list. On the other hand, keeping the default value in the edit form for existing issues is more debatable as it raises the same issue (form content not consistent with the details view) and would cause an unexpected behaviour: a user who just enters a note will also generate a journal saying "custom field set to X".

Good point. I had read this expectation as if it was about the new form, my mistake. So the conclusion of this could be that it's not viable to have #21074 fixed without side-effects mentioned in that issue and/or the ones described in this issue.

Jean-Philippe Lang wrote:

Here is a simple fix for this that should also address the problem reported by Mischa for the parent issues (not tested).

Confirmed that this patch fixes both problems. It is however (also) reverting the solution (r14773) which was added to fix the initial #21074. This is confirmed by the failing test:

  1) Failure:
Redmine::ApiTest::IssuesTest#test_PUT_/issues/:id.json_with_tracker_change_and_omitted_custom_field_specific_to_that_tracker_should_set_default_value [test/integration/api_test/issues_test.rb:752]:
Expected: "Default" 
  Actual: nil

Jean-Philippe Lang wrote:

We should probably apply this fix and give the option of applying the default to all issues as proposed by Jens.

[...]

• Target version changed from 3.2.7 to 3.5.0

Does this scheduling also cover delivery of an intermediate fix (like your proposed patch)? I'm asking because IMHO, the side-effects of the changes fixing #21074 are more disruptive than the initial problem they are supposed to fix (which already leads to users patching their instances/data: eg. Migration 3.0.3 -> 3.3.3 : Custom Fields and default values). Based on that, I would suggest to include the proposed (albeit #21074-reintroducing) patch in the next 3.2 and 3.3 maintenance releases and in the upcoming 3.4 stable release line until we have a solution (that removes the root cause of #21074 altogether) of handling custom field default values, like as proposed by Jens, that can be included in a future 3.5 feature release.
What do you think?