From 23223d0806bcd4056ca8571ef49112b3ec089881 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Oliver=20G=C3=BCnther?= <mail@oliverguenther.de>
Date: Mon, 4 Mar 2019 09:55:53 +0100
Subject: [PATCH] [29652] Avoid duplicated deferred parent insertion

When the parent work package is not in the current page,
but the grandparent is, the parent was added once per child
to the deferred render list and that list is not kept unique.

https://community.openproject.com/wp/29652
---
 .../modes/hierarchy/hierarchy-render-pass.ts  |  7 ++-
 .../hierarchy/hierarchy_parent_below_spec.rb  | 49 +++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/frontend/src/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts b/frontend/src/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts
index 5b34b8b41f..149d14e3d5 100644
--- a/frontend/src/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts
+++ b/frontend/src/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts
@@ -115,11 +115,14 @@ export class HierarchyRenderPass extends PrimaryRenderPass {
 
       if (inTable) {
         // Get the current elements
-        const elements = this.deferred[parent.id] || [];
+        let elements = this.deferred[parent.id] || [];
         // Append to them the child and all children below
         let newElements:WorkPackageResource[] = ancestorChain.slice(i + 1, ancestorChain.length);
         newElements = newElements.map(child => this.wpCacheService.state(child.id).value!);
-        this.deferred[parent.id] = elements.concat(newElements);
+        // Append all new elements
+        elements = elements.concat(newElements);
+        // Remove duplicates (Regression #29652)
+        this.deferred[parent.id] = _.uniqBy(elements, el => el.id);
         return true;
       }
       // Otherwise, continue the chain upwards
diff --git a/spec/features/work_packages/table/hierarchy/hierarchy_parent_below_spec.rb b/spec/features/work_packages/table/hierarchy/hierarchy_parent_below_spec.rb
index 739b0023dd..8b5f0d3cb3 100644
--- a/spec/features/work_packages/table/hierarchy/hierarchy_parent_below_spec.rb
+++ b/spec/features/work_packages/table/hierarchy/hierarchy_parent_below_spec.rb
@@ -92,4 +92,53 @@ describe 'Work Package table hierarchy parent below', js: true do
       wp_table.expect_work_package_order(child.id, grandparent.id)
     end
   end
+
+  describe 'grand-parent of 2+ children visible anywhere on the page, but parent is not (Regression #29652)' do
+    let(:child) { FactoryBot.create(:work_package, subject: 'AA Child WP', project: project, type: type_task) }
+    let(:child2) { FactoryBot.create(:work_package, subject: 'BB Child WP', project: project, type: type_task) }
+    let(:parent) { FactoryBot.create(:work_package, subject: 'ZZ Parent WP', project: project, type: type_task) }
+    let(:grandparent) { FactoryBot.create(:work_package, subject: 'Grandparent', project: project, type: type_task) }
+
+    let(:query) do
+      query              = FactoryBot.build(:query, user: user, project: project)
+      query.column_names = %w(id subject)
+      query.sort_criteria = [%w(subject asc)]
+      query.show_hierarchies = true
+
+      query.save!
+      query
+    end
+
+    before do
+      child
+      parent
+      grandparent
+
+      child.update(parent_id: parent.id)
+      child2.update(parent_id: parent.id)
+      parent.update(parent_id: grandparent.id)
+
+      allow(Setting).to receive(:per_page_options).and_return '3'
+      query
+    end
+
+    it 'shows hierarchy correctly' do
+      wp_table.visit_query query
+
+      wp_table.expect_work_package_listed(child, child2, parent, grandparent)
+
+      # Expect pagination to be correct
+      expect(page).to have_selector('.pagination--item.-current', text: '3')
+
+      # Expect count to be correct (one additional parent shown)
+      expect(page).to have_selector('.wp-table--row', count: 4)
+
+      # Double order result from regression
+      wp_table.expect_work_package_order(grandparent.id, parent.id, child.id, child2.id)
+
+      # Enable hierarchy mode, should sort according to spec above
+      hierarchy.expect_hierarchy_at(grandparent, parent)
+      hierarchy.expect_leaf_at(child, child2)
+    end
+  end
 end