Shiloh/brotherton-erpnext
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(Org Chart): use attribute selectors instead of ID selector for node IDs with special characters (#27717)

Browse Source 
* fix(Org Chart): use attribute selectors instead of ID selector for node IDs with special chars

* fix: UI tests
This commit is contained in:
Rucha Mahabal 2021-09-30 18:32:10 +05:30 committed by GitHub
parent 273f3fbe0f
commit 9e08229b7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 27 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cypress/integration/test_organizational_chart_desktop.js
View File

@ -6,7 +6,7 @@ context('Organizational Chart', () => {
it('navigates to org chart', () => { it('navigates to org chart', () => {
cy.visit('/app'); cy.visit('/app');
cy.awesomebar('Organizational Chart'); cy.visit('/app/organizational-chart');
cy.url().should('include', '/organizational-chart'); cy.url().should('include', '/organizational-chart');
cy.window().its('frappe.csrf_token').then(csrf_token => { cy.window().its('frappe.csrf_token').then(csrf_token => {

2
cypress/integration/test_organizational_chart_mobile.js
View File

@ -7,7 +7,7 @@ context('Organizational Chart Mobile', () => {
it('navigates to org chart', () => { it('navigates to org chart', () => {
cy.viewport(375, 667); cy.viewport(375, 667);
cy.visit('/app'); cy.visit('/app');
cy.awesomebar('Organizational Chart'); cy.visit('/app/organizational-chart');
cy.url().should('include', '/organizational-chart'); cy.url().should('include', '/organizational-chart');
cy.window().its('frappe.csrf_token').then(csrf_token => { cy.window().its('frappe.csrf_token').then(csrf_token => {

24
erpnext/public/js/hierarchy_chart/hierarchy_chart_desktop.js
View File

@ -63,7 +63,7 @@ erpnext.HierarchyChart = class {
}); });
node.parent.append(node_card); node.parent.append(node_card);
node.$link = $(`#${node.id}`); node.$link = $(`[id="${node.id}"]`);
} }
show() { show() {
@ -223,7 +223,7 @@ erpnext.HierarchyChart = class {
let node = undefined; let node = undefined;
$.each(r.message, (_i, data) => { $.each(r.message, (_i, data) => {
if ($(`#${data.id}`).length) if ($(`[id="${data.id}"]`).length)
return; return;
node = new me.Node({ node = new me.Node({
@ -263,7 +263,7 @@ erpnext.HierarchyChart = class {
this.refresh_connectors(node.parent_id); this.refresh_connectors(node.parent_id);
// rebuild incoming connections // rebuild incoming connections
let grandparent = $(`#${node.parent_id}`).attr('data-parent'); let grandparent = $(`[id="${node.parent_id}"]`).attr('data-parent');
this.refresh_connectors(grandparent); this.refresh_connectors(grandparent);
} }
@ -282,7 +282,7 @@ erpnext.HierarchyChart = class {
show_active_path(node) { show_active_path(node) {
// mark node parent on active path // mark node parent on active path
$(`#${node.parent_id}`).addClass('active-path'); $(`[id="${node.parent_id}"]`).addClass('active-path');
} }
load_children(node, deep=false) { load_children(node, deep=false) {
@ -317,7 +317,7 @@ erpnext.HierarchyChart = class {
render_child_nodes(node, child_nodes) { render_child_nodes(node, child_nodes) {
const last_level = this.$hierarchy.find('.level:last').index(); const last_level = this.$hierarchy.find('.level:last').index();
const current_level = $(`#${node.id}`).parent().parent().parent().index(); const current_level = $(`[id="${node.id}"]`).parent().parent().parent().index();
if (last_level === current_level) { if (last_level === current_level) {
this.$hierarchy.append(` this.$hierarchy.append(`
@ -382,7 +382,7 @@ erpnext.HierarchyChart = class {
node.$children = $('<ul class="node-children"></ul>'); node.$children = $('<ul class="node-children"></ul>');
const last_level = this.$hierarchy.find('.level:last').index(); const last_level = this.$hierarchy.find('.level:last').index();
const node_level = $(`#${node.id}`).parent().parent().parent().index(); const node_level = $(`[id="${node.id}"]`).parent().parent().parent().index();
if (last_level === node_level) { if (last_level === node_level) {
this.$hierarchy.append(` this.$hierarchy.append(`
@ -489,7 +489,7 @@ erpnext.HierarchyChart = class {
set_path_attributes(path, parent_id, child_id) { set_path_attributes(path, parent_id, child_id) {
path.setAttribute("data-parent", parent_id); path.setAttribute("data-parent", parent_id);
path.setAttribute("data-child", child_id); path.setAttribute("data-child", child_id);
const parent = $(`#${parent_id}`); const parent = $(`[id="${parent_id}"]`);
if (parent.hasClass('active')) { if (parent.hasClass('active')) {
path.setAttribute("class", "active-connector"); path.setAttribute("class", "active-connector");
@ -513,7 +513,7 @@ erpnext.HierarchyChart = class {
} }
collapse_previous_level_nodes(node) { collapse_previous_level_nodes(node) {
let node_parent = $(`#${node.parent_id}`); let node_parent = $(`[id="${node.parent_id}"]`);
let previous_level_nodes = node_parent.parent().parent().children('li'); let previous_level_nodes = node_parent.parent().parent().children('li');
let node_card = undefined; let node_card = undefined;
@ -545,7 +545,7 @@ erpnext.HierarchyChart = class {
setup_node_click_action(node) { setup_node_click_action(node) {
let me = this; let me = this;
let node_element = $(`#${node.id}`); let node_element = $(`[id="${node.id}"]`);
node_element.click(function() { node_element.click(function() {
const is_sibling = me.selected_node.parent_id === node.parent_id; const is_sibling = me.selected_node.parent_id === node.parent_id;
@ -563,7 +563,7 @@ erpnext.HierarchyChart = class {
} }
setup_edit_node_action(node) { setup_edit_node_action(node) {
let node_element = $(`#${node.id}`); let node_element = $(`[id="${node.id}"]`);
let me = this; let me = this;
node_element.find('.btn-edit-node').click(function() { node_element.find('.btn-edit-node').click(function() {
@ -572,7 +572,7 @@ erpnext.HierarchyChart = class {
} }
remove_levels_after_node(node) { remove_levels_after_node(node) {
let level = $(`#${node.id}`).parent().parent().parent().index(); let level = $(`[id="${node.id}"]`).parent().parent().parent().index();
level = $('.hierarchy > li:eq('+ level + ')'); level = $('.hierarchy > li:eq('+ level + ')');
level.nextAll('li').remove(); level.nextAll('li').remove();
@ -595,7 +595,7 @@ erpnext.HierarchyChart = class {
const parent = $(path).data('parent'); const parent = $(path).data('parent');
const child = $(path).data('child'); const child = $(path).data('child');
if ($(`#${parent}`).length && $(`#${child}`).length) if ($(`[id="${parent}"]`).length && $(`[id="${child}"]`).length)
return; return;
$(path).remove(); $(path).remove();

22
erpnext/public/js/hierarchy_chart/hierarchy_chart_mobile.js
View File

@ -54,7 +54,7 @@ erpnext.HierarchyChartMobile = class {
}); });
node.parent.append(node_card); node.parent.append(node_card);
node.$link = $(`#${node.id}`); node.$link = $(`[id="${node.id}"]`);
node.$link.addClass('mobile-node'); node.$link.addClass('mobile-node');
} }
@ -184,7 +184,7 @@ erpnext.HierarchyChartMobile = class {
this.refresh_connectors(node.parent_id, node.id); this.refresh_connectors(node.parent_id, node.id);
// rebuild incoming connections of parent // rebuild incoming connections of parent
let grandparent = $(`#${node.parent_id}`).attr('data-parent'); let grandparent = $(`[id="${node.parent_id}"]`).attr('data-parent');
this.refresh_connectors(grandparent, node.parent_id); this.refresh_connectors(grandparent, node.parent_id);
} }
@ -221,7 +221,7 @@ erpnext.HierarchyChartMobile = class {
show_active_path(node) { show_active_path(node) {
// mark node parent on active path // mark node parent on active path
$(`#${node.parent_id}`).addClass('active-path'); $(`[id="${node.parent_id}"]`).addClass('active-path');
} }
load_children(node) { load_children(node) {
@ -256,7 +256,7 @@ erpnext.HierarchyChartMobile = class {
if (child_nodes) { if (child_nodes) {
$.each(child_nodes, (_i, data) => { $.each(child_nodes, (_i, data) => {
this.add_node(node, data); this.add_node(node, data);
$(`#${data.id}`).addClass('active-child'); $(`[id="${data.id}"]`).addClass('active-child');
setTimeout(() => { setTimeout(() => {
this.add_connector(node.id, data.id); this.add_connector(node.id, data.id);
@ -293,9 +293,9 @@ erpnext.HierarchyChartMobile = class {
let connector = undefined; let connector = undefined;
if ($(`#${parent_id}`).hasClass('active')) { if ($(`[id="${parent_id}"]`).hasClass('active')) {
connector = this.get_connector_for_active_node(parent_node, child_node); connector = this.get_connector_for_active_node(parent_node, child_node);
} else if ($(`#${parent_id}`).hasClass('active-path')) { } else if ($(`[id="${parent_id}"]`).hasClass('active-path')) {
connector = this.get_connector_for_collapsed_node(parent_node, child_node); connector = this.get_connector_for_collapsed_node(parent_node, child_node);
} }
@ -351,7 +351,7 @@ erpnext.HierarchyChartMobile = class {
set_path_attributes(path, parent_id, child_id) { set_path_attributes(path, parent_id, child_id) {
path.setAttribute("data-parent", parent_id); path.setAttribute("data-parent", parent_id);
path.setAttribute("data-child", child_id); path.setAttribute("data-child", child_id);
const parent = $(`#${parent_id}`); const parent = $(`[id="${parent_id}"]`);
if (parent.hasClass('active')) { if (parent.hasClass('active')) {
path.setAttribute("class", "active-connector"); path.setAttribute("class", "active-connector");
@ -374,7 +374,7 @@ erpnext.HierarchyChartMobile = class {
setup_node_click_action(node) { setup_node_click_action(node) {
let me = this; let me = this;
let node_element = $(`#${node.id}`); let node_element = $(`[id="${node.id}"]`);
node_element.click(function() { node_element.click(function() {
let el = undefined; let el = undefined;
@ -398,7 +398,7 @@ erpnext.HierarchyChartMobile = class {
} }
setup_edit_node_action(node) { setup_edit_node_action(node) {
let node_element = $(`#${node.id}`); let node_element = $(`[id="${node.id}"]`);
let me = this; let me = this;
node_element.find('.btn-edit-node').click(function() { node_element.find('.btn-edit-node').click(function() {
@ -512,7 +512,7 @@ erpnext.HierarchyChartMobile = class {
} }
remove_levels_after_node(node) { remove_levels_after_node(node) {
let level = $(`#${node.id}`).parent().parent().index(); let level = $(`[id="${node.id}"]`).parent().parent().index();
level = $('.hierarchy-mobile > li:eq('+ level + ')'); level = $('.hierarchy-mobile > li:eq('+ level + ')');
level.nextAll('li').remove(); level.nextAll('li').remove();
@ -533,7 +533,7 @@ erpnext.HierarchyChartMobile = class {
const parent = $(path).data('parent'); const parent = $(path).data('parent');
const child = $(path).data('child'); const child = $(path).data('child');
if ($(`#${parent}`).length && $(`#${child}`).length) if ($(`[id="${parent}"]`).length && $(`[id="${child}"]`).length)
return; return;
$(path).remove(); $(path).remove();

2
erpnext/tests/ui_test_helpers.py
View File

@ -7,6 +7,8 @@ def create_employee_records():
create_company() create_company()
create_missing_designation() create_missing_designation()
frappe.db.sql("DELETE FROM tabEmployee WHERE company='Test Org Chart'")
emp1 = create_employee('Test Employee 1', 'CEO') emp1 = create_employee('Test Employee 1', 'CEO')
emp2 = create_employee('Test Employee 2', 'CTO') emp2 = create_employee('Test Employee 2', 'CTO')
emp3 = create_employee('Test Employee 3', 'Head of Marketing and Sales', emp1) emp3 = create_employee('Test Employee 3', 'Head of Marketing and Sales', emp1)