Hi hackers,
We tried to extract and refactor generate_url function in node.js and
collection.js. Please see the patch attached.
Thanks,
Violet & Sarah
diff --git a/web/pgadmin/browser/static/js/collection.js
b/web/pgadmin/browser/static/js/collection.js
index f97edcc0..c0cb0eb4 100644
--- a/web/pgadmin/browser/static/js/collection.js
+++ b/web/pgadmin/browser/static/js/collection.js
@@ -1,8 +1,8 @@
define([
'sources/gettext', 'jquery', 'underscore', 'underscore.string',
'sources/pgadmin',
- 'backbone', 'alertify', 'backform', 'backgrid', 'pgadmin.backform',
'pgadmin.backgrid',
+ 'backbone', 'alertify', 'backform', 'backgrid',
'sources/browser/generate_url', 'pgadmin.backform', 'pgadmin.backgrid',
'pgadmin.browser.node'
-], function(gettext, $, _, S, pgAdmin, Backbone, Alertify, Backform, Backgrid)
{
+], function(gettext, $, _, S, pgAdmin, Backbone, Alertify, Backform, Backgrid,
generateUrl) {
var pgBrowser = pgAdmin.Browser = pgAdmin.Browser || {};
@@ -135,41 +135,22 @@ define([
}
})
},
- generate_url: function(item, type, d) {
- var url = pgAdmin.Browser.URL + '{TYPE}/{REDIRECT}{REF}',
- /*
- * Using list, and collection functions of a node to get the nodes
- * under the collection, and properties of the collection respectively.
- */
- opURL = {
- 'properties': 'obj', 'children': 'nodes'
+ generate_url: function(item, type) {
+ /*
+ * Using list, and collection functions of a node to get the nodes
+ * under the collection, and properties of the collection respectively.
+ */
+ var opURL = {
+ 'properties': 'obj', 'children': 'nodes',
},
- ref = '', self = this;
-
- _.each(
- _.sortBy(
- _.values(
- _.pick(
- this.getTreeNodeHierarchy(item), function(v, k, o) {
- return (k != self.type);
- })
- ),
- function(o) { return o.priority; }
- ),
- function(o) {
- ref = S('%s/%s').sprintf(ref, encodeURI(o._id)).value();
- });
-
- var args = {
- 'TYPE': self.node,
- 'REDIRECT': (type in opURL ? opURL[type] : type),
- 'REF': S('%s/').sprintf(ref).value()
+ self = this;
+ var collectionPickFunction = function (treeInfoValue, treeInfoKey) {
+ return (treeInfoKey != self.type);
};
-
- return url.replace(/{(\w+)}/g, function(match, arg) {
- return args[arg];
- });
- }
+ var treeInfo = this.getTreeNodeHierarchy(item);
+ var actionType = type in opURL ? opURL[type] : type;
+ return generateUrl.generate_url(pgAdmin.Browser.URL, treeInfo,
actionType, self.node, collectionPickFunction);
+ },
});
return pgBrowser.Collection;
diff --git a/web/pgadmin/browser/static/js/node.js
b/web/pgadmin/browser/static/js/node.js
index f501d020..5d05fde1 100644
--- a/web/pgadmin/browser/static/js/node.js
+++ b/web/pgadmin/browser/static/js/node.js
@@ -2,8 +2,8 @@ define(
'pgadmin.browser.node', [
'sources/gettext', 'jquery', 'underscore', 'underscore.string',
'sources/pgadmin',
'pgadmin.browser.menu', 'backbone', 'pgadmin.alertifyjs',
'pgadmin.browser.datamodel',
- 'backform', 'pgadmin.browser.utils', 'pgadmin.backform'
-], function(gettext, $, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser,
Backform) {
+ 'backform', 'sources/browser/generate_url', 'pgadmin.browser.utils',
'pgadmin.backform'
+], function(gettext, $, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser,
Backform, generateUrl) {
var wcDocker = window.wcDocker,
keyCode = {
@@ -1428,26 +1428,27 @@ define(
* depends, statistics
*/
generate_url: function(item, type, d, with_id, info) {
- var url = pgBrowser.URL + '{TYPE}/{REDIRECT}{REF}',
- opURL = {
+
+ var opURL = {
'create': 'obj', 'drop': 'obj', 'edit': 'obj',
'properties': 'obj', 'statistics': 'stats'
- },
- ref = '', self = this,
+ }, self = this,
priority = -Infinity;
- info = (_.isUndefined(item) || _.isNull(item)) ?
+ var treeInfo = (_.isUndefined(item) || _.isNull(item)) ?
info || {} : this.getTreeNodeHierarchy(item);
+ var actionType = type in opURL ? opURL[type] : type;
+ var itemID = with_id && d._type == self.type ? encodeURIComponent(d._id)
: '';
if (self.parent_type) {
if (_.isString(self.parent_type)) {
- var p = info[self.parent_type];
+ var p = treeInfo[self.parent_type];
if (p) {
priority = p.priority;
}
} else {
_.each(self.parent_type, function(o) {
- var p = info[o];
+ var p = treeInfo[o];
if (p) {
if (priority < p.priority) {
priority = p.priority;
@@ -1456,34 +1457,10 @@ define(
});
}
}
-
- _.each(
- _.sortBy(
- _.values(
- _.pick(info,
- function(v, k, o) {
- return (v.priority <= priority);
- })
- ),
- function(o) { return o.priority; }
- ),
- function(o) {
- ref = S('%s/%s').sprintf(ref, encodeURIComponent(o._id)).value();
- });
-
- ref = S('%s/%s').sprintf(
- ref, with_id && d._type == self.type ? encodeURIComponent(d._id) : ''
- ).value();
-
- var args = {
- 'TYPE': self.type,
- 'REDIRECT': (type in opURL ? opURL[type] : type),
- 'REF': ref
+ var nodePickFunction = function (treeInfoValue) {
+ return (treeInfoValue.priority <= priority);
};
-
- return url.replace(/{(\w+)}/g, function(match, arg) {
- return args[arg];
- });
+ return generateUrl.generate_url(pgBrowser.URL, treeInfo, actionType,
self.type, nodePickFunction, itemID);
},
// Base class for Node Data Collection
Collection: pgBrowser.DataCollection,
diff --git a/web/pgadmin/static/js/browser/generate_url.js
b/web/pgadmin/static/js/browser/generate_url.js
new file mode 100644
index 00000000..a5cf2361
--- /dev/null
+++ b/web/pgadmin/static/js/browser/generate_url.js
@@ -0,0 +1,23 @@
+import _ from 'underscore';
+
+function generate_url(baseUrl, treeInfo, actionType, nodeType, pickFunction,
itemDataID) {
+ let ref = '';
+ _.each(
+ _.sortBy(
+ _.pick(treeInfo, pickFunction),
+ function (treeInfoItems) {
+ return treeInfoItems.priority;
+ }
+ ),
+ function (treeInfoItems) {
+ ref = `${ref}/${encodeURI(treeInfoItems._id)}`;
+ }
+ );
+ ref = itemDataID ? `${ref}/${itemDataID}` : `${ref}/`;
+
+ return `${baseUrl}${nodeType}/${actionType}${ref}`;
+}
+
+module.exports = {
+ generate_url: generate_url,
+};
diff --git a/web/regression/javascript/browser/generate_url_spec.js
b/web/regression/javascript/browser/generate_url_spec.js
new file mode 100644
index 00000000..db5f250b
--- /dev/null
+++ b/web/regression/javascript/browser/generate_url_spec.js
@@ -0,0 +1,95 @@
+import {generate_url} from 'sources/browser/generate_url';
+
+describe('generate_url', () => {
+ describe('in collection', () => {
+ let baseUrl, treeInfo, actionType, nodeType, pickFunction;
+ beforeEach(() => {
+ baseUrl = 'http://base/and-extension/';
+ treeInfo = {
+ treeNode1: {
+ _id: 'an_id',
+ priority: 1000,
+ },
+ };
+ actionType = 'actionType';
+ nodeType = 'nodeType';
+ pickFunction = () => {
+ return true;
+ };
+ });
+ it('returns a correctly formatted URL', () => {
+ let formattedUrl = generate_url(baseUrl, treeInfo, actionType, nodeType,
pickFunction);
+
+
expect(formattedUrl).toEqual('http://base/and-extension/nodeType/actionType/an_id/');
+ });
+
+ describe('given there are multiple treeInfoItems', () => {
+ beforeEach(() => {
+ treeInfo['treeNode2'] = {
+ _id: 'another_id',
+ priority: 500,
+ };
+ treeInfo['treeNode3'] = {
+ _id: 'a_third_id',
+ priority: 100,
+ };
+
+ pickFunction = (value, key) => {
+ return key != 'treeNode2';
+ };
+ });
+
+ it('chooses the correct treeInfo', () => {
+ let formattedUrl = generate_url(baseUrl, treeInfo, actionType,
nodeType, pickFunction);
+
+
expect(formattedUrl).toEqual('http://base/and-extension/nodeType/actionType/a_third_id/an_id/');
+ });
+ });
+ });
+ describe('in node', () => {
+ let baseUrl, treeInfo, actionType, nodeType, pickFunction, itemDataID;
+ beforeEach(() => {
+ baseUrl = 'http://base/and-extension/';
+ treeInfo = {
+ treeNode1: {
+ _id: 'an_id',
+ priority: 1000,
+ },
+ };
+ actionType = 'actionType';
+ nodeType = 'nodeType';
+ pickFunction = () => {
+ return true;
+ };
+ itemDataID = 'item1';
+ });
+ it('returns a correctly formatted URL', () => {
+ let formattedUrl = generate_url(baseUrl, treeInfo, actionType, nodeType,
pickFunction, itemDataID);
+
+
expect(formattedUrl).toEqual('http://base/and-extension/nodeType/actionType/an_id/item1');
+ });
+
+ describe('given there are multiple treeInfoItems', () => {
+ beforeEach(() => {
+ treeInfo['treeNode2'] = {
+ _id: 'another_id',
+ priority: 500,
+ };
+ treeInfo['treeNode3'] = {
+ _id: 'a_third_id',
+ priority: 100,
+ };
+
+ pickFunction = (value) => {
+ return value.priority > 100;
+ };
+ });
+
+ it('chooses the correct treeInfo', () => {
+ let formattedUrl = generate_url(baseUrl, treeInfo, actionType,
nodeType, pickFunction, itemDataID);
+
+
expect(formattedUrl).toEqual('http://base/and-extension/nodeType/actionType/another_id/an_id/item1');
+ });
+ });
+ });
+});
