https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292022-05-24T01:07:49ZRedmineRedmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067602022-05-24T01:07:49ZGo MAEDA
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/27848">Defect #27848</a>: The progress exceeding 99.5% is displayed as 100%</i> added</li></ul> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067622022-05-24T02:02:52ZGo MAEDA
<ul></ul><p>The following change works in the reported case.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/models/issue.rb b/app/models/issue.rb
index ab9f794db..d957d5933 100644
</span><span class="gd">--- a/app/models/issue.rb
</span><span class="gi">+++ b/app/models/issue.rb
</span><span class="p">@@ -1851,7 +1851,7 @@</span> class Issue < ActiveRecord::Base
estimated * ratio
end
progress = done / (average * children.count)
<span class="gd">- p.done_ratio = progress.floor
</span><span class="gi">+ p.done_ratio = progress.to_f.floor
</span> end
end
end
</pre></pre>
<pre>
irb(main):029:0> done
=> 0.109e5
irb(main):030:0> average
=> 0.9909090909090909090909090909090909091e1
irb(main):031:0> children.count
=> 11
irb(main):032:0> progress
=> 0.99999999999999999999999999999999999999082568807e2
irb(main):033:0> progress.floor
=> 99
irb(main):034:0> progress.to_f
=> 100.0
irb(main):035:0> progress.to_f.floor
=> 100</pre></code></pre> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067632022-05-24T02:21:17ZGo MAEDA
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/33576">Defect #33576</a>: Done ratio of a parent issue may be shown as 99% even though all subtasks are completed</i> added</li></ul> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067652022-05-24T03:35:38ZGo MAEDA
<ul></ul><p>Another solution. Probably this is more accurate than <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100% (Closed)" href="https://www.redmine.org/issues/37151#note-2">#37151#note-2</a>.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/models/issue.rb b/app/models/issue.rb
index ab9f794db..519b26fd8 100644
</span><span class="gd">--- a/app/models/issue.rb
</span><span class="gi">+++ b/app/models/issue.rb
</span><span class="p">@@ -1838,19 +1838,20 @@</span> class Issue < ActiveRecord::Base
if children.any?
child_with_total_estimated_hours = children.select {|c| c.total_estimated_hours.to_f > 0.0}
if child_with_total_estimated_hours.any?
<span class="gd">- average =
- child_with_total_estimated_hours.sum(&:total_estimated_hours).to_d /
- child_with_total_estimated_hours.count
</span><span class="gi">+ average = Rational(
+ child_with_total_estimated_hours.sum(&:total_estimated_hours).to_s,
+ child_with_total_estimated_hours.count
+ )
</span> else
<span class="gd">- average = BigDecimal('1.0')
</span><span class="gi">+ average = Rational('1.0')
</span> end
done = children.sum do |c|
<span class="gd">- estimated = (c.total_estimated_hours || 0.0).to_d
</span><span class="gi">+ estimated = Rational(c.total_estimated_hours.to_f.to_s)
</span> estimated = average unless estimated > 0.0
ratio = c.closed? ? 100 : (c.done_ratio || 0)
estimated * ratio
end
<span class="gd">- progress = done / (average * children.count)
</span><span class="gi">+ progress = Rational(done, average * children.count)
</span> p.done_ratio = progress.floor
end
end
</code></pre>
<pre>
irb(main):023:0> done
=> (10900/1)
irb(main):024:0> average
=> (109/11)
irb(main):025:0> children.count
=> 11
irb(main):026:0> progress
=> (100/1)
irb(main):027:0> progress.floor
=> 100
</pre> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067672022-05-24T08:41:12ZGo MAEDA
<ul><li><strong>File</strong> <a href="/attachments/29231">37151.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29231/37151.patch">37151.patch</a> added</li></ul><p>Attaching a patch.</p> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067702022-05-24T09:30:09ZGo MAEDA
<ul><li><strong>Target version</strong> set to <i>Candidate for next minor release</i></li></ul> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067722022-05-24T09:46:23ZHolger Just
<ul></ul><p>I think your latter patch with the Rationals is more correct than mixing Decimals and Floats would be. Thank you Maeda-san!</p>
<p>As a slight optimization however, I don't think it is necessary to use <code>to_s</code> in the numbers before parsing the Rationals. I think <code>Kernel#Rational</code> also accepts Floats and Integers at least down to Ruby 2.4.</p>
<p>Thus, I believe this should be functionally equivalent to your patch in <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Defect: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100% (Closed)" href="https://www.redmine.org/issues/37151#note-5">#37151#note-5</a> above:</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/models/issue.rb b/app/models/issue.rb
index ab9f794db..519b26fd8 100644
</span><span class="gd">--- a/app/models/issue.rb
</span><span class="gi">+++ b/app/models/issue.rb
</span><span class="p">@@ -1838,19 +1838,20 @@</span> class Issue < ActiveRecord::Base
if children.any?
child_with_total_estimated_hours = children.select {|c| c.total_estimated_hours.to_f > 0.0}
if child_with_total_estimated_hours.any?
<span class="gd">- average =
- child_with_total_estimated_hours.sum(&:total_estimated_hours).to_d /
- child_with_total_estimated_hours.count
</span><span class="gi">+ average = Rational(
+ child_with_total_estimated_hours.sum(&:total_estimated_hours),
+ child_with_total_estimated_hours.count
+ )
</span> else
<span class="gd">- average = BigDecimal('1.0')
</span><span class="gi">+ average = Rational(1)
</span> end
done = children.sum do |c|
<span class="gd">- estimated = (c.total_estimated_hours || 0.0).to_d
- estimated = average unless estimated > 0.0
</span><span class="gi">+ estimated = Rational(c.total_estimated_hours.to_f)
+ estimated = average unless estimated > 0
</span> ratio = c.closed? ? 100 : (c.done_ratio || 0)
estimated * ratio
end
progress = done / (average * children.count)
p.done_ratio = progress.floor
end
end
</code></pre> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067732022-05-24T14:32:03ZGo MAEDA
<ul></ul><p>Holger Just wrote:</p>
<blockquote>
<p>As a slight optimization however, I don't think it is necessary to use <code>to_s</code> in the numbers before parsing the Rationals. I think <code>Kernel#Rational</code> also accepts Floats and Integers at least down to Ruby 2.4.</p>
</blockquote>
<p>Thank you for optimizing the patch. But I found that it breaks an existing test.</p>
<pre>
Failure:
IssueSubtaskingTest#test_done_ratio_of_parent_with_completed_children_should_not_be_99 [test/unit/issue_subtasking_test.rb:250]:
Expected: 100
Actual: 99
rails test test/unit/issue_subtasking_test.rb:244
</pre>
<p>This is because the value of <code>Rational("8.1")</code> and <code>Rational(8.1)</code> are not the same. <code>Rational("8.1")</code> represents 0.8 while <code>Rational(8.1)</code> represents 8.09999999999999964472.... So, I think it is safe to convert a float value to string before passing the value to Kernel.#Rational.</p>
<pre>
irb(main):001:0> Rational("0.1")
=> (1/10)
irb(main):002:0> Rational(0.1)
=> (3602879701896397/36028797018963968)
</pre> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067742022-05-24T23:49:17ZGo MAEDA
<ul></ul><p>Using <code>Float#to_d</code> instead of <code>Float#to_s</code> also works as expected. It may be better because using <code>Float#to_d</code> makes it clear that floating-point errors are taken into account.</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/models/issue.rb b/app/models/issue.rb
index ab9f794db..2b488c0fd 100644
</span><span class="gd">--- a/app/models/issue.rb
</span><span class="gi">+++ b/app/models/issue.rb
</span><span class="p">@@ -1838,19 +1838,20 @@</span> class Issue < ActiveRecord::Base
if children.any?
child_with_total_estimated_hours = children.select {|c| c.total_estimated_hours.to_f > 0.0}
if child_with_total_estimated_hours.any?
<span class="gd">- average =
- child_with_total_estimated_hours.sum(&:total_estimated_hours).to_d /
- child_with_total_estimated_hours.count
</span><span class="gi">+ average = Rational(
+ child_with_total_estimated_hours.sum(&:total_estimated_hours).to_d,
+ child_with_total_estimated_hours.count
+ )
</span> else
<span class="gd">- average = BigDecimal('1.0')
</span><span class="gi">+ average = Rational(1)
</span> end
done = children.sum do |c|
<span class="gd">- estimated = (c.total_estimated_hours || 0.0).to_d
</span><span class="gi">+ estimated = Rational(c.total_estimated_hours&.to_d || 0)
</span> estimated = average unless estimated > 0.0
ratio = c.closed? ? 100 : (c.done_ratio || 0)
estimated * ratio
end
<span class="gd">- progress = done / (average * children.count)
</span><span class="gi">+ progress = Rational(done, average * children.count)
</span> p.done_ratio = progress.floor
end
end
</code></pre> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1067942022-05-26T14:31:35ZGo MAEDA
<ul><li><strong>File</strong> <a href="/attachments/29238">37151-v2.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29238/37151-v2.patch">37151-v2.patch</a> added</li><li><strong>Target version</strong> changed from <i>Candidate for next minor release</i> to <i>5.0.2</i></li></ul><p>Setting the target version to 5.0.2.</p> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1068972022-06-08T15:40:59ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Resolved</i></li><li><strong>Assignee</strong> set to <i>Go MAEDA</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Committed the patch.</p> Redmine - Defect #37151: The done ratio of a parent issue may not be 100% even if all subtasks have a done ratio of 100%https://www.redmine.org/issues/37151?journal_id=1069122022-06-10T09:17:41ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li></ul><p>Merged to 5.0-stable in <a class="changeset" title="Merged r21626 from the trunk to 5.0-stable (#37151)." href="https://www.redmine.org/projects/redmine/repository/svn/revisions/21628">r21628</a>.</p>