Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[50953] Progress reporting for work package hierarchies: Change calculation and name of Work and Remaining work #14216

Merged
99 changes: 53 additions & 46 deletions spec/services/work_packages/update_ancestors_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -575,67 +575,74 @@
end

context 'for the new ancestors chain' do
shared_context 'when called with children' do |children_remaining_hours:|
let(:children) do
(children_remaining_hours.size - 1).downto(0).map do |i|
create(:work_package,
subject: "child #{i}",
parent:,
status: open_status,
remaining_hours: children_remaining_hours[i],
derived_remaining_hours: children_remaining_hours[i])
end
end
context 'with parent having no remaining work' do
let_work_packages(<<~TABLE)
hierarchy | remaining work |
parent | |
child1 | 0h |
child2 | |
child3 | 2.5h |
TABLE

subject(:call_result) do
described_class.new(user:, work_package: children.first)
described_class.new(user:, work_package: child1)
.call(%i(remaining_hours))
end
end

shared_examples 'derived remaining hours' do |children_remaining_hours:, expected_derived_remaining_hours:|
context "for #{children_remaining_hours.count} children with remaining hours being #{children_remaining_hours.inspect}" do
include_context('when called with children', children_remaining_hours:)

it "sets parent derived remaining hours to #{expected_derived_remaining_hours}", :aggregate_failures do
expect(call_result).to be_success
expect(call_result.dependent_results.map(&:result))
.to contain_exactly(parent)
expect(call_result.dependent_results.first.result.derived_remaining_hours)
.to eq(expected_derived_remaining_hours)
end
it 'sets parent derived remaining work to the sum of children remaining work' do
expect(call_result).to be_success
updated_work_packages = call_result.dependent_results.map(&:result)
expect_work_packages(updated_work_packages, <<~TABLE)
subject | derived remaining work
parent | 2.5h
TABLE
end
end

context 'with parent having no remaining hours' do
include_examples 'derived remaining hours',
children_remaining_hours: [0.0, 2.0, nil],
expected_derived_remaining_hours: 2.0
context 'with parent having some remaining work' do
let_work_packages(<<~TABLE)
hierarchy | remaining work |
parent | 5.25h |
child1 | 0h |
child2 | |
child3 | 2.5h |
TABLE

include_examples 'derived remaining hours',
children_remaining_hours: [1, 2, 5, 42],
expected_derived_remaining_hours: 50
end

context 'with parent having 2.0 remaining hours' do
before do
parent.update(remaining_hours: 2.0)
subject(:call_result) do
described_class.new(user:, work_package: child1)
.call(%i(remaining_hours))
end

include_examples 'derived remaining hours',
children_remaining_hours: [0.0, 2.0, nil],
expected_derived_remaining_hours: 4.0

include_examples 'derived remaining hours',
children_remaining_hours: [1, 2, 5, 42],
expected_derived_remaining_hours: 52
it 'sets parent derived remaining work to the sum of itself and children remaining work' do
expect(call_result).to be_success
updated_work_packages = call_result.dependent_results.map(&:result)
expect_work_packages(updated_work_packages, <<~TABLE)
subject | derived remaining work
parent | 7.75h
TABLE
end
end

context 'with no remaining hours' do
include_context('when called with children', children_remaining_hours: [nil, nil, nil])
context 'with parent and children having no remaining work' do
let_work_packages(<<~TABLE)
hierarchy | remaining work |
parent | |
child1 | |
child2 | |
TABLE

subject(:call_result) do
described_class.new(user:, work_package: child1)
.call(%i(remaining_hours))
end

it 'does not update the parent derived remaining hours' do
it 'does not update the parent derived remaining work' do
expect(call_result).to be_success
expect(call_result.dependent_results).to be_empty
expect_work_packages([parent.reload], <<~TABLE)
subject | derived remaining work
parent |
TABLE
end
end
end
Expand Down
51 changes: 51 additions & 0 deletions spec/support/table_helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++

Dir[Rails.root.join('spec/support/table_helpers/*.rb')].each { |f| require f }

RSpec.configure do |config|
config.extend TableHelpers::LetWorkPackages
config.include TableHelpers::ExampleMethods

RSpec::Matchers.define :match_table do |expected|
match do |actual_work_packages|
expected_data = TableHelpers::TableData.for(expected)
actual_data = TableHelpers::TableData.from_work_packages(actual_work_packages, expected_data.columns)

representer = TableHelpers::TableRepresenter.new(tables_data: [expected_data, actual_data],
columns: expected_data.columns)
@expected = representer.render(expected_data)
@actual = representer.render(actual_data)

values_match? @expected, @actual
end

diffable
attr_reader :expected, :actual
end
end
176 changes: 176 additions & 0 deletions spec/support/table_helpers/column.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
# frozen_string_literal: true

#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++

require_relative 'identifier'

module TableHelpers
module Column
extend Identifier

def self.for(header)
case header
when /\A\s*estimated hours/i
raise ArgumentError, 'Please use "work" instead of "estimated hours"'
when /derived estimated hours/i
raise ArgumentError, 'Please use "derived work" instead of "derived estimated hours"'
when /derived remaining hours/i
raise ArgumentError, 'Please use "derived remaining work" instead of "derived remaining hours"'
when /\A\s*remaining hours/i
raise ArgumentError, 'Please use "remaining work" instead of "remaining hours"'
when /\A\s*work/i
Duration.new(header:, attribute: :estimated_hours)
when /derived work/i
Duration.new(header:, attribute: :derived_estimated_hours)
when /\A\s*remaining work/i
Duration.new(header:, attribute: :remaining_hours)
when /derived remaining work/i
Duration.new(header:, attribute: :derived_remaining_hours)
when /subject/
Subject.new(header:)
when /hierarchy/
Hierarchy.new(header:)
else
assert_work_package_attribute_exists(header)
Generic.new(header:)
end
end

def self.assert_work_package_attribute_exists(attribute)
attribute = to_identifier(attribute).to_s
return if WorkPackage.attribute_names.include?(attribute)

raise ArgumentError, "WorkPackage does not have an attribute named #{attribute.inspect}"
end

class Generic
include Identifier

attr_reader :attribute, :title, :raw_header

def initialize(header:, attribute: nil)
@raw_header = header
@title = header.strip
@attribute = attribute || to_identifier(title)
end

def format(value)
value.to_s
end

def cell_format(value, size)
format(value).send(text_align, size)
end

def parse(raw_value)
raw_value.strip
end

def text_align
:ljust
end

def read_and_update_work_packages_data(work_packages_data)
work_packages_data.each do |work_package_data|
work_package_data => { attributes:, row: }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to look up what this does. Very cool that it works somewhat like a Hash destructuring.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pattern matching. I'm still not sure if I like it or not, so I try using it from times to times to get a better feeling about it.

raw_value = row[raw_header]
work_package_data.merge!(metadata_for_value(raw_value))
attributes.merge!(attributes_for_raw_value(raw_value, work_package_data, work_packages_data))
end
end

def attributes_for_raw_value(raw_value, _data, _work_packages_data)
{ attribute => parse(raw_value) }
end

def metadata_for_value(_raw_value)
{}
end
end

module Identifiable
include Identifier

def metadata_for_value(raw_value, *)
super.merge(identifier: to_identifier(raw_value))
end
end

class Duration < Generic
def text_align
:rjust
end

def format(value)
if value.nil?
''
elsif value == value.truncate
"%sh" % value.to_i
else
"%sh" % value
end
end

def parse(raw_value)
raw_value.blank? ? nil : raw_value.to_f
end
end

class Subject < Generic
include Identifiable
end

class Hierarchy < Generic
include Identifiable

def attributes_for_raw_value(raw_value, data, work_packages_data)
{
parent: find_parent(data, work_packages_data),
subject: parse(raw_value)
}
end

def metadata_for_value(raw_value)
super.merge(hierarchy_indent: raw_value[/\A */].size)
end

private

def find_parent(data, work_packages_data)
return if data[:hierarchy_indent] == 0

work_packages_data
.slice(0, data[:index])
.reverse
.find { _1[:hierarchy_indent] < data[:hierarchy_indent] }
.then { _1&.fetch(:identifier) }
end
end
end
end
Loading