Merge pull request #7485 from opf/feature/derived-estimated-time

allow editing estimated time for parents, show derived separately
pull/7525/head
Markus Kahl 5 years ago committed by GitHub
commit 5bfa246527
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 8
      app/assets/stylesheets/content/work_packages/inplace_editing/_display_fields.sass
  2. 6
      app/contracts/work_packages/base_contract.rb
  3. 40
      app/models/queries/work_packages/columns/work_package_column.rb
  4. 1
      app/models/work_package/journalized.rb
  5. 37
      app/services/work_packages/update_ancestors_service.rb
  6. 7
      config/locales/js-en.yml
  7. 118
      db/migrate/20190722082648_add_derived_estimated_hours_to_work_packages.rb
  8. 78
      frontend/src/app/modules/fields/display/field-types/wp-display-duration-field.module.ts
  9. 13
      lib/api/v3/work_packages/work_package_representer.rb
  10. 2
      modules/backlogs/features/team_member.feature
  11. 11
      modules/backlogs/lib/open_project/backlogs/patches/update_ancestors_service_patch.rb
  12. 8
      modules/costs/spec/features/time_entries_spec.rb
  13. 2
      modules/costs/spec/features/view_own_rates_spec.rb
  14. 2
      modules/reporting/spec/features/my_time_spec.rb
  15. 27
      spec/contracts/work_packages/base_contract_spec.rb
  16. 168
      spec/features/work_packages/estimated_hours_display_spec.rb
  17. 16
      spec/lib/api/v3/work_packages/schema/specific_work_package_schema_spec.rb
  18. 12
      spec/lib/api/v3/work_packages/work_package_representer_spec.rb
  19. 34
      spec/models/queries/work_packages/columns/work_package_column_spec.rb
  20. 3
      spec/services/work_packages/destroy_service_integration_spec.rb
  21. 12
      spec/services/work_packages/update_ancestors_service_spec.rb
  22. 4
      spec/services/work_packages/update_service_integration_spec.rb
  23. 12
      spec/support/work_packages.rb

@ -36,6 +36,14 @@
&.-placeholder
font-style: italic
span.-derived
@include varprop(color, gray-dark)
font-style: italic
font-weight: bold
span.-with-actual
margin-left: 0.5em
.wp-edit-field--text,
wp-edit-field
width: 100%

@ -59,10 +59,8 @@ module WorkPackages
model.leaf? && Setting.work_package_done_ratio == 'field'
}
attribute :estimated_hours,
writeable: ->(*) {
model.leaf?
}
attribute :estimated_hours
attribute :derived_estimated_hours, writeable: false
attribute :parent_id,
permission: :manage_subtasks

@ -42,46 +42,12 @@ class Queries::WorkPackages::Columns::WorkPackageColumn < Queries::Columns::Base
end
def sum_of(work_packages)
wps = filter_for_sum work_packages
if wps.is_a?(Array)
if work_packages.is_a?(Array)
# TODO: Sums::grouped_sums might call through here without an AR::Relation
# Ensure that this also calls using a Relation and drop this (slow!) implementation
wps.map { |wp| value(wp) }.compact.reduce(:+)
else
wps.sum(name)
end
end
##
# Sometimes we don't want to consider all work packages when calculating
# the sum for a certain column.
#
# Specifically we don't want to consider child work packages when summing up
# the estimated hours for work packages since the estimated hours of
# parent work packages already include those of their children.
#
# Right now we cover only this one case here explicilty.
# As soon as there are more cases to be considered this shall be
# refactored into something more generic and outside of this class.
def filter_for_sum(work_packages)
if name == :estimated_hours
filter_for_sum_of_estimated_hours work_packages
else
work_packages
end
end
def filter_for_sum_of_estimated_hours(work_packages)
# Why an array? See TODO above in #sum_of.
if work_packages.is_a? Array
work_packages.reject { |wp| !wp.children.empty? && work_packages.any? { |x| x.parent_id == wp.id } }
work_packages.map { |wp| value(wp) }.compact.reduce(:+)
else
# @TODO Replace with CTE once we dropped MySQL support (MySQL only supports CTEs from version 8 onwards).
# With a CTE (common table expression) we only need to query the work packages once and can then
# drill the results down to the desired subset. Right now we run the work packages query a second
# time in a sub query.
work_packages.without_children in: work_packages
work_packages.sum(name)
end
end
end

@ -81,6 +81,7 @@ module WorkPackage::Journalized
register_on_journal_formatter(:id, 'parent_id')
register_on_journal_formatter(:fraction, 'estimated_hours')
register_on_journal_formatter(:fraction, 'derived_estimated_hours')
register_on_journal_formatter(:decimal, 'done_ratio')
register_on_journal_formatter(:diff, 'description')
register_on_journal_formatter(:attachment, /attachments_?\d+/)

@ -73,22 +73,34 @@ class WorkPackages::UpdateAncestorsService
parent = WorkPackage.find(previous_parent_id)
([parent] + parent.ancestors).each do |ancestor|
inherit_attributes(ancestor, %i(estimated_hours done_ratio))
# pass :parent to force update of all inherited attributes
inherit_attributes(ancestor, %i(parent))
end.select(&:changed?)
end
def inherit_attributes(ancestor, attributes)
return unless attributes_justify_inheritance?(attributes)
leaves = ancestor
leaves = leaves_for_ancestor ancestor
inherit_from_leaves ancestor: ancestor, leaves: leaves, attributes: attributes
end
def leaves_for_ancestor(ancestor)
ancestor
.leaves
.select(selected_leaf_attributes)
.distinct(true) # Be explicit that this is a distinct (wrt ID) query
.includes(:status).to_a
end
inherit_done_ratio(ancestor, leaves)
def inherit_from_leaves(ancestor:, leaves:, attributes:)
inherit_done_ratio ancestor, leaves if inherit? attributes, :done_ratio
derive_estimated_hours ancestor, leaves if inherit? attributes, :estimated_hours
end
inherit_estimated_hours(ancestor, leaves)
def inherit?(attributes, attribute)
[attribute, :parent].any? { |attr| attributes.include? attr }
end
def set_journal_note(work_packages)
@ -156,13 +168,18 @@ class WorkPackages::UpdateAncestorsService
summands.sum
end
def inherit_estimated_hours(ancestor, leaves)
ancestor.estimated_hours = all_estimated_hours(leaves).sum.to_f
ancestor.estimated_hours = nil if ancestor.estimated_hours.zero?
def derive_estimated_hours(ancestor, leaves)
ancestor.derived_estimated_hours = not_zero all_estimated_hours(leaves, derived: true).sum.to_f
end
def not_zero(value)
value unless value.zero?
end
def all_estimated_hours(work_packages)
work_packages.map(&:estimated_hours).reject { |hours| hours.to_f.zero? }
def all_estimated_hours(work_packages, derived: false)
work_packages
.map { |wp| (derived && wp.derived_estimated_hours) || wp.estimated_hours }
.reject { |hours| hours.to_f.zero? }
end
##
@ -191,6 +208,6 @@ class WorkPackages::UpdateAncestorsService
end
def selected_leaf_attributes
%i(id done_ratio estimated_hours status_id)
%i(id done_ratio derived_estimated_hours estimated_hours status_id)
end
end

@ -335,6 +335,7 @@ en:
label_total_progress: "%{percent}% Total progress"
label_total_amount: "Total: %{amount}"
label_updated_on: "updated on"
label_value_derived_from_children: "(value derived from children)"
label_warning: "Warning"
label_work_package: "Work package"
label_work_package_plural: "Work packages"
@ -885,9 +886,9 @@ en:
one: "one child work package"
other: "%{count} work package children"
hour:
one: "1 hour"
other: "%{count} hours"
zero: "0 hours"
one: "1 h"
other: "%{count} h"
zero: "0 h"
zen_mode:
button_activate: 'Activate zen mode'
button_deactivate: 'Deactivate zen mode'

@ -0,0 +1,118 @@
class AddDerivedEstimatedHoursToWorkPackages < ActiveRecord::Migration[5.2]
class WorkPackageWithRelations < ActiveRecord::Base
self.table_name = "work_packages"
scope :with_children, ->(*args) do
rel = "relations"
wp = "work_packages"
query = "EXISTS (SELECT 1 FROM #{rel} WHERE #{rel}.from_id = #{wp}.id AND #{rel}.hierarchy > 0 LIMIT 1)"
where(query)
end
end
def change
add_column :work_packages, :derived_estimated_hours, :float
add_column :work_package_journals, :derived_estimated_hours, :float
reversible do |change|
change.up do
WorkPackage.transaction do
migrate_to_derived_estimated_hours!
end
end
end
end
# Before this migration all work packages who have children had their
# estimated hours set based on their children through the UpdateAncestorsService.
#
# We move this value to the derived_estimated_hours column and clear
# the estimated_hours column. In the future users can estimte the time
# for parent work pacages separately there while the UpdateAncestorsService
# only touches the derived_estimated_hours column.
def migrate_to_derived_estimated_hours!
last_id = Journal.order(id: :desc).limit(1).pluck(:id).first || 0
wp_journals = "work_package_journals"
work_packages = WorkPackageWithRelations.with_children
work_packages.update_all("derived_estimated_hours = estimated_hours, estimated_hours = NULL")
create_journals_for work_packages
create_work_package_journals last_id: last_id
end
##
# Creates a new journal for each work package with the next version.
# The respective work_package journal is created in a separate step.
def create_journals_for(work_packages, author: journal_author, notes: journal_notes)
WorkPackage.connection.execute("
INSERT INTO #{Journal.table_name} (journable_type, journable_id, user_id, notes, created_at, version, activity_type)
SELECT
'WorkPackage',
parents.id,
#{author.id},
#{WorkPackage.connection.quote(notes)},
NOW(),
(SELECT MAX(version) FROM journals WHERE journable_id = parents.id AND journable_type = 'WorkPackage') + 1,
'work_packages'
FROM (
#{work_packages.select(:id).to_sql}
) AS parents
")
end
def journal_author
@journal_author ||= User.system
end
def journal_notes
"_'Estimated hours' changed to 'Derived estimated hours'_"
end
##
# Creates work package journals for the move of estimated_hours to derived_estimated_hours.
#
# For each newly created journal (see above) it inserts the respective work package's
# current estimated_hours (deleted) and derived estimated hours (previously estimated hours).
# All other attributes of the work package journal entry are copied from the previous
# work package journal entry (i.e. the values are not changed).
#
# @param last_id [Integer] The ID of the last journal before the journals for the migration were created.
def create_work_package_journals(last_id:)
journals = "journals"
wp_journals = "work_package_journals"
work_packages = "work_packages"
WorkPackage.connection.execute("
INSERT INTO #{wp_journals} (
journal_id, type_id, project_id, subject, description, due_date, category_id, status_id,
assigned_to_id, priority_id, fixed_version_id, author_id, done_ratio,
start_date, parent_id, responsible_id, cost_object_id, story_points, remaining_hours,
estimated_hours, derived_estimated_hours
)
SELECT *
FROM (
SELECT
#{journals}.id, #{wp_journals}.type_id, #{wp_journals}.project_id, #{wp_journals}.subject,
#{wp_journals}.description, #{wp_journals}.due_date, #{wp_journals}.category_id, #{wp_journals}.status_id,
#{wp_journals}.assigned_to_id, #{wp_journals}.priority_id, #{wp_journals}.fixed_version_id, #{wp_journals}.author_id,
#{wp_journals}.done_ratio, #{wp_journals}.start_date, #{wp_journals}.parent_id, #{wp_journals}.responsible_id,
#{wp_journals}.cost_object_id, #{wp_journals}.story_points, #{wp_journals}.remaining_hours,
#{work_packages}.estimated_hours, #{work_packages}.derived_estimated_hours
FROM #{journals} -- take the journal ID from here (ID of newly created journals from above)
LEFT JOIN #{work_packages} -- take the current (derived) estimated hours from here
ON #{work_packages}.id = #{journals}.journable_id AND #{journals}.journable_type = 'WorkPackage'
LEFT JOIN #{wp_journals} -- keep everything else the same
ON #{wp_journals}.journal_id = (
SELECT MAX(id)
FROM #{journals}
WHERE journable_id = #{work_packages}.id AND journable_type = 'WorkPackage' AND #{journals}.id <= #{last_id}
-- we are selecting the latest previous (hence <= last_id) work package journal here to copy its values
)
WHERE #{journals}.id > #{last_id} -- make sure to only create entries for the newly created journals
) AS results
")
end
end

@ -32,12 +32,88 @@ import {TimezoneService} from 'core-components/datetime/timezone.service';
export class DurationDisplayField extends DisplayField {
private timezoneService:TimezoneService = this.$injector.get(TimezoneService);
private derivedText = this.I18n.t('js.label_value_derived_from_children');
public get valueString() {
return this.timezoneService.formattedDuration(this.value);
}
/**
* Duration fields may have an additional derived value
*/
public get derivedPropertyName() {
return "derived" + this.name.charAt(0).toUpperCase() + this.name.slice(1);
}
public get derivedValue():string|null {
return this.resource[this.derivedPropertyName];
}
public get derivedValueString():string {
const value = this.derivedValue;
if (value) {
return this.timezoneService.formattedDuration(value);
} else {
return this.placeholder;
}
}
public render(element:HTMLElement, displayText:string):void {
if (this.isEmpty()) {
element.textContent = this.placeholder;
return;
}
let value = this.value;
let actual:number = (value && this.timezoneService.toHours(value)) || 0;
if (actual !== 0) {
this.renderActual(element, displayText);
}
let derived = this.derivedValue;
if (derived && this.timezoneService.toHours(derived) !== 0) {
this.renderDerived(element, this.derivedValueString, actual !== 0);
}
}
public renderActual(element:HTMLElement, displayText:string):void {
const span = document.createElement('span');
span.textContent = displayText;
span.title = this.valueString;
element.appendChild(span);
}
public renderDerived(element:HTMLElement, displayText:string, actualPresent:boolean):void {
const span = document.createElement('span');
span.setAttribute('title', this.texts.empty);
span.textContent = "(" + (actualPresent ? "+" : "") + displayText + ")";
span.title = `${this.derivedValueString} ${this.derivedText}`;
span.classList.add("-derived");
if (actualPresent) {
span.classList.add("-with-actual");
}
element.appendChild(span);
}
public get title():string|null {
return null; // we want to render separate titles ourselves
}
public isEmpty():boolean {
return this.timezoneService.toHours(this.value) === 0;
const value = this.value;
const derived = this.derivedValue;
const valueEmpty = !value || this.timezoneService.toHours(value) === 0;
const derivedEmpty = !derived || this.timezoneService.toHours(derived) === 0;
return valueEmpty && derivedEmpty;
}
}

@ -370,6 +370,14 @@ module API
end,
render_nil: true
property :derived_estimated_time,
exec_context: :decorator,
getter: ->(*) do
datetime_formatter.format_duration_from_hours(represented.derived_estimated_hours,
allow_nil: true)
end,
render_nil: true
property :spent_time,
exec_context: :decorator,
getter: ->(*) do
@ -545,6 +553,11 @@ module API
allow_nil: true)
end
def derived_estimated_time=(value)
represented.derived_estimated_hours = datetime_formatter
.parse_duration_to_hours(value, 'derivedEstimatedTime', allow_nil: true)
end
def spent_time=(value)
# noop
end

@ -169,4 +169,4 @@ Feature: Team Member
And task A Whole New Task should have remaining_hours set to 3
And task A Whole New Task should have estimated_hours set to 8
And story Story 1 should have remaining_hours set to 4
And story Story 1 should have estimated_hours set to 10
And story Story 1 should have derived_estimated_hours set to 10

@ -40,11 +40,16 @@ module OpenProject::Backlogs::Patches::UpdateAncestorsServicePatch
module InstanceMethods
private
# piggybacking the method because it has the correct signature
# and is called in the desired places
def inherit_estimated_hours(ancestor, leaves)
##
# Overrides method in original UpdateAncestorsService.
def inherit_from_leaves(ancestor:, leaves:, attributes:)
super
inherit_remaining_hours ancestor, leaves if inherit? attributes, :remaining_hours
end
def inherit_remaining_hours(ancestor, leaves)
ancestor.remaining_hours = all_remaining_hours(leaves).sum.to_f
ancestor.remaining_hours = nil if ancestor.remaining_hours == 0.0
end

@ -64,8 +64,8 @@ describe 'Work Package table cost entries', type: :feature, js: true do
parent_row = wp_table.row(parent)
wp_row = wp_table.row(work_package)
expect(parent_row).to have_selector('.wp-edit-field.spentTime', text: '12.5 hours')
expect(wp_row).to have_selector('.wp-edit-field.spentTime', text: '2.5 hours')
expect(parent_row).to have_selector('.wp-edit-field.spentTime', text: '12.5 h')
expect(wp_row).to have_selector('.wp-edit-field.spentTime', text: '2.5 h')
end
it 'creates an activity' do
@ -76,7 +76,7 @@ describe 'Work Package table cost entries', type: :feature, js: true do
find('#show_cost_objects').set true
click_on 'Apply'
expect(page).to have_selector('.time-entry a', text: '10.0 hours')
expect(page).to have_selector('.time-entry a', text: '2.5 hours')
expect(page).to have_selector('.time-entry a', text: '10.0 h')
expect(page).to have_selector('.time-entry a', text: '2.5 h')
end
end

@ -83,7 +83,7 @@ describe 'Only see your own rates', type: :feature, js: true do
it 'only displays own entries and rates' do
# All the values do not include the entries made by the other user
wp_page.expect_attributes spent_time: '1 hour',
wp_page.expect_attributes spent_time: '1 h',
costs_by_type: '2 Translations',
overall_costs: '24.00 EUR',
labor_costs: '10.00 EUR',

@ -34,7 +34,7 @@ describe 'Cost report showing my own times', type: :feature, js: true do
it 'shows my time' do
expect(page).to have_no_selector('.report')
expect(page).to have_selector('.generic-table--no-results-title')
expect(page).not_to have_text '10.00' # 1 EUR x 10
expect(page).not_to have_text '10.0' # 1 EUR x 10
end
end
end

@ -185,8 +185,6 @@ describe WorkPackages::BaseContract do
end
describe 'estimated hours' do
it_behaves_like 'a parent unwritable property', :estimated_hours
let(:estimated_hours) { 1 }
before do
@ -238,6 +236,31 @@ describe WorkPackages::BaseContract do
end
end
describe 'derived estimated hours' do
let(:changed_values) { [] }
let(:attribute) { :derived_estimated_hours }
before do
allow(work_package).to receive(:changed).and_return(changed_values.map(&:to_s))
contract.validate
end
context 'has not changed' do
let(:changed_values) { [] }
it('is valid') { expect(contract.errors).to be_empty }
end
context 'has changed' do
let(:changed_values) { [attribute] }
it('is invalid (read only)') do
expect(contract.errors.symbols_for(attribute)).to match_array([:error_readonly])
end
end
end
shared_examples_for 'a date attribute' do |attribute|
context 'a date' do
before do

@ -0,0 +1,168 @@
#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# 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-2017 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 docs/COPYRIGHT.rdoc for more details.
#++
require 'spec_helper'
RSpec.feature 'Estimated hours display' do
let(:user) { FactoryBot.create :admin }
let(:project) { FactoryBot.create :project }
let(:hierarchy) { [] }
let!(:work_packages) do
build_work_package_hierarchy(
hierarchy,
:subject,
:estimated_hours,
shared_attributes: {
project: project
}
)
end
let(:parent) { work_packages.first }
let(:child) { work_packages.last }
let!(:query) do
query = FactoryBot.build :query, user: user, project: project
query.column_names = %w[id subject estimated_hours]
query.save!
query
end
let(:wp_table) { Pages::WorkPackagesTable.new project }
before do
WorkPackages::UpdateAncestorsService
.new(user: user, work_package: child)
.call([:estimated_hours])
login_as(user)
end
context "with both estimated and derived estimated time" do
let(:hierarchy) do
[
{
["Parent", 1] => [
["Child", 3]
]
}
]
end
scenario 'work package index', js: true do
wp_table.visit_query query
wp_table.expect_work_package_listed parent, child
expect(page).to have_content("Parent\n1 h(+3 h)")
end
scenario 'work package details', js: true do
visit work_package_path(parent.id)
expect(page).to have_content("Estimated time\n1 h(+3 h)")
end
end
context "with just estimated time" do
let(:hierarchy) do
[
{
["Parent", 1] => [
["Child", 0]
]
}
]
end
scenario 'work package index', js: true do
wp_table.visit_query query
wp_table.expect_work_package_listed parent, child
expect(page).to have_content("Parent\n1 h")
end
scenario 'work package details', js: true do
visit work_package_path(parent.id)
expect(page).to have_content("Estimated time\n1 h")
end
end
context "with just derived estimated time" do
let(:hierarchy) do
[
{
["Parent", 0] => [
["Child", 3]
]
}
]
end
scenario 'work package index', js: true do
wp_table.visit_query query
wp_table.expect_work_package_listed parent, child
expect(page).to have_content("Parent\n(3 h)")
end
scenario 'work package details', js: true do
visit work_package_path(parent.id)
expect(page).to have_content("Estimated time\n(3 h)")
end
end
context "with neither estimated nor derived estimated time" do
let(:hierarchy) do
[
{
["Parent", 0] => [
["Child", 0]
]
}
]
end
scenario 'work package index', js: true do
wp_table.visit_query query
wp_table.expect_work_package_listed parent, child
expect(page).to have_content("Parent\n-")
end
scenario 'work package details', js: true do
visit work_package_path(parent.id)
expect(page).to have_content("Estimated time\n-")
end
end
end

@ -227,9 +227,9 @@ describe ::API::V3::WorkPackages::Schema::SpecificWorkPackageSchema do
end
context 'estimated time' do
it 'is not writable when the work package is a parent' do
it 'is writable when the work package is a parent' do
allow(work_package).to receive(:leaf?).and_return(false)
expect(subject.writable?(:estimated_time)).to be false
expect(subject.writable?(:estimated_time)).to be true
end
it 'is writable when the work package is a leaf' do
@ -238,6 +238,18 @@ describe ::API::V3::WorkPackages::Schema::SpecificWorkPackageSchema do
end
end
context 'derived estimated time' do
it 'is not writable when the work package is a parent' do
allow(work_package).to receive(:leaf?).and_return(false)
expect(subject.writable?(:derived_estimated_time)).to be false
end
it 'is not writable when the work package is a leaf' do
allow(work_package).to receive(:leaf?).and_return(true)
expect(subject.writable?(:derived_estimated_time)).to be false
end
end
context 'start date' do
it 'is not writable when the work package is a parent' do
allow(work_package).to receive(:leaf?).and_return(false)

@ -240,6 +240,18 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
it { is_expected.to be_json_eql('PT6H30M'.to_json).at_path('estimatedTime') }
end
describe 'derivedEstimatedTime' do
let(:work_package) do
FactoryBot.build_stubbed(:work_package,
id: 42,
created_at: DateTime.now,
updated_at: DateTime.now,
derived_estimated_hours: 3.75)
end
it { is_expected.to be_json_eql('PT3H45M'.to_json).at_path('derivedEstimatedTime') }
end
describe 'spentTime' do
describe '#content' do
context 'no view_time_entries permission' do

@ -42,34 +42,34 @@ describe Queries::WorkPackages::Columns::WorkPackageColumn, type: :model do
context "with work packages in a hierarchy" do
let(:work_packages) do
hierarchy = [
["Single", 1],
["Single", 1, 0],
{
["Parent", 3] => [
["Child 1 of Parent", 1],
["Child 2 of Parent", 1],
["Hidden Child 3 of Parent", 1]
["Parent", 1, 3] => [
["Child 1 of Parent", 1, 0],
["Child 2 of Parent", 1, 0],
["Hidden Child 3 of Parent", 1, 0]
]
},
{
["Hidden Parent", 4] => [
["Child of Hidden Parent", 1],
["Hidden Child", 3]
["Hidden Parent", 5, 4] => [
["Child of Hidden Parent", 1, 0],
["Hidden Child", 3, 0]
]
},
{
["Parent 2", 3] => [
["Child 1 of Parent 2", 1],
["Parent 2", 1, 3] => [
["Child 1 of Parent 2", 1, 0],
{
["Nested Parent", 2] => [
["Child 1 of Nested Parent", 1],
["Child 2 of Nested Parent", 1]
["Nested Parent", 0, 2] => [
["Child 1 of Nested Parent", 1, 0],
["Child 2 of Nested Parent", 1, 0]
]
}
]
}
]
build_work_package_hierarchy hierarchy, :subject, :estimated_hours
build_work_package_hierarchy hierarchy, :subject, :estimated_hours, :derived_estimated_hours
end
let(:result_set) { WorkPackage.where("NOT subject LIKE 'Hidden%'") }
@ -83,8 +83,10 @@ describe Queries::WorkPackages::Columns::WorkPackageColumn, type: :model do
end
it "yields the correct sum, not counting any children (of parents in the result set) twice" do
expect(column.sum_of(result_set)).to eq 7 # (Single + Child 1 of Parent + Child 2 of Parent + Child of Hidden Parent + Parent 2)
expect(column.sum_of(WorkPackage.all)).to eq 11 # (Single + Parent + Hidden Parent + Parent 2)
# Single + Parent + Child 1 of Parent + Child 2 of Parent + Child of Hidden Parent + Parent 2 + Child 1 of Parent 2
# + Nested Parent + Child 1 of Nested Parent + Child 2 of Nested Parent
expect(column.sum_of(result_set)).to eq 9
expect(column.sum_of(WorkPackage.all)).to eq 18 # the above + Hidden Child 3 of Parent + Hidden Parent + Hidden Child
end
end
end

@ -68,7 +68,8 @@ describe WorkPackages::DeleteService, 'integration', type: :model do
it 'updates the parent estimated_hours' do
expect(child.estimated_hours).to eq 123
expect(parent.estimated_hours).to eq 123
expect(parent.derived_estimated_hours).to eq 123
expect(parent.estimated_hours).to eq nil
expect(subject).to be_success

@ -53,8 +53,8 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
.to eq aggregate_done_ratio
end
it 'has the expected estimated_hours' do
expect(subject.dependent_results.first.result.estimated_hours)
it 'has the expected derived estimated_hours' do
expect(subject.dependent_results.first.result.derived_estimated_hours)
.to eq aggregate_estimated_hours
end
@ -240,7 +240,7 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
end
it 'updates the estimated_hours of the former parent' do
expect(parent.reload(select: :estimated_hours).estimated_hours)
expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
.to eql sibling_estimated_hours
end
@ -250,7 +250,7 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
end
it 'updates the estimated_hours of the former grandparent' do
expect(grandparent.reload(select: :estimated_hours).estimated_hours)
expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
.to eql sibling_estimated_hours
end
end
@ -305,7 +305,7 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
end
it 'updates the estimated_hours of the new parent' do
expect(parent.reload(select: :estimated_hours).estimated_hours)
expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
.to eql estimated_hours
end
@ -315,7 +315,7 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
end
it 'updates the estimated_hours of the new grandparent' do
expect(grandparent.reload(select: :estimated_hours).estimated_hours)
expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
.to eql estimated_hours
end
end

@ -489,8 +489,8 @@ describe WorkPackages::UpdateService, 'integration tests', type: :model do
wp.reload
expect(wp.estimated_hours)
.to eql(sum)
expect(wp.estimated_hours).to eql(nil)
expect(wp.derived_estimated_hours).to eql(sum)
end
# sibling hours are unchanged

@ -62,17 +62,21 @@ def become_member_with_move_work_package_permissions
become_member_with_permissions [:move_work_packages]
end
def build_work_package_hierarchy(data, *attributes, parent: nil)
def build_work_package_hierarchy(data, *attributes, parent: nil, shared_attributes: {})
work_packages = []
Array(data).each do |attr|
if attr.is_a? Hash
parent_wp = FactoryBot.create :work_package, **attributes.zip(attr.keys.first).to_h
parent_wp = FactoryBot.create(
:work_package, shared_attributes.merge(**attributes.zip(attr.keys.first).to_h)
)
work_packages << parent_wp
work_packages += build_work_package_hierarchy(attr.values.first, *attributes, parent: parent_wp)
work_packages += build_work_package_hierarchy(
attr.values.first, *attributes, parent: parent_wp, shared_attributes: shared_attributes
)
else
wp = FactoryBot.create :work_package, **attributes.zip(attr).to_h
wp = FactoryBot.create :work_package, shared_attributes.merge(**attributes.zip(attr).to_h)
parent.children << wp if parent

Loading…
Cancel
Save