Subject: [pgadmin4][patch] Initial patch to decouple from ACI Tree

From: Joao De Almeida Pereira <[email-masked]>
Date: Mon, 02 Apr 2018 18:45:32 +0000

Hi Hackers,

Attached you can find the patch that will start to decouple pgAdmin from
ACITree library.
This patch is intended to be merged after 3.0, because we do not want to
cause any entropy or delay the release, but we want to start the discussion
and show some code.

This job that we started is a massive tech debt chore that will take some
time to finalize and we would love the help of the community to do it.

*Summary of the patch:*
0001 patch:
 - Creates a new tree that will allow us to create a separation between the
application and ACI Tree
 - Creates a Fake Tree (Test double, for reference on the available test
doubles: https://martinfowler.com/bliki/TestDouble.html) that can be used
to inplace to replace the ACITree and also encapsulate the new tree
behavior on our tests
 - Adds tests for all the tree functionalities

0002 patch:
 - Extracts, refactors, adds tests and remove dependency from ACI Tree on:
- getTreeNodeHierarchy
- on backup.js: menu_enabled, menu_enabled_server,
start_backup_global_server, backup_objects
- on datagrid.js: show_data_grid, get_panel_title, show_query_tool
- Start using sprintf-js as Underscore.String is deprecating sprintf
function

This patch represents only 10 calls to ACITree.itemData out of 176 that are
spread around our code

*In Depth look on the process behind the patch:*

We started writing this patch with the idea that we need to decouple
pgAdmin4 from ACITree, because ACITree is no longer supported, the
documentation is non existent and ACITree is no longer being actively
developed.

Our process:
1. We "randomly" selected a function that is part of the ACITree. From this
point we decided to replace that function with our own version. The
function that we choose was "itemData".
The function gives us all the "data" that a specific node of the tree find.
Given in order to replace the tree we would need to have a function that
would give us the same information. We had 2 options:
  a) Create a tree with a function called itemData
Pros:
 - At first view this was the simpler solution
 - Would keep the status quo
Cons:
 - Not a OOP approach
 - Not very flexible
  b) Create a tree that would return a node given an ID and then the node
would be responsible for giving it's data.
Pros:
 - OOP Approach
 - More flexible and we do not need to bring the tree around, just a node
Cons:
 - Break the current status quo

Given these 2 options we decided to go for a more OOP approach creating a
Tree and a TreeNode classes, that in the future will be renamed to
ACITreeWrapper and TreeNode.

2. After we decided on the starting point we searched for occurrences of
the function "itemData" and we found out that there were 303 occurrences of
"itemData" in the code and roughly 176 calls to the function itself (some
of the hits were variable names).

3. We selected the first file on the search and found the function that was
responsible for calling the itemData function.

4. Extracted the function to a separate file

5. Wrap this function with tests

6. Refactor the function to ES6, give more declarative names to variables
and break the functions into smaller chunks

7. When all the tests were passing we replaced ACITree with our Tree

8. We ensured that all tests were passing

9. Remove function from the original file and use the new function

10. Ensure everything still works

11. Find the next function and execute from step 4 until all the functions
are replaced, refactored and tested.

As you can see by the process this is a pretty huge undertake, because of
the number of calls to the function. This is just the first step on the
direction of completely isolating the ACITree so that we can solve the
problem with a large number of elements on the tree.

*What is on our radar that we need to address:*
 - Finish the complete decoupling of the ACITree
 - Performance of the current tree implementation
 - Tweak the naming of the Tree class to explicitly tell us this is to use
only with ACITree.

Thanks
Joao

---

From: Dave Page <[email-masked]>
Date: Wed, 4 Apr 2018 17:17:01 +0100

Khushboo, Murtuza,

Can you spend some time reviewing this please? I've started playing with it
as well - the first thing that's irking me somewhat is the lack of
comments. Descriptive function names are all well and good, but sometimes a
little more is needed, especially for less experienced developers or
newcomers to the application!

On Mon, Apr 2, 2018 at 7:45 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

>  - Creates a new tree that will allow us to create a separation between
> the application and ACI Tree
> This patch represents only 10 calls to ACITree.itemData out of 176 that
> are spread around our code
>
>
> 1. We "randomly" selected a function that is part of the ACITree. From
> this point we decided to replace that function with our own version. The
> 3. We selected the first file on the search and found the function that
> was responsible for calling the itemData function.
>

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 5 Apr 2018 09:05:40 +0530

On Wed, Apr 4, 2018 at 9:47 PM, Dave Page <[email-masked]> wrote:

> Khushboo, Murtuza,
>
> Can you spend some time reviewing this please? I've started playing with
> it as well - the first thing that's irking me somewhat is the lack of
> Sure, already in our list.

>> doubles: https://martinfowler.com/bliki/TestDouble.html) that can be
>> used to inplace to replace the ACITree and also encapsulate the new tree
>> behavior on our tests
>> 11. Find the next function and execute from step 4 until all the
>> functions are replaced, refactored and tested.
>>
>>  - Tweak the naming of the Tree class to explicitly tell us this is to
>> use only with ACITree.
>>
>>
>> Thanks
>> Joao
>>
>>
>
>
> --
> Dave Page

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 5 Apr 2018 16:01:48 +0530

Hi Joao,

Can you please rebase the second patch?

Thanks,
Khushboo

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Thu, 05 Apr 2018 13:38:31 +0000

Hi Khushboo,
Attached you can find both patches rebased

Thanks

>>  - Tweak the naming of the Tree class to explicitly tell us this is to
diff --git a/web/pgadmin/static/js/tree/tree.js b/web/pgadmin/static/js/tree/tree.js
new file mode 100644
index 00000000..ed67aa85
--- /dev/null
+++ b/web/pgadmin/static/js/tree/tree.js
@@ -0,0 +1,134 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+export class TreeNode {
+  constructor(id, data, parent) {
+    this.id = id;
+    this.data = data;
+    this.parentNode = parent;
+    this.path = this.id;
+    if (parent !== null && parent !== undefined && parent.path !== undefined) {
+      this.path = parent.path + '.' + id;
+    }
+    this.children = [];
+  }
+
+  hasParent() {
+    return this.parentNode !== null && this.parentNode !== undefined;
+  }
+
+  parent() {
+    return this.parentNode;
+  }
+
+  getData() {
+    if (this.data === undefined) {
+      return undefined;
+    } else if (this.data === null) {
+      return null;
+    }
+    return Object.assign({}, this.data);
+  }
+}
+
+export class Tree {
+  constructor() {
+    this.rootNode = new TreeNode(undefined, {});
+    this.aciTreeApi = undefined;
+  }
+
+  addNewNode(id, data, path) {
+    const parent = this.findNode(path);
+    return this.createOrUpdateNode(id, data, parent);
+  }
+
+  findNode(path) {
+    if (path.length === 0) {
+      return this.rootNode;
+    }
+    return findInTree(this.rootNode, path.join('.'));
+  }
+
+  findNodeByDomElement(domElement) {
+    const path = this.translateTreeNodeIdFromACITree(domElement);
+    if(!path || !path[0]) {
+      return undefined;
+    }
+
+    return this.findNode(path);
+  }
+
+  selected() {
+    return this.aciTreeApi.selected();
+  }
+
+  register($treeJQuery) {
+    $treeJQuery.on('acitree', function (event, api, item, eventName) {
+      if (api.isItem(item)) {
+        if (eventName === 'added') {
+          const id = api.getId(item);
+          const data = api.itemData(item);
+          const parentId = this.translateTreeNodeIdFromACITree(api.parent(item));
+          this.addNewNode(id, data, parentId);
+        }
+      }
+    }.bind(this));
+    this.aciTreeApi = $treeJQuery.aciTree('api');
+  }
+
+  createOrUpdateNode(id, data, parent) {
+    const oldNode = this.findNode([parent.path, id]);
+    if (oldNode !== null) {
+      oldNode.data = Object.assign({}, data);
+      return oldNode;
+    }
+
+    const node = new TreeNode(id, data, parent);
+    if (parent === this.rootNode) {
+      node.parentNode = null;
+    }
+    parent.children.push(node);
+    return node;
+  }
+
+  translateTreeNodeIdFromACITree(aciTreeNode) {
+    let currentTreeNode = aciTreeNode;
+    let path = [];
+    while (currentTreeNode !== null && currentTreeNode !== undefined && currentTreeNode.length > 0) {
+      path.unshift(this.aciTreeApi.getId(currentTreeNode));
+      if (this.aciTreeApi.hasParent(currentTreeNode)) {
+        currentTreeNode = this.aciTreeApi.parent(currentTreeNode);
+      } else {
+        break;
+      }
+    }
+    return path;
+  }
+}
+
+export let tree = new Tree();
+
+function findInTree(rootNode, path) {
+  if (path === null) {
+    return rootNode;
+  }
+  return (function findInNode(currentNode) {
+    for (let i = 0, length = currentNode.children.length; i < length; i++) {
+      const calculatedNode = findInNode(currentNode.children[i]);
+      if (calculatedNode !== null) {
+        return calculatedNode;
+      }
+    }
+
+    if (currentNode.path === path) {
+      return currentNode;
+    } else {
+      return null;
+    }
+  })(rootNode);
+}
diff --git a/web/regression/javascript/tree/tree_fake.js b/web/regression/javascript/tree/tree_fake.js
new file mode 100644
index 00000000..a0983c67
--- /dev/null
+++ b/web/regression/javascript/tree/tree_fake.js
@@ -0,0 +1,55 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {Tree} from "../../../pgadmin/static/js/tree/tree";
+
+export class TreeFake extends Tree {
+  constructor() {
+    super();
+    this.aciTreeToOurTreeTranslator = {};
+  }
+
+  addNewNode(id, data, path) {
+    this.aciTreeToOurTreeTranslator[id] = path.concat(id);
+    return super.addNewNode(id, data, path);
+  }
+
+  hasParent(aciTreeNode) {
+    return this.translateTreeNodeIdFromACITree(aciTreeNode).length > 1;
+  }
+
+  parent(aciTreeNode) {
+    if (this.hasParent(aciTreeNode)) {
+      let path = this.translateTreeNodeIdFromACITree(aciTreeNode);
+      return [{id: this.findNode(path).parent().id}];
+    }
+
+    return null;
+  }
+
+  translateTreeNodeIdFromACITree(aciTreeNode) {
+    return this.aciTreeToOurTreeTranslator[aciTreeNode[0].id];
+  }
+
+  itemData(aciTreeNode) {
+    let path = this.translateTreeNodeIdFromACITree(aciTreeNode);
+    if(path === undefined || path === null) {
+      return undefined;
+    }
+    return this.findNode(path).getData();
+  }
+
+  selected() {
+    return this.selectedNode;
+  }
+
+  setSelectedNode(selectedNode) {
+    this.selectedNode = selectedNode;
+  }
+}
diff --git a/web/regression/javascript/tree/tree_spec.js b/web/regression/javascript/tree/tree_spec.js
new file mode 100644
index 00000000..e8930d84
--- /dev/null
+++ b/web/regression/javascript/tree/tree_spec.js
@@ -0,0 +1,284 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+import {Tree, TreeNode} from "../../../pgadmin/static/js/tree/tree";
+import {TreeFake} from "./tree_fake";
+
+const context = describe;
+
+const treeTests = (treeClass, setDefaultCallBack) => {
+  let tree;
+  beforeEach(() => {
+    tree = new treeClass();
+  });
+
+  describe('#addNewNode', () => {
+    describe('when add a new root element', () => {
+      beforeEach(() => {
+        tree.addNewNode('some new node', {data: 'interesting'}, []);
+      });
+
+      it('can be retrieved', () => {
+        const node = tree.findNode(['some new node']);
+        expect(node.data).toEqual({data: 'interesting'});
+      });
+
+      it('return false for #hasParent()', () => {
+        const node = tree.findNode(['some new node']);
+        expect(node.hasParent()).toBe(false);
+      });
+
+      it('return null for #parent()', () => {
+        const node = tree.findNode(['some new node']);
+        expect(node.parent()).toBeNull();
+      });
+    });
+
+    describe('when add a new element as a child', () => {
+      let parentNode;
+      beforeEach(() => {
+        parentNode = tree.addNewNode('parent node', {data: 'parent data'}, []);
+        tree.addNewNode('some new node', {data: 'interesting'}, ['parent' +
+        ' node']);
+      });
+
+      it('can be retrieved', () => {
+        const node = tree.findNode(['parent node', 'some new node']);
+        expect(node.data).toEqual({data: 'interesting'});
+      });
+
+      it('return true for #hasParent()', () => {
+        const node = tree.findNode(['parent node', 'some new node']);
+        expect(node.hasParent()).toBe(true);
+      });
+
+      it('return "parent node" object for #parent()', () => {
+        const node = tree.findNode(['parent node', 'some new node']);
+        expect(node.parent()).toEqual(parentNode);
+      });
+    });
+
+    describe('when add an element that already exists under a parent', () => {
+      let parentNode;
+      beforeEach(() => {
+        parentNode = tree.addNewNode('parent node', {data: 'parent data'}, []);
+        tree.addNewNode('some new node', {data: 'interesting'}, ['parent' +
+        ' node']);
+      });
+
+      it('does not add a new child', () => {
+        tree.addNewNode('some new node', {data: 'interesting 1'}, ['parent' +
+        ' node']);
+        const parentNode = tree.findNode(['parent node']);
+        expect(parentNode.children.length).toBe(1);
+      });
+
+      it('updates the existing node data', () => {
+        tree.addNewNode('some new node', {data: 'interesting 1'}, ['parent' +
+        ' node']);
+        const node = tree.findNode(['parent node', 'some new node']);
+        expect(node.data).toEqual({data: 'interesting 1'});
+      });
+    });
+  });
+
+  describe('#translateTreeNodeIdFromACITree', () => {
+    let aciTreeApi;
+    beforeEach(() => {
+      aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+        'hasParent',
+        'parent',
+        'getId',
+      ]);
+
+      aciTreeApi.getId.and.callFake((node) => {
+        return node[0].id;
+      });
+      tree.aciTreeApi = aciTreeApi;
+    });
+
+    describe('When tree as a single level', () => {
+      beforeEach(() => {
+        aciTreeApi.hasParent.and.returnValue(false);
+      });
+
+      it('returns an array with the ID of the first level', () => {
+        let node = [{
+          id: 'some id',
+        }];
+        tree.addNewNode('some id', {}, []);
+
+        expect(tree.translateTreeNodeIdFromACITree(node)).toEqual(['some id']);
+      });
+    });
+
+    describe('When tree as a 2 levels', () => {
+      describe('When we try to retrieve the node in the second level', () => {
+        it('returns an array with the ID of the first level and second level', () => {
+          aciTreeApi.hasParent.and.returnValues(true, false);
+          aciTreeApi.parent.and.returnValue([{
+            id: 'parent id'
+          }]);
+          let node = [{
+            id: 'some id',
+          }];
+
+          tree.addNewNode('parent id', {}, []);
+          tree.addNewNode('some id', {}, ['parent id']);
+
+          expect(tree.translateTreeNodeIdFromACITree(node))
+            .toEqual(['parent id', 'some id']);
+        });
+      });
+    });
+  });
+
+  describe('#selected', () => {
+    context('a node in the tree is selected', () => {
+      it('returns that node object', () => {
+        let selectedNode = new TreeNode('bamm', {}, []);
+        setDefaultCallBack(tree, selectedNode)
+        expect(tree.selected()).toEqual(selectedNode);
+      });
+    });
+  });
+
+  describe('#findNodeByTreeElement', () => {
+    context('retrieve data from node not found', () => {
+      it('return undefined', () => {
+        let aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+          'hasParent',
+          'parent',
+          'getId',
+        ]);
+
+        aciTreeApi.getId.and.callFake((node) => {
+          return node[0].id;
+        });
+        tree.aciTreeApi = aciTreeApi;
+        expect(tree.findNodeByDomElement(['<li>something</li>'])).toBeUndefined();
+      });
+    });
+  });
+};
+
+describe('tree tests', () => {
+  describe('TreeNode', () => {
+    describe('#hasParent', () => {
+      context('parent is null', () => {
+        it('returns false', () => {
+          let treeNode = new TreeNode('123', {}, null);
+          expect(treeNode.hasParent()).toBe(false);
+        });
+      });
+      context('parent is undefined', () => {
+        it('returns false', () => {
+          let treeNode = new TreeNode('123', {}, undefined);
+          expect(treeNode.hasParent()).toBe(false);
+        });
+      });
+      context('parent exists', () => {
+        it('returns true', () => {
+          let parentNode = new TreeNode('456', {}, undefined);
+          let treeNode = new TreeNode('123', {}, parentNode);
+          expect(treeNode.hasParent()).toBe(true);
+        });
+      });
+    });
+  });
+
+  describe('Tree', () => {
+    function realTreeSelectNode(tree, selectedNode) {
+      let aciTreeApi = jasmine.createSpyObj('ACITreeApi', [
+        'selected',
+      ]);
+      tree.aciTreeApi = aciTreeApi;
+      aciTreeApi.selected.and.returnValue(selectedNode);
+    }
+
+    treeTests(Tree, realTreeSelectNode);
+  });
+
+  describe('TreeFake', () => {
+    function fakeTreeSelectNode(tree, selectedNode) {
+      tree.setSelectedNode(selectedNode);
+    }
+
+    treeTests(TreeFake, fakeTreeSelectNode);
+
+    describe('#hasParent', () => {
+      context('tree contains multiple levels', () => {
+        let tree;
+        beforeEach(() => {
+          tree = new TreeFake();
+          tree.addNewNode('level1', {data: 'interesting'}, []);
+          tree.addNewNode('level2', {data: 'interesting'}, ['level1']);
+        });
+
+        context('node is at the first level', () => {
+          it('returns false', () => {
+            expect(tree.hasParent([{id: 'level1'}])).toBe(false);
+          });
+        });
+
+        context('node is at the second level', () => {
+          it('returns true', () => {
+            expect(tree.hasParent([{id: 'level2'}])).toBe(true);
+          });
+        });
+      });
+    });
+
+    describe('#parent', () => {
+      let tree;
+      beforeEach(() => {
+        tree = new TreeFake();
+        tree.addNewNode('level1', {data: 'interesting'}, []);
+        tree.addNewNode('level2', {data: 'interesting'}, ['level1']);
+      });
+
+      context('node is the root', () => {
+        it('returns null', () => {
+          expect(tree.parent([{id: 'level1'}])).toBeNull();
+        });
+      });
+
+      context('node is not root', () => {
+        it('returns root element', () => {
+          expect(tree.parent([{id: 'level2'}])).toEqual([{id: 'level1'}]);
+        });
+      });
+    });
+
+    describe('#itemData', () => {
+      let tree;
+      beforeEach(() => {
+        tree = new TreeFake();
+        tree.addNewNode('level1', {data: 'interesting'}, []);
+        tree.addNewNode('level2', {data: 'expected data'}, ['level1']);
+      });
+
+      context('retrieve data from the node', () => {
+        it('return the node data', () => {
+          expect(tree.itemData([{id: 'level2'}])).toEqual({
+            data: 'expected' +
+            ' data'
+          })
+        });
+      });
+
+      context('retrieve data from node not found', () => {
+        it('return undefined', () => {
+          expect(tree.itemData([{id: 'bamm'}])).toBeUndefined();
+        });
+      });
+    });
+  });
+});
+
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema.js
index a7fd4c7c..10dc7e3b 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema.js
@@ -450,29 +450,7 @@ define('pgadmin.node.schema', [
     });
 
     pgBrowser.tableChildTreeNodeHierarchy = function(i) {
-      var idx = 0,
-        res = {},
-        t = pgBrowser.tree;
-
-      do {
-        var d = t.itemData(i);
-        if (
-          d._type in pgBrowser.Nodes && pgBrowser.Nodes[d._type].hasId
-        ) {
-          if (d._type === 'partition' || d._type === 'table') {
-            if (!('table' in res)) {
-              res['table'] = _.extend({}, d, {'priority': idx});
-              idx -= 1;
-            }
-          } else {
-            res[d._type] = _.extend({}, d, {'priority': idx});
-            idx -= 1;
-          }
-        }
-        i = t.hasParent(i) ? t.parent(i) : null;
-      } while (i);
-
-      return res;
+      return this.getTreeNodeHierarchy(i);
     };
   }
 
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
index 72b6eb0c..1fdacf52 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
@@ -88,7 +88,6 @@ define('pgadmin.node.column', [
 
   if (!pgBrowser.Nodes['column']) {
     pgBrowser.Nodes['column'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       parent_type: ['table', 'view', 'mview'],
       collection_type: ['coll-table', 'coll-view', 'coll-mview'],
       type: 'column',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/check_constraint/static/js/check_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/check_constraint/static/js/check_constraint.js
index bf7ef556..b1cb2518 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/check_constraint/static/js/check_constraint.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/check_constraint/static/js/check_constraint.js
@@ -8,7 +8,6 @@ define('pgadmin.node.check_constraint', [
   // Check Constraint Node
   if (!pgBrowser.Nodes['check_constraint']) {
     pgAdmin.Browser.Nodes['check_constraint'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       type: 'check_constraint',
       label: gettext('Check'),
       collection_type: 'coll-constraints',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.js
index 0bbf66a1..adccf2e9 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.js
@@ -605,7 +605,6 @@ define('pgadmin.node.exclusion_constraint', [
   // Extend the browser's node class for exclusion constraint node
   if (!pgBrowser.Nodes['exclusion_constraint']) {
     pgAdmin.Browser.Nodes['exclusion_constraint'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       type: 'exclusion_constraint',
       label: gettext('Exclusion constraint'),
       collection_type: 'coll-constraints',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.js
index 4997d175..788cecfc 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.js
@@ -603,7 +603,6 @@ define('pgadmin.node.foreign_key', [
   // Extend the browser's node class for foreign key node
   if (!pgBrowser.Nodes['foreign_key']) {
     pgAdmin.Browser.Nodes['foreign_key'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       type: 'foreign_key',
       label: gettext('Foreign key'),
       collection_type: 'coll-constraints',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.js
index d3a6cff4..0ad0f054 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.js
@@ -20,7 +20,6 @@ define('pgadmin.node.primary_key', [
       parent_type: ['table','partition'],
       canDrop: true,
       canDropCascade: true,
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       Init: function() {
         /* Avoid multiple registration of menus */
         if (this.initialized)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.js
index 769185d6..18d3ca33 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.js
@@ -20,7 +20,6 @@ define('pgadmin.node.unique_constraint', [
       parent_type: ['table','partition'],
       canDrop: true,
       canDropCascade: true,
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       Init: function() {
         /* Avoid multiple registration of menus */
         if (this.initialized)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/static/js/constraints.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/static/js/constraints.js
index cb242cd2..9c0e24fe 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/static/js/constraints.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/static/js/constraints.js
@@ -12,14 +12,12 @@ define('pgadmin.node.constraints', [
         node: 'constraints',
         label: gettext('Constraints'),
         type: 'coll-constraints',
-        getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
         columns: ['name', 'comment'],
       });
   }
 
   if (!pgBrowser.Nodes['constraints']) {
     pgAdmin.Browser.Nodes['constraints'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       type: 'constraints',
       label: gettext('Constraints'),
       collection_type: 'coll-constraints',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.js
index ec2b4da1..2b687c1a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.js
@@ -13,7 +13,6 @@ define('pgadmin.node.index', [
         node: 'index',
         label: gettext('Indexes'),
         type: 'coll-index',
-        getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
         sqlAlterHelp: 'sql-alterindex.html',
         sqlCreateHelp: 'sql-createindex.html',
         dialogHelp: url_for('help.static', {'filename': 'index_dialog.html'}),
@@ -215,7 +214,6 @@ define('pgadmin.node.index', [
 
   if (!pgBrowser.Nodes['index']) {
     pgAdmin.Browser.Nodes['index'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       parent_type: ['table', 'view', 'mview', 'partition'],
       collection_type: ['coll-table', 'coll-view'],
       sqlAlterHelp: 'sql-alterindex.html',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/partitions/static/js/partition.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/partitions/static/js/partition.js
index fd53f743..ef8eb534 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/partitions/static/js/partition.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/partitions/static/js/partition.js
@@ -1,10 +1,12 @@
 define([
+  'sources/tree/pgadmin_tree_node',
   'sources/gettext', 'sources/url_for', 'jquery', 'underscore',
   'underscore.string', 'sources/pgadmin', 'pgadmin.browser',
   'pgadmin.alertifyjs', 'pgadmin.backform', 'pgadmin.backgrid',
   'pgadmin.browser.collection', 'pgadmin.browser.table.partition.utils',
 ],
 function(
+  pgadminTreeNode,
   gettext, url_for, $, _, S, pgAdmin, pgBrowser, Alertify, Backform, Backgrid
 ) {
 
@@ -13,7 +15,6 @@ function(
       pgAdmin.Browser.Collection.extend({
         node: 'partition',
         label: gettext('Partitions'),
-        getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
         type: 'coll-partition',
         columns: [
           'name', 'schema', 'partition_value', 'is_partitioned', 'description',
@@ -80,36 +81,6 @@ function(
         },
         ]);
       },
-      getTreeNodeHierarchy: function(i) {
-        var idx = 0,
-          res = {},
-          t = pgBrowser.tree;
-
-        do {
-          var d = t.itemData(i);
-          if (
-            d._type in pgBrowser.Nodes && pgBrowser.Nodes[d._type].hasId
-          ) {
-            if (d._type == 'partition' && 'partition' in res) {
-              if (!('table' in res)) {
-                res['table'] = _.extend({}, d, {'priority': idx});
-                idx -= 1;
-              }
-            } else if (d._type == 'table') {
-              if (!('table' in res)) {
-                res['table'] = _.extend({}, d, {'priority': idx});
-                idx -= 1;
-              }
-            } else {
-              res[d._type] = _.extend({}, d, {'priority': idx});
-              idx -= 1;
-            }
-          }
-          i = t.hasParent(i) ? t.parent(i) : null;
-        } while (i);
-
-        return res;
-      },
       generate_url: function(item, type, d, with_id, info) {
         if (_.indexOf([
           'stats', 'statistics', 'dependency', 'dependent', 'reset',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/static/js/rule.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/static/js/rule.js
index 3af61754..354909f3 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/static/js/rule.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/rules/static/js/rule.js
@@ -16,7 +16,6 @@ define('pgadmin.node.rule', [
         node: 'rule',
         label: gettext('Rules'),
         type: 'coll-rule',
-        getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
         columns: ['name', 'owner', 'comment'],
       });
   }
@@ -35,7 +34,6 @@ define('pgadmin.node.rule', [
    */
   if (!pgBrowser.Nodes['rule']) {
     pgAdmin.Browser.Nodes['rule'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       parent_type: ['table','view', 'partition'],
       type: 'rule',
       sqlAlterHelp: 'sql-alterrule.html',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.js
index 647d1bfe..99d60243 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.js
@@ -26,7 +26,6 @@ define('pgadmin.node.table', [
 
   if (!pgBrowser.Nodes['table']) {
     pgBrowser.Nodes['table'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       type: 'table',
       label: gettext('Table'),
       collection_type: 'coll-table',
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/static/js/trigger.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/static/js/trigger.js
index fbe7659e..9a1ce2e0 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/static/js/trigger.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/triggers/static/js/trigger.js
@@ -29,14 +29,12 @@ define('pgadmin.node.trigger', [
         node: 'trigger',
         label: gettext('Triggers'),
         type: 'coll-trigger',
-        getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
         columns: ['name', 'description'],
       });
   }
 
   if (!pgBrowser.Nodes['trigger']) {
     pgAdmin.Browser.Nodes['trigger'] = pgBrowser.Node.extend({
-      getTreeNodeHierarchy: pgBrowser.tableChildTreeNodeHierarchy,
       parent_type: ['table', 'view', 'partition'],
       collection_type: ['coll-table', 'coll-view'],
       type: 'trigger',
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 0f640a90..c94616dd 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -1,4 +1,5 @@
 define('pgadmin.browser', [
+  'sources/tree/tree',
   'sources/gettext', 'sources/url_for', 'require', 'jquery', 'underscore', 'underscore.string',
   'bootstrap', 'sources/pgadmin', 'pgadmin.alertifyjs', 'bundled_codemirror',
   'sources/check_node_visibility', 'sources/modify_animation', 'pgadmin.browser.utils', 'wcdocker',
@@ -10,6 +11,7 @@ define('pgadmin.browser', [
   'sources/codemirror/addon/fold/pgadmin-sqlfoldcode',
   'pgadmin.browser.keyboard',
 ], function(
+  tree,
   gettext, url_for, require, $, _, S, Bootstrap, pgAdmin, Alertify,
   codemirror, checkNodeVisibility, modifyAnimation
 ) {
@@ -86,6 +88,7 @@ define('pgadmin.browser', [
       });
 
       b.tree = $('#tree').aciTree('api');
+      b.treeMenu.register($('#tree'));
     };
 
   // Extend the browser class attributes
@@ -100,6 +103,7 @@ define('pgadmin.browser', [
     editor:null,
     // Left hand browser tree
     tree:null,
+    treeMenu: new tree.Tree(),
     // list of script to be loaded, when a certain type of node is loaded
     // It will be used to register extensions, tools, child node scripts,
     // etc.
diff --git a/web/pgadmin/browser/static/js/node.js b/web/pgadmin/browser/static/js/node.js
index ad582483..3baf554a 100644
--- a/web/pgadmin/browser/static/js/node.js
+++ b/web/pgadmin/browser/static/js/node.js
@@ -1,9 +1,12 @@
 define('pgadmin.browser.node', [
+  'sources/tree/pgadmin_tree_node',
   'sources/gettext', 'jquery', 'underscore', 'underscore.string', 'sources/pgadmin',
   'pgadmin.browser.menu', 'backbone', 'pgadmin.alertifyjs', 'pgadmin.browser.datamodel',
   'backform', 'sources/browser/generate_url', 'sources/utils', 'pgadmin.browser.utils',
   'pgadmin.backform',
-], function(gettext, $, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser, Backform, generateUrl, commonUtils) {
+], function(
+  pgadminTreeNode,
+  gettext, $, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser, Backform, generateUrl, commonUtils) {
 
   var wcDocker = window.wcDocker,
     keyCode = {
@@ -1566,7 +1569,6 @@ define('pgadmin.browser.node', [
      * depends, statistics
      */
     generate_url: function(item, type, d, with_id, info) {
-
       var opURL = {
           'create': 'obj',
           'drop': 'obj',
@@ -1608,24 +1610,7 @@ define('pgadmin.browser.node', [
     Collection: pgBrowser.DataCollection,
     // Base class for Node Data Model
     Model: pgBrowser.DataModel,
-    getTreeNodeHierarchy: function(i) {
-      var idx = 0,
-        res = {},
-        t = pgBrowser.tree,
-        d;
-      do {
-        d = t.itemData(i);
-        if (d._type in pgBrowser.Nodes && pgBrowser.Nodes[d._type].hasId) {
-          res[d._type] = _.extend({}, d, {
-            'priority': idx,
-          });
-          idx -= 1;
-        }
-        i = t.hasParent(i) ? t.parent(i) : null;
-      } while (i);
-
-      return res;
-    },
+    getTreeNodeHierarchy: pgadminTreeNode.getTreeNodeHierarchyFromIdentifier.bind(pgBrowser),
     cache: function(url, node_info, level, data) {
       var cached = this.cached = this.cached || {},
         hash = url,
diff --git a/web/pgadmin/static/js/backup/backup_dialog.js b/web/pgadmin/static/js/backup/backup_dialog.js
new file mode 100644
index 00000000..779170ee
--- /dev/null
+++ b/web/pgadmin/static/js/backup/backup_dialog.js
@@ -0,0 +1,142 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import gettext from '../gettext';
+import {sprintf} from 'sprintf-js';
+import Backform from '../backform.pgadmin';
+import {DialogFactory} from './backup_dialog_factory';
+
+export class BackupDialog {
+  constructor(pgBrowser, $, alertify, BackupModel, backform = Backform) {
+    this.pgBrowser = pgBrowser;
+    this.jquery = $;
+    this.alertify = alertify;
+    this.backupModel = BackupModel;
+    this.backform = backform;
+  }
+
+  draw(action, item, params) {
+    const serverInformation = this.retrieveAncestorOfTypeServer(item);
+
+    if (!serverInformation) {
+      return;
+    }
+
+    if (!this.hasBinariesConfiguration(serverInformation)) {
+      return;
+    }
+
+    const typeOfDialog = BackupDialog.typeOfDialog(params);
+
+    const dialog = this.createOrGetDialog(typeOfDialog);
+    dialog(true).resizeTo('60%', '50%');
+  }
+
+  static typeOfDialog(params) {
+    if(params === null) {
+      return 'backup_objects';
+    }
+    let typeOfDialog = 'server';
+    if (!_.isUndefined(params['globals']) && params['globals']) {
+      typeOfDialog = 'globals';
+    }
+    return typeOfDialog;
+  }
+
+  static dialogTitle(typeOfDialog) {
+    if(typeOfDialog === 'backup_objects') {
+      return null;
+    }
+    return ((typeOfDialog === 'globals') ?
+      gettext('Backup Globals...') :
+      gettext('Backup Server...'));
+  }
+
+  createOrGetDialog(typeOfDialog) {
+    const dialogName = this.dialogName(typeOfDialog);
+    const dialogTitle = BackupDialog.dialogTitle(typeOfDialog);
+
+    if (!this.alertify[dialogName]) {
+      const self = this;
+      this.alertify.dialog(dialogName, function factory() {
+        return self.dialogFactory(dialogTitle, typeOfDialog);
+      });
+    }
+    return this.alertify[dialogName];
+  }
+
+  dialogName(typeOfDialog) {
+    if(typeOfDialog === 'backup_objects') {
+      return typeOfDialog;
+    }
+    return 'BackupDialog_' + typeOfDialog;
+  }
+
+  hasBinariesConfiguration(serverInformation) {
+    const module = 'paths';
+    let preference_name = 'pg_bin_dir';
+    let msg = gettext('Please configure the PostgreSQL Binary Path in the Preferences dialog.');
+
+    if ((serverInformation.type && serverInformation.type === 'ppas') ||
+      serverInformation.server_type === 'ppas') {
+      preference_name = 'ppas_bin_dir';
+      msg = gettext('Please configure the EDB Advanced Server Binary Path in the Preferences dialog.');
+    }
+    const preference = this.pgBrowser.get_preference(module, preference_name);
+
+    if (preference) {
+      if (!preference.value) {
+        this.alertify.alert(gettext('Configuration required'), msg);
+        return false;
+      }
+    } else {
+      this.alertify.alert(
+        gettext('Backup Error'),
+        sprintf(gettext('Failed to load preference %s of module %s'), preference_name, module)
+      );
+      return false;
+    }
+    return true;
+  }
+
+  retrieveAncestorOfTypeServer(item) {
+    let serverInformation = null;
+    let aciTreeItem = item || this.pgBrowser.treeMenu.selected();
+    let treeNode = this.pgBrowser.treeMenu.findNodeByDomElement(aciTreeItem);
+
+    while (treeNode) {
+      const node_data = treeNode.getData();
+      if (node_data._type === 'server') {
+        serverInformation = node_data;
+        break;
+      }
+
+      if (treeNode.hasParent()) {
+        treeNode = treeNode.parent();
+      } else {
+        this.alertify.alert(
+          gettext('Backup Error'),
+          gettext('Please select server or child node from the browser tree.')
+        );
+        break;
+      }
+    }
+    return serverInformation;
+  }
+
+  dialogFactory(dialogTitle, typeOfDialog) {
+    const factory = new DialogFactory(
+      this.pgBrowser,
+      this.jquery,
+      this.alertify,
+      this.backupModel,
+      this.backform);
+    return factory.create(dialogTitle, typeOfDialog);
+  }
+}
diff --git a/web/pgadmin/static/js/backup/backup_dialog_factory.js b/web/pgadmin/static/js/backup/backup_dialog_factory.js
new file mode 100644
index 00000000..2fd491d6
--- /dev/null
+++ b/web/pgadmin/static/js/backup/backup_dialog_factory.js
@@ -0,0 +1,56 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import * as commonUtils from '../utils';
+import {DialogWrapper} from './backup_dialog_wrapper';
+
+export class DialogFactory {
+  constructor(pgBrowser, $, alertify, BackupModel, backform) {
+    this.pgBrowser = pgBrowser;
+    this.jquery = $;
+    this.alertify = alertify;
+    this.backupModel = BackupModel;
+    this.backform = backform;
+  }
+
+  create(dialogTitle, typeOfDialog) {
+    const dialogContainerSelector = '<div class=\'backup_dialog\'></div>';
+    return new DialogWrapper(dialogContainerSelector, this, dialogTitle, typeOfDialog);
+  }
+
+  static wasBackupButtonPressed(event) {
+    return event.button['data-btn-name'] === 'backup';
+  }
+
+  static wasHelpButtonPressed(e) {
+    return e.button.element.name === 'dialog_help'
+      || e.button.element.name === 'object_help';
+  }
+
+  static getSelectedNode(selectedTreeNode) {
+    if (!DialogFactory.isNodeSelected(selectedTreeNode)) {
+      return undefined;
+    }
+    const treeNodeData = selectedTreeNode.getData();
+    if (treeNodeData) {
+      return treeNodeData;
+    }
+    return undefined;
+  }
+
+  static focusOnDialog(dialog) {
+    const container = dialog.$el.find('.tab-content:first > .tab-pane.active:first');
+    commonUtils.findAndSetFocus(container);
+  }
+
+  static isNodeSelected(selectedTreeNode) {
+    return selectedTreeNode;
+  }
+}
+
diff --git a/web/pgadmin/static/js/backup/backup_dialog_wrapper.js b/web/pgadmin/static/js/backup/backup_dialog_wrapper.js
new file mode 100644
index 00000000..e5c803c0
--- /dev/null
+++ b/web/pgadmin/static/js/backup/backup_dialog_wrapper.js
@@ -0,0 +1,270 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {getTreeNodeHierarchyFromElement} from '../tree/pgadmin_tree_node';
+import axios from 'axios/index';
+import {DialogFactory} from './backup_dialog_factory';
+import gettext from '../gettext';
+import url_for from '../url_for';
+import _ from 'underscore';
+
+export class DialogWrapper {
+  constructor(dialogContainerSelector, factory, dialogTitle, typeOfDialog) {
+    this.hooks = {
+      // Triggered when the dialog is closed
+      onclose: function () {
+        if (this.view) {
+          // clear our backform model/view
+          this.view.remove({
+            data: true,
+            internal: true,
+            silent: true,
+          });
+        }
+      },
+    };
+    this.dialogContainerSelector = dialogContainerSelector;
+    this.factory = factory;
+    this.dialogTitle = dialogTitle;
+    this.typeOfDialog = typeOfDialog;
+  }
+
+  main(title) {
+    this.set('title', title);
+  }
+
+  build() {
+    this.factory.alertify.pgDialogBuild.apply(this);
+  }
+
+  setup() {
+    return {
+      buttons: [{
+        text: '',
+        className: 'btn btn-default pull-left fa fa-lg fa-info',
+        attrs: {
+          name: 'object_help',
+          type: 'button',
+          url: 'backup.html',
+          label: gettext('Backup'),
+        },
+      }, {
+        text: '',
+        key: 112,
+        className: 'btn btn-default pull-left fa fa-lg fa-question',
+        attrs: {
+          name: 'dialog_help',
+          type: 'button',
+          label: gettext('Backup'),
+          url: url_for('help.static', {
+            'filename': 'backup_dialog.html',
+          }),
+        },
+      }, {
+        text: gettext('Backup'),
+        key: 13,
+        className: 'btn btn-primary fa fa-lg fa-save pg-alertify-button',
+        'data-btn-name': 'backup',
+      }, {
+        text: gettext('Cancel'),
+        key: 27,
+        className: 'btn btn-danger fa fa-lg fa-times pg-alertify-button',
+        'data-btn-name': 'cancel',
+      }],
+      // Set options for dialog
+      options: {
+        title: this.dialogTitle,
+        //disable both padding and overflow control.
+        padding: !1,
+        overflow: !1,
+        model: 0,
+        resizable: true,
+        maximizable: true,
+        pinnable: false,
+        closableByDimmer: false,
+        modal: false,
+      },
+    };
+  }
+
+  prepare() {
+    this.disableBackupButton();
+
+    const $container = this.factory.jquery(this.dialogContainerSelector);
+    const selectedTreeNode = this.getSelectedNode();
+    const selectedTreeNodeData = DialogFactory.getSelectedNode(selectedTreeNode);
+    if (!selectedTreeNodeData) {
+      return;
+    }
+
+    const node = this.factory.pgBrowser.Nodes[selectedTreeNodeData._type];
+    if (this.dialogTitle === null) {
+      const title = `Backup (${node.label}: ${selectedTreeNodeData.label})`;
+      this.main(title);
+    }
+
+    const treeInfo = getTreeNodeHierarchyFromElement(this.factory.pgBrowser, selectedTreeNode);
+    const dialog = this.createDialog(node, treeInfo, this.typeOfDialog, $container);
+    this.addAlertifyClassToBackupNodeChildNodes();
+    dialog.render();
+
+    this.elements.content.appendChild($container.get(0));
+
+    DialogFactory.focusOnDialog(dialog);
+    this.setListenersForFilenameChanges();
+  }
+
+  callback(event) {
+    const selectedTreeNode = this.getSelectedNode();
+    const selectedTreeNodeData = DialogFactory.getSelectedNode(selectedTreeNode);
+    const node = selectedTreeNodeData && this.factory.pgBrowser.Nodes[selectedTreeNodeData._type];
+
+    if (DialogFactory.wasHelpButtonPressed(event)) {
+      event.cancel = true;
+      this.factory.pgBrowser.showHelp(
+        event.button.element.name,
+        event.button.element.getAttribute('url'),
+        node,
+        selectedTreeNode,
+        event.button.element.getAttribute('label')
+      );
+      return;
+    }
+
+    if (DialogFactory.wasBackupButtonPressed(event)) {
+
+      if (!selectedTreeNodeData)
+        return;
+
+      const serverIdentifier = this.retrieveServerIdentifier(node, selectedTreeNode);
+
+      const factory = this.factory;
+      let urlShortcut = 'backup.create_server_job';
+      if (this.typeOfDialog === 'backup_objects') {
+        urlShortcut = 'backup.create_object_job';
+      }
+      const baseUrl = url_for(urlShortcut, {
+        'sid': serverIdentifier,
+      });
+
+      this.setExtraParameters(selectedTreeNode);
+
+      let service = axios.create({});
+      service.post(
+        baseUrl,
+        this.view.model.toJSON()
+      ).then(function () {
+        factory.alertify.success(gettext('Backup job created.'), 5);
+        factory.pgBrowser.Events.trigger('pgadmin-bgprocess:created', factory);
+      }).catch(function (error) {
+        try {
+          const err = error.response.data;
+          factory.alertify.alert(
+            gettext('Backup job failed.'),
+            err.errormsg
+          );
+        } catch (e) {
+          console.warn(e.stack || e);
+        }
+      });
+    }
+  }
+
+  addAlertifyClassToBackupNodeChildNodes() {
+    this.factory.jquery(this.elements.body.childNodes[0]).addClass(
+      'alertify_tools_dialog_properties obj_properties'
+    );
+  }
+
+  getSelectedNode() {
+    const tree = this.factory.pgBrowser.treeMenu;
+    const selectedNode = tree.selected();
+    if (selectedNode) {
+      return tree.findNodeByDomElement(selectedNode);
+    } else {
+      return undefined;
+    }
+  }
+
+  disableBackupButton() {
+    this.__internal.buttons[2].element.disabled = true;
+  }
+
+  enableBackupButton() {
+    this.__internal.buttons[2].element.disabled = false;
+  }
+
+  createDialog(node, treeInfo, typeOfDialog, $container) {
+    let attributes = {};
+    if (typeOfDialog !== 'backup_objects') {
+      attributes['type'] = typeOfDialog;
+    }
+    // Instance of backbone model
+    const newModel = new this.factory.backupModel(attributes, {
+      node_info: treeInfo,
+    });
+    const fields = this.factory.backform.generateViewSchema(
+      treeInfo, newModel, 'create', node, treeInfo.server, true
+    );
+
+    return this.view = new this.factory.backform.Dialog({
+      el: $container,
+      model: newModel,
+      schema: fields,
+    });
+  }
+
+  retrieveServerIdentifier(node, selectedTreeNode) {
+    const treeInfo = getTreeNodeHierarchyFromElement(
+      this.factory.pgBrowser,
+      selectedTreeNode
+    );
+    return treeInfo.server._id;
+  }
+
+  setListenersForFilenameChanges() {
+    const self = this;
+
+    this.view.model.on('change', function () {
+      if (!_.isUndefined(this.get('file')) && this.get('file') !== '') {
+        this.errorModel.clear();
+        self.enableBackupButton();
+      } else {
+        self.disableBackupButton();
+        this.errorModel.set('file', gettext('Please provide a filename'));
+      }
+    });
+  }
+
+  setExtraParameters(selectedTreeNode) {
+    if (this.typeOfDialog === 'backup_objects') {
+      const treeInfo = getTreeNodeHierarchyFromElement(
+        this.factory.pgBrowser,
+        selectedTreeNode
+      );
+
+      this.view.model.set('database', treeInfo.database._label);
+
+      const nodeData = selectedTreeNode.getData();
+      if (nodeData._type === 'schema') {
+        this.view.model.set('schemas', [nodeData._label]);
+      }
+
+      if (nodeData._type === 'table') {
+        this.view.model.set('tables', [
+          [treeInfo.schema._label, nodeData._label],
+        ]);
+      }
+
+      if (_.isEmpty(this.view.model.get('ratio'))) {
+        this.view.model.unset('ratio');
+      }
+    }
+  }
+}
diff --git a/web/pgadmin/static/js/backup/menu_utils.js b/web/pgadmin/static/js/backup/menu_utils.js
new file mode 100644
index 00000000..f67a436a
--- /dev/null
+++ b/web/pgadmin/static/js/backup/menu_utils.js
@@ -0,0 +1,60 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+export const backupSupportedNodes = [
+  'database', 'schema', 'table', 'partition',
+];
+
+function isNodeTypeSupported(nodeDataType, parentNodeType) {
+  return _.indexOf(backupSupportedNodes, nodeDataType) !== -1
+    && parentNodeType !== 'catalog';
+}
+
+function isProvidedDataValid(treeNodeData) {
+  return !_.isUndefined(treeNodeData) && !_.isNull(treeNodeData);
+}
+
+function doesNodeHaveMenu(treeNodeData) {
+  return (treeNodeData._type === 'database' && treeNodeData.allowConn)
+    || treeNodeData._type !== 'database';
+}
+
+function retrieveParentNodeType(treeNode) {
+  if (!treeNode.hasParent()) {
+    return null;
+  }
+  if(treeNode.parent().getData() === null) {
+    return null;
+  }
+  return treeNode.parent().getData()._type;
+}
+
+export function menuEnabled(treeNodeData, domTreeNode) {
+  let treeNode = this.treeMenu.findNodeByDomElement(domTreeNode);
+  if(!treeNode) {
+    return false;
+  }
+  let parentNodeType = retrieveParentNodeType(treeNode);
+
+  if (isProvidedDataValid(treeNodeData) && !_.isNull(parentNodeType)) {
+    return isNodeTypeSupported(treeNodeData._type, parentNodeType)
+      && doesNodeHaveMenu(treeNodeData);
+  } else {
+    return false;
+  }
+}
+
+function isNodeAServerAndConnected(treeNodeData) {
+  return (('server' === treeNodeData._type) && treeNodeData.connected);
+}
+
+export function menuEnabledServer(treeNodeData) {
+  return isProvidedDataValid(treeNodeData)
+    && isNodeAServerAndConnected(treeNodeData);
+}
diff --git a/web/pgadmin/static/js/datagrid/get_panel_title.js b/web/pgadmin/static/js/datagrid/get_panel_title.js
new file mode 100644
index 00000000..f5a5664d
--- /dev/null
+++ b/web/pgadmin/static/js/datagrid/get_panel_title.js
@@ -0,0 +1,33 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {getTreeNodeHierarchyFromIdentifier} from '../tree/pgadmin_tree_node';
+
+function getDatabaseLabel(parentData) {
+  return parentData.database ? parentData.database.label
+    : parentData.server.db;
+}
+
+function isServerInformationAvailable(parentData) {
+  return parentData.server === undefined;
+}
+
+export function getPanelTitle(pgBrowser) {
+  const selected_item = pgBrowser.treeMenu.selected();
+
+  const parentData = getTreeNodeHierarchyFromIdentifier
+    .call(pgBrowser, selected_item);
+  if (isServerInformationAvailable(parentData)) {
+    return;
+  }
+
+  const db_label = getDatabaseLabel(parentData);
+
+  return `${db_label} on ${parentData.server.user.name}@${parentData.server.label}`;
+}
diff --git a/web/pgadmin/static/js/datagrid/show_data.js b/web/pgadmin/static/js/datagrid/show_data.js
new file mode 100644
index 00000000..a414fd77
--- /dev/null
+++ b/web/pgadmin/static/js/datagrid/show_data.js
@@ -0,0 +1,92 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+import gettext from '../gettext';
+import url_for from '../url_for';
+import {getTreeNodeHierarchyFromIdentifier} from '../../../static/js/tree/pgadmin_tree_node';
+
+export function showDataGrid(
+  datagrid,
+  pgBrowser,
+  alertify,
+  connectionData,
+  aciTreeIdentifier
+) {
+  const node = pgBrowser.treeMenu.findNodeByDomElement(aciTreeIdentifier);
+  if (node === undefined || !node.getData()) {
+    alertify.alert(
+      gettext('Data Grid Error'),
+      gettext('No object selected.')
+    );
+    return;
+  }
+
+  const parentData = getTreeNodeHierarchyFromIdentifier.call(
+    pgBrowser,
+    aciTreeIdentifier
+  );
+
+  if (hasServerOrDatabaseConfiguration(parentData)
+    || !hasSchemaOrCatalogOrViewInformation(parentData)) {
+    return;
+  }
+
+  let namespaceName = retrieveNameSpaceName(parentData);
+  const baseUrl = generateUrl(connectionData, node.getData(), parentData);
+  const grid_title = generateDatagridTitle(parentData, namespaceName, node.getData());
+
+  datagrid.create_transaction(
+    baseUrl,
+    null,
+    'false',
+    parentData.server.server_type,
+    '',
+    grid_title,
+    ''
+  );
+}
+
+
+function retrieveNameSpaceName(parentData) {
+  if (parentData.schema !== undefined) {
+    return parentData.schema.label;
+  }
+  else if (parentData.view !== undefined) {
+    return parentData.view.label;
+  }
+  else if (parentData.catalog !== undefined) {
+    return parentData.catalog.label;
+  }
+  return '';
+}
+
+function generateUrl(connectionData, nodeData, parentData) {
+  const url_params = {
+    'cmd_type': connectionData.mnuid,
+    'obj_type': nodeData._type,
+    'sgid': parentData.server_group._id,
+    'sid': parentData.server._id,
+    'did': parentData.database._id,
+    'obj_id': nodeData._id,
+  };
+
+  return url_for('datagrid.initialize_datagrid', url_params);
+}
+
+function hasServerOrDatabaseConfiguration(parentData) {
+  return parentData.server === undefined || parentData.database === undefined;
+}
+
+function hasSchemaOrCatalogOrViewInformation(parentData) {
+  return parentData.schema !== undefined || parentData.view !== undefined ||
+    parentData.catalog !== undefined;
+}
+
+function generateDatagridTitle(parentData, namespaceName, nodeData) {
+  return `${parentData.server.label} - ${parentData.database.label} - ${namespaceName}.${nodeData.label}`;
+}
diff --git a/web/pgadmin/static/js/datagrid/show_query_tool.js b/web/pgadmin/static/js/datagrid/show_query_tool.js
new file mode 100644
index 00000000..0436e0fd
--- /dev/null
+++ b/web/pgadmin/static/js/datagrid/show_query_tool.js
@@ -0,0 +1,63 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import gettext from '../gettext';
+import url_for from '../url_for';
+import {getTreeNodeHierarchyFromIdentifier} from '../../../static/js/tree/pgadmin_tree_node';
+
+function hasDatabaseInformation(parentData) {
+  return parentData.database;
+}
+
+function generateUrl(parentData) {
+  let url_endpoint = 'datagrid.initialize_query_tool';
+  let url_params = {
+    'sgid': parentData.server_group._id,
+    'sid': parentData.server._id,
+  };
+
+  if (hasDatabaseInformation(parentData)) {
+    url_params['did'] = parentData.database._id;
+    url_endpoint = 'datagrid.initialize_query_tool_with_did';
+  }
+
+  return url_for(url_endpoint, url_params);
+}
+
+function hasServerInformations(parentData) {
+  return parentData.server === undefined;
+}
+
+export function showQueryTool(datagrid, pgBrowser, alertify, url,
+                              aciTreeIdentifier, panelTitle) {
+  const sURL = url || '';
+  const queryToolTitle = panelTitle || '';
+
+  const currentNode = pgBrowser.treeMenu.findNodeByDomElement(aciTreeIdentifier);
+  if (currentNode === undefined) {
+    alertify.alert(
+      gettext('Query Tool Error'),
+      gettext('No object selected.')
+    );
+    return;
+  }
+
+  const parentData = getTreeNodeHierarchyFromIdentifier.call(
+    pgBrowser, aciTreeIdentifier);
+
+  if (hasServerInformations(parentData)) {
+    return;
+  }
+
+  const baseUrl = generateUrl(parentData);
+
+  datagrid.create_transaction(
+    baseUrl, null, 'true',
+    parentData.server.server_type, sURL, queryToolTitle, '', false);
+}
diff --git a/web/pgadmin/static/js/tree/pgadmin_tree_node.js b/web/pgadmin/static/js/tree/pgadmin_tree_node.js
new file mode 100644
index 00000000..bccbe587
--- /dev/null
+++ b/web/pgadmin/static/js/tree/pgadmin_tree_node.js
@@ -0,0 +1,43 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+export function getTreeNodeHierarchyFromElement(pgBrowser, treeNode) {
+  return getTreeNodeHierarchy.call(pgBrowser, treeNode);
+}
+
+export function getTreeNodeHierarchyFromIdentifier(aciTreeNodeIdentifier) {
+  let identifier = this.treeMenu.translateTreeNodeIdFromACITree(aciTreeNodeIdentifier);
+  let currentNode = this.treeMenu.findNode(identifier);
+  return getTreeNodeHierarchy.call(this, currentNode);
+}
+
+export function getTreeNodeHierarchy(currentNode) {
+  let idx = 0;
+  let result = {};
+
+  do {
+    const currentNodeData = currentNode.getData();
+    if (currentNodeData._type in this.Nodes && this.Nodes[currentNodeData._type].hasId) {
+      const nodeType = mapType(currentNodeData._type);
+      if (result[nodeType] === undefined) {
+        result[nodeType] = _.extend({}, currentNodeData, {
+          'priority': idx,
+        });
+        idx -= 1;
+      }
+    }
+    currentNode = currentNode.hasParent() ? currentNode.parent() : null;
+  } while (currentNode);
+
+  return result;
+}
+
+function mapType(type) {
+  return type === 'partition' ? 'table' : type;
+}
diff --git a/web/pgadmin/tools/backup/static/js/backup.js b/web/pgadmin/tools/backup/static/js/backup.js
index ddbfaee4..ffbae127 100644
--- a/web/pgadmin/tools/backup/static/js/backup.js
+++ b/web/pgadmin/tools/backup/static/js/backup.js
@@ -3,9 +3,10 @@ define([
   'sources/gettext', 'sources/url_for', 'jquery', 'underscore',
   'underscore.string', 'pgadmin.alertifyjs', 'backbone', 'pgadmin.backgrid',
   'pgadmin.backform', 'pgadmin.browser', 'sources/utils',
+  'sources/backup/menu_utils', 'sources/backup/backup_dialog',
 ], function(
   gettext, url_for, $, _, S, alertify, Backbone, Backgrid, Backform, pgBrowser,
-commonUtils
+commonUtils, menuUtils, globalBackupDialog
 ) {
 
   // if module is already initialized, refer to that.
@@ -394,48 +395,6 @@ commonUtils
 
       this.initialized = true;
 
-      // Define list of nodes on which backup context menu option appears
-      var backup_supported_nodes = [
-        'database', 'schema', 'table', 'partition',
-      ];
-
-      /**
-        Enable/disable backup menu in tools based
-        on node selected
-        if selected node is present in supported_nodes,
-        menu will be enabled otherwise disabled.
-        Also, hide it for system view in catalogs
-      */
-      var menu_enabled = function(itemData, item) {
-        var t = pgBrowser.tree,
-          i = item,
-          d = itemData,
-          parent_item = t.hasParent(i) ? t.parent(i) : null,
-          parent_data = parent_item ? t.itemData(parent_item) : null;
-
-        if (!_.isUndefined(d) && !_.isNull(d) && !_.isNull(parent_data)) {
-          if (_.indexOf(backup_supported_nodes, d._type) !== -1 &&
-            parent_data._type != 'catalog') {
-            if (d._type == 'database' && d.allowConn)
-              return true;
-            else if (d._type != 'database')
-              return true;
-            else
-              return false;
-          } else
-            return false;
-        } else
-          return false;
-      };
-
-      var menu_enabled_server = function(itemData) {
-        // If server node selected && connected
-        if (!_.isUndefined(itemData) && !_.isNull(itemData))
-          return (('server' === itemData._type) && itemData.connected);
-        else
-          return false;
-      };
-
       // Define the nodes on which the menus to be appear
       var menus = [{
         name: 'backup_global',
@@ -445,7 +404,7 @@ commonUtils
         priority: 12,
         label: gettext('Backup Globals...'),
         icon: 'fa fa-floppy-o',
-        enable: menu_enabled_server,
+        enable: menuUtils.menuEnabledServer,
       }, {
         name: 'backup_server',
         module: this,
@@ -454,7 +413,7 @@ commonUtils
         priority: 12,
         label: gettext('Backup Server...'),
         icon: 'fa fa-floppy-o',
-        enable: menu_enabled_server,
+        enable: menuUtils.menuEnabledServer,
       }, {
         name: 'backup_global_ctx',
         module: this,
@@ -464,7 +423,7 @@ commonUtils
         priority: 12,
         label: gettext('Backup Globals...'),
         icon: 'fa fa-floppy-o',
-        enable: menu_enabled_server,
+        enable: menuUtils.menuEnabledServer,
       }, {
         name: 'backup_server_ctx',
         module: this,
@@ -474,7 +433,7 @@ commonUtils
         priority: 12,
         label: gettext('Backup Server...'),
         icon: 'fa fa-floppy-o',
-        enable: menu_enabled_server,
+        enable: menuUtils.menuEnabledServer,
       }, {
         name: 'backup_object',
         module: this,
@@ -483,20 +442,20 @@ commonUtils
         priority: 11,
         label: gettext('Backup...'),
         icon: 'fa fa-floppy-o',
-        enable: menu_enabled,
+        enable: menuUtils.menuEnabled.bind(pgBrowser),
       }];
 
-      for (var idx = 0; idx < backup_supported_nodes.length; idx++) {
+      for (var idx = 0; idx < menuUtils.backupSupportedNodes.length; idx++) {
         menus.push({
-          name: 'backup_' + backup_supported_nodes[idx],
-          node: backup_supported_nodes[idx],
+          name: 'backup_' + menuUtils.backupSupportedNodes[idx],
+          node: menuUtils.backupSupportedNodes[idx],
           module: this,
           applies: ['context'],
           callback: 'backup_objects',
           priority: 11,
           label: gettext('Backup...'),
           icon: 'fa fa-floppy-o',
-          enable: menu_enabled,
+          enable: menuUtils.menuEnabled.bind(pgBrowser),
         });
       }
 
@@ -521,531 +480,25 @@ commonUtils
     },
 
     // Callback to draw Backup Dialog for globals/server
-    start_backup_global_server: function(action, item, params) {
-      var i = item || pgBrowser.tree.selected(),
-        server_data = null;
-
-      while (i) {
-        var node_data = pgBrowser.tree.itemData(i);
-        if (node_data._type == 'server') {
-          server_data = node_data;
-          break;
-        }
-
-        if (pgBrowser.tree.hasParent(i)) {
-          i = $(pgBrowser.tree.parent(i));
-        } else {
-          alertify.alert(
-            gettext('Backup Error'),
-            gettext('Please select server or child node from the browser tree.')
-          );
-          break;
-        }
-      }
-
-      if (!server_data) {
-        return;
-      }
-
-      var module = 'paths',
-        preference_name = 'pg_bin_dir',
-        msg = gettext('Please configure the PostgreSQL Binary Path in the Preferences dialog.');
-
-      if ((server_data.type && server_data.type == 'ppas') ||
-        server_data.server_type == 'ppas') {
-        preference_name = 'ppas_bin_dir';
-        msg = gettext('Please configure the EDB Advanced Server Binary Path in the Preferences dialog.');
-      }
-
-      var preference = pgBrowser.get_preference(module, preference_name);
-
-      if (preference) {
-        if (!preference.value) {
-          alertify.alert(gettext('Configuration required'), msg);
-          return;
-        }
-      } else {
-        alertify.alert(
-          gettext('Backup Error'),
-          S(gettext('Failed to load preference %s of module %s')).sprintf(preference_name, module).value()
-        );
-        return;
-      }
-
-      var of_type = undefined;
-
-      // Set Notes according to type of backup
-      if (!_.isUndefined(params['globals']) && params['globals']) {
-        of_type = 'globals';
-      } else {
-        of_type = 'server';
-      }
-
-      var DialogName = 'BackupDialog_' + of_type,
-        DialogTitle = ((of_type == 'globals') ?
-          gettext('Backup Globals...') :
-          gettext('Backup Server...'));
-
-      if (!alertify[DialogName]) {
-        alertify.dialog(DialogName, function factory() {
-          return {
-            main: function(title) {
-              this.set('title', title);
-            },
-            build: function() {
-              alertify.pgDialogBuild.apply(this);
-            },
-            setup: function() {
-              return {
-                buttons: [{
-                  text: '',
-                  className: 'btn btn-default pull-left fa fa-lg fa-info',
-                  attrs: {
-                    name: 'object_help',
-                    type: 'button',
-                    url: 'backup.html',
-                    label: gettext('Backup'),
-                  },
-                }, {
-                  text: '',
-                  key: 112,
-                  className: 'btn btn-default pull-left fa fa-lg fa-question',
-                  attrs: {
-                    name: 'dialog_help',
-                    type: 'button',
-                    label: gettext('Backup'),
-                    url: url_for('help.static', {
-                      'filename': 'backup_dialog.html',
-                    }),
-                  },
-                }, {
-                  text: gettext('Backup'),
-                  key: 13,
-                  className: 'btn btn-primary fa fa-lg fa-save pg-alertify-button',
-                  'data-btn-name': 'backup',
-                }, {
-                  text: gettext('Cancel'),
-                  key: 27,
-                  className: 'btn btn-danger fa fa-lg fa-times pg-alertify-button',
-                  'data-btn-name': 'cancel',
-                }],
-                // Set options for dialog
-                options: {
-                  title: DialogTitle,
-                  //disable both padding and overflow control.
-                  padding: !1,
-                  overflow: !1,
-                  model: 0,
-                  resizable: true,
-                  maximizable: true,
-                  pinnable: false,
-                  closableByDimmer: false,
-                  modal: false,
-                },
-              };
-            },
-            hooks: {
-              // Triggered when the dialog is closed
-              onclose: function() {
-                if (this.view) {
-                  // clear our backform model/view
-                  this.view.remove({
-                    data: true,
-                    internal: true,
-                    silent: true,
-                  });
-                }
-              },
-            },
-            prepare: function() {
-              var self = this;
-              // Disable Backup button until user provides Filename
-              this.__internal.buttons[2].element.disabled = true;
-
-              var $container = $('<div class=\'backup_dialog\'></div>');
-              // Find current/selected node
-              var t = pgBrowser.tree,
-                i = t.selected(),
-                d = i && i.length == 1 ? t.itemData(i) : undefined,
-                node = d && pgBrowser.Nodes[d._type];
-
-              if (!d)
-                return;
-              // Create treeInfo
-              var treeInfo = node.getTreeNodeHierarchy.apply(node, [i]);
-              // Instance of backbone model
-              var newModel = new BackupModel({
-                  type: of_type,
-                }, {
-                  node_info: treeInfo,
-                }),
-                fields = Backform.generateViewSchema(
-                  treeInfo, newModel, 'create', node, treeInfo.server, true
-                );
-
-              var view = this.view = new Backform.Dialog({
-                el: $container,
-                model: newModel,
-                schema: fields,
-              });
-              // Add our class to alertify
-              $(this.elements.body.childNodes[0]).addClass(
-                'alertify_tools_dialog_properties obj_properties'
-              );
-              // Render dialog
-              view.render();
-
-              this.elements.content.appendChild($container.get(0));
-
-              var container = view.$el.find('.tab-content:first > .tab-pane.active:first');
-              commonUtils.findAndSetFocus(container);
-
-              // Listen to model & if filename is provided then enable Backup button
-              this.view.model.on('change', function() {
-                if (!_.isUndefined(this.get('file')) && this.get('file') !== '') {
-                  this.errorModel.clear();
-                  self.__internal.buttons[2].element.disabled = false;
-                } else {
-                  self.__internal.buttons[2].element.disabled = true;
-                  this.errorModel.set('file', gettext('Please provide a filename'));
-                }
-              });
-            },
-            // Callback functions when click on the buttons of the Alertify dialogs
-            callback: function(e) {
-              // Fetch current server id
-              var t = pgBrowser.tree,
-                i = t.selected(),
-                d = i && i.length == 1 ? t.itemData(i) : undefined,
-                node = d && pgBrowser.Nodes[d._type];
-
-              if (e.button.element.name == 'dialog_help' || e.button.element.name == 'object_help') {
-                e.cancel = true;
-                pgBrowser.showHelp(e.button.element.name, e.button.element.getAttribute('url'),
-                  node, i, e.button.element.getAttribute('label'));
-                return;
-              }
-
-              if (e.button['data-btn-name'] === 'backup') {
-
-                if (!d)
-                  return;
-
-                var treeInfo = node.getTreeNodeHierarchy.apply(node, [i]);
-
-                var self = this,
-                  baseUrl = url_for('backup.create_server_job', {
-                    'sid': treeInfo.server._id,
-                  }),
-                  args = this.view.model.toJSON();
-
-                $.ajax({
-                  url: baseUrl,
-                  method: 'POST',
-                  data: {
-                    'data': JSON.stringify(args),
-                  },
-                  success: function(res) {
-                    if (res.success) {
-                      alertify.success(gettext('Backup job created.'), 5);
-                      pgBrowser.Events.trigger('pgadmin-bgprocess:created', self);
-                    } else {
-                      console.warn(res);
-                    }
-                  },
-                  error: function(xhr) {
-                    try {
-                      var err = $.parseJSON(xhr.responseText);
-                      alertify.alert(
-                        gettext('Backup job failed.'),
-                        err.errormsg
-                      );
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
-                  },
-                });
-              }
-            },
-          };
-        });
-      }
-      alertify[DialogName](true).resizeTo('60%', '50%');
+    start_backup_global_server: function(action, treeItem, params) {
+      let dialog = new globalBackupDialog.BackupDialog(
+        pgBrowser,
+        $,
+        alertify,
+        BackupModel
+      );
+      dialog.draw(action, treeItem, params);
     },
 
     // Callback to draw Backup Dialog for objects
     backup_objects: function(action, treeItem) {
-
-      var i = treeItem || pgBrowser.tree.selected(),
-        server_data = null;
-
-      while (i) {
-        var node_data = pgBrowser.tree.itemData(i);
-        if (node_data._type == 'server') {
-          server_data = node_data;
-          break;
-        }
-
-        if (pgBrowser.tree.hasParent(i)) {
-          i = $(pgBrowser.tree.parent(i));
-        } else {
-          alertify.alert(
-            gettext('Backup Error'),
-            gettext('Please select server or child node from tree.')
-          );
-          break;
-        }
-      }
-
-      if (!server_data) {
-        return;
-      }
-
-      var module = 'paths',
-        preference_name = 'pg_bin_dir',
-        msg = gettext('Please set binary path for PostgreSQL Server from preferences.');
-
-      if ((server_data.type && server_data.type == 'ppas') ||
-        server_data.server_type == 'ppas') {
-        preference_name = 'ppas_bin_dir';
-        msg = gettext('Please set binary path for EDB Postgres Advanced Server from preferences.');
-      }
-
-      var preference = pgBrowser.get_preference(module, preference_name);
-
-      if (preference) {
-        if (!preference.value) {
-          alertify.alert(gettext('Configuration required'), msg);
-          return;
-        }
-      } else {
-        alertify.alert(
-          gettext('Backup Error'),
-          S(gettext('Failed to load preference %s of module %s')).sprintf(preference_name, module).value()
-        );
-        return;
-      }
-
-      var title = S(gettext('Backup (%s: %s)')),
-        tree = pgBrowser.tree,
-        item = treeItem || tree.selected(),
-        data = item && item.length == 1 && tree.itemData(item),
-        node = data && data._type && pgBrowser.Nodes[data._type];
-
-      if (!node)
-        return;
-
-      title = title.sprintf(node.label, data.label).value();
-
-      if (!alertify.backup_objects) {
-        // Create Dialog title on the fly with node details
-        alertify.dialog('backup_objects', function factory() {
-          return {
-            main: function(title) {
-              this.set('title', title);
-            },
-            build: function() {
-              alertify.pgDialogBuild.apply(this);
-            },
-            setup: function() {
-              return {
-                buttons: [{
-                  text: '',
-                  className: 'btn btn-default pull-left fa fa-lg fa-info',
-                  attrs: {
-                    name: 'object_help',
-                    type: 'button',
-                    url: 'backup.html',
-                    label: gettext('Backup'),
-                  },
-                }, {
-                  text: '',
-                  key: 112,
-                  className: 'btn btn-default pull-left fa fa-lg fa-question',
-                  attrs: {
-                    name: 'dialog_help',
-                    type: 'button',
-                    label: gettext('Backup'),
-                    url: url_for('help.static', {
-                      'filename': 'backup_dialog.html',
-                    }),
-                  },
-                }, {
-                  text: gettext('Backup'),
-                  key: 13,
-                  className: 'btn btn-primary fa fa-lg fa-save pg-alertify-button',
-                  'data-btn-name': 'backup',
-                }, {
-                  text: gettext('Cancel'),
-                  key: 27,
-                  className: 'btn btn-danger fa fa-lg fa-times pg-alertify-button',
-                  'data-btn-name': 'cancel',
-                }],
-                // Set options for dialog
-                options: {
-                  title: title,
-                  //disable both padding and overflow control.
-                  padding: !1,
-                  overflow: !1,
-                  model: 0,
-                  resizable: true,
-                  maximizable: true,
-                  pinnable: false,
-                  closableByDimmer: false,
-                  modal: false,
-                },
-              };
-            },
-            hooks: {
-              // triggered when the dialog is closed
-              onclose: function() {
-                if (this.view) {
-                  this.view.remove({
-                    data: true,
-                    internal: true,
-                    silent: true,
-                  });
-                }
-              },
-            },
-            prepare: function() {
-              var self = this;
-              // Disable Backup button until user provides Filename
-              this.__internal.buttons[2].element.disabled = true;
-              var $container = $('<div class=\'backup_dialog\'></div>');
-              var t = pgBrowser.tree,
-                i = t.selected(),
-                d = i && i.length == 1 ? t.itemData(i) : undefined,
-                node = d && pgBrowser.Nodes[d._type];
-
-              if (!d)
-                return;
-
-              var treeInfo = node.getTreeNodeHierarchy.apply(node, [i]);
-
-              var newModel = new BackupObjectModel({}, {
-                  node_info: treeInfo,
-                }),
-                fields = Backform.generateViewSchema(
-                  treeInfo, newModel, 'create', node, treeInfo.server, true
-                );
-
-              var view = this.view = new Backform.Dialog({
-                el: $container,
-                model: newModel,
-                schema: fields,
-              });
-
-              $(this.elements.body.childNodes[0]).addClass(
-                'alertify_tools_dialog_properties obj_properties'
-              );
-
-              view.render();
-
-              this.elements.content.appendChild($container.get(0));
-
-              if(view) {
-                view.$el.attr('tabindex', -1);
-                // var dialogTabNavigator = pgBrowser.keyboardNavigation.getDialogTabNavigator(view);
-                pgBrowser.keyboardNavigation.getDialogTabNavigator(view);
-                var container = view.$el.find('.tab-content:first > .tab-pane.active:first');
-                commonUtils.findAndSetFocus(container);
-              }
-              // Listen to model & if filename is provided then enable Backup button
-              this.view.model.on('change', function() {
-                if (!_.isUndefined(this.get('file')) && this.get('file') !== '') {
-                  this.errorModel.clear();
-                  self.__internal.buttons[2].element.disabled = false;
-                } else {
-                  self.__internal.buttons[2].element.disabled = true;
-                  this.errorModel.set('file', gettext('Please provide filename'));
-                }
-              });
-
-            },
-            // Callback functions when click on the buttons of the Alertify dialogs
-            callback: function(e) {
-              // Fetch current server id
-              var t = pgBrowser.tree,
-                i = t.selected(),
-                d = i && i.length == 1 ? t.itemData(i) : undefined,
-                node = d && pgBrowser.Nodes[d._type];
-
-              if (e.button.element.name == 'dialog_help' || e.button.element.name == 'object_help') {
-                e.cancel = true;
-                pgBrowser.showHelp(e.button.element.name, e.button.element.getAttribute('url'),
-                  node, i, e.button.element.getAttribute('label'));
-                return;
-              }
-
-              if (e.button['data-btn-name'] === 'backup') {
-                if (!d)
-                  return;
-
-                var treeInfo = node.getTreeNodeHierarchy.apply(node, [i]);
-
-                // Set current database into model
-                this.view.model.set('database', treeInfo.database._label);
-
-                // We will remove once object tree is implemented
-                // If selected node is Schema then add it in model
-                if (d._type == 'schema') {
-                  var schemas = [];
-                  schemas.push(d._label);
-                  this.view.model.set('schemas', schemas);
-                }
-                // If selected node is Table then add it in model along with
-                // its schema
-                if (d._type == 'table') {
-                  this.view.model.set(
-                    'tables', [
-                      [treeInfo.schema._label, d._label],
-                    ]
-                  );
-                }
-
-                // Remove ratio attribute from model if it has empty string.
-                // The valid value can be between 0 to 9.
-                if (_.isEmpty(this.view.model.get('ratio'))) {
-                  this.view.model.unset('ratio');
-                }
-
-                var self = this,
-                  baseUrl = url_for('backup.create_object_job', {
-                    'sid': treeInfo.server._id,
-                  }),
-                  args = this.view.model.toJSON();
-
-                $.ajax({
-                  url: baseUrl,
-                  method: 'POST',
-                  data: {
-                    'data': JSON.stringify(args),
-                  },
-                  success: function(res) {
-                    if (res.success) {
-                      alertify.success(gettext('Backup job created.'), 5);
-                      pgBrowser.Events.trigger('pgadmin-bgprocess:created', self);
-                    }
-                  },
-                  error: function(xhr) {
-                    try {
-                      var err = $.parseJSON(xhr.responseText);
-                      alertify.alert(
-                        gettext('Backup job failed.'),
-                        err.errormsg
-                      );
-                    } catch (e) {
-                      console.warn(e.stack || e);
-                    }
-                  },
-                });
-              }
-            },
-          };
-        });
-      }
-      alertify.backup_objects(title).resizeTo('65%', '60%');
+      let dialog = new globalBackupDialog.BackupDialog(
+        pgBrowser,
+        $,
+        alertify,
+        BackupObjectModel
+      );
+      dialog.draw(action, treeItem, null);
     },
   };
   return pgBrowser.Backup;
diff --git a/web/pgadmin/tools/datagrid/static/js/datagrid.js b/web/pgadmin/tools/datagrid/static/js/datagrid.js
index 473f20ad..d8d1c902 100644
--- a/web/pgadmin/tools/datagrid/static/js/datagrid.js
+++ b/web/pgadmin/tools/datagrid/static/js/datagrid.js
@@ -1,10 +1,13 @@
 define('pgadmin.datagrid', [
   'sources/gettext', 'sources/url_for', 'jquery', 'underscore',
   'pgadmin.alertifyjs', 'sources/pgadmin', 'bundled_codemirror',
-  'sources/sqleditor_utils', 'backbone', 'wcdocker',
+  'sources/sqleditor_utils', 'backbone', 'sources/datagrid/show_data',
+  'sources/datagrid/get_panel_title',
+  'sources/datagrid/show_query_tool',
+  'wcdocker',
 ], function(
   gettext, url_for, $, _, alertify, pgAdmin, codemirror, sqlEditorUtils,
-  Backbone
+  Backbone, showData, panelTitle, showQueryTool
 ) {
   // Some scripts do export their object in the window only.
   // Generally the one, which do no have AMD support.
@@ -161,55 +164,7 @@ define('pgadmin.datagrid', [
 
       // This is a callback function to show data when user click on menu item.
       show_data_grid: function(data, i) {
-        var self = this,
-          d = pgAdmin.Browser.tree.itemData(i);
-        if (d === undefined) {
-          alertify.alert(
-            gettext('Data Grid Error'),
-            gettext('No object selected.')
-          );
-          return;
-        }
-
-        // Get the parent data from the tree node hierarchy.
-        var node = pgBrowser.Nodes[d._type],
-          parentData = node.getTreeNodeHierarchy(i);
-
-        // If server, database or schema is undefined then return from the function.
-        if (parentData.server === undefined || parentData.database === undefined) {
-          return;
-        }
-        // If schema, view, catalog object all are undefined then return from the function.
-        if (parentData.schema === undefined && parentData.view === undefined &&
-             parentData.catalog === undefined) {
-          return;
-        }
-
-        var nsp_name = '';
-
-        if (parentData.schema != undefined) {
-          nsp_name = parentData.schema.label;
-        }
-        else if (parentData.view != undefined) {
-          nsp_name = parentData.view.label;
-        }
-        else if (parentData.catalog != undefined) {
-          nsp_name = parentData.catalog.label;
-        }
-        var url_params = {
-          'cmd_type': data.mnuid,
-          'obj_type': d._type,
-          'sgid': parentData.server_group._id,
-          'sid': parentData.server._id,
-          'did': parentData.database._id,
-          'obj_id': d._id,
-        };
-
-        var baseUrl = url_for('datagrid.initialize_datagrid', url_params);
-        var grid_title = parentData.server.label + ' - ' + parentData.database.label + ' - '
-                        + nsp_name + '.' + d.label;
-
-        self.create_transaction(baseUrl, null, 'false', parentData.server.server_type, '', grid_title, '');
+        showData.showDataGrid(this, pgBrowser, alertify, data, i);
       },
 
       // This is a callback function to show filtered data when user click on menu item.
@@ -382,63 +337,12 @@ define('pgadmin.datagrid', [
       },
 
       get_panel_title: function() {
-        // Get the parent data from the tree node hierarchy.
-        var tree = pgAdmin.Browser.tree,
-          selected_item = tree.selected(),
-          item_data = tree.itemData(selected_item);
-
-        var node = pgBrowser.Nodes[item_data._type],
-          parentData = node.getTreeNodeHierarchy(selected_item);
-
-        // If server, database is undefined then return from the function.
-        if (parentData.server === undefined) {
-          return;
-        }
-        // If Database is not available then use default db
-        var db_label = parentData.database ? parentData.database.label
-                                           : parentData.server.db;
-
-        var grid_title = db_label + ' on ' + parentData.server.user.name + '@' +
-                parentData.server.label;
-        return grid_title;
+        return panelTitle.getPanelTitle(pgBrowser);
       },
       // This is a callback function to show query tool when user click on menu item.
-      show_query_tool: function(url, i, panel_title) {
-        var sURL = url || '',
-          d = pgAdmin.Browser.tree.itemData(i);
-
-        panel_title = panel_title || '';
-        if (d === undefined) {
-          alertify.alert(
-            gettext('Query Tool Error'),
-            gettext('No object selected.')
-          );
-          return;
-        }
-
-        // Get the parent data from the tree node hierarchy.
-        var node = pgBrowser.Nodes[d._type],
-          parentData = node.getTreeNodeHierarchy(i);
-
-        // If server, database is undefined then return from the function.
-        if (parentData.server === undefined) {
-          return;
-        }
-
-        var url_params = {
-            'sgid': parentData.server_group._id,
-            'sid': parentData.server._id,
-          },
-          url_endpoint = 'datagrid.initialize_query_tool';
-        // If database not present then use Maintenance database
-        // We will handle this at server side
-        if (parentData.database) {
-          url_params['did'] = parentData.database._id;
-          url_endpoint = 'datagrid.initialize_query_tool_with_did';
-        }
-        var baseUrl = url_for(url_endpoint, url_params);
-
-        this.create_transaction(baseUrl, null, 'true', parentData.server.server_type, sURL, panel_title, '', false);
+      show_query_tool: function(url, aciTreeIdentifier, panelTitle) {
+        showQueryTool.showQueryTool(this, pgBrowser, alertify, url,
+          aciTreeIdentifier, panelTitle);
       },
       create_transaction: function(baseUrl, target, is_query_tool, server_type, sURL, panel_title, sql_filter, recreate) {
         var self = this;
diff --git a/web/regression/javascript/backup/backup_dialog_spec.js b/web/regression/javascript/backup/backup_dialog_spec.js
new file mode 100644
index 00000000..e272f094
--- /dev/null
+++ b/web/regression/javascript/backup/backup_dialog_spec.js
@@ -0,0 +1,651 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+import {BackupDialog} from '../../../pgadmin/static/js/backup/backup_dialog';
+import {TreeFake} from '../tree/tree_fake';
+import MockAdapter from 'axios-mock-adapter';
+import axios from 'axios/index';
+import {FakeModel} from '../fake_model';
+import {DialogWrapper} from "../../../pgadmin/static/js/backup/backup_dialog_wrapper";
+
+const context = describe;
+
+describe('ObjectBackupDialog', () => {
+  let backupDialog;
+  let backupNode;
+  let pgBrowser;
+  let jquerySpy;
+  let alertifySpy;
+  let backupModelSpy;
+  let backform;
+
+
+  let rootNode;
+  let serverTreeNode;
+  let databaseTreeNode;
+  let ppasServerTreeNode;
+  let noDataNode;
+
+  beforeEach(() => {
+    pgBrowser = {
+      treeMenu: new TreeFake(),
+      Nodes: {
+        server: {
+          hasId: true,
+          label: 'server',
+          getTreeNodeHierarchy: jasmine.createSpy('server.getTreeNodeHierarchy'),
+        },
+        database: {
+          hasId: true,
+          label: 'database',
+          getTreeNodeHierarchy: jasmine.createSpy('db.getTreeNodeHierarchy'),
+        },
+        schema: {
+          hasId: true,
+          label: 'schema',
+          getTreeNodeHierarchy: jasmine.createSpy('db.getTreeNodeHierarchy'),
+        },
+      }
+    };
+    pgBrowser.Nodes.server.hasId = true;
+    pgBrowser.Nodes.database.hasId = true;
+    jquerySpy = jasmine.createSpy('jquerySpy');
+    backupModelSpy = jasmine.createSpy('backupModelSpy');
+    backupNode = {};
+    backform = jasmine.createSpyObj('backform', ['generateViewSchema', 'Dialog']);
+
+    rootNode = pgBrowser.treeMenu.addNewNode('level1', {}, []);
+    serverTreeNode = pgBrowser.treeMenu.addNewNode('level1.1', {
+      _type: 'server',
+      _id: 10,
+    }, ['level1']);
+    databaseTreeNode = pgBrowser.treeMenu.addNewNode(
+      'level1.1.1', {
+        _type: 'database',
+        _id: 11,
+        label: 'some_database',
+        _label: 'some_database_label',
+      }, ['level1', 'level1.1']);
+    ppasServerTreeNode = pgBrowser.treeMenu.addNewNode('level1.2', {
+      _type: 'server',
+      server_type: 'ppas',
+    }, ['level1']);
+    pgBrowser.treeMenu.addNewNode('level3', {}, ['level1', 'level1.2']);
+    noDataNode = pgBrowser.treeMenu.addNewNode(
+      'level3.1', undefined, ['level1', 'level1.2', 'level3']);
+  });
+
+  describe('#draw', () => {
+    beforeEach(() => {
+      alertifySpy = jasmine.createSpyObj('alertify', ['alert', 'dialog']);
+      alertifySpy['backup_objects'] = jasmine.createSpy('backup_objects');
+      backupDialog = new BackupDialog(
+        pgBrowser,
+        jquerySpy,
+        alertifySpy,
+        backupModelSpy
+      );
+
+      pgBrowser.get_preference = jasmine.createSpy('get_preferences');
+    });
+
+    context('there are no ancestors of the type server', () => {
+      it('does not create a dialog', () => {
+        pgBrowser.treeMenu.setSelectedNode([rootNode]);
+        backupDialog.draw(null, null, null);
+        expect(alertifySpy['backup_objects']).not.toHaveBeenCalled();
+      });
+
+      it('display an alert with a Backup Error', () => {
+        backupDialog.draw(null, [rootNode], null);
+        expect(alertifySpy.alert).toHaveBeenCalledWith(
+          'Backup Error',
+          'Please select server or child node from the browser tree.'
+        );
+      });
+    });
+
+    context('there is an ancestor of the type server', () => {
+      context('no preference can be found', () => {
+        beforeEach(() => {
+          pgBrowser.get_preference.and.returnValue(undefined);
+        });
+
+        context('server is a ppas server', () => {
+          it('display an alert with "Backup Error"', () => {
+            backupDialog.draw(null, [serverTreeNode], null);
+            expect(alertifySpy.alert).toHaveBeenCalledWith(
+              'Backup Error',
+              'Failed to load preference pg_bin_dir of module paths'
+            );
+          });
+        });
+
+        context('server is not a ppas server', () => {
+          it('display an alert with "Backup Error"', () => {
+            backupDialog.draw(null, [ppasServerTreeNode], null);
+            expect(alertifySpy.alert).toHaveBeenCalledWith(
+              'Backup Error',
+              'Failed to load preference ppas_bin_dir of module paths'
+            );
+          });
+        });
+      });
+
+      context('preference can be found', () => {
+        context('binary folder is not configured', () => {
+          beforeEach(() => {
+            pgBrowser.get_preference.and.returnValue({});
+          });
+
+          context('server is a ppas server', () => {
+            it('display an alert with "Configuration required"', () => {
+              backupDialog.draw(null, [serverTreeNode], null);
+              expect(alertifySpy.alert).toHaveBeenCalledWith(
+                'Configuration required',
+                'Please configure the PostgreSQL Binary Path in the Preferences dialog.'
+              );
+            });
+          });
+
+          context('server is not a ppas server', () => {
+            it('display an alert with "Configuration required"', () => {
+              backupDialog.draw(null, [ppasServerTreeNode], null);
+              expect(alertifySpy.alert).toHaveBeenCalledWith(
+                'Configuration required',
+                'Please configure the EDB Advanced Server Binary Path in the Preferences dialog.'
+              );
+            });
+          });
+        });
+
+        context('binary folder is configured', () => {
+          let backupDialogResizeToSpy;
+          let serverResizeToSpy;
+          beforeEach(() => {
+            backupDialogResizeToSpy = jasmine.createSpyObj('backupDialogResizeToSpy', ['resizeTo']);
+            alertifySpy['backup_objects'].and
+              .returnValue(backupDialogResizeToSpy);
+            pgBrowser.get_preference.and.returnValue({value: '/some/path'});
+          });
+
+          it('displays the dialog', () => {
+            backupDialog.draw(null, [serverTreeNode], null);
+            expect(alertifySpy['backup_objects']).toHaveBeenCalledWith(true);
+            expect(backupDialogResizeToSpy.resizeTo).toHaveBeenCalledWith('60%', '50%');
+          });
+        });
+      });
+    });
+  });
+
+  describe('#dialogFactory', () => {
+    describe('#prepare', () => {
+      let backupJQueryContainer;
+      let viewSchema;
+      let generatedBackupModel;
+      let backupNodeChildNode;
+      beforeEach(() => {
+        backupJQueryContainer = jasmine.createSpyObj('backupJQueryContainer', ['get']);
+        backupJQueryContainer.get.and.returnValue(backupJQueryContainer);
+        viewSchema = {};
+        generatedBackupModel = {};
+        backupNode.__internal = {
+          buttons: [
+            {}, {},
+            {
+              element: {
+                disabled: false,
+              }
+            }
+          ]
+        };
+        backupNode.elements = {
+          body: {
+            childNodes: [
+              {}
+            ]
+          },
+          content: jasmine.createSpyObj('content', ['appendChild']),
+        };
+
+        backform.generateViewSchema.and.returnValue(viewSchema);
+        backupModelSpy.and.returnValue(generatedBackupModel);
+
+        backupDialog = new BackupDialog(
+          pgBrowser,
+          jquerySpy,
+          alertifySpy,
+          backupModelSpy,
+          backform
+        );
+
+        backupNodeChildNode = jasmine.createSpyObj('something', ['addClass']);
+
+        jquerySpy.and.callFake((selector) => {
+          if (selector === '<div class=\'backup_dialog\'></div>') {
+            return backupJQueryContainer;
+          } else if (selector === backupNode.elements.body.childNodes[0]) {
+            return backupNodeChildNode;
+          }
+        })
+      });
+
+      context('selected tree node has no data', () => {
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([noDataNode]);
+          const dialog = backupDialog.dialogFactory(null, 'backup_objects');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('does not create a backform dialog', () => {
+          expect(backform.Dialog).not.toHaveBeenCalledWith();
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+      });
+
+      context('no tree element is selected', () => {
+        beforeEach(() => {
+          const dialog = backupDialog.dialogFactory(null, 'backup_objects');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('does not create a backform dialog', () => {
+          expect(backform.Dialog).not.toHaveBeenCalledWith();
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+      });
+
+      context('tree element is selected', () => {
+        let treeHierarchyInformation;
+        let dialogSpy;
+        let dialog;
+        beforeEach(() => {
+          treeHierarchyInformation = {
+            database: {
+              _type: 'database',
+              _id: 11,
+              label: 'some_database',
+              _label: 'some_database_label',
+              priority: 0,
+            },
+            server: {
+              _type: 'server',
+              _id: 10,
+              priority: -1,
+            }
+          };
+          pgBrowser.treeMenu.setSelectedNode([{id: 'level1.1.1'}]);
+          pgBrowser.Nodes['database'].getTreeNodeHierarchy.and
+            .returnValue(treeHierarchyInformation);
+          dialogSpy = jasmine.createSpyObj('newView', ['render']);
+          dialogSpy.$el = jasmine.createSpyObj('$el', ['find']);
+          dialogSpy.model = jasmine.createSpyObj('model', ['on'])
+          dialogSpy.$el.find.and.returnValue([]);
+
+          backform.Dialog.and.returnValue(dialogSpy);
+
+          dialog = backupDialog.dialogFactory(null, 'backup_objects');
+          dialog.set = jasmine.createSpy('dialog.set');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('creates a backform dialog and displays it', () => {
+          expect(backform.Dialog).toHaveBeenCalledWith({
+            el: backupJQueryContainer,
+            model: generatedBackupModel,
+            schema: viewSchema,
+          });
+
+          expect(dialogSpy.render).toHaveBeenCalled();
+        });
+
+        it('add alertify classes to backup node childnode', () => {
+          expect(backupNodeChildNode.addClass)
+            .toHaveBeenCalledWith('alertify_tools_dialog_properties obj_properties');
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+
+        it('generates a new backup model', () => {
+          expect(backupModelSpy).toHaveBeenCalledWith(
+            {},
+            {node_info: treeHierarchyInformation}
+          );
+        });
+
+        it('add the new dialog to the backup node HTML', () => {
+          expect(backupNode.elements.content.appendChild).toHaveBeenCalledWith(backupJQueryContainer);
+        });
+
+        it('sets the title to "Backup (database: some_database)"', () => {
+          expect(dialog.set).toHaveBeenCalledWith('title', 'Backup (database: some_database)');
+        });
+      });
+    });
+
+    describe('#onButtonClicked', () => {
+      let backupJQueryContainer;
+      let backupNodeChildNode;
+      let dialog;
+      let networkMock;
+
+      beforeEach(() => {
+        networkMock = new MockAdapter(axios);
+        backupJQueryContainer = jasmine.createSpyObj('backupJQueryContainer', ['get']);
+        backupJQueryContainer.get.and.returnValue(backupJQueryContainer);
+        pgBrowser.showHelp = jasmine.createSpy('showHelp');
+        alertifySpy = jasmine.createSpyObj('alertify', ['success', 'alert']);
+
+        backupDialog = new BackupDialog(
+          pgBrowser,
+          jquerySpy,
+          alertifySpy,
+          backupModelSpy,
+          backform
+        );
+
+        backupNodeChildNode = jasmine.createSpyObj('something', ['addClass']);
+
+        dialog = Object.assign(
+          backupDialog.dialogFactory(null, 'backup_objects'),
+          backupNode
+        );
+      });
+
+      afterEach(() => {
+        networkMock.restore();
+      });
+
+      context('dialog help button was pressed', () => {
+        let networkCalled;
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([{id: 'level1.1.1'}]);
+          networkCalled = false;
+          networkMock.onAny(/.+/).reply(() => {
+            networkCalled = true;
+            return [200, {}];
+          });
+
+          const event = {
+            button: {
+              element: {
+                name: 'dialog_help',
+                getAttribute: (attributeName) => {
+                  if (attributeName === 'url') {
+                    return 'http://someurl';
+                  } else if (attributeName === 'label') {
+                    return 'some label';
+                  }
+                }
+              }
+            }
+          };
+          dialog.callback(event);
+        });
+
+        it('displays help for dialog', () => {
+          expect(pgBrowser.showHelp).toHaveBeenCalledWith(
+            'dialog_help',
+            'http://someurl',
+            pgBrowser.Nodes['database'],
+            databaseTreeNode,
+            'some label'
+          )
+        });
+
+        it('does not start the backup', () => {
+          expect(networkCalled).toBe(false);
+        });
+      });
+
+      context('object help button was pressed', () => {
+        let networkCalled;
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([{id: 'level1.1.1'}]);
+          networkCalled = false;
+          networkMock.onAny(/.+/).reply(() => {
+            networkCalled = true;
+            return [200, {}];
+          });
+
+          const event = {
+            button: {
+              element: {
+                name: 'object_help',
+                getAttribute: (attributeName) => {
+                  if (attributeName === 'url') {
+                    return 'http://someurl';
+                  } else if (attributeName === 'label') {
+                    return 'some label';
+                  }
+                }
+              }
+            }
+          };
+          dialog.callback(event);
+        });
+
+        it('displays help for dialog', () => {
+          expect(pgBrowser.showHelp).toHaveBeenCalledWith(
+            'object_help',
+            'http://someurl',
+            pgBrowser.Nodes['database'],
+            databaseTreeNode,
+            'some label'
+          )
+        });
+
+        it('does not start the backup', () => {
+          expect(networkCalled).toBe(false);
+        });
+      });
+
+      context('backup button was pressed', () => {
+        context('no tree node is selected', () => {
+          let networkCalled;
+          beforeEach(() => {
+            networkCalled = false;
+            networkMock.onPost('/backup/job/10/object').reply(() => {
+              networkCalled = true;
+              return [200, {}];
+            });
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            dialog.callback(event);
+          });
+
+          it('does not start the backup', () => {
+            expect(networkCalled).toBe(false);
+          });
+        });
+
+        context('selected node has no data', () => {
+          let networkCalled;
+          beforeEach(() => {
+            pgBrowser.treeMenu.setSelectedNode([noDataNode]);
+            networkCalled = false;
+            networkMock.onPost('/backup/job/10/object').reply(() => {
+              networkCalled = true;
+              return [200, {}];
+            });
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            dialog.callback(event);
+          });
+
+          it('does not start the backup', () => {
+            expect(networkCalled).toBe(false);
+          });
+        });
+
+        context('selected node has data', () => {
+          beforeEach(() => {
+            pgBrowser.treeMenu.setSelectedNode([{id: 'level1.1.1'}]);
+            pgBrowser.Events = jasmine.createSpyObj('Events', ['trigger']);
+
+            backupNode.view = {
+              model: new FakeModel(),
+            };
+            backupNode.view.model.set('dqoute', false);
+            backupNode.view.model.set('file', 'test');
+            backupNode.view.model.set('verbose', true);
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            Object.assign(dialog, backupNode).callback(event);
+          });
+          context('creation of backup job successfull', () => {
+            let dataSentToServer;
+            beforeEach(() => {
+              networkMock.onPost('/backup/job/10/object').reply((request) => {
+                dataSentToServer = request.data;
+                return [200, {}];
+              });
+            });
+
+            it('creates an success alert box', (done) => {
+              setTimeout(() => {
+                expect(alertifySpy.success).toHaveBeenCalledWith('Backup job' +
+                  ' created.', 5);
+                done();
+              }, 0);
+            });
+
+            it('trigger background process', (done) => {
+              setTimeout(() => {
+                expect(pgBrowser.Events.trigger).toHaveBeenCalledWith(
+                  'pgadmin-bgprocess:created',
+                  jasmine.anything()
+                );
+                done();
+              }, 0);
+            });
+
+            it('send correct data to server', () => {
+              expect(dataSentToServer).toEqual(JSON.stringify({
+                dqoute: false,
+                file: 'test',
+                verbose: true,
+                database: 'some_database_label'
+              }));
+            });
+          });
+
+          context('error creating backup job', () => {
+            beforeEach(() => {
+              const response = {
+                errormsg: 'some error message'
+              };
+
+              networkMock.onPost('/backup/job/10/object').reply(() => {
+                return [500, response];
+              });
+            });
+
+            it('creates an alert box', (done) => {
+              setTimeout(() => {
+                expect(alertifySpy.alert).toHaveBeenCalledWith('Backup job' +
+                  ' failed.', 'some error message');
+                done();
+              }, 0);
+            });
+          });
+        });
+      });
+    });
+  });
+
+  describe('BackupDialogWrapper', () => {
+    describe('#setExtraParameters', () => {
+      let model;
+      let dialog;
+      let schemaNode;
+      let tableNode;
+      beforeEach(() => {
+        model = new FakeModel();
+        dialog = new DialogWrapper({}, {
+          pgBrowser: pgBrowser,
+        }, '', 'backup_objects');
+        dialog.view = {
+          model: model,
+        };
+        schemaNode = pgBrowser.treeMenu.addNewNode(
+          'schema', {
+            _label: 'schema_label',
+            _type: 'schema',
+          }, ['level1', 'level1.1', 'level1.1.1']);
+        tableNode = pgBrowser.treeMenu.addNewNode(
+          'table', {
+            _label: 'table_label',
+            _type: 'table',
+          }, ['level1', 'level1.1', 'level1.1.1', 'schema']);
+      });
+
+      context('type is schema', () => {
+        it('add schema array to the model', () => {
+          dialog.setExtraParameters(schemaNode);
+          expect(model.get('schemas')).toEqual(['schema_label']);
+        });
+      });
+
+      context('type is table', () => {
+        it('add schema and table to the model', () => {
+          dialog.setExtraParameters(tableNode);
+          expect(model.get('tables')).toEqual([['schema_label', 'table_label']]);
+        });
+      });
+    });
+  });
+});
diff --git a/web/regression/javascript/backup/global_server_backup_dialog_spec.js b/web/regression/javascript/backup/global_server_backup_dialog_spec.js
new file mode 100644
index 00000000..0b229038
--- /dev/null
+++ b/web/regression/javascript/backup/global_server_backup_dialog_spec.js
@@ -0,0 +1,585 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+import {BackupDialog} from '../../../pgadmin/static/js/backup/backup_dialog';
+import {TreeFake} from '../tree/tree_fake';
+import MockAdapter from 'axios-mock-adapter';
+import axios from 'axios/index';
+import {FakeModel} from '../fake_model';
+
+const context = describe;
+
+describe('GlobalServerBackupDialog', () => {
+  let backupDialog;
+  let backupNode;
+  let pgBrowser;
+  let jquerySpy;
+  let alertifySpy;
+  let backupModelSpy;
+  let backform;
+
+
+  let rootNode;
+  let serverTreeNode;
+  let ppasServerTreeNode;
+  let noDataNode;
+
+  beforeEach(() => {
+    pgBrowser = {
+      treeMenu: new TreeFake(),
+      Nodes: {
+        server: jasmine.createSpyObj('Node[server]', ['getTreeNodeHierarchy']),
+      }
+    };
+    pgBrowser.Nodes.server.hasId = true;
+    jquerySpy = jasmine.createSpy('jquerySpy');
+    backupModelSpy = jasmine.createSpy('backupModelSpy');
+    backupNode = {};
+    backform = jasmine.createSpyObj('backform', ['generateViewSchema', 'Dialog']);
+
+    rootNode = pgBrowser.treeMenu.addNewNode('level1', {}, []);
+    serverTreeNode = pgBrowser.treeMenu.addNewNode('level1.1', {
+      _type: 'server',
+      _id: 10,
+    }, ['level1']);
+    ppasServerTreeNode = pgBrowser.treeMenu.addNewNode('level1.2', {
+      _type: 'server',
+      server_type: 'ppas',
+    }, ['level1']);
+    pgBrowser.treeMenu.addNewNode('level3', {}, ['level1', 'level1.2']);
+    noDataNode = pgBrowser.treeMenu.addNewNode(
+      'level3.1', undefined, ['level1', 'level1.2', 'level3']);
+  });
+
+  describe('#draw', () => {
+    beforeEach(() => {
+      alertifySpy = jasmine.createSpyObj('alertify', ['alert', 'dialog']);
+      alertifySpy['BackupDialog_globals'] = jasmine.createSpy('BackupDialog_globals');
+      alertifySpy['BackupDialog_server'] = jasmine.createSpy('BackupDialog_server');
+      backupDialog = new BackupDialog(
+        pgBrowser,
+        jquerySpy,
+        alertifySpy,
+        backupModelSpy
+      );
+
+      pgBrowser.get_preference = jasmine.createSpy('get_preferences');
+    });
+
+    context('there are no ancestors of the type server', () => {
+      it('does not create a dialog', () => {
+        pgBrowser.treeMenu.setSelectedNode([{id: 'level1'}]);
+        backupDialog.draw(null, null, null);
+        expect(alertifySpy['BackupDialog_globals']).not.toHaveBeenCalled();
+        expect(alertifySpy['BackupDialog_server']).not.toHaveBeenCalled();
+      });
+
+      it('display an alert with a Backup Error', () => {
+        backupDialog.draw(null, [rootNode], null);
+        expect(alertifySpy.alert).toHaveBeenCalledWith(
+          'Backup Error',
+          'Please select server or child node from the browser tree.'
+        );
+      });
+    });
+
+    context('there is an ancestor of the type server', () => {
+      context('no preference can be found', () => {
+        beforeEach(() => {
+          pgBrowser.get_preference.and.returnValue(undefined);
+        });
+
+        context('server is a ppas server', () => {
+          it('display an alert with "Backup Error"', () => {
+            backupDialog.draw(null, [serverTreeNode], null);
+            expect(alertifySpy.alert).toHaveBeenCalledWith(
+              'Backup Error',
+              'Failed to load preference pg_bin_dir of module paths'
+            );
+          });
+        });
+
+        context('server is not a ppas server', () => {
+          it('display an alert with "Backup Error"', () => {
+            backupDialog.draw(null, [ppasServerTreeNode], null);
+            expect(alertifySpy.alert).toHaveBeenCalledWith(
+              'Backup Error',
+              'Failed to load preference ppas_bin_dir of module paths'
+            );
+          });
+        });
+      });
+
+      context('preference can be found', () => {
+        context('binary folder is not configured', () => {
+          beforeEach(() => {
+            pgBrowser.get_preference.and.returnValue({});
+          });
+
+          context('server is a ppas server', () => {
+            it('display an alert with "Configuration required"', () => {
+              backupDialog.draw(null, [serverTreeNode], null);
+              expect(alertifySpy.alert).toHaveBeenCalledWith(
+                'Configuration required',
+                'Please configure the PostgreSQL Binary Path in the Preferences dialog.'
+              );
+            });
+          });
+
+          context('server is not a ppas server', () => {
+            it('display an alert with "Configuration required"', () => {
+              backupDialog.draw(null, [ppasServerTreeNode], null);
+              expect(alertifySpy.alert).toHaveBeenCalledWith(
+                'Configuration required',
+                'Please configure the EDB Advanced Server Binary Path in the Preferences dialog.'
+              );
+            });
+          });
+        });
+
+        context('binary folder is configured', () => {
+          let globalResizeToSpy;
+          let serverResizeToSpy;
+          beforeEach(() => {
+            globalResizeToSpy = jasmine.createSpyObj('globals', ['resizeTo']);
+            alertifySpy['BackupDialog_globals'].and
+              .returnValue(globalResizeToSpy);
+            serverResizeToSpy = jasmine.createSpyObj('server', ['resizeTo']);
+            alertifySpy['BackupDialog_server'].and
+              .returnValue(serverResizeToSpy);
+            pgBrowser.get_preference.and.returnValue({value: '/some/path'});
+          });
+
+          context('dialog for global backup', () => {
+            it('displays the dialog', () => {
+              backupDialog.draw(null, [serverTreeNode], {globals: true});
+              expect(alertifySpy['BackupDialog_globals']).toHaveBeenCalledWith(true);
+              expect(globalResizeToSpy.resizeTo).toHaveBeenCalledWith('60%', '50%');
+            });
+          });
+
+          context('dialog for global backup', () => {
+            it('displays the dialog', () => {
+              backupDialog.draw(null, [serverTreeNode], {server: true});
+              expect(alertifySpy['BackupDialog_server']).toHaveBeenCalledWith(true);
+              expect(serverResizeToSpy.resizeTo).toHaveBeenCalledWith('60%', '50%');
+            });
+          });
+        });
+      });
+    });
+  });
+
+  describe('#dialogFactory', () => {
+    describe('#prepare', () => {
+      let backupJQueryContainer;
+      let viewSchema;
+      let generatedBackupModel;
+      let backupNodeChildNode;
+      beforeEach(() => {
+        backupJQueryContainer = jasmine.createSpyObj('backupJQueryContainer', ['get']);
+        backupJQueryContainer.get.and.returnValue(backupJQueryContainer);
+        viewSchema = {};
+        generatedBackupModel = {};
+        backupNode.__internal = {
+          buttons: [
+            {}, {},
+            {
+              element: {
+                disabled: false,
+              }
+            }
+          ]
+        };
+        backupNode.elements = {
+          body: {
+            childNodes: [
+              {}
+            ]
+          },
+          content: jasmine.createSpyObj('content', ['appendChild']),
+        };
+
+        backform.generateViewSchema.and.returnValue(viewSchema);
+        backupModelSpy.and.returnValue(generatedBackupModel);
+
+        backupDialog = new BackupDialog(
+          pgBrowser,
+          jquerySpy,
+          alertifySpy,
+          backupModelSpy,
+          backform
+        );
+
+        backupNodeChildNode = jasmine.createSpyObj('something', ['addClass']);
+
+        jquerySpy.and.callFake((selector) => {
+          if (selector === '<div class=\'backup_dialog\'></div>') {
+            return backupJQueryContainer;
+          } else if (selector === backupNode.elements.body.childNodes[0]) {
+            return backupNodeChildNode;
+          }
+        })
+      });
+
+      context('selected tree node has no data', () => {
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([noDataNode]);
+          const dialog = backupDialog.dialogFactory('Some title', 'globals');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('does not create a backform dialog', () => {
+          expect(backform.Dialog).not.toHaveBeenCalledWith();
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+      });
+
+      context('no tree element is selected', () => {
+        beforeEach(() => {
+          const dialog = backupDialog.dialogFactory('Some title', 'globals');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('does not create a backform dialog', () => {
+          expect(backform.Dialog).not.toHaveBeenCalledWith();
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+      });
+
+      context('tree element is selected', () => {
+        let treeHierarchyInformation;
+        let dialogSpy;
+        beforeEach(() => {
+          treeHierarchyInformation = {
+            server: {
+              _type: 'server',
+              _id: 10,
+              priority: 0,
+            }
+          };
+          pgBrowser.treeMenu.setSelectedNode([serverTreeNode]);
+          pgBrowser.Nodes['server'].getTreeNodeHierarchy.and
+            .returnValue(treeHierarchyInformation);
+          dialogSpy = jasmine.createSpyObj('newView', ['render']);
+          dialogSpy.$el = jasmine.createSpyObj('$el', ['find']);
+          dialogSpy.model = jasmine.createSpyObj('model', ['on'])
+          dialogSpy.$el.find.and.returnValue([]);
+
+          backform.Dialog.and.returnValue(dialogSpy);
+
+          const dialog = backupDialog.dialogFactory('Some title', 'globals');
+          Object.assign(dialog, backupNode).prepare();
+        });
+
+        it('creates a backform dialog and displays it', () => {
+          expect(backform.Dialog).toHaveBeenCalledWith({
+            el: backupJQueryContainer,
+            model: generatedBackupModel,
+            schema: viewSchema,
+          });
+
+          expect(dialogSpy.render).toHaveBeenCalled();
+        });
+
+        it('add alertify classes to backup node childnode', () => {
+          expect(backupNodeChildNode.addClass)
+            .toHaveBeenCalledWith('alertify_tools_dialog_properties obj_properties');
+        });
+
+        it('disables the button submit button until a filename is selected', () => {
+          expect(backupNode.__internal.buttons[2].element.disabled).toBe(true);
+        });
+
+        it('generates a new backup model', () => {
+          expect(backupModelSpy).toHaveBeenCalledWith(
+            {type: 'globals'},
+            {node_info: treeHierarchyInformation}
+          );
+        });
+
+        it('add the new dialog to the backup node HTML', () => {
+          expect(backupNode.elements.content.appendChild).toHaveBeenCalledWith(backupJQueryContainer);
+        });
+      });
+    });
+
+    describe('#onButtonClicked', () => {
+      let backupJQueryContainer;
+      let backupNodeChildNode;
+      let dialog;
+      let networkMock;
+
+      beforeEach(() => {
+        networkMock = new MockAdapter(axios);
+        backupJQueryContainer = jasmine.createSpyObj('backupJQueryContainer', ['get']);
+        backupJQueryContainer.get.and.returnValue(backupJQueryContainer);
+        pgBrowser.showHelp = jasmine.createSpy('showHelp');
+        alertifySpy = jasmine.createSpyObj('alertify', ['success', 'alert']);
+
+        backupDialog = new BackupDialog(
+          pgBrowser,
+          jquerySpy,
+          alertifySpy,
+          backupModelSpy,
+          backform
+        );
+
+        backupNodeChildNode = jasmine.createSpyObj('something', ['addClass']);
+
+        dialog = Object.assign(backupDialog.dialogFactory('Some title', 'server'), backupNode);
+      });
+
+      afterEach(() => {
+        networkMock.restore();
+      });
+
+      context('dialog help button was pressed', () => {
+        let networkCalled;
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([serverTreeNode]);
+          networkCalled = false;
+          networkMock.onAny(/.+/).reply(() => {
+            networkCalled = true;
+            return [200, {}];
+          });
+
+          const event = {
+            button: {
+              element: {
+                name: 'dialog_help',
+                getAttribute: (attributeName) => {
+                  if (attributeName === 'url') {
+                    return 'http://someurl';
+                  } else if (attributeName === 'label') {
+                    return 'some label';
+                  }
+                }
+              }
+            }
+          };
+          dialog.callback(event);
+        });
+
+        it('displays help for dialog', () => {
+          expect(pgBrowser.showHelp).toHaveBeenCalledWith(
+            'dialog_help',
+            'http://someurl',
+            pgBrowser.Nodes['server'],
+            serverTreeNode,
+            'some label'
+          )
+        });
+
+        it('does not start the backup', () => {
+          expect(networkCalled).toBe(false);
+        });
+      });
+
+      context('object help button was pressed', () => {
+        let networkCalled;
+        beforeEach(() => {
+          pgBrowser.treeMenu.setSelectedNode([serverTreeNode]);
+          networkCalled = false;
+          networkMock.onAny(/.+/).reply(() => {
+            networkCalled = true;
+            return [200, {}];
+          });
+
+          const event = {
+            button: {
+              element: {
+                name: 'object_help',
+                getAttribute: (attributeName) => {
+                  if (attributeName === 'url') {
+                    return 'http://someurl';
+                  } else if (attributeName === 'label') {
+                    return 'some label';
+                  }
+                }
+              }
+            }
+          };
+          dialog.callback(event);
+        });
+
+        it('displays help for dialog', () => {
+          expect(pgBrowser.showHelp).toHaveBeenCalledWith(
+            'object_help',
+            'http://someurl',
+            pgBrowser.Nodes['server'],
+            serverTreeNode,
+            'some label'
+          )
+        });
+
+        it('does not start the backup', () => {
+          expect(networkCalled).toBe(false);
+        });
+      });
+
+      context('backup button was pressed', () => {
+        context('no tree node is selected', () => {
+          let networkCalled;
+          beforeEach(() => {
+            networkCalled = false;
+            networkMock.onAny(/.+/).reply(() => {
+              networkCalled = true;
+              return [200, {}];
+            });
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            dialog.callback(event);
+          });
+
+          it('does not start the backup', () => {
+            expect(networkCalled).toBe(false);
+          });
+        });
+
+        context('selected node has no data', () => {
+          let networkCalled;
+          beforeEach(() => {
+            pgBrowser.treeMenu.setSelectedNode([noDataNode]);
+            networkCalled = false;
+            networkMock.onAny(/.+/).reply(() => {
+              networkCalled = true;
+              return [200, {}];
+            });
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            dialog.callback(event);
+          });
+
+          it('does not start the backup', () => {
+            expect(networkCalled).toBe(false);
+          });
+        });
+
+        context('selected node has data', () => {
+          beforeEach(() => {
+            pgBrowser.treeMenu.setSelectedNode([serverTreeNode]);
+            pgBrowser.Events = jasmine.createSpyObj('Events', ['trigger']);
+
+            backupNode.view = {
+              model: new FakeModel(),
+            };
+            backupNode.view.model.set('dqoute', false);
+            backupNode.view.model.set('file', 'test');
+            backupNode.view.model.set('type', 'globals');
+            backupNode.view.model.set('verbose', true);
+
+            const event = {
+              button: {
+                'data-btn-name': 'backup',
+                element: {
+                  getAttribute: (attributeName) => {
+                    if (attributeName === 'url') {
+                      return 'http://someurl';
+                    } else if (attributeName === 'label') {
+                      return 'some label';
+                    }
+                  }
+                }
+              }
+            };
+            Object.assign(dialog, backupNode).callback(event);
+          });
+          context('creation of backup job successfull', () => {
+            let dataSentToServer;
+            beforeEach(() => {
+              networkMock.onPost('/backup/job/10').reply((request) => {
+                dataSentToServer = request.data;
+                return [200, {}];
+              });
+            });
+
+            it('creates an success alert box', (done) => {
+              setTimeout(() => {
+                expect(alertifySpy.success).toHaveBeenCalledWith('Backup job' +
+                  ' created.', 5);
+                done();
+              }, 0);
+            });
+
+            it('trigger background process', (done) => {
+              setTimeout(() => {
+                expect(pgBrowser.Events.trigger).toHaveBeenCalledWith(
+                  'pgadmin-bgprocess:created',
+                  jasmine.anything()
+                );
+                done();
+              }, 0);
+            });
+
+            it('send correct data to server', () => {
+              expect(dataSentToServer).toEqual(JSON.stringify({
+                dqoute: false,
+                file: 'test',
+                type: 'globals',
+                verbose: true
+              }));
+            });
+          });
+
+          context('error creating backup job', () => {
+            beforeEach(() => {
+              const response = {
+                errormsg: 'some error message'
+              };
+
+              networkMock.onPost('/backup/job/10').reply(() => {
+                return [500, response];
+              });
+            });
+
+            it('creates an alert box', (done) => {
+              setTimeout(() => {
+                expect(alertifySpy.alert).toHaveBeenCalledWith('Backup job' +
+                  ' failed.', 'some error message');
+                done();
+              }, 0);
+            });
+          });
+        });
+      });
+    });
+  });
+});
diff --git a/web/regression/javascript/backup/menu_utils_spec.js b/web/regression/javascript/backup/menu_utils_spec.js
new file mode 100644
index 00000000..8f776976
--- /dev/null
+++ b/web/regression/javascript/backup/menu_utils_spec.js
@@ -0,0 +1,167 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+
+import {TreeFake} from "../tree/tree_fake";
+import {
+  menuEnabledServer,
+  menuEnabled
+} from "../../../pgadmin/static/js/backup/menu_utils";
+
+const context = describe;
+
+describe('backup.menuUtils', () => {
+  describe('#menuEnabled', () => {
+    let ourBrowser;
+    beforeEach(() => {
+      const tree = new TreeFake();
+      ourBrowser = {
+        treeMenu: tree,
+      };
+      tree.addNewNode('level1', {}, []);
+      tree.addNewNode('level1.1', {_type: 'catalog'}, ['level1']);
+      tree.addNewNode('level1.1.1', {_type: 'database'}, ['level1', 'level1.1']);
+      tree.addNewNode('level1.2', {_type: 'bamm'}, ['level1']);
+      tree.addNewNode('level1.2.1', {
+        _type: 'database',
+        allowConn: true
+      }, ['level1', 'level1.2']);
+      tree.addNewNode('level1.2.2', {
+        _type: 'database',
+        allowConn: false
+      }, ['level1', 'level1.2']);
+      tree.addNewNode('level1.2.3', {
+        _type: 'table'
+      }, ['level1', 'level1.2']);
+
+      tree.addNewNode('level2', {}, []);
+      tree.addNewNode('level2.1', null, ['level2']);
+      tree.addNewNode('level2.1.1', {}, ['level2', 'level2.1']);
+    });
+
+    context('When the current node is a root node', () => {
+      it('return false', () => {
+        expect(menuEnabled.apply(ourBrowser, [{}, [{id: 'level1'}]])).toBe(false);
+      });
+    });
+
+    context('when current node does not exist', () => {
+      it('return false', () => {
+        expect(menuEnabled.apply(ourBrowser, [{}, [{id: 'bamm'}]])).toBe(false);
+      });
+    });
+
+    context('When the current node is not a root node', () => {
+      context('parent data does not exist', () => {
+        it('returns false', () => {
+          expect(menuEnabled.apply(ourBrowser, [{}, [{id: 'level2.1.1'}]])).toBe(false);
+        });
+      });
+
+      context('parent as data', () => {
+        context('the current node type is in the supported node types', () => {
+          context('the parent is of the type catalog', () => {
+            it('returns false', () => {
+              expect(menuEnabled.apply(ourBrowser, [
+                {_type: 'schema'},
+                [{id: 'level1.1.1'}]
+              ])).toBe(false);
+            });
+          });
+          context('the parent is not of the type catalog', () => {
+            context('current node is of the type database', () => {
+              context('current node allows connection', () => {
+                it('returns true', () => {
+                  expect(menuEnabled.apply(ourBrowser, [{
+                    _type: 'database',
+                    allowConn: true
+                  }, [{id: 'level1.2.1'}]])).toBe(true);
+                });
+              });
+              context('current node do not allow connection', () => {
+                it('returns false', () => {
+                  expect(menuEnabled.apply(ourBrowser, [{
+                    _type: 'database',
+                    allowConn: false
+                  }, [{id: 'level1.2.2'}]])).toBe(false);
+                });
+              });
+            });
+            context('current node is not of the type database', () => {
+              it('returns true', () => {
+                expect(menuEnabled.apply(ourBrowser, [{
+                  _type: 'schema'
+                }, [{id: 'level1.2.3'}]])).toBe(true);
+              });
+            });
+          });
+        });
+        context('the current node type is not in the supported node types', () => {
+          it('returns false', () => {
+            expect(menuEnabled.apply(ourBrowser, [{_type: 'catalog'}, [{id: 'level1.1'}]])).toBe(false);
+          });
+        });
+      });
+    });
+
+    context('provided node data is undefined', () => {
+      it('returns false', () => {
+        expect(menuEnabled.apply(ourBrowser, [undefined, [{id: 'level1'}]])).toBe(false);
+      });
+    });
+
+    context('provided node data is null', () => {
+      it('returns false', () => {
+        expect(menuEnabled.apply(ourBrowser, [null, [{id: 'level1'}]])).toBe(false);
+      });
+    });
+  });
+
+  describe('#menuEnabledServer', () => {
+    let ourBrowser;
+    beforeEach(() => {
+      const tree = new TreeFake();
+      ourBrowser = {
+        treeMenu: tree,
+      };
+    });
+
+    context('provided node data is undefined', () => {
+      it('returns false', () => {
+        expect(menuEnabledServer(undefined)).toBe(false);
+      });
+    });
+
+    context('provided node data is null', () => {
+      it('returns false', () => {
+        expect(menuEnabledServer(null)).toBe(false);
+      });
+    });
+
+    context('current node type is not of the type server', () => {
+      it('returns false', () => {
+        expect(menuEnabledServer({_type: 'schema'})).toBe(false);
+      });
+    });
+
+    context('current node type is of the type server', () => {
+      context('is connected', () => {
+        it('returns true', () => {
+          expect(menuEnabledServer({_type: 'server', connected: true})).toBe(true);
+        });
+      });
+      context('is not connected', () => {
+        it('returns false', () => {
+          expect(menuEnabledServer({_type: 'server', connected: false})).toBe(false);
+        });
+      });
+    });
+  });
+});
+
diff --git a/web/regression/javascript/datagrid/get_panel_title_spec.js b/web/regression/javascript/datagrid/get_panel_title_spec.js
new file mode 100644
index 00000000..3baedaa1
--- /dev/null
+++ b/web/regression/javascript/datagrid/get_panel_title_spec.js
@@ -0,0 +1,79 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {getPanelTitle} from "../../../pgadmin/static/js/datagrid/get_panel_title";
+import {TreeFake} from "../tree/tree_fake";
+
+const context = describe;
+const xcontext = xdescribe;
+
+describe('#getPanelTitle', () => {
+  let pgBrowser;
+  let tree;
+  beforeEach(() => {
+    tree = new TreeFake();
+    pgBrowser = {
+      treeMenu: tree,
+      Nodes: {
+        server: {
+          hasId: true,
+          _type: 'server',
+        },
+        database: {
+          hasId: true,
+          _type: 'database',
+        },
+      }
+    };
+  });
+
+  context('selected node does not belong to a server', () => {
+    it('returns undefined', () => {
+      tree.addNewNode('level1', {_type: 'server_groups'}, []);
+      tree.addNewNode('level1.1', {_type: 'other'}, ['level1']);
+      tree.setSelectedNode([{id: 'level1'}]);
+      expect(getPanelTitle(pgBrowser)).toBeUndefined();
+    });
+  });
+
+  context('selected node belong to a server', () => {
+    context('selected node does not belong to a database', () => {
+      it('returns the server label and the username', () => {
+        tree.addNewNode('level1', {
+          _type: 'server',
+          db: 'other db label',
+          user: {name: 'some user name'},
+          label: 'server label'
+        }, []);
+        tree.setSelectedNode([{id: 'level1'}]);
+        expect(getPanelTitle(pgBrowser))
+          .toBe('other db label on some user name@server label');
+      });
+    });
+
+    context('selected node belongs to a database', () => {
+      it('returns the database label and the username', () => {
+        tree.addNewNode('level1', {
+          _type: 'server',
+          db: 'other db label',
+          user: {name: 'some user name'},
+          label: 'server label'
+        }, []);
+        tree.addNewNode('level1.1', {
+          _type: 'database',
+          label: 'db label'
+        }, ['level1']);
+        tree.addNewNode('level1.1.1', {_type: 'table'}, ['level1', 'level1.1']);
+        tree.setSelectedNode([{id: 'level1.1.1'}]);
+        expect(getPanelTitle(pgBrowser))
+          .toBe('db label on some user name@server label');
+      });
+    });
+  });
+});
diff --git a/web/regression/javascript/datagrid/show_data_spec.js b/web/regression/javascript/datagrid/show_data_spec.js
new file mode 100644
index 00000000..e0ef8783
--- /dev/null
+++ b/web/regression/javascript/datagrid/show_data_spec.js
@@ -0,0 +1,159 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {showDataGrid} from "../../../pgadmin/static/js/datagrid/show_data";
+import {TreeFake} from "../tree/tree_fake";
+
+const context = describe;
+
+describe('#show_data', () => {
+  let datagrid;
+  let pgBrowser;
+  let alertify;
+  beforeEach(() => {
+    alertify = jasmine.createSpyObj('alertify', ['alert']);
+    datagrid = {
+      create_transaction: jasmine.createSpy('create_transaction'),
+    };
+    pgBrowser = {
+      treeMenu: new TreeFake(),
+      Nodes: {
+        server_group: {
+          _type: 'server_group',
+          hasId: true,
+        },
+        server: {
+          _type: 'server',
+          hasId: true,
+        },
+        database: {
+          _type: 'database',
+          hasId: true,
+        },
+        schema: {
+          _type: 'schema',
+          hasId: true,
+        },
+        view: {
+          _type: 'view',
+          hasId: true,
+        },
+        catalog: {
+          _type: 'catalog',
+          hasId: true,
+        },
+      }
+    };
+    pgBrowser.treeMenu.addNewNode('parent', {_type: 'parent'}, []);
+    pgBrowser.treeMenu.addNewNode('server_group1', {
+      _type: 'server_group',
+      _id: 1,
+    }, ['parent']);
+    pgBrowser.treeMenu.addNewNode('server1', {
+      _type: 'server',
+      label: 'server1',
+      server_type: 'pg',
+      _id: 2,
+    }, ['parent', 'server_group1']);
+    pgBrowser.treeMenu.addNewNode('database1', {
+      _type: 'database',
+      label: 'database1',
+      _id: 3,
+    }, ['parent', 'server_group1', 'server1']);
+    pgBrowser.treeMenu.addNewNode('schema1', {
+      _type: 'schema',
+      label: 'schema1',
+      _id: 4,
+    }, ['parent', 'server_group1', 'server1', 'database1']);
+    pgBrowser.treeMenu.addNewNode('view1', {
+      _type: 'view',
+      label: 'view1',
+      _id: 5,
+    }, ['parent', 'server_group1', 'server1', 'database1']);
+    pgBrowser.treeMenu.addNewNode('catalog1', {
+      _type: 'catalog',
+      label: 'catalog1',
+      _id: 6,
+    }, ['parent', 'server_group1', 'server1', 'database1']);
+  });
+
+  context('cannot find the tree node', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {}, [{id: '10'}]);
+      expect(datagrid.create_transaction).not.toHaveBeenCalled();
+    });
+
+    it('display alert', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {}, [{id: '10'}]);
+      expect(alertify.alert).toHaveBeenCalledWith(
+        'Data Grid Error',
+        'No object selected.'
+      );
+    });
+  });
+
+  context('current node is not underneath a server', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {}, [{id: 'parent'}]);
+      expect(datagrid.create_transaction).not.toHaveBeenCalled();
+    });
+  });
+
+  context('current node is not underneath a schema or view or catalog', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {}, [{id: 'database1'}]);
+      expect(datagrid.create_transaction).not.toHaveBeenCalled();
+    });
+  });
+
+  context('current node is underneath a schema', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {mnuid: 11}, [{id: 'schema1'}]);
+      expect(datagrid.create_transaction).toHaveBeenCalledWith(
+        '/initialize/datagrid/11/schema/1/2/3/4',
+        null,
+        'false',
+        'pg',
+        '',
+        'server1 - database1 - schema1.schema1',
+        ''
+      );
+    });
+  });
+
+  context('current node is underneath a view', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {mnuid: 11}, [{id: 'view1'}]);
+      expect(datagrid.create_transaction).toHaveBeenCalledWith(
+        '/initialize/datagrid/11/view/1/2/3/5',
+        null,
+        'false',
+        'pg',
+        '',
+        'server1 - database1 - view1.view1',
+        ''
+      );
+    });
+  });
+
+  context('current node is underneath a catalog', () => {
+    it('does not create a transaction', () => {
+      showDataGrid(datagrid, pgBrowser, alertify, {mnuid: 11}, [{id: 'catalog1'}]);
+      expect(datagrid.create_transaction).toHaveBeenCalledWith(
+        '/initialize/datagrid/11/catalog/1/2/3/6',
+        null,
+        'false',
+        'pg',
+        '',
+        'server1 - database1 - catalog1.catalog1',
+        ''
+      );
+    });
+  });
+});
diff --git a/web/regression/javascript/datagrid/show_query_tool_spec.js b/web/regression/javascript/datagrid/show_query_tool_spec.js
new file mode 100644
index 00000000..b36dc4a7
--- /dev/null
+++ b/web/regression/javascript/datagrid/show_query_tool_spec.js
@@ -0,0 +1,119 @@
+/////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////
+
+import {TreeFake} from '../tree/tree_fake';
+import {showQueryTool} from '../../../pgadmin/static/js/datagrid/show_query_tool';
+
+const context = describe;
+
+describe('#showQueryTool', () => {
+  let queryTool;
+  let pgBrowser;
+  let alertify;
+  beforeEach(() => {
+    alertify = jasmine.createSpyObj('alertify', ['alert']);
+    queryTool = {
+      create_transaction: jasmine.createSpy('create_transaction'),
+    };
+    pgBrowser = {
+      treeMenu: new TreeFake(),
+      Nodes: {
+        server_group: {
+          _type: 'server_group',
+          hasId: true,
+        },
+        server: {
+          _type: 'server',
+          hasId: true,
+        },
+        database: {
+          _type: 'database',
+          hasId: true,
+        },
+      }
+    };
+    pgBrowser.treeMenu.addNewNode('parent', {_type: 'parent'}, []);
+    pgBrowser.treeMenu.addNewNode('server_group1', {
+      _type: 'server_group',
+      _id: 1,
+    }, ['parent']);
+    pgBrowser.treeMenu.addNewNode('server1', {
+      _type: 'server',
+      label: 'server1',
+      server_type: 'pg',
+      _id: 2,
+    }, ['parent', 'server_group1']);
+    pgBrowser.treeMenu.addNewNode('database1', {
+      _type: 'database',
+      label: 'database1',
+      _id: 3,
+    }, ['parent', 'server_group1', 'server1']);
+  });
+
+  context('cannot find the tree node', () => {
+    beforeEach(() => {
+      showQueryTool(queryTool, pgBrowser, alertify, '', [{id: '10'}], 'title');
+    });
+    it('does not create a transaction', () => {
+      expect(queryTool.create_transaction).not.toHaveBeenCalled();
+    });
+
+    it('display alert', () => {
+      expect(alertify.alert).toHaveBeenCalledWith(
+        'Query Tool Error',
+        'No object selected.'
+      );
+    });
+  });
+
+  context('current node is not underneath a server', () => {
+    it('does not create a transaction', () => {
+      showQueryTool(queryTool, pgBrowser, alertify, '', [{id: 'parent'}], 'title');
+      expect(queryTool.create_transaction).not.toHaveBeenCalled();
+    });
+
+    it('no alert is displayed', () => {
+      expect(alertify.alert).not.toHaveBeenCalled();
+    });
+  });
+
+  context('current node is underneath a server', () => {
+    context('current node is not underneath a database', () => {
+      it('creates a transaction', () => {
+        showQueryTool(queryTool, pgBrowser, alertify, 'http://someurl', [{id: 'server1'}], 'title');
+        expect(queryTool.create_transaction).toHaveBeenCalledWith(
+          '/initialize/query_tool/1/2',
+          null,
+          'true',
+          'pg',
+          'http://someurl',
+          'title',
+          '',
+          false
+        );
+      });
+    });
+
+    context('current node is underneath a database', () => {
+      it('creates a transaction', () => {
+        showQueryTool(queryTool, pgBrowser, alertify, 'http://someurl', [{id: 'database1'}], 'title');
+        expect(queryTool.create_transaction).toHaveBeenCalledWith(
+          '/initialize/query_tool/1/2/3',
+          null,
+          'true',
+          'pg',
+          'http://someurl',
+          'title',
+          '',
+          false
+        );
+      });
+    });
+  });
+});
diff --git a/web/regression/javascript/fake_endpoints.js b/web/regression/javascript/fake_endpoints.js
index 63ab05dc..e4ae7020 100644
--- a/web/regression/javascript/fake_endpoints.js
+++ b/web/regression/javascript/fake_endpoints.js
@@ -11,6 +11,12 @@ define(function () {
   return {
     'static': '/base/pgadmin/static/<path:filename>',
     'sqleditor.poll': '/sqleditor/query_tool/poll/<path:trans_id>',
-    'sqleditor.query_tool_start': '/sqleditor/query_tool/start/<path:trans_id>'
+    'sqleditor.query_tool_start': '/sqleditor/query_tool/start/<path:trans_id>',
+    'backup.create_server_job':  '/backup/job/<int:sid>',
+    'backup.create_object_job':  '/backup/job/<int:sid>/object',
+    'datagrid.initialize_datagrid': '/initialize/datagrid/<int:cmd_type>/<obj_type>/<int:sgid>/<int:sid>/<int:did>/<int:obj_id>',
+    'datagrid.initialize_query_tool': '/initialize/query_tool/<int:sgid>/<int:sid>',
+    'datagrid.initialize_query_tool_with_did': '/initialize/query_tool/<int:sgid>/<int:sid>/<int:did>',
+
   };
 });
diff --git a/web/regression/javascript/fake_model.js b/web/regression/javascript/fake_model.js
new file mode 100644
index 00000000..acfaa532
--- /dev/null
+++ b/web/regression/javascript/fake_model.js
@@ -0,0 +1,21 @@
+export class FakeModel {
+  constructor() {
+    this.values = {};
+  }
+
+  set(key, value) {
+    this.values[key] = value;
+  }
+
+  get(key) {
+    return this.values[key];
+  }
+
+  unset(key) {
+    delete this.values[key];
+  }
+
+  toJSON() {
+    return Object.assign({}, this.values);
+  }
+}
diff --git a/web/regression/javascript/tree/pgadmin_tree_node_spec.js b/web/regression/javascript/tree/pgadmin_tree_node_spec.js
new file mode 100644
index 00000000..1cab65f0
--- /dev/null
+++ b/web/regression/javascript/tree/pgadmin_tree_node_spec.js
@@ -0,0 +1,353 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+import {
+  getTreeNodeHierarchyFromElement,
+  getTreeNodeHierarchyFromIdentifier
+} from "../../../pgadmin/static/js/tree/pgadmin_tree_node";
+import {Tree} from "../../../pgadmin/static/js/tree/tree";
+
+const context = describe;
+
+describe('tree#node#getTreeNodeHierarchy', () => {
+  let browser;
+  let newTree;
+  let translateTreeNodeIdFromACITreeSpy;
+  beforeEach(() => {
+    newTree = new Tree();
+    browser = {
+      Nodes: {
+        'special one': {hasId: true},
+        'child special': {hasId: true},
+        'other type': {hasId: true},
+        'table': {hasId: true},
+        'partition': {hasId: true},
+        'no id': {hasId: false},
+      }
+    };
+    browser.treeMenu = newTree;
+    translateTreeNodeIdFromACITreeSpy = spyOn(newTree, 'translateTreeNodeIdFromACITree');
+  });
+
+  context('getTreeNodeHierarchy is called with aciTreeNode object', () => {
+    describe('When the current node is root element', () => {
+      beforeEach(() => {
+        newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'special one'
+        }, []);
+        translateTreeNodeIdFromACITreeSpy.and.returnValue(['root']);
+      });
+
+      it('returns a object with the element type passed data and priority == 0', () => {
+        const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('root');
+        expect(result).toEqual({
+          'special one': {
+            'some key': 'some value',
+            '_type': 'special one',
+            'priority': 0,
+          }
+        });
+      });
+    });
+
+    describe('When the current node is not of a known type', () => {
+      beforeEach(() => {
+        newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'do not exist'
+        }, []);
+        translateTreeNodeIdFromACITreeSpy.and.returnValue(['root']);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('root');
+        expect(result).toEqual({});
+      });
+    });
+
+    describe('When the current node type has no id', () => {
+      beforeEach(() => {
+        newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'no id'
+        }, []);
+        translateTreeNodeIdFromACITreeSpy.and.returnValue(['root']);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('root');
+        expect(result).toEqual({});
+      });
+    });
+
+    describe('When the current node is at the second level', () => {
+      beforeEach(() => {
+        newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'special one'
+        }, []);
+        newTree.addNewNode('first child', {
+          'some key': 'some other value',
+          '_type': 'child special'
+        }, ['root']);
+        translateTreeNodeIdFromACITreeSpy.and.returnValue(['root', 'first' +
+        ' child']);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('first child');
+        expect(result).toEqual({
+          'child special': {
+            'some key': 'some other value',
+            '_type': 'child special',
+            'priority': 0,
+          },
+          'special one': {
+            'some key': 'some value',
+            '_type': 'special one',
+            'priority': -1,
+          },
+        });
+      });
+    });
+
+    context('When tree as "special type"', () => {
+      context('When "special type" have "other type"', () => {
+        context('When "other type" have "special type"', () => {
+          describe('When retrieving lead node', () => {
+            it('does not override previous node type data', () => {
+              newTree.addNewNode('root', {
+                'some key': 'some value',
+                '_type': 'special one'
+              }, []);
+              newTree.addNewNode('level 1', {
+                'some key': 'some value',
+                '_type': 'other type'
+              }, ['root']);
+              newTree.addNewNode('level 2', {
+                'some key': 'expected value',
+                '_type': 'special one',
+                'some other key': 'some other value',
+              }, ['root', 'level 1']);
+
+              translateTreeNodeIdFromACITreeSpy.and.returnValue(['root', 'level 1', 'level 2']);
+
+              const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('level 2');
+              expect(result).toEqual({
+                'special one': {
+                  'some key': 'expected value',
+                  '_type': 'special one',
+                  'some other key': 'some other value',
+                  'priority': 0,
+                },
+                'other type': {
+                  'some key': 'some value',
+                  '_type': 'other type',
+                  'priority': -1,
+                },
+              });
+            });
+          });
+        });
+      });
+    });
+
+    context('When tree has table', () => {
+      context('when table has partition', () => {
+        it('returns table with partition parameters', () => {
+          newTree.addNewNode('root', {
+            'some key': 'some value',
+            '_type': 'special one'
+          }, []);
+          newTree.addNewNode('level 1', {
+            'some key': 'some value',
+            '_type': 'table'
+          }, ['root']);
+          newTree.addNewNode('level 2', {
+            'some key': 'expected value',
+            '_type': 'partition',
+            'some other key': 'some other value',
+          }, ['root', 'level 1']);
+
+          translateTreeNodeIdFromACITreeSpy.and.returnValue(['root', 'level 1', 'level 2']);
+
+          const result = getTreeNodeHierarchyFromIdentifier.bind(browser)('level 2');
+          expect(result).toEqual({
+            'special one': {
+              'some key': 'some value',
+              '_type': 'special one',
+              'priority': -1,
+            },
+            'table': {
+              'some key': 'expected value',
+              'some other key': 'some other value',
+              '_type': 'partition',
+              'priority': 0,
+            },
+          });
+        });
+      });
+    });
+  });
+
+  context('getTreeNodeHierarchy is called with TreeNode object', () => {
+    let treeNode;
+    describe('When the current node is root element', () => {
+      beforeEach(() => {
+        treeNode = newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'special one'
+        }, []);
+      });
+
+      it('returns a object with the element type passed data and priority == 0', () => {
+        const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+        expect(result).toEqual({
+          'special one': {
+            'some key': 'some value',
+            '_type': 'special one',
+            'priority': 0,
+          }
+        });
+      });
+    });
+
+    describe('When the current node is not of a known type', () => {
+      beforeEach(() => {
+        treeNode = newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'do not exist'
+        }, []);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+        expect(result).toEqual({});
+      });
+    });
+
+    describe('When the current node type has no id', () => {
+      beforeEach(() => {
+        treeNode = newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'no id'
+        }, []);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+        expect(result).toEqual({});
+      });
+    });
+
+    describe('When the current node is at the second level', () => {
+      beforeEach(() => {
+        newTree.addNewNode('root', {
+          'some key': 'some value',
+          '_type': 'special one'
+        }, []);
+        treeNode = newTree.addNewNode('first child', {
+          'some key': 'some other value',
+          '_type': 'child special'
+        }, ['root']);
+      });
+
+      it('returns a empty object', () => {
+        const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+        expect(result).toEqual({
+          'child special': {
+            'some key': 'some other value',
+            '_type': 'child special',
+            'priority': 0,
+          },
+          'special one': {
+            'some key': 'some value',
+            '_type': 'special one',
+            'priority': -1,
+          },
+        });
+      });
+    });
+
+    context('When tree as "special type"', () => {
+      context('When "special type" have "other type"', () => {
+        context('When "other type" have "special type"', () => {
+          describe('When retrieving lead node', () => {
+            it('does not override previous node type data', () => {
+              newTree.addNewNode('root', {
+                'some key': 'some value',
+                '_type': 'special one'
+              }, []);
+              newTree.addNewNode('level 1', {
+                'some key': 'some value',
+                '_type': 'other type'
+              }, ['root']);
+              treeNode = newTree.addNewNode('level 2', {
+                'some key': 'expected value',
+                '_type': 'special one',
+                'some other key': 'some other value',
+              }, ['root', 'level 1']);
+
+              const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+              expect(result).toEqual({
+                'special one': {
+                  'some key': 'expected value',
+                  '_type': 'special one',
+                  'some other key': 'some other value',
+                  'priority': 0,
+                },
+                'other type': {
+                  'some key': 'some value',
+                  '_type': 'other type',
+                  'priority': -1,
+                },
+              });
+            });
+          });
+        });
+      });
+    });
+
+    context('When tree has table', () => {
+      context('when table has partition', () => {
+        it('returns table with partition parameters', () => {
+          newTree.addNewNode('root', {
+            'some key': 'some value',
+            '_type': 'special one'
+          }, []);
+          newTree.addNewNode('level 1', {
+            'some key': 'some value',
+            '_type': 'table'
+          }, ['root']);
+          treeNode = newTree.addNewNode('level 2', {
+            'some key': 'expected value',
+            '_type': 'partition',
+            'some other key': 'some other value',
+          }, ['root', 'level 1']);
+
+          const result = getTreeNodeHierarchyFromElement(browser, treeNode);
+          expect(result).toEqual({
+            'special one': {
+              'some key': 'some value',
+              '_type': 'special one',
+              'priority': -1,
+            },
+            'table': {
+              'some key': 'expected value',
+              'some other key': 'some other value',
+              '_type': 'partition',
+              'priority': 0,
+            },
+          });
+        });
+      });
+    });
+  });
+});

---

From: Murtuza Zabuawala <[email-masked]>
Date: Fri, 6 Apr 2018 14:18:21 +0530

Hi Joao,

Patch looks good and working as expected.

I also agree with Dave, Can we please add some comments in each file which
can help us to understand the flow, I'm saying because now the code is
segregated in so many separate files it will be hard to keep track of the
flow from one file to another when debugging.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Apr 5, 2018 at 7:08 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

>>> This job that we started is a massive tech debt chore that will take
>>> some time to finalize and we would love the help of the community to do it.
>>>
>>> *Summary of the patch:*
>>> 0001 patch:
>>>   b) Create a tree that would return a node given an ID and then the
>>> node would be responsible for giving it's data.
>>> Pros:
>>>  - OOP Approach
>>> Given these 2 options we decided to go for a more OOP approach creating
>>> a Tree and a TreeNode classes, that in the future will be renamed to
>>> 6. Refactor the function to ES6, give more declarative names to
>>> variables and break the functions into smaller chunks
>>>
>>> As you can see by the process this is a pretty huge undertake, because
>>> of the number of calls to the function. This is just the first step on the

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Fri, 06 Apr 2018 19:13:22 +0000

Hello Murtuza/Dave,
Yes now the extracted functions are spread into different files. The intent
would be to make the files as small as possible, and also to group and name
them in a way that would be easy to understand what each file is doing
without the need of opening it.
As a example:
static/js/backup will contain all the backup related functionality
inside of this folder we can see the file:
menu_utils.js At this moment in time we decided to group all the functions
that are related to the menu, but we can split that also if we believe it
is easier to see.

static/js/datagrid folder contains all the datagrid related functionality
Inside of the folder we can see the files:
get_panel_title.js is responsible for retrieving the name of the panel
show_data.js is responsible for showing the datagrid
show_query_tool.js is responsible for showing the query tool

Does this structure make sense?
Can you give an example of a comment that you think is missing and that
could help?

As a personal note, unless the algorithm is very obscure or very
complicated, I believe that if the code needs comments it is a signal that
something needs to change in terms of naming, structure of the part in
question. This being said, I am open to add some comments that might help
people.

Thanks
Joao
​

On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <
[email-masked]> wrote:

>>>> Attached you can find the patch that will start to decouple pgAdmin
>>>> from ACITree library.
>>>> This patch is intended to be merged after 3.0, because we do not want
>>>> to cause any entropy or delay the release, but we want to start the
>>>> discussion and show some code.
>>>>
>>>>  - Creates a Fake Tree (Test double, for reference on the available
>>>> test doubles: https://martinfowler.com/bliki/TestDouble.html) that can
>>>> be used to inplace to replace the ACITree and also encapsulate the new tree
>>>> behavior on our tests
>>>> Given in order to replace the tree we would need to have a function
>>>> that would give us the same information. We had 2 options:
>>>> 2. After we decided on the starting point we searched for occurrences
>>>> of the function "itemData" and we found out that there were 303 occurrences
>>>> of "itemData" in the code and roughly 176 calls to the function itself
>>>> (some of the hits were variable names).
>>>>

---

From: Khushboo Vashi <[email-masked]>
Date: Mon, 9 Apr 2018 11:29:38 +0530

Hi Joao,

I have reviewed your patch and have some suggestions.

On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Hello Murtuza/Dave,
> Yes now the extracted functions are spread into different files. The
> intent would be to make the files as small as possible, and also to group
> and name them in a way that would be easy to understand what each file is
> doing without the need of opening it.
> As a example:
> static/js/backup will contain all the backup related functionality inside
> of this folder we can see the file:
>
> that are related to the menu, but we can split that also if we believe it
It's really very good to see the separated code for backup module. As we
have done for backup, we would like do it for other PG utilities like
restore, maintenance etc.
Considering this, we should separate the code in a way that some of the
common functionalities can be used for other modules  like menu (as you
have mentioned above), dialogue factory etc.
Also, I think these functionalities should be in their respective static
folder instead of pgadmin/static.

> static/js/datagrid folder contains all the datagrid related functionality
Same as backup module,  this should be in it's respective static/js folder.

> question. This being said, I am open to add some comments that might help
You are right, with the help of naming convention and structure of the
code, any one can get the idea about the code. But it is very useful to
understand the code
very easily with the proper comments especially when there are multiple
developers working on a single project.

I found some of the places where it would be great to have comments.

- treeMenu: new tree.Tree()  in a browser.js
- tree.js  (especially Tree class)

Thanks
Khushboo

>> I also agree with Dave, Can we please add some comments in each file
>> which can help us to understand the flow, I'm saying because now the code
>> is segregated in so many separate files it will be hard to keep track of
>> the flow from one file to another when debugging.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>>>>>  - Creates a new tree that will allow us to create a separation
>>>>> between the application and ACI Tree
>>>>> test doubles: https://martinfowler.com/bliki/TestDouble.html) that
>>>>> can be used to inplace to replace the ACITree and also encapsulate the new
>>>>> tree behavior on our tests
>>>>> This patch represents only 10 calls to ACITree.itemData out of 176
>>>>> that are spread around our code
>>>>>
>>>>>
>>>>> Given these 2 options we decided to go for a more OOP approach
>>>>> creating a Tree and a TreeNode classes, that in the future will be renamed
>>>>> to ACITreeWrapper and TreeNode.
>>>>>
>>>>> 3. We selected the first file on the search and found the function
>>>>> that was responsible for calling the itemData function.
>>>>>

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Tue, 10 Apr 2018 16:00:05 +0000

Hello Khushboo

On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
[email-masked]> wrote:

> menu_utils.js At this moment in time we decided to group all the
>> functions that are related to the menu, but we can split that also if we
>> believe it is easier to see.
>>

About the location of the files. The move of the files to pgadmin/static/js
was made on purpose in order to clearly separate Javascript from python
code.
The rational behind it was
- Create a clear separation between the backend and frontend
- Having Javascript code concentrated in a single place, hopefully, will
encourage to developers to look for a functionality, that is already
implemented in another modules, because they are right there. (When we
started this journey we realized that the 'nodes' have a big groups of code
that could be shared, but because the Javascript is spread everywhere it is
much harder to look for it)

There are some drawbacks of this separation:
- When creating a new module we will need to put the javascript in a
separate location from the backend code

> - tree.js  (especially Tree class)
About the comment point I need a more clear understanding on what kind of
comments you are looking for. Because when you read the function names you
understand the intent, what they are doing. The parameters also explain
what you need to pass into them.

If what you are looking for in these comments is the reasoning being the
change itself, then that should be present in the commit message. Specially
because this is going to be a very big patch with a very big number of
changes.

>>>>>> 1. We "randomly" selected a function that is part of the ACITree.
>>>>>> From this point we decided to replace that function with our own version.
>>>>>> The function that we choose was "itemData".
>>>>>> As you can see by the process this is a pretty huge undertake,
>>>>>> because of the number of calls to the function. This is just the first step
>>>>>> on the direction of completely isolating the ACITree so that we can solve
>>>>>> the problem with a large number of elements on the tree.
>>>>>>
>>>>>>  - Tweak the naming of the Tree class to explicitly tell us this is
>>>>>> to use only with ACITree.
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>>
>>>>>
>>>

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Wed, 18 Apr 2018 14:22:58 +0000

Hi Hackers,
Any other comment about this patch?

Thanks
Joao

On Tue, Apr 10, 2018 at 12:00 PM Joao De Almeida Pereira <
[email-masked]> wrote:

> About the location of the files. The move of the files to
> pgadmin/static/js was made on purpose in order to clearly separate
> Javascript from python code.
>>>>>>> This patch is intended to be merged after 3.0, because we do not
>>>>>>> want to cause any entropy or delay the release, but we want to start the
>>>>>>> 2. After we decided on the starting point we searched for
>>>>>>> occurrences of the function "itemData" and we found out that there were 303
>>>>>>> occurrences of "itemData" in the code and roughly 176 calls to the function
>>>>>>> itself (some of the hits were variable names).
>>>>>>>

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Tue, 24 Apr 2018 19:13:37 +0000

Hi Hackers,

Can someone review and merge this patch?

Thanks
Joao

On Wed, Apr 18, 2018 at 10:23 AM Joao De Almeida Pereira <
[email-masked]> wrote:

>>>>>>>> This job that we started is a massive tech debt chore that will
>>>>>>>> take some time to finalize and we would love the help of the community to
>>>>>>>> do it.
>>>>>>>>
>>>>>>>> *Summary of the patch:*
>>>>>>>> 0001 patch:
>>>>>>>> - Start using sprintf-js as Underscore.String is deprecating
>>>>>>>> sprintf function
>>>>>>>>
>>>>>>>> We started writing this patch with the idea that we need to
>>>>>>>> decouple pgAdmin4 from ACITree, because ACITree is no longer supported, the
>>>>>>>>   b) Create a tree that would return a node given an ID and then
>>>>>>>> the node would be responsible for giving it's data.
>>>>>>>> Pros:
>>>>>>>>  - OOP Approach

---

From: Dave Page <[email-masked]>
Date: Wed, 25 Apr 2018 08:52:34 +0100

Ashesh; you had agreed to work on this early this week. Please ensure you
do so today.

Thanks.

On Tue, Apr 24, 2018 at 8:13 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

>>>> It's really very good to see the separated code for backup module. As
>>>> we have done for backup, we would like do it for other PG utilities like
>>>> Also, I think these functionalities should be in their respective
>>>> static folder instead of pgadmin/static.
>>>>
>>>
>>>>> Can you give an example of a comment that you think is missing and
>>>>> that could help?
>>>>>
>>> About the comment point I need a more clear understanding on what kind
>>> of comments you are looking for. Because when you read the function names
>>> you understand the intent, what they are doing. The parameters also explain
>>>>> On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <murtuza.zabuawala@
>>>>> enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Joao,
>>>>>>
>>>>>>>>> Attached you can find the patch that will start to decouple
>>>>>>>>> pgAdmin from ACITree library.
>>>>>>>>>  - Creates a Fake Tree (Test double, for reference on the
>>>>>>>>> available test doubles: https://martinfowler.
>>>>>>>>> com/bliki/TestDouble.html) that can be used to inplace to replace
>>>>>>>>> the ACITree and also encapsulate the new tree behavior on our tests
>>>>>>>>> Given in order to replace the tree we would need to have a
>>>>>>>>> function that would give us the same information. We had 2 options:
>>>>>>>>>  - Tweak the naming of the Tree class to explicitly tell us this
>>>>>>>>> is to use only with ACITree.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Joao
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>

---

From: Dave Page <[email-masked]>
Date: Fri, 27 Apr 2018 10:15:38 +0100

How is your work on this going Ashesh? Will you be committing today?

On Wed, Apr 25, 2018 at 8:52 AM, Dave Page <[email-masked]> wrote:

>>>>> Considering this, we should separate the code in a way that some of
>>>>> the common functionalities can be used for other modules  like menu (as you
>>>> - Having Javascript code concentrated in a single place, hopefully,
>>>> will encourage to developers to look for a functionality, that is already
>>>>> very easily with the proper comments especially when there are
>>>>> multiple developers working on a single project.
>>>>>
>>>> If what you are looking for in these comments is the reasoning being
>>>> the change itself, then that should be present in the commit message.
>>>> Specially because this is going to be a very big patch with a very big
>>>> number of changes.
>>>>
>>>>>
>>>>> Thanks
>>>>>> Joao
>>>>>> ​
>>>>>>
>>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>>
>>>>>>>>>> com/bliki/TestDouble.html) that can be used to inplace to
>>>>>>>>>> replace the ACITree and also encapsulate the new tree behavior on our tests
>>>>>>>>>> This patch represents only 10 calls to ACITree.itemData out of
>>>>>>>>>> 176 that are spread around our code
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 3. We selected the first file on the search and found the
>>>>>>>>>> function that was responsible for calling the itemData function.
>>>>>>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Fri, 27 Apr 2018 09:34:36 +0000

I have quite a few comments for the patch.
I will send them soon.

On Fri, Apr 27, 2018, 14:45 Dave Page <[email-masked]> wrote:

>>>>>> You are right, with the help of naming convention and structure of
>>>>>> the code, any one can get the idea about the code. But it is very useful to
>>>>>> understand the code
>>>>>>>> I also agree with Dave, Can we please add some comments in each
>>>>>>>> file which can help us to understand the flow, I'm saying because now the
>>>>>>>> code is segregated in so many separate files it will be hard to keep track
>>>>>>>> of the flow from one file to another when debugging.
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Regards,
>>>>>>>> Murtuza Zabuawala
>>>>>>>>>>> available test doubles:
>>>>>>>>>>> https://martinfowler.com/bliki/TestDouble.html) that can be
>>>>>>>>>>> 1. We "randomly" selected a function that is part of the
>>>>>>>>>>> ACITree. From this point we decided to replace that function with our own
>>>>>>>>>>> version. The function that we choose was "itemData".

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Fri, 27 Apr 2018 22:25:41 +0000

Hi Hackers,
As you are aware we kept on working on the patch, so we are attaching to
this email a new version of the patch.
This new version contains all the changes in the previous one plus more
extractions of functions and refactoring of code.

The objective of this patch is to create a separation between pgAdmin and
the ACI Tree. We are doing this because we realized that at this point in
time we have the ACI Tree all over the code of pgAdmin. I found a very
interesting article that really talks about this:
https://medium.freecodecamp.org/code-dependencies-are-the-devil-35ed28b556d

In this patch there are some visions and ideas about the location of the
code, the way to organize it and also try to pave the future for a
application that is stable, easy to develop on and that can be release at a
times notice.

We are investing a big chunk of our time in doing this refactoring, but
while doing that we also try to respond to the patches sent to the mailing
list. We would like the feedback from the community because we believe this
is a changing point for the application. The idea is to change the way we
develop this application, instead of only correcting a bug of developing a
feature, with every commit we should correct the bug or develop a feature
but leave the code a little better than we found it (Refactoring,
refactoring, refactoring). This is hard work but that is what the users
from pgAdmin expect from this community of developers.

======

It is a huge patch
                          86 files changed, 5492 inserts, 1840 deletions
and we would like to get your feedback as soon as possible, because we are
continuing to work on it which means it is going to grow in size.

At this point in time we still have 124 of 176 calls to the function
itemData from ACITree.

What does each patch contain:
0001:
  Very simple patch, we found out that the linter was not looking into all
the javascript test files, so this patch will ensure it is

0002:
  New Tree abstraction. This patch contains the new Tree that works as an
adaptor for ACI Tree and is going to be used on all the extractions that we
are doing

0003:
  Code that extracts, wrap with tests and replace ACI Tree invocations.
  We start creating new pattern for the location of Javascript files and
their structure.
  Create patterns for creation of dialogs (backup and restore)

Thanks
Joao

On Fri, Apr 27, 2018 at 5:34 AM Ashesh Vashi <[email-masked]>
wrote:

>>> Ashesh; you had agreed to work on this early this week. Please ensure
>>> you do so today.
>>>
>>> Thanks.
>>>
>>>>>>>> Yes now the extracted functions are spread into different files.
>>>>>>>> The intent would be to make the files as small as possible, and also to
>>>>>>>> group and name them in a way that would be easy to understand what each
>>>>>>>> file is doing without the need of opening it.
>>>>>>>> As a example:
>>>>>>> It's really very good to see the separated code for backup module.
>>>>>>> As we have done for backup, we would like do it for other PG utilities like
>>>>>> About the comment point I need a more clear understanding on what
>>>>>> kind of comments you are looking for. Because when you read the function
>>>>>> names you understand the intent, what they are doing. The parameters also
>>>>>> explain what you need to pass into them.
>>>>>>
>>>>>>>>>>>> This patch is intended to be merged after 3.0, because we do
>>>>>>>>>>>> not want to cause any entropy or delay the release, but we want to start
>>>>>>>>>>>> the discussion and show some code.
>>>>>>>>>>>>
>>>>>>>>>>>>   b) Create a tree that would return a node given an ID and
>>>>>>>>>>>> then the node would be responsible for giving it's data.
>>>>>>>>>>>> Pros:
>>>>>>>>>>>>  - OOP Approach
>>>>>>>>>>>> 11. Find the next function and execute from step 4 until all
>>>>>>>>>>>> the functions are replaced, refactored and tested.
>>>>>>>>>>>>
>>>>>>>>>>>>  - Tweak the naming of the Tree class to explicitly tell us
>>>>>>>>>>>> this is to use only with ACITree.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>> Joao
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>
>>>
>>> --
>>> Dave Page

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 11:46:47 +0530

>   Create patterns for creation of dialogs (backup and restore)

Do you have some TODO left for the same?
Or, is this the final one? Because - it gives us the better understanding
during reviewing the patch.

-- Thanks, Ashesh

>
>
> Thanks
> Joao
>
>
> On Fri, Apr 27, 2018 at 5:34 AM Ashesh Vashi <
> [email-masked]> wrote:
>
>>>>>>>>> static/js/backup will contain all the backup related
>>>>>>>>> functionality inside of this folder we can see the file:
>>>>>>>>>
>>>>>>>>>>>>> This job that we started is a massive tech debt chore that
>>>>>>>>>>>>> will take some time to finalize and we would love the help of the community
>>>>>>>>>>>>> to do it.
>>>>>>>>>>>>>
>>>>>>>>>>>>> *Summary of the patch:*
>>>>>>>>>>>>> 0001 patch:
>>>>>>>>>>>>>  - Creates a new tree that will allow us to create a
>>>>>>>>>>>>> separation between the application and ACI Tree
>>>>>>>>>>>>> 6. Refactor the function to ES6, give more declarative names
>>>>>>>>>>>>> to variables and break the functions into smaller chunks
>>>>>>>>>>>>>

---

From: Anthony Emengo <[email-masked]>
Date: Mon, 30 Apr 2018 06:21:08 -0400

Hi there,

Yes, there's a lot of TODO that we intend for this effort - to be
submitted. We'd like to remove as much *direct* invocations on the ACI Tree
library, as it causes a lot of coupling to the library. It is not the final
patch, but we cannot come up with a definitive list of the things we intend
to do, at this time.

On Mon, Apr 30, 2018 at 2:16 AM, Ashesh Vashi <[email-masked]

>> and we would like to get your feedback as soon as possible, because we
>> are continuing to work on it which means it is going to grow in size.
>>
>>
>>   Very simple patch, we found out that the linter was not looking into
>> all the javascript test files, so this patch will ensure it is
>>
>> 0002:
>>>>>>>>> Considering this, we should separate the code in a way that some
>>>>>>>>> of the common functionalities can be used for other modules  like menu (as
>>>>>>>>> you have mentioned above), dialogue factory etc.
>>>>>>>> - When creating a new module we will need to put the javascript in
>>>>>>>> a separate location from the backend code
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Can you give an example of a comment that you think is missing
>>>>>>>>>> and that could help?
>>>>>>>>>>
>>>>>>>> If what you are looking for in these comments is the reasoning
>>>>>>>> being the change itself, then that should be present in the commit message.
>>>>>>>>>>>>>> This patch represents only 10 calls to ACITree.itemData out
>>>>>>>>>>>>>> of 176 that are spread around our code
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Given these 2 options we decided to go for a more OOP
>>>>>>>>>>>>>> approach creating a Tree and a TreeNode classes, that in the future will be
>>>>>>>>>>>>>> renamed to ACITreeWrapper and TreeNode.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As you can see by the process this is a pretty huge
>>>>>>>>>>>>>> undertake, because of the number of calls to the function. This is just the
>>>>>>>>>>>>>> first step on the direction of completely isolating the ACITree so that we
>>>>>>>>>>>>>> can solve the problem with a large number of elements on the tree.
>>>>>>>>>>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 15:54:58 +0530

On Mon, Apr 30, 2018 at 3:51 PM, Anthony Emengo <[email-masked]> wrote:

> submitted. We'd like to remove as much *direct* invocations on the ACI
> Tree library, as it causes a lot of coupling to the library. It is not the
> final patch, but we cannot come up with a definitive list of the things we
> intend to do, at this time.
>
Is there any known TODO list?
So that - I can help you figure out (if any more).

-- Thanks, Ashesh

>
> On Mon, Apr 30, 2018 at 2:16 AM, Ashesh Vashi <
>>> The objective of this patch is to create a separation between pgAdmin
>>> and the ACI Tree. We are doing this because we realized that at this point
>>> in time we have the ACI Tree all over the code of pgAdmin. I found a very
>>>   New Tree abstraction. This patch contains the new Tree that works as
>>> an adaptor for ACI Tree and is going to be used on all the extractions that
>>> we are doing
>>>
>>> 0003:
>>>>>>>>>> It's really very good to see the separated code for backup
>>>>>>>>>> module. As we have done for backup, we would like do it for other PG
>>>>>>>>>> utilities like restore, maintenance etc.
>>>>>>>>> - Having Javascript code concentrated in a single place,
>>>>>>>>> hopefully, will encourage to developers to look for a functionality, that
>>>>>>>>> is already implemented in another modules, because they are right there.
>>>>>>>>> (When we started this journey we realized that the 'nodes' have a big
>>>>>>>>> groups of code that could be shared, but because the Javascript is spread
>>>>>>>>> everywhere it is much harder to look for it)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> You are right, with the help of naming convention and structure
>>>>>>>>>> of the code, any one can get the idea about the code. But it is very useful
>>>>>>>>>> to understand the code

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Mon, 30 Apr 2018 14:57:21 +0000

Despite this being a work-in-progress. We've provided committable code:
We've ensured it works like before.

For right now, our strategy is looking for calls to `itemData(`. We've
noticed that these surface a lot of calls that are duplicated throughout
the code base. We'd like to reduce the amount of duplication as much as
possible for multiple reasons:
- we've noticed that it makes the code easier to reason about.
- we've noticed that it makes the code much easier to test (there's a lot
of functionality we'd like to have coverage for).
- ultimately will make the goal of decoupling the code from the aci tree
library easier.

For instance, the following lines in the picture below show up in too many
places.

You can expect in the next patches to see more and more functions extracted
with the goal of decoupling the tree in mind.

Anthony && Joao

On Mon, Apr 30, 2018 at 6:25 AM Ashesh Vashi <[email-masked]>
wrote:

>>>> As you are aware we kept on working on the patch, so we are attaching
>>>> to this email a new version of the patch.
>>>> In this patch there are some visions and ideas about the location of
>>>> the code, the way to organize it and also try to pave the future for a
>>>>   We start creating new pattern for the location of Javascript files
>>>> and their structure.
>>> Or, is this the final one? Because - it gives us the better
>>> understanding during reviewing the patch.
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>>
>>>>>>> Ashesh; you had agreed to work on this early this week. Please
>>>>>>> ensure you do so today.
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>>>>>>> Yes now the extracted functions are spread into different
>>>>>>>>>>>> files. The intent would be to make the files as small as possible, and also
>>>>>>>>>>>> to group and name them in a way that would be easy to understand what each
>>>>>>>>>>> menu_utils.js At this moment in time we decided to group all
>>>>>>>>>>>> the functions that are related to the menu, but we can split that also if
>>>>>>>>>>>> we believe it is easier to see.
>>>>>>>>>>>>
>>>>>>>>>>> Also, I think these functionalities should be in their
>>>>>>>>>>> respective static folder instead of pgadmin/static.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - When creating a new module we will need to put the javascript
>>>>>>>>>> in a separate location from the backend code
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> As a personal note, unless the algorithm is very obscure or
>>>>>>>>>>>> very complicated, I believe that if the code needs comments it is a signal
>>>>>>>>>>>> that something needs to change in terms of naming, structure of the part in
>>>>>>>>>>>>> I also agree with Dave, Can we please add some comments in
>>>>>>>>>>>>> each file which can help us to understand the flow, I'm saying because now
>>>>>>>>>>>>> the code is segregated in so many separate files it will be hard to keep
>>>>>>>>>>>>> track of the flow from one file to another when debugging.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Murtuza Zabuawala
>>>>>>>>>>>>>>>> This patch is intended to be merged after 3.0, because we
>>>>>>>>>>>>>>>> do not want to cause any entropy or delay the release, but we want to start
>>>>>>>>>>>>>>>> https://martinfowler.com/bliki/TestDouble.html) that can
>>>>>>>>>>>>>>>> - Start using sprintf-js as Underscore.String is
>>>>>>>>>>>>>>>> deprecating sprintf function
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 6. Refactor the function to ES6, give more declarative
>>>>>>>>>>>>>>>> names to variables and break the functions into smaller chunks
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 11. Find the next function and execute from step 4 until
>>>>>>>>>>>>>>>> all the functions are replaced, refactored and tested.
>>>>>>>>>>>>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 20:30:49 +0530

> the javascript test files, so this patch will ensure it is
Committed the patch along with the regression introduced because of this
patch.

> are doing.

I was expecting a separate layer between the tree implementation, and
aciTree adaptor.
Please find the patch for the example.

It will separate the two layers, and easy to replace with the new
implemenation in future.

>   Code that extracts, wrap with tests and replace ACI Tree invocations.
There are many small cases left in the patches.
Hence - I would like to know the TODO list created by you.

e.g. When we remove any of the object from the database server, we're not
yet removing the respective node from the new implementation, and its
children.

>   We start creating new pattern for the location of Javascript files and
I would not like to see that changes in this patch.
I would like us to come up with the actual design about the hot
pluggability before going in this direction.

>   Create patterns for creation of dialogs (backup and restore)
It's better - we don't change the directory structure at the moment.

I am not against dividing the big javascript files in small chunks, but - I
would like us to discuss first about the hot plugins design first.

-- Thanks, Ashesh

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 20:33:03 +0530

On Mon, Apr 30, 2018 at 8:30 PM, Ashesh Vashi <[email-masked]

> It will separate the two layers, and easy to replace with the new

Oops forgot to attach the patch.
Please find the patch attached.

-- Thanks, Ashesh

> I am not against dividing the big javascript files in small chunks, but -
> I would like us to discuss first about the hot plugins design first.
>
> -- Thanks, Ashesh
>
>>
>>
>
>>
>> Thanks
>> Joao
>>
>>

---

From: Anthony Emengo <[email-masked]>
Date: Mon, 30 Apr 2018 11:28:17 -0400

> implementation in future.

In general, we want defer the separation of the layers for now. Even though
we might assume that this is the direction we want to go in. It's simply
too early to be making such an architectural leap. For right now, we just
know that we need the decoupling, but don't know what if we'd need the 2
layers *as implemented*. The principle we're adhering to here is the Last
Responsible Moment principle, which states that you only make the changes
that you feel is necessary for the given problems you're facing:
https://blog.codinghorror.com/the-last-responsible-moment/

> pluggability before going in this direction.

In our point of view these 2 changes are not related. One thing is the
internal code organization of the application, other thing is allowing
third party to drop code in the application and it just work. These 2
should be talked separately, but the hot pluggability is not something that
will be address by this work we are doing right now.

Anthony && Joao

On Mon, Apr 30, 2018 at 11:03 AM, Ashesh Vashi <
[email-masked]> wrote:

>
>
>
> On Mon, Apr 30, 2018 at 8:30 PM, Ashesh Vashi <
>>> we are doing.

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 21:03:17 +0530

On Mon, Apr 30, 2018 at 8:58 PM, Anthony Emengo <[email-masked]> wrote:

> In general, we want defer the separation of the layers for now. Even
> though we might assume that this is the direction we want to go in. It's
> simply too early to be making such an architectural leap. For right now, we
> just know that we need the decoupling, but don't know what if we'd need the
> 2 layers *as implemented*. The principle we're adhering to here is the
> Last Responsible Moment principle, which states that you only make the
> changes that you feel is necessary for the given problems you're facing:
Neither - it should be part of this change.
It should be addressed separately, and discussed.

-- Thanks, Ashesh

>>> e.g. When we remove any of the object from the database server, we're
>>> not yet removing the respective node from the new implementation, and its
>>> children.
>>>
>>>>
>>> I am not against dividing the big javascript files in small chunks, but
>>> - I would like us to discuss first about the hot plugins design first.
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>>
>>>
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>>

---

From: Dave Page <[email-masked]>
Date: Mon, 30 Apr 2018 16:37:01 +0100

On Mon, Apr 30, 2018 at 4:33 PM, Ashesh Vashi <[email-masked]

> It should be addressed separately, and discussed.

I agree. As long as this work doesn't make the pluggability problem worse,
that problem should be considered separately.

So given Anthony's comments, are you happy with this patch?

>>>>> This new version contains all the changes in the previous one plus
>>>>> more extractions of functions and refactoring of code.
>>>>>
>>>>> We are investing a big chunk of our time in doing this refactoring,
>>>>> but while doing that we also try to respond to the patches sent to the
>>>>> mailing list. We would like the feedback from the community because we
>>>>> believe this is a changing point for the application. The idea is to change
>>>>> the way we develop this application, instead of only correcting a bug of
>>>>> developing a feature, with every commit we should correct the bug or
>>>>> develop a feature but leave the code a little better than we found it
>>>>> (Refactoring, refactoring, refactoring). This is hard work but that is what
>>>>> the users from pgAdmin expect from this community of developers.
>>>>>
>>>>>
>>>>> ======
>>>>>
>>>>>
>>>>>
>>>>> It is a huge patch
>>>> Committed the patch along with the regression introduced because of
>>>> this patch.
>>>>
>>>>>
>>>>> 0002:
>>>>>>>>>>>> Considering this, we should separate the code in a way that
>>>>>>>>>>>> some of the common functionalities can be used for other modules  like menu
>>>>>>>>>>>> (as you have mentioned above), dialogue factory etc.
>>>>>>>>>>> About the comment point I need a more clear understanding on
>>>>>>>>>>> what kind of comments you are looking for. Because when you read the
>>>>>>>>>>> function names you understand the intent, what they are doing. The
>>>>>>>>>>> parameters also explain what you need to pass into them.
>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Attached you can find the patch that will start to
>>>>>>>>>>>>>>>>> decouple pgAdmin from ACITree library.
>>>>>>>>>>>>>>>>> This patch represents only 10 calls to ACITree.itemData
>>>>>>>>>>>>>>>>> out of 176 that are spread around our code
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> We started writing this patch with the idea that we need
>>>>>>>>>>>>>>>>> to decouple pgAdmin4 from ACITree, because ACITree is no longer supported,
>>>>>>>>>>>>>>>>> the documentation is non existent and ACITree is no longer being actively
>>>>>>>>>>>>>>>>> developed.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Our process:
>>>>>>>>>>>>>>>>>   b) Create a tree that would return a node given an ID
>>>>>>>>>>>>>>>>> and then the node would be responsible for giving it's data.
>>>>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>>>>>  - OOP Approach
>>>>>>>>>>>>>>>>>  - Tweak the naming of the Tree class to explicitly tell
>>>>>>>>>>>>>>>>> us this is to use only with ACITree.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 30 Apr 2018 21:15:21 +0530

On Mon, Apr 30, 2018 at 9:07 PM, Dave Page <[email-masked]> wrote:

>
>
> On Mon, Apr 30, 2018 at 4:33 PM, Ashesh Vashi <
I liked the design so far.
But - as Khushboo mentioned ealier - it is missing at some places.
I had to read through the code to understand the execution flow for some.

And, there is still a lot missing bits, and pieces to consider for commit
in the repo.

-- Thanks, Ashesh

>>>>>> and we would like to get your feedback as soon as possible, because
>>>>>> we are continuing to work on it which means it is going to grow in size.
>>>>>>
>>>>>>
>>>>>>   Very simple patch, we found out that the linter was not looking
>>>>>> into all the javascript test files, so this patch will ensure it is
>>>>>>
>>>>>>   New Tree abstraction. This patch contains the new Tree that works
>>>>>> as an adaptor for ACI Tree and is going to be used on all the extractions
>>>>>> that we are doing.
>>>>>>
>>>>>
>>>>> I am not against dividing the big javascript files in small chunks,
>>>>> but - I would like us to discuss first about the hot plugins design first.
>>>>>
>>>>> -- Thanks, Ashesh
>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>>
>>>>>>>>>>>>>> Can you give an example of a comment that you think is
>>>>>>>>>>>>>> missing and that could help?
>>>>>>>>>>>>>>
>>>>>>>>>>>>> You are right, with the help of naming convention and
>>>>>>>>>>>>> structure of the code, any one can get the idea about the code. But it is
>>>>>>>>>>>>> very useful to understand the code
>>>>>>>>>>>>>>>>>> This job that we started is a massive tech debt chore
>>>>>>>>>>>>>>>>>> that will take some time to finalize and we would love the help of the
>>>>>>>>>>>>>>>>>> community to do it.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> *Summary of the patch:*
>>>>>>>>>>>>>>>>>> 0001 patch:
>>>>>>>>>>>>>>>>>> com/bliki/TestDouble.html) that can be used to inplace
>>>>>>>>>>>>>>>>>> to replace the ACITree and also encapsulate the new tree behavior on our
>>>>>>>>>>>>>>>>>> tests
>>>>>>>>>>>>>>>>>> Given in order to replace the tree we would need to have
>>>>>>>>>>>>>>>>>> a function that would give us the same information. We had 2 options:

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Mon, 30 Apr 2018 17:57:56 +0000

You've lost us with the last reply. We'd love to know what we'd have to do
to get this patch committed. You mention that "some things are missing at
some places" and that "there are missing bits". Please note that this is
not a complete solution that we've offered so far. But only one step in a
grander effort to effect a more cleaner, more maintainable, more testable,
code base.

Thanks
Joao and Anthony.

On Mon, Apr 30, 2018 at 11:45 AM Ashesh Vashi <[email-masked]>
wrote:

>> I agree. As long as this work doesn't make the pluggability problem
>> worse, that problem should be considered separately.
>>
>>>>>>> As you are aware we kept on working on the patch, so we are
>>>>>>> attaching to this email a new version of the patch.
>>>>>>> The objective of this patch is to create a separation between
>>>>>>> pgAdmin and the ACI Tree. We are doing this because we realized that at
>>>>>>> this point in time we have the ACI Tree all over the code of pgAdmin. I
>>>>>>> found a very interesting article that really talks about this:
>>>>>>>>>>>>> - When creating a new module we will need to put the
>>>>>>>>>>>>> javascript in a separate location from the backend code
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> very easily with the proper comments especially when there
>>>>>>>>>>>>>> are multiple developers working on a single project.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida Pereira
>>>>>>>>>>>>>>>>>> <[email-masked]> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> This patch is intended to be merged after 3.0, because
>>>>>>>>>>>>>>>>>>> we do not want to cause any entropy or delay the release, but we want to
>>>>>>>>>>>>>>>>>>> start the discussion and show some code.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>  - Creates a Fake Tree (Test double, for reference on
>>>>>>>>>>>>>>>>>>> the available test doubles:
>>>>>>>>>>>>>>>>>>> https://martinfowler.com/bliki/TestDouble.html) that
>>>>>>>>>>>>>>>>>>> 2. After we decided on the starting point we searched
>>>>>>>>>>>>>>>>>>> for occurrences of the function "itemData" and we found out that there were
>>>>>>>>>>>>>>>>>>> 303 occurrences of "itemData" in the code and roughly 176 calls to the
>>>>>>>>>>>>>>>>>>> function itself (some of the hits were variable names).
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> 3. We selected the first file on the search and found
>>>>>>>>>>>>>>>>>>> the function that was responsible for calling the itemData function.
>>>>>>>>>>>>>>>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Wed, 2 May 2018 11:00:11 +0530

On Mon, Apr 30, 2018 at 11:27 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> some places" and that "there are missing bits".
>
I've already mentioned few of them in the earlier mails about them.

> Please note that this is not a complete solution that we've offered so far.
>
I know and, hence - asked for your understanding of the code (and, asked
for the TODOs.)

> But only one step in a grander effort to effect a more cleaner, more
> maintainable, more testable, code base.
>
Yes - we all want cleaner, maintainable, and testable code base.
As per my understading comments are required for better understanding, and
makes the code more maintainable.

But - if we're going to change everything in one go, it is difficult to
understand the changes.
Let's avoid mix match multiple things in a single patch. And, concerntrate
one functionality at a time.

-- Thanks, Ashesh

>
> Thanks
> Joao and Anthony.
>
> On Mon, Apr 30, 2018 at 11:45 AM Ashesh Vashi <
>>>>> 2 layers *as implemented*. The principle we're adhering to here is
>>>>> the Last Responsible Moment principle, which states that you only make the
>>>>>>>> In this patch there are some visions and ideas about the location
>>>>>>>> of the code, the way to organize it and also try to pave the future for a
>>>>>>>> At this point in time we still have 124 of 176 calls to the
>>>>>>>> function itemData from ACITree.
>>>>>>>>
>>>>>>> I was expecting a separate layer between the tree implementation,
>>>>>>> and aciTree adaptor.
>>>>>>> e.g. When we remove any of the object from the database server,
>>>>>>> we're not yet removing the respective node from the new implementation, and
>>>>>>> its children.
>>>>>>>
>>>>>>>>
>>>>>>>>   We start creating new pattern for the location of Javascript
>>>>>>>> files and their structure.
>>>>>>>>
>>>>>>>>>>>>>>> menu_utils.js At this moment in time we decided to group
>>>>>>>>>>>>>>>> all the functions that are related to the menu, but we can split that also
>>>>>>>>>>>>>>>> if we believe it is easier to see.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If what you are looking for in these comments is the
>>>>>>>>>>>>>> reasoning being the change itself, then that should be present in the
>>>>>>>>>>>>>> commit message. Specially because this is going to be a very big patch with
>>>>>>>>>>>>>> a very big number of changes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>>> ​
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> the available test doubles: https://martinfowler.
>>>>>>>>>>>>>>>>>>>> We started writing this patch with the idea that we
>>>>>>>>>>>>>>>>>>>> need to decouple pgAdmin4 from ACITree, because ACITree is no longer
>>>>>>>>>>>>>>>>>>>> supported, the documentation is non existent and ACITree is no longer being
>>>>>>>>>>>>>>>>>>>> actively developed.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Our process:
>>>>>>>>>>>>>>>>>>>> 1. We "randomly" selected a function that is part of
>>>>>>>>>>>>>>>>>>>> the ACITree. From this point we decided to replace that function with our
>>>>>>>>>>>>>>>>>>>> own version. The function that we choose was "itemData".
>>>>>>>>>>>>>>>>>>>> Given in order to replace the tree we would need to
>>>>>>>>>>>>>>>>>>>> have a function that would give us the same information. We had 2 options:
>>>>>>>>>>>>>>>>>>>> 11. Find the next function and execute from step 4
>>>>>>>>>>>>>>>>>>>> until all the functions are replaced, refactored and tested.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>  - Tweak the naming of the Tree class to explicitly
>>>>>>>>>>>>>>>>>>>> tell us this is to use only with ACITree.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page

---

From: Dave Page <[email-masked]>
Date: Wed, 2 May 2018 09:08:00 +0100

On Wed, May 2, 2018 at 6:30 AM, Ashesh Vashi <[email-masked]>
wrote:

>> You've lost us with the last reply. We'd love to know what we'd have to
>> do to get this patch committed. You mention that "some things are missing
>> at some places" and that "there are missing bits".
>>

So if I understand you correctly, your concerns are:

- The changes should cover specific units of code at once.

- There should be high-level comments to help guide new or less experienced
developers.

The first I agree with - and I think (if I understand correctly, so do Joao
and Anthony). That's why they've concentrated on itemData first.

The second I also agree with. It's one thing being able to follow a simple
function without comments, but that doesn't give you an easy to understand
high level view of how it all ties together without non-trivial amounts of
work. I'll be the first to admit that we're not the best at that either,
but we do try, and I'd like us to get better.

>>>>>> In our point of view these 2 changes are not related. One thing is
>>>>>> the internal code organization of the application, other thing is allowing
>>> And, there is still a lot missing bits, and pieces to consider for
>>> commit in the repo.
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>>
>>>>>
>>>>> -- Thanks, Ashesh
>>>>>
>>>>>>
>>>>>> Anthony && Joao
>>>>>>
>>>>>>>>> We are investing a big chunk of our time in doing this
>>>>>>>>> refactoring, but while doing that we also try to respond to the patches
>>>>>>>>> sent to the mailing list. We would like the feedback from the community
>>>>>>>>> because we believe this is a changing point for the application. The idea
>>>>>>>>> is to change the way we develop this application, instead of only
>>>>>>>>> correcting a bug of developing a feature, with every commit we should
>>>>>>>>> correct the bug or develop a feature but leave the code a little better
>>>>>>>>> than we found it (Refactoring, refactoring, refactoring). This is hard work
>>>>>>>>> but that is what the users from pgAdmin expect from this community of
>>>>>>>>> developers.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ======
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It is a huge patch
>>>>>>>>> and we would like to get your feedback as soon as possible,
>>>>>>>>> because we are continuing to work on it which means it is going to grow in
>>>>>>>>> size.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>   New Tree abstraction. This patch contains the new Tree that
>>>>>>>>> works as an adaptor for ACI Tree and is going to be used on all the
>>>>>>>>> extractions that we are doing.
>>>>>>>>>
>>>>>>>>
>>>>>>>>>>>>>>>>> As a personal note, unless the algorithm is very obscure
>>>>>>>>>>>>>>>>> or very complicated, I believe that if the code needs comments it is a
>>>>>>>>>>>>>>>>> signal that something needs to change in terms of naming, structure of the
>>>>>>>>>>>>>>>>> part in question. This being said, I am open to add some comments that
>>>>>>>>>>>>>>>>> might help people.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I also agree with Dave, Can we please add some comments
>>>>>>>>>>>>>>>>>> in each file which can help us to understand the flow, I'm saying because
>>>>>>>>>>>>>>>>>> now the code is segregated in so many separate files it will be hard to
>>>>>>>>>>>>>>>>>> keep track of the flow from one file to another when debugging.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>>> Murtuza Zabuawala
>>>>>>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida
>>>>>>>>>>>>>>>>>>>> Pereira <[email-masked]> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> com/bliki/TestDouble.html) that can be used to
>>>>>>>>>>>>>>>>>>>>> inplace to replace the ACITree and also encapsulate the new tree behavior
>>>>>>>>>>>>>>>>>>>>> on our tests
>>>>>>>>>>>>>>>>>>>>> This patch represents only 10 calls to
>>>>>>>>>>>>>>>>>>>>> ACITree.itemData out of 176 that are spread around our code
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>   b) Create a tree that would return a node given an
>>>>>>>>>>>>>>>>>>>>> ID and then the node would be responsible for giving it's data.
>>>>>>>>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>>>>>>>>>  - OOP Approach

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Wed, 02 May 2018 14:54:16 +0000

HI Hackers,

The next image depicts what we have been talking in the previous email
"TODO List". On the left side you can see the master branch utilizations of
the function itemData, and on the left side what master will look like when
you merge this patch.

As you can see this is a huge job that should be committed in smaller chunk
as soon as they are completed or else we will lose all traceability of the
changes and it will become harder and harder to merge and review any patch.
We can split this patch into small patchs, one for each unit(Unit can be
considered an application feature like handling tables or server
information). This will make the patches smaller and easier to
review/merge.

> one functionality at a time.

 You are talking about the change in the locations of the file + removal of
the refactor the Tree integration, correct? If so you are right, but we
have to consider the following in order to refactor the Tree we need to
extract the functions from the place where they are so that they can be
wrapped with tests, only after that the Refactor should happen. By doing
this we will have to extract them to separate file(s). This leaves us with
2 options, one is to put these files next to the files we extract the
functions from the other is to try to create more intuitive structure for
the code. We hoped to go with the second options, because we feel that this
will make the application easier to understand and to develop on.

What is preventing us from merging this patch?
Do you believe we didn't explain the intent of this patch well?

Any information that you can provide to us that would help us understand
what is required to merge this patch would be welcomed. Because after one
month of back and forth with this patch it is time to give it some closure.

Thanks
Joao & Victoria

On Wed, May 2, 2018 at 4:08 AM Dave Page <[email-masked]> wrote:

>
>
> On Wed, May 2, 2018 at 6:30 AM, Ashesh Vashi <
>> As per my understading comments are required for better understanding,
>> and makes the code more maintainable.
>>
>> Let's avoid mix match multiple things in a single patch. And,
>> concerntrate one functionality at a time.
>>
>
> - There should be high-level comments to help guide new or less
> experienced developers.
>
> The first I agree with - and I think (if I understand correctly, so do
> Joao and Anthony). That's why they've concentrated on itemData first.
>
>>>>>>>>>> This new version contains all the changes in the previous one
>>>>>>>>>> plus more extractions of functions and refactoring of code.
>>>>>>>>>>
>>>>>>>>> Committed the patch along with the regression introduced because
>>>>>>>>> of this patch.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 0002:
>>>>>>>>> I am not against dividing the big javascript files in small
>>>>>>>>> chunks, but - I would like us to discuss first about the hot plugins design
>>>>>>>>> first.
>>>>>>>>>
>>>>>>>>> -- Thanks, Ashesh
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Joao
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>>>>>>> Considering this, we should separate the code in a way
>>>>>>>>>>>>>>>>> that some of the common functionalities can be used for other modules  like
>>>>>>>>>>>>>>>>> menu (as you have mentioned above), dialogue factory etc.
>>>>>>>>>>>>>>>> About the comment point I need a more clear understanding
>>>>>>>>>>>>>>>> on what kind of comments you are looking for. Because when you read the
>>>>>>>>>>>>>>>>>>> On Thu, Apr 5, 2018 at 7:08 PM, Joao De Almeida Pereira
>>>>>>>>>>>>>>>>>>>>>> This patch is intended to be merged after 3.0,
>>>>>>>>>>>>>>>>>>>>>> because we do not want to cause any entropy or delay the release, but we
>>>>>>>>>>>>>>>>>>>>>> want to start the discussion and show some code.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> 6. Refactor the function to ES6, give more
>>>>>>>>>>>>>>>>>>>>>> declarative names to variables and break the functions into smaller chunks
>>>>>>>>>>>>>>>>>>>>>>

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Fri, 04 May 2018 22:01:14 +0000

Hi Hackers,

*Be prepared this is going to be a big email.*
Objectives of this patch:

   1. Start the work to split ACI from pgAdmin code
   2. Add more tests around the front end components
   3. Start the discussion on application architecture

1. Start the work to split ACI from pgAdmin code Why

This journey started on our first attempt to store state of the current
tree, so that when a user accessed again pgAdmin
there would be no need to reopen all the nodes and make the flow better.
Another problem users brought to us was related
to the UI behavior when the number of objects was too big.

In order to do implement this features we would have to play around with
aciTree or some functions from our code that
called and leveraged the aciTree functionalities. To do this we needed to
have some confidence that our changes would
implement correctly the new feature and it would not break the current
functionality.
The process that we used was first extract the functions that interacted
with the tree, wrap them with
tests and then refactor them in some way. While doing this work we realized
that the tree started spreading roots
throughout the majority of the application.
How

Options:

   1. Rewrite the front end
   2. One bang replace ACI Tree without change on the application
   3. Create an abstraction layer between ACI Tree and the application

1. Rewrite the front end
Pros Const
Achieve decoupling by recreating the frontend Cost is too high
Do not rely on deprecated or unmaintained libraries Turn around is too long
One shot change 2. One bang replace ACI Tree
Pros Const
Achieve decoupling by recreating the frontend Cost is too high
Does not achieve the decoupling unless we change the application
Need to create an adaptor
No garantee of success
One shot change 3. Create an abstraction layer between ACI Tree and the
application
Pros Const
Achieve decoupling of the application and the tree 90% of the time on Code
archaeology
Lower cost
Ability to change or keep ACI Tree, decision TBD
Can be done in a iterative way

We decided that options 3 looked more attractive specially because we could
do it in a iterative way.

The next image depicts the current state of the interactions between the
application and the ACI and the place we want
to be in.

for retrieving of data from the ACI Tree that then is used in different
ways.

Because we are doing this in a iterative process we need to keep the
current tree working and create our adaptor next
to it. In order to do that we created a new property on PGBrowser class
called treeMenu and when ACI Tree receives
the node information we also populate the new Tree with all the needed
information.

This approach allow us not to worry, for now, with data retrieval, URL
system and other issues that should be addressed
in the future when the adaptor is done.

This first patch tries to deal with the low hanging fruit, functions that
are triggered by events from ACI Tree.
Cases like registering to events and triggering events need to be handled
in the future as well.
2. Add more tests around the front end components Why

Go Fast Forever: https://builttoadapt.io/why-tdd-489fdcdda05e
I think this is a very good summary of why do we need tests in our
applications.
3. Start the discussion on application architecture

Why should we care about location of files inside a our application?

Why is this way better the another?

These are 2 good questions that have very lengthy answers. Trying to more
or less summarize the answers we care about
the location of the files, because we want our application to communicate
intent and there are always pros and cons
on all the decisions that we make.

At this point the application structure follows our menu, this approach
eventually make is easier to follow the code
but at the same time if the menu changes down the line, will we change the
structure of our folders?

The proposal that we do with the last diff of this patch is to change to a
structure that slices vertically the
application. This way we can understand intent behind the code and more
easily find what we are looking for.

In the current structure if you want to see the tables code you need to go
to
pgAdmin/browser/server_groups/servers/databases/schemas/tables/ this is a
huge path to remember and to get to. What
do we win with this? If we open pgAdmin we know which nodes to click in
order to get to tables. But for development
every time that you are looking for a specific functionality you need to
run the application, navigate the menu so that
you know where you can find the code. This doesn’t sound very appealing.

What if our structure would look like this:

 - web
   - tables
     - controller
       - get_nodes.py
       - get_sql.py
       - __init__.py
     - frontend
        - component
            - ddl_component.js
        - services
            - table-service.js
   - schemas
   - servers
   - ....

This would saves us time because all the information that we need is what
are we working on and everything is right there.
Menu driven structure Intent Driven Structure
*Pros:* *Pros:*
Already in place Explicitly shows features
Self contained features Self contained features
Support for drop in features Support for drop in features
*Cons:* *Cons:*
Follows the menu, and it could change Need to change current code
Hard to find features Some additional plumbing might be needed
Drop in features need to be placed in a specific location according to the
menu location

What are your thought about this architecture?

Around minute 7 of this video <https://www.youtube.com/watch?v=hALFGQNeEnU>
Uncle Bob shows an application written
in rails to talk about architecture. It is a long KeyNote if you are
curious I would advise you to see the full video.
His approach to architecture of the application is pretty interesting.
------------------------------
Patches 0001 Change the order of the shims on the shim file

Simple change that orders the shims inside the shim file
0002 New tree implementation

This is the first version of our Tree implementation. At this point is a
very simple tree without no abstractions and
with code that eventually is not very performant, but this is only the
first iteration and we are trying to follow the
Last Responsible Moment Principle
<https://medium.com/@aidanjcasey/guiding-principles-for-an-evolutionary-software-architecture-b6dc2cb24680>
.

What can you find in this patch:

   - Creation of PGBrowser.treeMenu
   - Initial version of the Tree Adaptor web/pgadmin/static/js/tree/tree.js
   - TreeFake test double <https://martinfowler.com/bliki/TestDouble.html>
   that can replace the Tree for testing purposes
   - Tests. As an interesting asside because Fake’s need to behave like the
   real object you will noticed that there are
   tests for this type of double and they the same as of the real object.

0003 Extract, Test and Refactor Methods

This is the patch that contains the change that we talked a in the
objectives. It touches in a subset of the places
where itemData function is called. To have a sense of progress the
following image depicts, on the left all places
where this functions can be found in the code and on the right the places
where it still remains after this patch.

But this patch is not only related to the ACI Tree, it also creates some
abstractions from code that we found repeated.
Some examples of this are the dialogs for Backup and Restore, where the
majority of the logic was common, so we created
in web/pgadmin/static/js/alertify/ 3 different objects:
A Factory DialogFactory that is responsible for creating new Dialog and
these dialogs will use the DialogWrapper.

This is also a good example of the Last Responsible Moment Principle
<https://medium.com/@aidanjcasey/guiding-principles-for-an-evolutionary-software-architecture-b6dc2cb24680>
,
in Dialog there are still some functions that are used in a Recover and
Backup scenario. When we have more examples
we will be able to, if we want, to extract that into another function.

While doing some code refactoring we found out that the Right Click Menu as
some checks that can be centralized. Examples
of these are checks for Enable/Disable of an option of the display of the
Create/Edit/Drop option in some menus.
Check web/pgadmin/static/js/menu/ for more information. This simplified
code like:

      canCreate: function(itemData, item, data) {
-        //If check is false then , we will allow create menu
-        if (data && data.check == false)
-          return true;
-
-        var t = pgBrowser.tree, i = item, d = itemData;
-        // To iterate over tree to check parent node
-        while (i) {
-          // If it is schema then allow user to create domain
-          if (_.indexOf(['schema'], d._type) > -1)
-            return true;
-
-          if ('coll-domain' == d._type) {
-            //Check if we are not child of catalog
-            var prev_i = t.hasParent(i) ? t.parent(i) : null,
-              prev_d = prev_i ? t.itemData(prev_i) : null;
-            if( prev_d._type == 'catalog') {
-              return false;
-            } else {
-              return true;
-            }
-          }
-          i = t.hasParent(i) ? t.parent(i) : null;
-          d = i ? t.itemData(i) : null;
-        }
-        // by default we do not want to allow create menu
-        return true;
+        return canCreate.canCreate(pgBrowser, 'coll-domain', item, data);
      }

This refactor was made in a couple of places throughout the code.

Another refactor that had to be done was the creation of the functions
getTreeNodeHierarchyFromElement and
getTreeNodeHierarchyFromIdentifier that replace the old getTreeNodeHierarchy.
This implementation was done
because for the time being we have 2 different types of trees and calls to
them.

The rest of the changes resulted from our process of refactoring that we
are following:

   1. Find a location where itemData is used
   2. Extract that into a function outside of the current code
   3. Wrap it with tests
   4. When we have enough confidence on the tests, we refactor the function
   to use the new Tree
   5. We refactor the function to become more readable
   6. Start over

0004 Architecture

The proposed structure is our vision for the application, this is not the
state we will be in when this patch lands.
For now the only change we are doing is move the javascript into
static/js/FEATURE_NAME. This is a first step in the
direction that we would like to see the product heading.

Even if we decide to keep using the same architecture the move of
Javascript to a centralized place will
not break any plugability as webpack can retrieve all the Javascript from
any place in the code.

Have a nice weekend
Joao
​

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Fri, 04 May 2018 22:03:08 +0000

And now the patches......

On Fri, May 4, 2018 at 6:01 PM Joao De Almeida Pereira <
[email-masked]> wrote:

> tests and then refactor them in some way. While doing this work we
> realized that the tree started spreading roots
> Achieve decoupling of the application and the tree 90% of the time on
> Code archaeology
> Lower cost
> We decided that options 3 looked more attractive specially because we
> could do it in a iterative way.
>
> Around minute 7 of this video
> <https://www.youtube.com/watch?v=hALFGQNeEnU> Uncle Bob shows an
> application written
>    - Tests. As an interesting asside because Fake’s need to behave like
>    the real object you will noticed that there are
> While doing some code refactoring we found out that the Right Click Menu
> as some checks that can be centralized. Examples
> getTreeNodeHierarchyFromIdentifier that replace the old
> getTreeNodeHierarchy. This implementation was done
>    4. When we have enough confidence on the tests, we refactor the
>    function to use the new Tree

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Mon, 07 May 2018 19:01:08 +0000

Hi Hackers,
We noticed that some of the latest changes created some conflicts with this
patch. So you can find attached the patch rebased on the new master.

Thanks
Victoria && Joao

On Fri, May 4, 2018 at 6:03 PM Joao De Almeida Pereira <
[email-masked]> wrote:

>> but at the same time if the menu changes down the line, will we change
>> the structure of our folders?
>>
>> The proposal that we do with the last diff of this patch is to change to
>> a structure that slices vertically the
>> In the current structure if you want to see the tables code you need to
>> go to
>> pgAdmin/browser/server_groups/servers/databases/schemas/tables/ this is
>> a huge path to remember and to get to. What
>> Drop in features need to be placed in a specific location according to
>> the menu location
>>
>>    - TreeFake test double
>>    <https://martinfowler.com/bliki/TestDouble.html> that can replace the
>>    Tree for testing purposes
>> because for the time being we have 2 different types of trees and calls
>> to them.
>>

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 9 May 2018 16:32:56 +0530

Hi Joao,

Patch 0001 & 0002 (esp the new tree implementation) look good to me.

Patch 0003 - I have some concerns regarding the approach taken to separate
out the PG utilities. The dialogue factory is responsible for creating the
dialogue which uses dialogue wrapper. So, any module should call the
dialogue factory to create a new dialogue instance. Here dialogue itself
call the dialogue factory for creating it.

In the enable_disable_triggers.js file, can we just import alertifyjs
instead of passing in all the functions?

Patch 0004 - Right now with the new proposed architecture only JS files are
re-arranged. To maintain the pluggability I would also like to have some
discussion on the back end side. Right now, the code maintains everything.
Let say I want to add any node under schema I will just drop the related
folder and files in that hierarchy and it will work. But with the proposed
architecture I can not visualize the backend architecture and this should
not be ended with only JS restructuring.

Thanks,
Khushboo

On Tue, May 8, 2018 at 12:31 AM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Hi Hackers,
> We noticed that some of the latest changes created some conflicts with
> this patch. So you can find attached the patch rebased on the new master.
>
> Thanks
> Victoria && Joao
>
>>> called and leveraged the aciTree functionalities. To do this we needed
>>> to have some confidence that our changes would
>>> This first patch tries to deal with the low hanging fruit, functions
>>> that are triggered by events from ACI Tree.
>>> Cases like registering to events and triggering events need to be
>>> handled in the future as well.
>>> These are 2 good questions that have very lengthy answers. Trying to
>>> more or less summarize the answers we care about
>>> the location of the files, because we want our application to
>>> communicate intent and there are always pros and cons
>>> This would saves us time because all the information that we need is
>>> what are we working on and everything is right there.
>>>    <https://martinfowler.com/bliki/TestDouble.html> that can replace
>>>    the Tree for testing purposes
>>> where this functions can be found in the code and on the right the
>>> places where it still remains after this patch.
>>>
>>> of these are checks for Enable/Disable of an option of the display of
>>> the Create/Edit/Drop option in some menus.
>>> The proposed structure is our vision for the application, this is not
>>> the state we will be in when this patch lands.
>>> not break any plugability as webpack can retrieve all the Javascript
>>> from any place in the code.
>>>
>>> Have a nice weekend
>>> Joao
>>> ​
>>>
>>>

---

From: Akshay Joshi <[email-masked]>
Date: Wed, 9 May 2018 18:08:53 +0530

Hi Joao

Below are my review comments:

   - In *menu/can_create.js,* "parentCatalogOfTableChild" function name is
   unable to understand.
   - Below code is bit confusing in terms of naming convention canCreate.
   canCreate inside "canCreate".

  return canCreate.canCreate(pgBrowser, 'coll-collation', item, data);

}

Can we have something like "*menuCreator.canCreate*"?

   - Why we have "*menu_utils.js*" for Backup, Maintenance, Restore and
   Grant Wizard? Can we have one single file with all the supported node for
   respective module? Also I have seen that the folder hierarchy for grant
   wizard is "/*web/pgadmin/static/js/grant/wizard/menu_utils.js*" why
   there are two separate folders grant and wizard (sub folder of grant)?

On Wed, May 9, 2018 at 4:32 PM, Khushboo Vashi <
[email-masked]> wrote:

> Patch 0004 - Right now with the new proposed architecture only JS files
> are re-arranged. To maintain the pluggability I would also like to have
> some discussion on the back end side. Right now, the code maintains
> everything. Let say I want to add any node under schema I will just drop
> the related folder and files in that hierarchy and it will work. But with
> the proposed architecture I can not visualize the backend architecture and
> this should not be ended with only JS restructuring.
>
> Thanks,
> Khushboo
>
>>>> there would be no need to reopen all the nodes and make the flow
>>>> better. Another problem users brought to us was related
>>>> In order to do implement this features we would have to play around
>>>> with aciTree or some functions from our code that
>>>> The process that we used was first extract the functions that
>>>> interacted with the tree, wrap them with
>>>> One shot change 3. Create an abstraction layer between ACI Tree and
>>>> the application
>>>> Pros Const
>>>> The next image depicts the current state of the interactions between
>>>> the application and the ACI and the place we want
>>>> to be in.
>>>>
>>>>
>>>> to it. In order to do that we created a new property on PGBrowser
>>>> class called treeMenu and when ACI Tree receives
>>>> The proposal that we do with the last diff of this patch is to change
>>>> to a structure that slices vertically the
>>>> pgAdmin/browser/server_groups/servers/databases/schemas/tables/ this
>>>> is a huge path to remember and to get to. What
>>>> every time that you are looking for a specific functionality you need
>>>> to run the application, navigate the menu so that
>>>> This is the first version of our Tree implementation. At this point is
>>>> a very simple tree without no abstractions and
>>>>    - Tests. As an interesting asside because Fake’s need to behave
>>>>    like the real object you will noticed that there are
>>>> But this patch is not only related to the ACI Tree, it also creates
>>>> some abstractions from code that we found repeated.
>>>> A Factory DialogFactory that is responsible for creating new Dialog
>>>> and these dialogs will use the DialogWrapper.
>>>>
>>>> in Dialog there are still some functions that are used in a Recover
>>>> and Backup scenario. When we have more examples
>>>> While doing some code refactoring we found out that the Right Click
>>>> Menu as some checks that can be centralized. Examples
>>>> Check web/pgadmin/static/js/menu/ for more information. This
>>>> simplified code like:
>>>>
>>>> The rest of the changes resulted from our process of refactoring that
>>>> we are following:
>>>>

-- 
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Wed, 09 May 2018 15:00:01 +0000

Hi Hackers

There were very good points raised. Do you think that these are blockers
for 0001, 0002, and 0003 getting committed? In general, we felt like there
were points made about very specific issues, were there any broader
concerns about the patches? We don't need an agreement on architecture yet
for patches 0001, 0002, and 0003.

@Khushboo Vashi <[email-masked]>

> call the dialogue factory for creating it.
We did not fully understand what your concern is on this point.
*If you are concerned that we have a dialogWrapper and a dialog class and
don't understand the difference between the two?*
We don't feel too strongly about this decision. It seemed easier at the
time (to test) because the wrapper represents what will be passed into the
alertify library. We're certainly open to having this implementation change
in the future.

> instead of passing in all the functions?

 We made this decision to make testing of the functionality easier to test,
by using a principle called dependency injection.
https://martinfowler.com/articles/injection.html. If we import alertifyjs
directly, then we can't stub out its functionality.

> not be ended with only JS restructuring.

This patch in fact only moves Javascript files from one folder structure to
a different folder structure. The process we are following is an iterative
process and will be built upon itself. The end vision that we have for the
product is as depicted in the previous email an Intent Driven Structure and
in this structure each folder will contain a Feature that will be self
contained (Backend + Frontend). The major difference between what we have
now and what we envision this application to look like is that these
folders can live at the same level so that we know at a glance what this
application is capable of doing. This structure will also make it easier to
onboard someone that is looking to help building any feature because they
will have access to all of the features  in the "same screen" instead of
having to do a deep dive into the application code in order to find the
place to start.

As of right now we haven't though very deep on the backend side of things.
But looking at the current structure moving from a hierarchical file
structure into a horizontal one, doesn't look like a lot of plumbing.
Nevertheless this is something that we should address when we get there and
we believe we are not there yet.

 @Akshay Joshi <[email-masked]>

> In *menu/can_create.js,* "parentCatalogOfTableChild" function name is
> unable to understand.

The intent behind this function is to retrieve all the tree child node that
matches a given type argument and has a direct parent of the catalog type.
The name was in a hurry and could certainly be improved. Any suggestions
are welcome. How about **matchesTypeAndHasCatalogParent(self, arg)**?

Below code is bit confusing in terms of naming convention
> canCreate.canCreate inside "canCreate".

That sounds great. Let's do that.

Why we have "menu_utils.js" for Backup, Maintenance, Restore and Grant
> Wizard? Can we have one single file with all the supported node for
> respective module?

It was for expressing developer intent. The menu_utils.js that were placed
in discrete packages have functionality that pertain only to its package.
If we thought that the functionality were shared, we wouldn't have put them
in the shared folder `pgadmin4/web/pgadmin/static/js/menu/...`

> Also I have seen that the folder hierarchy for grant wizard is
> "/web/pgadmin/static/js/grant/wizard/menu_utils.js" why there are two
> separate folders grant and wizard (sub folder of grant)?

It makes sense to think of the wizard as separate entity from grants. You
could imagine that when we add additional functionality for grants, that
we'd want it alongside the wizard functionality. You could call it
premature at this point.

@pgadmin-hackers <[email-masked]>
We understand that our vision for the application is quite bold and if we,
as a community, decide that this is the way to go it is going to take some
time to accomplish and will involve effort. But  we believe this will take
the code into a better place.
Patches 1,2 and 3 are our priority at this point, because there are
features that we want to implement that depend on the separation from ACI
Tree, and we would like to see these in master as soon as possible.

Joao and Anthony

On Wed, May 9, 2018 at 8:38 AM Akshay Joshi <[email-masked]>
wrote:

>    - In *menu/can_create.js,* "parentCatalogOfTableChild" function name
>    is unable to understand.
>> Patch 0003 - I have some concerns regarding the approach taken to
>> separate out the PG utilities. The dialogue factory is responsible for
>> creating the dialogue which uses dialogue wrapper. So, any module should
>> call the dialogue factory to create a new dialogue instance. Here dialogue
>> itself call the dialogue factory for creating it.
>>
>>>>> This journey started on our first attempt to store state of the
>>>>> current tree, so that when a user accessed again pgAdmin
>>>>> for retrieving of data from the ACI Tree that then is used in
>>>>> different ways.
>>>>>
>>>>> At this point the application structure follows our menu, this
>>>>> approach eventually make is easier to follow the code
>>>>> application. This way we can understand intent behind the code and
>>>>> more easily find what we are looking for.
>>>>>
>>>>> In the current structure if you want to see the tables code you need
>>>>> to go to
>>>>> do we win with this? If we open pgAdmin we know which nodes to click
>>>>> in order to get to tables. But for development
>>>>> Some examples of this are the dialogs for Backup and Restore, where
>>>>> the majority of the logic was common, so we created
>>>>> because for the time being we have 2 different types of trees and
>>>>> calls to them.
>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Thu, 10 May 2018 12:22:29 +0530

Hi Joao,
On Wed, May 9, 2018 at 8:30 PM, Joao De Almeida Pereira <

> for patches 0001, 0002, and 0003.
I have already commited the patch - 0001.

I have two review comments for 0002 patch.
*In pgadmin/static/js/tree/tree.js:*
1. In TreeNode, we're keepging the reference of DOMElement, do we really
need it?
i.e.
*export class TreeNode {*
*  constructor(id, data, domNode, parent) {*
*    this.id <http://this.id> = id;*
*    this.data = data;*
*    this.setParent(parent);*
*    this.children = [];*
*    this.domNode = domNode;*
*  }*
*  ...*
*}*
*....*

2. We're exporting a tree object on line #189.
i.e.
*export let tree = new Tree();*

Are you expecting the tree class to be a singleton class?
I think not. Hence - it should not be exported.

I am currently reviewing 0003 patch.
Will share my comments soon.

We will talk about the architecture change in separate mail chain.

-- Thanks, Ashesh

>  We made this decision to make testing of the functionality easier to
> test, by using a principle called dependency injection.
> This patch in fact only moves Javascript files from one folder structure
> to a different folder structure. The process we are following is an
> iterative process and will be built upon itself. The end vision that we
> have for the product is as depicted in the previous email an Intent Driven
> Structure and in this structure each folder will contain a Feature that
> will be self contained (Backend + Frontend). The major difference between
> what we have now and what we envision this application to look like is that
> these folders can live at the same level so that we know at a glance what
> this application is capable of doing. This structure will also make it
> easier to onboard someone that is looking to help building any feature
> because they will have access to all of the features  in the "same screen"
> instead of having to do a deep dive into the application code in order to
> find the place to start.
>
> The intent behind this function is to retrieve all the tree child node
> that matches a given type argument and has a direct parent of the catalog
> type. The name was in a hurry and could certainly be improved. Any
> suggestions are welcome. How about **matchesTypeAndHasCatalogParent(self,
> arg)**?
>
>>>>>> called and leveraged the aciTree functionalities. To do this we
>>>>>> needed to have some confidence that our changes would
>>>>>> implement correctly the new feature and it would not break the
>>>>>> current functionality.
>>>>>> Achieve decoupling of the application and the tree 90% of the time
>>>>>> on Code archaeology
>>>>>> Lower cost
>>>>>> the node information we also populate the new Tree with all the
>>>>>> needed information.
>>>>>>
>>>>>> This approach allow us not to worry, for now, with data retrieval,
>>>>>> URL system and other issues that should be addressed
>>>>>> but at the same time if the menu changes down the line, will we
>>>>>> change the structure of our folders?
>>>>>>
>>>>>> Drop in features need to be placed in a specific location according
>>>>>> to the menu location
>>>>>>
>>>>>> This is the first version of our Tree implementation. At this point
>>>>>> is a very simple tree without no abstractions and
>>>>>> with code that eventually is not very performant, but this is only
>>>>>> the first iteration and we are trying to follow the
>>>>>> Another refactor that had to be done was the creation of the
>>>>>> functions getTreeNodeHierarchyFromElement and

---

From: Anthony Emengo <[email-masked]>
Date: Thu, 10 May 2018 14:38:31 +0000

> 1. In TreeNode, we're keepging the reference of DOMElement, do we really

As of right now, our Tree abstraction acts as an adapter to the aciTree
library. The aciTree library needs the domElement for most of its functions
(setInode, unload, etc). Thus this is the easiest way to introduce our
abstraction and keep the functionality as before - at least until we decide
that whether we want to switch out the library or not.

2. Are you expecting the tree class to be a singleton class

Since this tree is referenced throughout the codebase, considering it to be
a singleton seems like the most appropriate pattern for this usecase. It is
very much the same way how we create a single instance of the aciTree
library and use that throughout the codebase. Moreover, it opens up
opportunities to improve performance, for example caching lockups of nodes.
I’m not a fan of singletons myself, but I feel like we’re simply keeping
the architecture the same in the instance.
​

Sincerely,

Anthony and Victoria

---

From: Ashesh Vashi <[email-masked]>
Date: Fri, 11 May 2018 12:01:53 +0530

> that whether we want to switch out the library or not.
I understand that. But - I've not seen any reference of domElement the code
yet, hence - pointed that out.

> Since this tree is referenced throughout the codebase, considering it to
> be a singleton seems like the most appropriate pattern for this usecase. It
> is very much the same way how we create a single instance of the aciTree
Yeah - I don't see any usage of tree object from anywhere.
And, we're already creating new object in browser.js (and, not utitlizing
that instance anywhere.)

-- Thanks, Ashesh

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Fri, 11 May 2018 17:27:25 -0400

Hello Ashesh,

> I understand that. But - I've not seen any reference of domElement the
> code yet, hence - pointed that out.

If you look at the function: reload, unload you will see that domNode is
used to communicate with the ACITree
​

> And, we're already creating new object in browser.js (and, not utitlizing

You are right, we do not need to export tree as a singleton for now. The
line that exports the variable tree can be remove when applying the patch
number 2.
​

I think we addressed all the concern raised about this patch. Does this
mean that the patch is going to get committed?

We are looking forward to continue the development on this track of work,
but as you know, we are blocked until it gets committed into master. We
hope that this gets unblocked soon because this is a very big track of work
and the longer it is blocked, the longer it will take to get finalized.

Thanks
Joao

On Fri, May 11, 2018 at 2:32 AM Ashesh Vashi <[email-masked]>
wrote:

> I understand that. But - I've not seen any reference of domElement the
> code yet, hence - pointed that out.
>

---

From: Ashesh Vashi <[email-masked]>
Date: Sat, 12 May 2018 04:40:49 +0530

> mean that the patch is going to get committed?
Yes - from me for 0002.

-- Thanks, Ashesh

> On Fri, May 11, 2018 at 2:32 AM Ashesh Vashi <

---

From: Dave Page <[email-masked]>
Date: Mon, 14 May 2018 10:29:56 +0100

On Sat, May 12, 2018 at 12:10 AM, Ashesh Vashi <
[email-masked]> wrote:

>>> And, we're already creating new object in browser.js (and, not
>>> utitlizing that instance anywhere.)
>>
>>
>> line that exports the variable tree can be remove when applying the
>> patch number 2.
>> ​
>>

Can you do that today?

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 14 May 2018 18:08:46 +0530

On Mon, May 14, 2018 at 2:59 PM, Dave Page <[email-masked]> wrote:

>>>> As of right now, our Tree abstraction acts as an adapter to the
>>>>> aciTree library. The aciTree library needs the domElement for most of its
>>>>> functions (setInode, unload, etc). Thus this is the easiest way to
>>>>> introduce our abstraction and keep the functionality as before - at least
>>>>> until we decide that whether we want to switch out the library or not.
>>>>
>>> If you look at the function: reload, unload you will see that domNode
>>> is used to communicate with the ACITree
>>> ​
>>>
>>>> Since this tree is referenced throughout the codebase, considering it
>>>>> to be a singleton seems like the most appropriate pattern for this usecase.
>>>>> It is very much the same way how we create a single instance of the aciTree
Done.

-- Thanks, Ashesh

---

From: Dave Page <[email-masked]>
Date: Mon, 14 May 2018 13:40:16 +0100

On Mon, May 14, 2018 at 1:38 PM, Ashesh Vashi <[email-masked]

>>>> 1. In TreeNode, we're keepging the reference of DOMElement, do we
>>>>>> really need it?
>>>>>
>>>> You are right, we do not need to export tree as a singleton for now.
>>>> The line that exports the variable tree can be remove when applying
>>>> the patch number 2.
>>>> ​
>>>>

Great, thanks!

On to patch 0003 then :-)

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 14 May 2018 18:11:44 +0530

On Mon, May 14, 2018 at 6:10 PM, Dave Page <[email-masked]> wrote:

>
>
> On Mon, May 14, 2018 at 1:38 PM, Ashesh Vashi <
>>>>>> I understand that. But - I've not seen any reference of domElement
>>>>>> the code yet, hence - pointed that out.
>>>>>
>>>>> I think we addressed all the concern raised about this patch. Does
>>>>> this mean that the patch is going to get committed?
>>>>>
>>>> Yes - from me for 0002.
>>>>
>>>
>>> Can you do that today?
>>>
>> Done.
>>
>
> Great, thanks!
>
Yes - already working on it! :-)

-- Thanks, Ashesh

---

From: Ashesh Vashi <[email-masked]>
Date: Tue, 15 May 2018 16:07:08 +0530

Hi Joao,
On Mon, May 14, 2018 at 6:11 PM, Ashesh Vashi <[email-masked]

>>>>>>> Since this tree is referenced throughout the codebase, considering
>>>>>>>> it to be a singleton seems like the most appropriate pattern for this
>>>>>>>> usecase. It is very much the same way how we create a single instance of
>>>>>>>> the aciTree library and use that throughout the codebase. Moreover, it
>>>>>>>> opens up opportunities to improve performance, for example caching lockups
>>>>>>>> of nodes. I’m not a fan of singletons myself, but I feel like we’re simply
>>>>>>>> keeping the architecture the same in the instance.
>>>>>>>
Majority part of the 0003 patch looks good to me.
Except choice of the path of some of the file, and name of the functions.

Please find the updated patch.
I've moved files under the 'pgadmin/static/js/menu' directory under the
'pgadmin/static/js/tree', as they're using tree functionalities directly.

Please review it, and let me know your concern.

-- Thanks, Ashesh

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Tue, 15 May 2018 11:03:56 -0400

Hello Ashesh,

These are our comments to the patch:

   -

   Can you explain the reasoning behind the change you did on the canCreate
   function?
   Bind creates a new instance of a function, do we really need that?
   -

   isValidTreeNodeData
   1. If it is not Null or Undefined it really means that the data is
      Valid? Shouldn’t it be isDataDefined?
      2. This looks like a generic function that could be used with objects
      of any type not specific to TreeNodeData. So the file location
doesn’t look
      correct.
   -

   The tree folder is just a Tree that we use to store information. The
   menu uses a Tree but the 2 things should be separated.
   In our point of view the current entanglement of the ACITree into our
   code came from missing concepts into a single place (Menu + Storage of
   information).
   The idea behind having the Tree as a separate block was to ensure that
   we do not have the Menu and Tree coupling.
   -

   supportedNodesMenu.enabled what it does it check if a Node Menu should
   be enabled or not. The name of it maybe should be nodeMenu.enabled?

​

Thanks
Victoria & Joao

On Tue, May 15, 2018 at 6:37 AM Ashesh Vashi <[email-masked]>
wrote:

> Hi Joao,
> On Mon, May 14, 2018 at 6:11 PM, Ashesh Vashi <
>>>>>>> If you look at the function: reload, unload you will see that
>>>>>>> domNode is used to communicate with the ACITree
>>>>>>> ​
>>>>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Wed, 16 May 2018 12:50:10 +0530

Hi Joao,

On Tue, May 15, 2018 at 8:33 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

>    Can you explain the reasoning behind the change you did on the
>    canCreate function?
>
> I do agree as Akshay mentioned ealier, canCreate.canCreate does not look
right to me too, and neither directory structure, nor name of the script
does not make sense to me.
As those functions are always going to apply on the tree nodes only, moved
them under the 'sources/tree' directory, and renamed it as 'node_menu'.

> If you look at the original proposed patch, the implementation looks like
this.

*canCreate: function(itemData, item, data) {*
*       return canCreate.canCreate(pgBrowser, <collection_node_type>, item,
data);*
*}*

Here - we're already creating an anonymous function, which is an object
under the hood.
And - for every tree node, they will be a separate anonymous function
anyway.

Why not use better object oriented approach?

>       2. This looks like a generic function that could be used with
>       objects of any type not specific to TreeNodeData. So the file location
>       doesn’t look correct.
>
> I renamed it as isValidTreeNodeData, because - we were using it in for
testing the tree data.
I do agree - it is a generic function that could be used with objects of
any type.

But - when I moved the code, the function name was
'isProvidedDataValid(...)', which was present in
'sources/menu/menu_enabled' file, which was definitely not right place. :-)
Please suggest me the right place, and name.

>
>    - The tree folder is just a Tree that we use to store information. The
> I think - you're missing the point.

Here - we're dealing with two types of menus:
1. Contextual menu (which will always depend on the current selected tree
node)
2. Normal menus

> In my opinion - keeping them in them in different directories/files does
not make them decoupled to be honest.

The original patch was separating them in different places, but - still
uses some of the functionalities directly from the tree, which was
happening because we have contextual menu.
To give a better solution, I can think of putting the menus related code
understand 'sources/tree/menu' directory.
That will give clear distiguation between actual tree, and dependent code.

What do you say?

> nodeMenu.enabled(...) looks to general to me.
How about nodeMenu.isSupportedNode(...)?

If you look at the implementation of the function, it checks for the
current tree node is one of the supported nodes, or not.

-- Thanks, Ashesh

> ​
>
>
> Thanks
> Victoria & Joao
>
> On Tue, May 15, 2018 at 6:37 AM Ashesh Vashi <
>>>>>>>> You are right, we do not need to export tree as a singleton for
>>>>>>>> now. The line that exports the variable tree can be remove when
>>>>>>>> applying the patch number 2.
>>>>>>>> ​
>>>>>>>>

---

From: Anthony Emengo <[email-masked]>
Date: Wed, 16 May 2018 11:25:40 -0400

export function canCreate(pgBrowser, childOfCatalogType) {
  return canCreateObject.bind({
    browser: pgBrowser,
    childOfCatalogType: childOfCatalogType,
  });
}

With respect to the above code: this bind pattern looks good and seems like
the idiomatic way to handle this in JavaScript. On a lighter node, I don’t
even see the need for an additional method to wrap it. The invocation could
have easily been like canCreate: canCreateObject.bind({ browser: pgBrowser,
childOfCatalogType: childOfCatalogType }), I don’t feel too strongly here.

I renamed it as isValidTreeNodeData, because - we were using it in for
testing the tree data. Please suggest me the right place, and name.

We’re not sure; maybe after continued refactoring, we will come across more
generic functions. At that point we can revisit this and create a utils.js
file.

understand ‘sources/tree/menu’ directory.

We’re particularly worried because we’re trying to avoid the coupling that
we see in the code base today. We want to decouple *application state*
from *business
domain* logic as much as we can - because this makes the code much easier
to understand. We achieve lower coupling by have more suitable interfaces
to retrieve *application state* like: anyParent (the menu doesn’t care how
this happens). This is the direction that we’re trying to move towards, we
just don’t want the package structure to undermine that developer intent.

How about nodeMenu.isSupportedNode(…)?

Naming is one of the hardest problems in programming. I don’t feel too
strongly about this one. For now, let’s keep it as is

Thanks
Anthony && Victoria
​

---

From: Anthony Emengo <[email-masked]>
Date: Mon, 21 May 2018 09:46:02 -0400

Hey all,

We haven't heard from you all in a while regarding our last statements. Is
there any thing that I need to clarify? We feel left in the dark here and
just want to know what we can do help.

Cheers!
Anthony && Joao

---

From: Dave Page <[email-masked]>
Date: Mon, 21 May 2018 14:56:30 +0100

Ashesh, please prioritise this so we can move on.

Thanks.

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Wed, 23 May 2018 10:14:40 -0400

Hello Hackers,

Just to remind that if you need some more information from our side you can
ping us.
Can we expect and email tomorrow morning with the commit?

Thanks
Victoria & Joao

On Mon, May 21, 2018 at 9:56 AM Dave Page <[email-masked]> wrote:

>> We haven't heard from you all in a while regarding our last statements.
>> Is there any thing that I need to clarify? We feel left in the dark here
>> and just want to know what we can do help.
>>
>> Cheers!
>> Anthony && Joao
>>
>
>
>
> --
> Dave Page

---

From: Dave Page <[email-masked]>
Date: Wed, 23 May 2018 15:16:35 +0100

I spoke to Ashesh earlier - he's working on an update to the proposed patch
at the moment and will get back to you ASAP.

On Wed, May 23, 2018 at 3:14 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Hello Hackers,
>
> Just to remind that if you need some more information from our side you
> can ping us.

---

From: Ashesh Vashi <[email-masked]>
Date: Thu, 24 May 2018 12:29:16 +0530

Sorry for the late reply.
On Wed, May 16, 2018 at 8:55 PM, Anthony Emengo <[email-masked]> wrote:

> With respect to the above code: this bind pattern looks good and seems
> like the idiomatic way to handle this in JavaScript. On a lighter node, I
> don’t even see the need for an additional method to wrap it. The invocation
> could have easily been like canCreate: canCreateObject.bind({ browser:
> pgBrowser, childOfCatalogType: childOfCatalogType }), I don’t feel too
> strongly here.
>
I do agree - we can handle the same problem many ways.
I prefer object oriented pardigm more in general.
Any way - I have modified the code with some other changes.

> We’re not sure; maybe after continued refactoring, we will come across
> more generic functions. At that point we can revisit this and create a
> utils.js file.
>
Sure.

> from *business domain* logic as much as we can - because this makes the
> code much easier to understand. We achieve lower coupling by have more
> suitable interfaces to retrieve *application state* like: anyParent (the
> menu doesn’t care how this happens). This is the direction that we’re
> trying to move towards, we just don’t want the package structure to
> undermine that developer intent.
>
I realized after revisiting the code, menu/can_create.js was only
applicable to the children of the schema/catalog nodes, same as
'can_drop_child'.
We should have put both scripts in the same directory.

Please find the updated patch for the same.

Please review it, and let me know your concerns.

-- Thanks, Ashesh

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Thu, 24 May 2018 10:43:02 -0400

Hey, Thanks so much for the reply.

We've noticed that you've made several modifications on top of our original
patch. Unfortunately, we've found it very hard to follow. Could we please
get a brief synopsis of the changes you have made - just so we can better
understand the rationale behind them? Just like we've done for you
previously.

Let's keep in mind that the original intent was simply to introduce this
abstraction into the code base, which is a big enough task. I'd hate for
the scope of the changes we're making to expand beyond that.

Thanks
Joao && Anthony

On Thu, May 24, 2018 at 2:59 AM Ashesh Vashi <[email-masked]>
wrote:

>> We’re particularly worried because we’re trying to avoid the coupling
>> that we see in the code base today. We want to decouple *application
>> state* from *business domain* logic as much as we can - because this
>> makes the code much easier to understand. We achieve lower coupling by have
>> more suitable interfaces to retrieve *application state* like: anyParent
>> (the menu doesn’t care how this happens). This is the direction that we’re

---

From: Robert Eckhardt <[email-masked]>
Date: Thu, 31 May 2018 10:39:05 -0400

All,

These patches were first proposed on April 2 and the meaningful changes
have yet to be committed. ~8 weeks is long enough that my assumption now is
that these aren't going to be committed.

The goal of these patches is to begin to separate out the ACI tree in order
to allow us to do feature work on that chunk of the code. Are there any
alternate suggestions for this work?

Are there any ideas as to how we can meaningfully move forward with the
goal of allowing the tree view to support a very large number of tables?

-- Rob

On Thu, May 24, 2018 at 10:43 AM, Joao De Almeida Pereira <
[email-masked]> wrote:

> We've noticed that you've made several modifications on top of our
> original patch. Unfortunately, we've found it very hard to follow. Could we
> please get a brief synopsis of the changes you have made - just so we can
> better understand the rationale behind them? Just like we've done for you
> previously.
>
> On Thu, May 24, 2018 at 2:59 AM Ashesh Vashi <

---

From: Dave Page <[email-masked]>
Date: Thu, 31 May 2018 11:13:18 -0400

Rob,

My sincere apologies for the delay - I have told the team to prioritise
getting patch 0003 agreed and committed, and to get an understanding of
0004 and to ask any questions etc. they may have.

This *will* get done ASAP.

On Thu, May 31, 2018 at 10:39 AM, Robert Eckhardt <[email-masked]>
wrote:

> The goal of these patches is to begin to separate out the ACI tree in
> order to allow us to do feature work on that chunk of the code. Are there
> any alternate suggestions for this work?
>
>>>> To give a better solution, I can think of putting the menus related
>>>> code understand ‘sources/tree/menu’ directory.
>>>>
>>>> more suitable interfaces to retrieve *application state* like:
>>>> anyParent (the menu doesn’t care how this happens). This is the
>>>> direction that we’re trying to move towards, we just don’t want the package
>>>> structure to undermine that developer intent.
>>>>

---

From: Ashesh Vashi <[email-masked]>
Date: Fri, 1 Jun 2018 17:22:33 +0530

> better understand the rationale behind them? Just like we've done for you
Please find the changes from your original patch:

M webpack.shim.js
M webpack.test.config.js
- In order to specify the fake_browser in regression tests, we need to
use 'pgbrowser/browser' in the 'schema_child_tree_node.js' script.D
pgadmin/browser/server_groups/servers/databases/schemas/static/js/can_drop_child.js
- We don't need this with the new implementation.C
pgadmin/browser/server_groups/servers/databases/schemas/static/js/child.js
- All the children of schema node have common properties as
'parent_type', 'canDrop', 'canDropCascase', 'canCreate'.
  Hence - instead of defining them in each node, we have created a
base node, which will have all these properties.
  And, modified all schema children node to inherit from it.C
pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema_child_tree_node.js
- In this script, we're defining three functions
'childCreateMenuEnabled', 'isTreeItemOfChildOfSchema', &
'isTreeNodeOfSchemaChild', which are used by the 'SchemaChildNode'
objects.M pgadmin/browser/static/js/collection.js
- Fixed an issue related to wrongly defined 'error' function for the
Collection object.D pgadmin/static/js/menu/can_create.js
- It defined the function, which was defining a check for creation of
a schema child node, or not by looking at the parent node (i.e. a
schema/catalog node).
  The file was not defintely placed under the wrong directory, because
- the similar logic was under 'can_drop_child.js', and it was defined
under 'pgadmin/browser/server_groups/servers/databases/schemas/static/js'
directory.D pgadmin/static/js/menu/menu_enabled.jsC
pgadmin/static/js/nodes/supported_database_node.js
- Used by the external tools for checking whether the 'selected' tree-node is:
  + 'database' node, and it is allowed to connect it.
  + Or, it is one of the schema child (and, not 'catalog' child).
- Finding the correct location was difficult for this, as there is no
defined pattern, also it can be used by other functions too. Hence -
moved it out of 'pgadmin/static/js/menu' directory.M
pgadmin/static/js/tree/tree.js
- Introduced a function, which returns the ancestor node object, fow
which the condition is true.D
regression/javascript/menu/can_create_spec.js
D regression/javascript/menu/menu_enabled_spec.js
D regression/javascript/schema/can_drop_child_spec.jsC
regression/javascript/fake_browser/browser.js
C regression/javascript/nodes/schema/child_menu_spec.js
- Modified the regression to test the new functionalies.M
pgadmin/browser/server_groups/servers/databases/schemas/**/*.js
- Extending the schema child nodes from the 'SchemaChildNode' class
defined in 'pgadmin/.../schemas/static/js/child.js' script.

Let me know if you need more information.

> the scope of the changes we're making to expand beyond that.

I have the mutual feeling.

-- Thanks, Ashesh

---

From: Anthony Emengo <[email-masked]>
Date: Fri, 1 Jun 2018 11:17:45 -0400

Hey Ashesh,

Thanks for the explanation. It was great and it really helped!

C pgadmin/browser/server_groups/servers/databases/schemas/static/js/child.js
C pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema_child_tree_node.js

It makes sense to remove duplication by extracting these attributes out and
setting the canDrop and canCreate functions here. But is it possible to
combine these two files into one since they are related so we don’t need to
import schema_child_tree_node?

M pgadmin/static/js/tree/tree.js

The creation of the ancestorNode function feels like a pre-optimization.
That function is not used any where outside of the tree.js file, so it’s
more confusing to have it defined. On a lighter note, could we avoid the !!
syntax when possible? For example, instead of return !!obj, we could do
something like return obj === undefined or return _.isUndefined(obj) as
this is more intuitive.

https://softwareengineering.stackexchange.com/a/80092

In addition, please update this patch as it is out of sync with the latest
commit on the master branch. Otherwise, everything looks good!
​

Thanks
Anthony && Victoria

On Fri, Jun 1, 2018 at 7:52 AM Ashesh Vashi <[email-masked]>
wrote:

> - In order to specify the fake_browser in regression tests, we need to use 'pgbrowser/browser' in the 'schema_child_tree_node.js' script.D pgadmin/browser/server_groups/servers/databases/schemas/static/js/can_drop_child.js
> - We don't need this with the new implementation.C pgadmin/browser/server_groups/servers/databases/schemas/static/js/child.js
> - All the children of schema node have common properties as 'parent_type', 'canDrop', 'canDropCascase', 'canCreate'.
>   Hence - instead of defining them in each node, we have created a base node, which will have all these properties.
>   And, modified all schema children node to inherit from it.C pgadmin/browser/server_groups/servers/databases/schemas/static/js/schema_child_tree_node.js
> - In this script, we're defining three functions 'childCreateMenuEnabled', 'isTreeItemOfChildOfSchema', & 'isTreeNodeOfSchemaChild', which are used by the 'SchemaChildNode' objects.M pgadmin/browser/static/js/collection.js
> - Fixed an issue related to wrongly defined 'error' function for the Collection object.D pgadmin/static/js/menu/can_create.js
> - It defined the function, which was defining a check for creation of a schema child node, or not by looking at the parent node (i.e. a schema/catalog node).
>   The file was not defintely placed under the wrong directory, because - the similar logic was under 'can_drop_child.js', and it was defined under 'pgadmin/browser/server_groups/servers/databases/schemas/static/js' directory.D pgadmin/static/js/menu/menu_enabled.jsC pgadmin/static/js/nodes/supported_database_node.js
> - Finding the correct location was difficult for this, as there is no defined pattern, also it can be used by other functions too. Hence - moved it out of 'pgadmin/static/js/menu' directory.M pgadmin/static/js/tree/tree.js
> - Introduced a function, which returns the ancestor node object, fow which the condition is true.D regression/javascript/menu/can_create_spec.js
> D regression/javascript/schema/can_drop_child_spec.jsC regression/javascript/fake_browser/browser.js
> - Modified the regression to test the new functionalies.M pgadmin/browser/server_groups/servers/databases/schemas/**/*.js
> - Extending the schema child nodes from the 'SchemaChildNode' class defined in 'pgadmin/.../schemas/static/js/child.js' script.
>

---

From: Victoria Henry <[email-masked]>
Date: Fri, 1 Jun 2018 12:39:01 -0400

Hi Ashesh,

We just attempted to apply your patch over master but it did not work.  We
don't want to introduce any bugs or break any functionality.  Please update
the patch to make sure it is synced up with the master branch.

Sincerely,

Victoria

On Fri, Jun 1, 2018 at 11:18 AM Anthony Emengo <[email-masked]> wrote:

> It makes sense to remove duplication by extracting these attributes out
> and setting the canDrop and canCreate functions here. But is it possible
> to combine these two files into one since they are related so we don’t need
> to import schema_child_tree_node?
>
>>>>> could have easily been like canCreate: canCreateObject.bind({
>>>>> browser: pgBrowser, childOfCatalogType: childOfCatalogType }), I
>>>>> don’t feel too strongly here.
>>>>>
>>>>> The original patch was separating them in different places, but -
>>>>> still uses some of the functionalities directly from the tree, which was

---

From: Ashesh Vashi <[email-masked]>
Date: Sat, 2 Jun 2018 00:06:26 +0530

> That was the original plan, but 'pgadmin/browser/static/js//node.js'
script has too many dependecies, which are not easily portable.
And - that may lead to change the scope of the patch.

Hence - I decided to use the separate file to make sure we have enough test
coverage (which is more imprortant than changing the scope).

>> more confusing to have it defined.
>>
> It is being used in the latest changes. :-)

> On a lighter note, could we avoid the !! syntax when possible? For
>> example, instead of return !!obj, we could do something like return obj
>> === undefined or return _.isUndefined(obj) as this is more intuitive.
>>
> I am kind of disagree here. But - I have changed it anyway.

> Here - you go!

-- Thanks, Ashesh

> ​
>>
>> Thanks
>> Anthony && Victoria
>>
>> On Fri, Jun 1, 2018 at 7:52 AM Ashesh Vashi <
>>>> Let's keep in mind that the original intent was simply to introduce
>>>> this abstraction into the code base, which is a big enough task. I'd hate
>>>> for the scope of the changes we're making to expand beyond that.
>>>>
>>>
>>>>>> With respect to the above code: this bind pattern looks good and
>>>>>> seems like the idiomatic way to handle this in JavaScript. On a lighter
>>>>>> node, I don’t even see the need for an additional method to wrap it. The
>>>>>> invocation could have easily been like canCreate:
>>>>>> canCreateObject.bind({ browser: pgBrowser, childOfCatalogType:
>>>>>> childOfCatalogType }), I don’t feel too strongly here.
>>>>>>
>>>>>> I renamed it as isValidTreeNodeData, because - we were using it in
>>>>>> for testing the tree data. Please suggest me the right place, and name.
>>>>>>
>>>>>> We’re not sure; maybe after continued refactoring, we will come
>>>>>> across more generic functions. At that point we can revisit this and create
>>>>>> a utils.js file.
>>>>>>
>>>>> Sure.
>>>>>
>>>>>> Naming is one of the hardest problems in programming. I don’t feel
>>>>>> too strongly about this one. For now, let’s keep it as is
>>>>>>
>>>>>> Thanks
>>>>>> Anthony && Victoria
>>>>>> ​
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>

---

From: Victoria Henry <[email-masked]>
Date: Fri, 1 Jun 2018 15:17:23 -0400

Hey Ashesh,

LGTM!  The some of the CI tests failed but it looks like a flake.  But you
can go ahead and merge this.

Sincerely,

Victoria

On Fri, Jun 1, 2018 at 2:36 PM Ashesh Vashi <[email-masked]>
wrote:

>> We just attempted to apply your patch over master but it did not work.
>> We don't want to introduce any bugs or break any functionality.  Please
>> update the patch to make sure it is synced up with the master branch.
>>
>>> and setting the canDrop and canCreate functions here. But is it
>>> possible to combine these two files into one since they are related so we
>>> don’t need to import schema_child_tree_node?
>>>
> Hence - I decided to use the separate file to make sure we have enough
> test coverage (which is more imprortant than changing the scope).
>
>>> The creation of the ancestorNode function feels like a
>>> pre-optimization. That function is not used any where outside of the
>>> tree.js file, so it’s more confusing to have it defined.
>>>
>> In addition, please update this patch as it is out of sync with the
>>> latest commit on the master branch. Otherwise, everything looks good!
>>>
>> Here - you go!
>
> -- Thanks, Ashesh
>
>> ​
>>>
>>> Thanks
>>> Anthony && Victoria
>>>
>>>>>>> We’re particularly worried because we’re trying to avoid the
>>>>>>> coupling that we see in the code base today. We want to decouple *application
>>>>>>> state* from *business domain* logic as much as we can - because
>>>>>>> this makes the code much easier to understand. We achieve lower coupling by
>>>>>>> have more suitable interfaces to retrieve *application state* like:

---

From: Aditya Toshniwal <[email-masked]>
Date: Wed, 6 Jun 2018 16:37:50 +0530

Hi Anthony/Victoria/Joao,

I know I am very late to review on patch 004. The idea of moving js files
from tools to static folder looks good, but I have a few suggestions:

1) Why don't we start using webpack alias's instead of using absolute path.
For eg,
import {RestoreDialogWrapper} from
'../../../pgadmin/static/js/restore/restore_dialog_wrapper';
can be used as import {RestoreDialogWrapper} from
'pgadmin_static/js/restore/restore_dialog_wrapper';
by adding pgadmin_static alias to webpack config.

2) Few of the js are left behind, the ones which are used in python
__init__.py. Can't we move them too ? It would be nicer to not to leave
behind a single js.

Kindly let me know your views on this.

Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"

On Sat, Jun 2, 2018 at 12:47 AM, Victoria Henry <[email-masked]> wrote:

>>>> example, instead of return !!obj, we could do something like return
>>>> obj === undefined or return _.isUndefined(obj) as this is more
>>>> intuitive.
>>>>
>>>>>>>> have more suitable interfaces to retrieve *application state*
>>>>>>>> like: anyParent (the menu doesn’t care how this happens). This is
>>>>>>>> the direction that we’re trying to move towards, we just don’t want the
>>>>>>>> package structure to undermine that developer intent.
>>>>>>>>

---

From: Victoria Henry <[email-masked]>
Date: Wed, 6 Jun 2018 11:25:13 -0400

Hi Aditya,

> by adding pgadmin_static alias to webpack config.

Great point. In some areas of the code, we began making this change.  There
is already an alias in webpack shims for `../../../pgadmin/static/js`
called `sources`.  You can find an example of this in import statements for
`supported_database_node.js`

> __init__.py. Can't we move them too ? It would be nicer to not to leave

I'm not sure what you mean.  Could you point to an example of a single js
file?

Sincerely,

Victoria

On Wed, Jun 6, 2018 at 7:07 AM Aditya Toshniwal <
[email-masked]> wrote:

> 1) Why don't we start using webpack alias's instead of using absolute
> path. For eg,
>> LGTM!  The some of the CI tests failed but it looks like a flake.  But
>> you can go ahead and merge this.
>>
>> Sincerely,
>>
>> Victoria
>>
>> On Fri, Jun 1, 2018 at 2:36 PM Ashesh Vashi <
>>>>> It makes sense to remove duplication by extracting these attributes
>>>>> out and setting the canDrop and canCreate functions here. But is it
>>>>>>>> On Wed, May 16, 2018 at 8:55 PM, Anthony Emengo <[email-masked]
>>>>>>>> > wrote:
>>>>>>>>
>>>>>>>>> To give a better solution, I can think of putting the menus
>>>>>>>>> related code understand ‘sources/tree/menu’ directory.
>>>>>>>>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Thu, 7 Jun 2018 10:05:16 +0530

Hi Victoria,

On Wed, Jun 6, 2018 at 8:55 PM, Victoria Henry <[email-masked]> wrote:

>> import {RestoreDialogWrapper} from '../../../pgadmin/static/js/
>> restore/restore_dialog_wrapper';
> Great point. In some areas of the code, we began making this change.
> There is already an alias in webpack shims for `../../../pgadmin/static/js`

Sure. I did not find moving
web/pgadmin/tools/datagrid/static/js/datagrid.js. Please correct me if I am
missing anything.

>
> Sincerely,
>
> Victoria
>
> On Wed, Jun 6, 2018 at 7:07 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> import {RestoreDialogWrapper} from '../../../pgadmin/static/js/
>> restore/restore_dialog_wrapper';
>>>>> We just attempted to apply your patch over master but it did not
>>>>> work.  We don't want to introduce any bugs or break any functionality.
>>>>> Please update the patch to make sure it is synced up with the master branch.
>>>>>
>>>>>>>>>> I renamed it as isValidTreeNodeData, because - we were using it
>>>>>>>>>> in for testing the tree data. Please suggest me the right place, and name.
>>>>>>>>>>
>>>>>>>>>> like: anyParent (the menu doesn’t care how this happens). This
>>>>>>>>>> is the direction that we’re trying to move towards, we just don’t want the
>>>>>>>>>> Naming is one of the hardest problems in programming. I don’t
>>>>>>>>>> feel too strongly about this one. For now, let’s keep it as is
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Anthony && Victoria
>>>>>>>>>> ​
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>
>>

---

From: Khushboo Vashi <[email-masked]>
Date: Thu, 7 Jun 2018 13:11:33 +0530

Hi All,

> structure of our folders?
Right, currently it is very easy to follow the code as it is menu driven.
We can easily find the particular database object (node) and do whatever
required within that folder only.
We can also find the parent and its child nodes very easily, for this we
don't need to look into the code to find out who is parent and who is
child.  So, it is a huge advantage in terms of the application which is
specially designed for the database GUI. The downside of this is to
remember the path as you have mentioned. But after working for so long I
don't consider this as a downside :) but yes definitely for others who are
new to the project.

> The intent driven architecture is always good and you have also mentioned
the pros and cons for the same.

Let me understand the proposed structure correctly.
--  We would have all the nodes (browser, server_groups, servers, database
etc.) at same level i.e. in the web folder
--  The __init__.py file would have the main business logic and also the
routs same as the current approach.
--  The get_nodes.py, get_sql.py etc. would contain the functionality which
will be extracted from __init__.py
          - Please correct me if i am wrong
--  Frontend part would reside in the different folder same as the current
one.
--  All the static files will be in static folder (in their respected
folder).
         -  Right now the main frontend logic is in the static/js file, for
example, tables/static/js/tables.js file. So, as per me this should be here
only. Main advantage of this is, it is easy to locate
    and also good for pluggability. For example, If I want to
change/add/understand the code, it is always good to have in its respected
module and I don't see any downside for this.

> His approach to architecture of the application is pretty interesting.
So, at the end, I can see the main disadvantage of the menu driven
architecture is, missing intent (I realised after watching the video) and
hard to find the module.
And the main advantage right now is, find out the tree structure, easy to
plug-in / plug-out the feature and easy to understand to code.

So, both the architectures have pros and cons. I like the proposed
architecture with some changes as I mentioned and now as a community we
need to decide, should we go ahead with this proposed architecture (with
some cost i.e. additional plumbing) or not.

Thanks,
Khushboo

---

From: Victoria Henry <[email-masked]>
Date: Thu, 7 Jun 2018 11:21:52 -0400

Hi Aditya

> web/pgadmin/tools/datagrid/static/js/datagrid.js. Please correct me if I am

Generally speaking, I agree with moving/deleting files if it makes sense.
But in regards to web/pgadmin/tools/datagrid/static/js/datagrid.js, it
looks like this is still being used in
web/pgadmin/tools/sqleditor/static/js/sqleditor.js with
Datagrid.create_transaction

Sincerely,

Victoria
​

On Thu, Jun 7, 2018 at 12:35 AM Aditya Toshniwal <
[email-masked]> wrote:

>> There is already an alias in webpack shims for `
>> ../../../pgadmin/static/js` called `sources`.  You can find an example
>> of this in import statements for `supported_database_node.js`
>>
>>> I know I am very late to review on patch 004. The idea of moving js
>>> files from tools to static folder looks good, but I have a few suggestions:
>>>
>>>>>>>>> Let's keep in mind that the original intent was simply to
>>>>>>>>> introduce this abstraction into the code base, which is a big enough task.
>>>>>>>>> I'd hate for the scope of the changes we're making to expand beyond that.
>>>>>>>>>
>>>>>>>>
>>>>>>>>>>> The original patch was separating them in different places, but
>>>>>>>>>>> - still uses some of the functionalities directly from the tree, which was

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 11 Jun 2018 11:14:44 +0530

Hi Victoria,

On Thu, Jun 7, 2018 at 8:51 PM, Victoria Henry <[email-masked]> wrote:

> Hi Aditya
>
> Sure. I did not find moving web/pgadmin/tools/datagrid/static/js/datagrid.js.
>> Please correct me if I am missing anything.
>
> looks like this is still being used in web/pgadmin/tools/sqleditor/
> static/js/sqleditor.js with Datagrid.create_transaction
>
And this is another js which is not moved.(sqleditor.js). If we are moving
js files, why not this too.

> Sincerely,
>
> Victoria
> ​
>
>
>
> On Thu, Jun 7, 2018 at 12:35 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Victoria,
>>
>>> I'm not sure what you mean.  Could you point to an example of a single
>>> js file?
>>>
>>
>> Sure. I did not find moving web/pgadmin/tools/datagrid/static/js/datagrid.js.
>> Please correct me if I am missing anything.
>>
>>>
>>> Sincerely,
>>>
>>> Victoria
>>>
>>>>>>>> out and setting the canDrop and canCreate functions here. But is
>>>>>>>> it possible to combine these two files into one since they are related so
>>>>>>>> we don’t need to import schema_child_tree_node?
>>>>>>>>
>>>>>> Hence - I decided to use the separate file to make sure we have
>>>>>> enough test coverage (which is more imprortant than changing the scope).
>>>>>>
>>>>>>>>>> We've noticed that you've made several modifications on top of
>>>>>>>>>> our original patch. Unfortunately, we've found it very hard to follow.
>>>>>>>>>> Could we please get a brief synopsis of the changes you have made - just so
>>>>>>>>>> we can better understand the rationale behind them? Just like we've done
>>>>>>>>>> for you previously.
>>>>>>>>>>
>>>>>>>>>>>> With respect to the above code: this bind pattern looks good
>>>>>>>>>>>> and seems like the idiomatic way to handle this in JavaScript. On a lighter
>>>>>>>>>>>> state* from *business domain* logic as much as we can -
>>>>>>>>>>>> because this makes the code much easier to understand. We achieve lower
>>>>>>>>>>>> coupling by have more suitable interfaces to retrieve *application
>>>>>>>>>>>> state* like: anyParent (the menu doesn’t care how this
>>>>>>>>>>>> happens). This is the direction that we’re trying to move towards, we just
>>>>>>>>>>>> don’t want the package structure to undermine that developer intent.
>>>>>>>>>>>>
>>>>>>>>>>> I realized after revisiting the code, menu/can_create.js was
>>>>>>>>>>> only applicable to the children of the schema/catalog nodes, same as
>>>>>>>>>>> 'can_drop_child'.

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 11 Jun 2018 15:59:22 +0530

Hi Team,

On Sat, May 5, 2018 at 3:31 AM, Joao De Almeida Pereira <
[email-masked]> wrote:

> *...*
To be honest, it depends on how do you see the application, and its
expectations.
That question leads me to another question "What do we want from pgAdmin 4?"

> structure of our folders?
To be honest not menu driven. (Probably a wrong choice of word 'Menu'.)
But - only under 'browser' (You can also call it object browser/tree), it
is driven by the database object relation model.
And, each module maintains the parent-child relationship.

> do we win with this?
I agree - it is too deep structure.
But - it gives me the idea what's the structure of the database objection
relationship.

Does it hurt, I would say no?
Because - as a database developer, I know whats the relationship of
objects, and where I can find them.
(Until I heard Dave saying it is about to touch the limit of maximum file
system path.)

So - there is a scope for improvement there, we can leave without the
object relations (but - limited to the object browser I think).

> If we open pgAdmin we know which nodes to click in order to get to tables.
> But for development
What if our structure would look like this:
>
>  - web
>    - tables
>      - controller
>        - get_nodes.py
>        - get_sql.py
>        - __init__.py
>      - frontend
>         - component
>             - ddl_component.js
>         - services
>             - table-service.js
>    - schemas
>    - servers
>    - ....
>
> I think - there is nothing wrong with the current module structure. It is
more appealing to me than the above one.
The python package contains the backend code, and static contains all your
frontend.

I am not saying, we should not change anything.
We can definitely divide them in smaller chunks for both backend, and
frontend side.

> What are your thought about this architecture?
I am not a fan of flat file structure in application.
There are many reasons - why we need namespace in C++, same applies here.

Let me start from the question "What do we want from pgAdmin 4?"
* A object browser
* Miscallenous operations associated with the object shown in the object
browser
  + Reversed Enginering query (if any)
  + Properties Viewer
  + Edit/Create Viewer
  + Staticis
  + List of dependencies
  + List of dependents
* Query tool
* Data Viewer (table/view)
* Function debugger
* Extendability/Pluggability

According to me - these as the intent of the application, and not objects
assiciated with them.

I do agree, there is no boundry (or, very thin) between data & presentation
at the moment.
We can start from there.

I can think of the following directory structure ( which is not too
different from the current, but - still will give the developers a lot more
comfort as per your complain :-) ).

> pgadmin/
> - database_objects/
>   - server/
>     - create.py
>     - get.py
>     - sql.py
>     - update.py
>     - static/
>       - img/
>         - server.svg
>         - server_bad.svg
>       - javascripts/
>         - server.js
>         - ui_schema.js
>   - database/
>       - templates
>         ...
>     ...
>   + schema/
>   ...
>   - tests/
>     - api/
>       ...
>     - gui/
>       ...
>     - karma/
>       ...
> - browser/
>   + pgagents/
>   - server_groups/
>     - servers/
>       - databases/
>         - node.py
>           ...
>         ...
>       ...
>     ...
>   + tests/
> - tools/
>   + sqleditor/
>   + dataviewer/
>   + debugger/
> - misc/
>   + sql/
>   + statistics/
>   ...
> NOTE:
We also need to think about the facts that.
These changes may lead to a lot of night mare packagers to make changes in
the installers in upgrade mode.

Also - when I say pluggability/extendability, we should be able to create
plugins on top of pgAdmin 4.
Just take an example of this feature (Geospatial Query Viewer)
<https://www.postgresql.org/message-id/CAKyzeV1V1c_=KfLLxnzF7-fXWKLBiGgru1tz_+XRDXJWJ=[email-masked]>
.

I think - it is best implemented as an plugin, as not every pgAdmin user
will benificiary.
We need ability to load the modules as pluggable module same as we used to
have before we introduced the webpack to bundle everything as modules.
We also need to start thinking about it.

-- Thanks, Ashesh

---

From: Ashesh Vashi <[email-masked]>
Date: Mon, 11 Jun 2018 16:05:52 +0530

On Mon, Jun 11, 2018 at 3:59 PM, Ashesh Vashi <[email-masked]

> (Until I heard Dave saying it is about to touch the limit of maximum file
So - there is a scope for improvement there, we can leave *live* without
the object relations (but - limited to the object browser I think).

>
>
>> If we open pgAdmin we know which nodes to click in order to get to
>> tables. But for development
> I do agree, there is no boundry (or, very thin) between data &
> presentation at the moment.
> We can start from there.
>