Redmine

Just my two cents on this: if a feature like this is deemed eligible for core integration, I think it might be good to preemptively consider, explicitly, whether or not it might be better to not reuse the global issue_done_ratio_interval setting, but instead implement it independently as a custom field attribute so that each progress bar custom field can have its own dedicated done_ratio_interval value.

The first iteration of this which I had implemented at Planio actually had this setting on the custom field in place. We dropped it in the end to keep it simple and because we considered that someone wishing to have 5% intervals would likely want to have that level of precision everywhere.

But it certainly is cleaner to have the setting on the custom field, since these are not bound to issues only, but could be used anywhere, and then it is actually confusing when an issue tracking setting influences a progress bar field on a document, for example. So, +1 from me.

Marius BĂLTEANU wrote in #note-8:

If we really want to have a dependency on Issue Done Ratio Interval, we can set the default value of this new attribute to the issue_done_ratio_interval or add a special value that inherit the value of issue_done_ratio_interval.

I noticed that the default value of the new "Ratio interval" for custom fields introduced in the additional patch is set to 5%, whereas the existing "Done Ratio options interval" has a default of 10%. This difference might be confusing. For consistency, it may be better to either set the default to 10% or inherit the value from issue_done_ratio_interval.

Go MAEDA wrote in #note-10:

Marius BĂLTEANU wrote in #note-8:

[..]

I noticed that the default value of the new "Ratio interval" for custom fields introduced in the additional patch is set to 5%, whereas the existing "Done Ratio options interval" has a default of 10%. This difference might be confusing. For consistency, it may be better to either set the default to 10% or inherit the value from issue_done_ratio_interval.

I agree with you regarding 10% as default. Also, I think we should persist the value in the database for each custom field saved.

Marius BĂLTEANU wrote in #note-12:

I've updated the patch to persist the interval ratio for all custom fields of type progress bar. The default value is inherit from Setting.issue_done_ratio_interval.to_i.

Thank you for posting the patch.

I found that the value of Setting.issue_done_ratio_interval will not be reflected on /custom_fields/*/edit pages unless the Redmine application is restarted. This is because the constant DEFAULT_RATIO_INTERVAL is initialized only when the application starts.

To fix the issue, I suggest changing the patch as follows:

diff --git a/app/views/custom_fields/formats/_progressbar.html.erb b/app/views/custom_fields/formats/_progressbar.html.erb
index 9077aebaf..02613e5e5 100644
--- a/app/views/custom_fields/formats/_progressbar.html.erb
+++ b/app/views/custom_fields/formats/_progressbar.html.erb
@@ -1,3 +1,3 @@
 <p>
-  <%= f.select :ratio_interval, [5, 10].collect {|i| ["#{i} %", i]}, selected: Redmine::FieldFormat::ProgressbarFormat::DEFAULT_RATIO_INTERVAL, required: true %>
-</p>
\ No newline at end of file
+  <%= f.select :ratio_interval, [5, 10].collect {|i| ["#{i} %", i]}, selected: Redmine::FieldFormat::ProgressbarFormat.default_ratio_interval, required: true %>
+</p>
diff --git a/lib/redmine/field_format.rb b/lib/redmine/field_format.rb
index da46a4d5c..2a76f2c46 100644
--- a/lib/redmine/field_format.rb
+++ b/lib/redmine/field_format.rb
@@ -1092,7 +1092,9 @@ module Redmine

       # Take the default value from Setting.issue_done_ratio_interval.to_i
       # in order to have a consistent behaviour for default ratio interval.
-      DEFAULT_RATIO_INTERVAL = Setting.issue_done_ratio_interval.to_i
+      def self.default_ratio_interval
+        Setting.issue_done_ratio_interval.to_i
+      end

       def label
         "label_progressbar" 
@@ -1121,7 +1123,7 @@ module Redmine
         super

         if custom_field.ratio_interval.blank?
-          custom_field.ratio_interval = DEFAULT_RATIO_INTERVAL
+          custom_field.ratio_interval = self.class.default_ratio_interval
         end
       end

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 005864d4c..9b6feac5f 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -5987,7 +5987,6 @@ class IssuesControllerTest < Redmine::ControllerTest
     get(:edit, :params => {:id => 1})
     assert_response :success

-    puts response.body
     assert_select "select[id=?]", "issue_custom_field_values_#{cf.id}", 1
   end

Go MAEDA wrote in #note-13:

Marius BĂLTEANU wrote in #note-12:

I've updated the patch to persist the interval ratio for all custom fields of type progress bar. The default value is inherit from Setting.issue_done_ratio_interval.to_i.

Thank you for posting the patch.

I found that the value of Setting.issue_done_ratio_interval will not be reflected on /custom_fields/*/edit pages unless the Redmine application is restarted. This is because the constant DEFAULT_RATIO_INTERVAL is initialized only when the application starts.

To fix the issue, I suggest changing the patch as follows:

[...]

Thanks for catching and fixing those errors!