Merge pull request #5446 from opf/fix/25170/hierarchy-parent-position

[25170] When parent is visible, defer rendering of children below them
pull/5451/merge
ulferts 8 years ago committed by GitHub
commit c300b49abf
  1. 180
      frontend/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts
  2. 81
      frontend/app/components/wp-fast-table/builders/modes/hierarchy/hierarchy-rows-builder.ts
  3. 24
      frontend/app/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts
  4. 181
      spec/features/work_packages/table/hierarchy_spec.rb
  5. 29
      spec/support/components/work_packages/hierarchies.rb
  6. 16
      spec/support/pages/work_packages_table.rb

@ -0,0 +1,180 @@
import {WorkPackageTable} from "../../../wp-fast-table";
import {WorkPackageResourceInterface} from "../../../../api/api-v3/hal-resources/work-package-resource.service";
import {SingleHierarchyRowBuilder} from "./single-hierarchy-row-builder";
import {WorkPackageTableRow} from "../../../wp-table.interfaces";
import {hierarchyGroupClass} from "../../../helpers/wp-table-hierarchy-helpers";
export class HierarchyRenderPass {
// Remember which rows were already rendered
public rendered:{[workPackageId:string]: boolean};
// Remember the actual order of rendering
public renderedOrder:string[];
// Remember additional parents inserted that are not part of the results table
public additionalParents:{[workPackageId:string]: WorkPackageResourceInterface};
// Defer children to be rendered when their parent occurs later in the table
public deferred:{[parentId:string]: WorkPackageResourceInterface[]};
// The resulting rows fragment
public content:DocumentFragment;
constructor(public workPackageTable:WorkPackageTable, public rowBuilder:SingleHierarchyRowBuilder) {
this.rendered = {};
this.renderedOrder = [];
this.additionalParents = {};
this.deferred = {};
this.content = document.createDocumentFragment();
this.render();
}
/**
* Render the hierarchy table into the document fragment
*/
private render() {
this.workPackageTable.rows.forEach((wpId:string) => {
const row:WorkPackageTableRow = this.workPackageTable.rowIndex[wpId];
const workPackage:WorkPackageResourceInterface = row.object;
// If we need to defer this row, skip it for now
if (this.deferInsertion(workPackage)) {
return;
}
if (workPackage.ancestors.length) {
// If we have ancestors, render it
this.buildWithHierarchy(row);
} else {
// Render a work package root with no parents
let tr = this.rowBuilder.buildEmpty(workPackage);
row.element = tr;
this.content.appendChild(tr);
this.markRendered(workPackage);
}
// Render all potentially deferred rows
this.renderAllDeferredChildren(workPackage);
});
}
/**
* If the given work package has a visible parent in the table, return true
* and remember the work package until the parent is rendered.
* @param workPackage
* @returns {boolean}
*/
public deferInsertion(workPackage:WorkPackageResourceInterface):boolean {
const parentId = workPackage.parentId;
// Will only defer if parent exists
if (!parentId) {
return false;
}
// Will only defer is parent is
// 1. existent in the table results
// 1. yet to be rendered
if (this.workPackageTable.rowIndex[parentId] === undefined || this.rendered[parentId]) {
return false;
}
const elements = this.deferred[parentId] || [];
this.deferred[parentId] = elements.concat([workPackage]);
return true;
}
/**
* Render any deferred children of the given work package. If recursive children were
* deferred, each of them will be passed through renderCallback.
* @param workPackage
* @param renderCallback
*/
private renderAllDeferredChildren(workPackage:WorkPackageResourceInterface) {
const wpId = workPackage.id.toString();
const deferredChildren = this.deferred[wpId] || [];
// If the work package has deferred children to render,
// run them through the callback
deferredChildren.forEach((child:WorkPackageResourceInterface) => {
// Callback on the child itself
const row:WorkPackageTableRow = this.workPackageTable.rowIndex[child.id];
this.insertUnderParent(row, child.parentId.toString());
// Descend into any children the child WP might have and callback
this.renderAllDeferredChildren(child);
});
}
private buildWithHierarchy(row:WorkPackageTableRow) {
// Ancestor data [root, med, thisrow]
const ancestors = row.object.ancestors;
const ancestorGroups:string[] = [];
// Iterate ancestors
ancestors.forEach((ancestor:WorkPackageResourceInterface, index:number) => {
// If we see the parent the first time,
// build it as an additional row and insert it into the ancestry
if (!this.rendered[ancestor.id]) {
let ancestorRow = this.rowBuilder.buildAncestorRow(ancestor, ancestorGroups, index);
// Insert the ancestor row, either right here if it's a root node
// Or below the appropriate parent
if (index === 0) {
// Special case, first ancestor => root without parent
this.content.appendChild(ancestorRow);
this.markRendered(ancestor);
} else {
// This ancestor must be inserted in the last position of its root
const parent = ancestors[index - 1];
this.insertAtExistingHierarchy(ancestor, ancestorRow, parent.id);
}
// Remember we just added this extra ancestor row
this.additionalParents[ancestor.id] = ancestor;
// Push the correct ancestor groups for identifiying a hierarchy group
ancestorGroups.push(hierarchyGroupClass(ancestor.id));
}
});
// Insert this row to parent
const parent = _.last(ancestors);
this.insertUnderParent(row, parent.id);
}
/**
* Insert the given node as a child of the parent
* @param row
* @param parentId
*/
private insertUnderParent(row:WorkPackageTableRow, parentId:string) {
const tr = this.rowBuilder.buildEmpty(row.object);
row.element = tr;
this.insertAtExistingHierarchy(row.object, tr, parentId);
}
/**
* Mark the given work package as rendered
* @param workPackage
*/
private markRendered(workPackage:WorkPackageResourceInterface) {
this.rendered[workPackage.id] = true;
this.renderedOrder.push(workPackage.id);
}
/**
* Append a row to the given parent hierarchy group.
*/
private insertAtExistingHierarchy(workPackage:WorkPackageResourceInterface, el:HTMLElement, parentId:string) {
// Either append to the hierarchy group root (= the parentID row itself)
const hierarchyRoot = `.__hierarchy-root-${parentId}`;
// Or, if it has descendants, append to the LATEST of that set
const hierarchyGroup = `.__hierarchy-group-${parentId}`;
jQuery(this.content).find(`${hierarchyRoot},${hierarchyGroup}`).last().after(el);
this.markRendered(workPackage);
}
}

@ -4,10 +4,8 @@ import {States} from "../../../../states.service";
import {WorkPackageTableHierarchiesService} from "../../../state/wp-table-hierarchy.service";
import {WorkPackageTable} from "../../../wp-fast-table";
import {injectorBridge} from "../../../../angular/angular-injector-bridge.functions";
import {WorkPackageResourceInterface} from "../../../../api/api-v3/hal-resources/work-package-resource.service";
import {WorkPackageTableRow} from "../../../wp-table.interfaces";
import {SingleHierarchyRowBuilder} from "./single-hierarchy-row-builder";
import {hierarchyGroupClass} from "../../../helpers/wp-table-hierarchy-helpers";
import {HierarchyRenderPass} from "./hierarchy-render-pass";
export class HierarchyRowsBuilder extends PlainRowsBuilder {
@ -21,8 +19,6 @@ export class HierarchyRowsBuilder extends PlainRowsBuilder {
protected rowBuilder:SingleHierarchyRowBuilder;
protected refreshBuilder:SingleHierarchyRowBuilder;
// The group expansion state
constructor(public workPackageTable: WorkPackageTable) {
super(workPackageTable);
@ -41,85 +37,14 @@ export class HierarchyRowsBuilder extends PlainRowsBuilder {
* @param table
*/
public internalBuildRows(table:WorkPackageTable):DocumentFragment {
// Remember all additional rows drawn for hierarchy
const additional:{[workPackageId:string]: WorkPackageResourceInterface} = {};
const tbodyContent = document.createDocumentFragment();
table.rows.forEach((wpId:string) => {
let row:WorkPackageTableRow = table.rowIndex[wpId];
// If this row was already rendered in a hierarchy, ignore it here
if (additional[row.workPackageId]) {
return;
}
// If we have ancestors
if (row.object.ancestors.length) {
this.buildWithHierarchy(table, tbodyContent, row, additional);
} else {
let tr = this.buildEmptyRow(row);
row.element = tr;
tbodyContent.appendChild(tr);
}
additional[row.object.id] = row.object;
});
return tbodyContent;
const instance = new HierarchyRenderPass(table, this.rowBuilder);
return instance.content;
}
protected setupRowBuilders() {
this.rowBuilder = new SingleHierarchyRowBuilder(this.stopExisting$, this.workPackageTable);
this.refreshBuilder = this.rowBuilder;
}
private buildWithHierarchy(
table:WorkPackageTable,
tbody:DocumentFragment,
row:WorkPackageTableRow,
additional:{[workPackageId:string]: WorkPackageResourceInterface}) {
// Ancestor data [root, med, thisrow]
const ancestors = row.object.ancestors;
const ancestorGroups:string[] = [];
ancestors.forEach((ancestor:WorkPackageResourceInterface, index:number) => {
if (!additional[ancestor.id]) {
let ancestorRow = this.rowBuilder.buildAncestorRow(table, ancestor, ancestorGroups, index);
// special case, root without parent
if (index === 0) {
// Simply append the root here
tbody.appendChild(ancestorRow);
} else {
// This ancestor must be inserted in the last position of its root
const parent = ancestors[index-1];
this.insertIntoHierarchy(tbody, ancestorRow, parent.id);
}
additional[ancestor.id] = ancestor;
ancestorGroups.push(hierarchyGroupClass(ancestor.id));
}
});
// Insert this row to parent
const parent = _.last(ancestors);
const tr = this.buildEmptyRow(row);
row.element = tr;
this.insertIntoHierarchy(tbody, tr, parent.id);
}
/**
* Append a row to the given parent hierarchy group.
*/
private insertIntoHierarchy(tbody:DocumentFragment, tr:HTMLElement, parentId:string) {
// Either append to the hierarchy group root (= the parentID row itself)
const hierarchyRoot = `.__hierarchy-root-${parentId}`;
// Or, if it has descendants, append to the LATEST of that set
const hierarchyGroup = `.__hierarchy-group-${parentId}`;
jQuery(tbody).find(`${hierarchyRoot},${hierarchyGroup}`).last().after(tr);
}
}

@ -75,12 +75,11 @@ export class SingleHierarchyRowBuilder extends RowRefreshBuilder {
* Append an additional ancestor row that is not yet loaded
*/
public buildAncestorRow(
table:WorkPackageTable,
ancestor:WorkPackageResourceInterface,
ancestorGroups:string[],
index:number):HTMLElement {
const loadedRow = table.rowIndex[ancestor.id];
const loadedRow = this.workPackageTable.rowIndex[ancestor.id];
if (loadedRow) {
const tr = this.buildEmpty(loadedRow.object);
@ -178,22 +177,11 @@ export class SingleHierarchyRowBuilder extends RowRefreshBuilder {
return false; // Work Package has no children at all
}
const rows = this.workPackageTable.rows;
const total = rows.length;
if (rows[total - 1] === workPackage.id) {
return false; // Last element, can have no children
}
// If immediately following child has element
const row = this.workPackageTable.rowIndex[workPackage.id];
const nextIndex = row.position + 1;
if (nextIndex < total) {
const nextId = rows[nextIndex].toString();
return !!_.find(workPackage.children, (child:WorkPackageResourceInterface) => child.idFromLink === nextId);
}
return false;
// If any visible children in the table
return !!_.find(workPackage.children, (child:WorkPackageResourceInterface) => {
const childId = child.idFromLink!;
return this.workPackageTable.rowIndex[childId] !== undefined;
});
}
}

@ -7,18 +7,6 @@ describe 'Work Package table hierarchy', js: true do
let(:wp_table) { Pages::WorkPackagesTable.new(project) }
let(:hierarchy) { ::Components::WorkPackages::Hierarchies.new }
def expect_listed(*wps)
wps.each do |wp|
wp_table.expect_work_package_listed(wp)
end
end
def expect_hidden(*wps)
wps.each do |wp|
hierarchy.expect_hidden(wp)
end
end
before do
login_as(user)
end
@ -44,25 +32,23 @@ describe 'Work Package table hierarchy', js: true do
it 'shows hierarchy correctly' do
wp_table.visit!
expect_listed(wp_root, wp_inter, wp_leaf, wp_other)
wp_table.expect_work_package_listed(wp_root, wp_inter, wp_leaf, wp_other)
# Hierarchy mode is enabled by default
hierarchy.expect_hierarchy_at(wp_root)
hierarchy.expect_hierarchy_at(wp_inter)
hierarchy.expect_leaf_at(wp_leaf)
hierarchy.expect_leaf_at(wp_other)
hierarchy.expect_hierarchy_at(wp_root, wp_inter)
hierarchy.expect_leaf_at(wp_leaf, wp_other)
# Toggling hierarchies hides the inner children
hierarchy.toggle_row(wp_root)
# Root, other showing
expect_listed(wp_root, wp_other)
wp_table.expect_work_package_listed(wp_root, wp_other)
# Inter, Leaf hidden
expect_hidden(wp_inter, wp_leaf)
hierarchy.expect_hidden(wp_inter, wp_leaf)
# Show all again
hierarchy.toggle_row(wp_root)
expect_listed(wp_root, wp_other, wp_inter, wp_leaf)
wp_table.expect_work_package_listed(wp_root, wp_other, wp_inter, wp_leaf)
# Disable hierarchies
hierarchy.disable_hierarchy
@ -76,10 +62,8 @@ describe 'Work Package table hierarchy', js: true do
wp_table.expect_notification message: 'Successful update.'
wp_table.dismiss_notification!
hierarchy.expect_hierarchy_at(wp_root)
hierarchy.expect_hierarchy_at(wp_inter)
hierarchy.expect_leaf_at(wp_leaf)
hierarchy.expect_leaf_at(wp_other)
hierarchy.expect_hierarchy_at(wp_root, wp_inter)
hierarchy.expect_leaf_at(wp_leaf, wp_other)
# Disable hierarchy again
hierarchy.disable_hierarchy
@ -91,12 +75,10 @@ describe 'Work Package table hierarchy', js: true do
# Should only list the matching leaf
wp_table.expect_work_package_listed(wp_leaf)
hierarchy.expect_hierarchy_at(wp_root)
hierarchy.expect_hierarchy_at(wp_inter)
hierarchy.expect_hierarchy_at(wp_root, wp_inter)
hierarchy.toggle_row(wp_root)
expect_listed(wp_root)
expect_listed(wp_inter, wp_leaf)
wp_table.expect_work_package_listed(wp_root, wp_inter, wp_leaf)
# Disabling hierarchy hides them again
hierarchy.disable_hierarchy
@ -113,21 +95,22 @@ describe 'Work Package table hierarchy', js: true do
let(:global_table) { Pages::WorkPackagesTable.new }
it 'shows the hierarchy indicator only when the rows are both shown' do
wp_table.visit!
expect_listed(wp_root)
wp_table.expect_work_package_listed(wp_root)
wp_table.expect_work_package_not_listed(wp_inter)
hierarchy.expect_leaf_at(wp_root)
# Visit global table
global_table.visit!
expect_listed(wp_root, wp_inter)
wp_table.expect_work_package_listed(wp_root, wp_inter)
hierarchy.expect_hierarchy_at(wp_root)
hierarchy.expect_leaf_at(wp_inter)
end
end
describe 'sorting so that the parent appears below the child' do
let!(:wp_root) { FactoryGirl.create(:work_package, project: project, assigned_to: user) }
describe 'flat table such that the parent appears below the child' do
let!(:wp_root) { FactoryGirl.create(:work_package, project: project) }
let!(:wp_inter) { FactoryGirl.create(:work_package, project: project, parent: wp_root) }
let!(:wp_leaf) { FactoryGirl.create(:work_package, project: project, parent: wp_inter) }
let!(:query) do
query = FactoryGirl.build(:query, user: user, project: project)
@ -139,32 +122,144 @@ describe 'Work Package table hierarchy', js: true do
query
end
it 'removes the parent from the flow, moving it above' do
it 'removes the parent from the flow in hierarchy mode, moving it above' do
# Hierarchy disabled, expect wp_inter before wp_root
wp_table.visit_query query
wp_table.expect_work_package_listed(wp_inter)
wp_table.expect_work_package_listed(wp_root)
wp_table.expect_work_package_order(wp_inter.id, wp_root.id)
wp_table.expect_work_package_listed(wp_inter, wp_root, wp_leaf)
wp_table.expect_work_package_order(wp_inter.id, wp_root.id, wp_leaf.id)
hierarchy.expect_no_hierarchies
# Enable hierarchy mode, should move it above now
hierarchy.enable_hierarchy
hierarchy.expect_hierarchy_at(wp_root)
hierarchy.expect_leaf_at(wp_inter)
# Should not be marked as additional row (grey)
expect(page).to have_no_selector('.wp-table--hierarchy-aditional-row')
wp_table.expect_work_package_listed(wp_inter)
wp_table.expect_work_package_listed(wp_root)
wp_table.expect_work_package_order(wp_root.id, wp_inter.id)
hierarchy.expect_hierarchy_at(wp_root, wp_inter)
hierarchy.expect_leaf_at(wp_leaf)
wp_table.expect_work_package_listed(wp_inter, wp_root, wp_leaf)
wp_table.expect_work_package_order(wp_root.id, wp_inter.id, wp_leaf.id)
# Toggling hierarchies hides the inner children
hierarchy.toggle_row(wp_root)
# Root showing
expect_listed(wp_root)
wp_table.expect_work_package_listed(wp_root)
# Inter hidden
expect_hidden(wp_inter)
hierarchy.expect_hidden(wp_inter, wp_leaf)
end
end
describe 'sorting by assignee' do
include_context 'work package table helpers'
let!(:root_assigned) do
FactoryGirl.create(:work_package, subject: 'root_assigned', project: project, assigned_to: user)
end
let!(:inter_assigned) do
FactoryGirl.create(:work_package, subject: 'inter_assigned', project: project, assigned_to: user, parent: root_assigned)
end
let!(:inter) do
FactoryGirl.create(:work_package, subject: 'inter', project: project, parent: root_assigned)
end
let!(:leaf_assigned) do
FactoryGirl.create(:work_package, subject: 'leaf_assigned', project: project, assigned_to: user, parent: inter)
end
let!(:leaf) do
FactoryGirl.create(:work_package, subject: 'leaf', project: project, parent: inter)
end
let!(:root) do
FactoryGirl.create(:work_package, project: project)
end
let(:user) do
FactoryGirl.create :user,
member_in_project: project,
member_through_role: role
end
let(:permissions) { %i(view_work_packages add_work_packages) }
let(:role) { FactoryGirl.create :role, permissions: permissions }
let!(:query) do
query = FactoryGirl.build(:query, user: user, project: project)
query.column_names = ['subject', 'assigned_to']
query.filters.clear
query.sort_criteria = [['assigned_to', 'asc']]
query.show_hierarchies = false
query.save!
query
end
it 'shows the respective order' do
wp_table.visit_query query
wp_table.expect_work_package_listed(leaf, inter, root)
wp_table.expect_work_package_listed(leaf_assigned, inter_assigned, root_assigned)
wp_table.expect_work_package_order(
leaf_assigned.id, inter_assigned.id, root_assigned.id,
leaf.id, inter.id, root.id
)
# Hierarchy should be disabled
hierarchy.expect_no_hierarchies
# Enable hierarchy mode, should sort according to spec above
hierarchy.enable_hierarchy
hierarchy.expect_hierarchy_at(root_assigned, inter)
hierarchy.expect_leaf_at(root, leaf, leaf_assigned, inter_assigned)
# When ascending, order should be:
# ├──root_assigned
# | ├─ inter_assigned
# | ├─ inter
# | | ├─ leaf_assigned
# | | ├─ leaf
# ├──root
wp_table.expect_work_package_order(
root_assigned.id,
inter_assigned.id,
inter.id,
leaf_assigned.id,
leaf.id,
root.id
)
# Test collapsing of rows
hierarchy.toggle_row(root_assigned)
wp_table.expect_work_package_listed(root, root_assigned)
hierarchy.expect_hidden(inter, inter_assigned, leaf, leaf_assigned)
hierarchy.toggle_row(root_assigned)
# Sort descending
sort_wp_table_by 'Assignee', order: :desc
loading_indicator_saveguard
wp_table.expect_work_package_listed(root, root_assigned)
# When descending, order should be:
# ├──root
# ├──root
# | ├─ inter
# | | ├─ leaf
# | | ├─ leaf_assigned
# | ├─ inter_assigned
wp_table.expect_work_package_order(
root.id,
root_assigned.id,
inter.id,
inter_assigned.id,
leaf.id,
leaf_assigned.id
)
# Disable hierarchy mode
hierarchy.disable_hierarchy
wp_table.expect_work_package_order(
leaf.id, inter.id, root.id,
leaf_assigned.id, inter_assigned.id, root_assigned.id
)
end
end
end

@ -47,24 +47,31 @@ module Components
expect(page).to have_no_selector('.wp-table--hierarchy-span')
end
def expect_leaf_at(work_package)
expect(page).to have_selector("#wp-row-#{work_package.id} .wp-table--leaf-indicator")
def expect_leaf_at(*work_packages)
work_packages.each do |wp|
expect(page).to have_selector("#wp-row-#{wp.id} .wp-table--leaf-indicator")
end
end
def expect_hierarchy_at(work_package, collapsed = false)
selector = "#wp-row-#{work_package.id} .wp-table--hierarchy-indicator"
def expect_hierarchy_at(*work_packages, collapsed: false)
collapsed_sel = ".-hierarchy-collapsed"
if collapsed
expect(page).to have_selector("#{selector}#{collapsed_sel}")
else
expect(page).to have_selector(selector)
expect(page).to have_no_selector("#{selector}#{collapsed_sel}")
work_packages.each do |wp|
selector = "#wp-row-#{wp.id} .wp-table--hierarchy-indicator"
if collapsed
expect(page).to have_selector("#{selector}#{collapsed_sel}")
else
expect(page).to have_selector(selector)
expect(page).to have_no_selector("#{selector}#{collapsed_sel}")
end
end
end
def expect_hidden(work_package)
expect(page).to have_selector("#wp-row-#{work_package.id}", visible: :hidden)
def expect_hidden(*work_packages)
work_packages.each do |wp|
expect(page).to have_selector("#wp-row-#{wp.id}", visible: :hidden)
end
end
def toggle_row(work_package)

@ -40,17 +40,21 @@ module Pages
visit "#{path}?query_id=#{query.id}"
end
def expect_work_package_listed(work_package)
def expect_work_package_listed(*work_packages)
within(table_container) do
expect(page).to have_selector("#wp-row-#{work_package.id} td.subject",
text: work_package.subject)
work_packages.each do |wp|
expect(page).to have_selector("#wp-row-#{wp.id} td.subject",
text: wp.subject)
end
end
end
def expect_work_package_not_listed(work_package)
def expect_work_package_not_listed(*work_packages)
within(table_container) do
expect(page).to have_no_selector("#wp-row-#{work_package.id} td.subject",
text: work_package.subject)
work_packages.each do |wp|
expect(page).to have_no_selector("#wp-row-#{wp.id} td.subject",
text: wp.subject)
end
end
end

Loading…
Cancel
Save