[pgAdmin4][patch] extract generate_url function from node.js and collection.js

2017-08-10 Thread Violet Cheng
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(
-_.

Re: [pgAdmin4][patch] update the alert style in the sub-navigation

2017-08-10 Thread Violet Cheng
Hi Surinder!

Are you referring to the green message popup? If so, it also appears to be
happening on master. We'll log a bug in our backlog and Redmine and
prioritize it. We agree that it needs to be fixed, but don't think it's
unrelated to this patch.

Thanks!
Violet & Sarah

On Thu, Aug 10, 2017 at 2:32 PM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Please find attached screenshot.
>
> On Thu, Aug 10, 2017 at 11:30 AM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi Sarah,
>>
>> We noticed that due to this patch, the alert style of "Database
>> connected" message is changed.
>> Can you please look into this?
>>
>> Thanks,
>> Surinder
>>
>> On Wed, Aug 9, 2017 at 4:43 PM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> ​Hi,
>>>
>>> ​The updated patch looks good to me.
>>>
>>> On Wed, Aug 9, 2017 at 4:15 PM, Sarah McAlear 
>>> wrote:
>>>
 As discussed with Surinder, we have created a Redmine ticket for his
 4th comment regarding the error message not showing up when the app can't
 be reached. This issue existed prior to this patch and should be
 prioritized.

 https://redmine.postgresql.org/issues/2640

 Thanks!
 Matt & Sarah

 On Wed, Aug 9, 2017 at 4:06 PM, Sarah McAlear 
 wrote:

> Hi Surinder!
>
> I am not able to see anything different from what I see on Master with
> or without the patch applied. I tried adjusting the preferences. I did
> update the dashboard.js to instantiate a new object, great idea!
>
> Thanks,
> Sarah
>
>
>
> On Wed, Aug 9, 2017 at 1:42 PM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi Wenlin,
>>
>> On Tue, Aug 8, 2017 at 3:15 PM, Wenlin Zhang 
>> wrote:
>>
>>> Hi Surinder,
>>>
>>>Thanks for your review.
>>>
>>>We have changed the indentation for _dashboard.scss file and
>>> also removed the style about icon-postgres:before, like
>>> margin-top,etc, but we are not sure if it is perfectly aligned now, you 
>>> can
>>> add further change to it.
>>>
>>> As the second comment, I'm sorry I'm not sure what's the
>>> problem, could you please clarify it? Because we replace css with scss
>>> right now, dashboard.css doesn't exist. So maybe you are looking at the 
>>> css
>>> file that are complied by the scss?
>>>
>> ​Sorry I​ typed 'dashboard.css' instead of 'dashboard.js'.
>> ​In dashboard.js can we change `return DashboardAlert;` to `return
>> new DashboardAlert();`
>> and then we can remove the instances being created(var alertDashboard
>> = new AlertDashboard();) from dashboard.js, and simply
>> use `AlertDashboard.info('message')`.
>>
>>
>>> For the fourth comment, we tried the steps you suggested on
>>> master branch, the error is not shown either. So it should be an 
>>> existing
>>> issue. But if you want to see the error message, navigate to "Servers" 
>>> at
>>> the top of browser, then navigate back to postgresql server, then you 
>>> will
>>> see the error message.
>>>
>> ​The error in console will appear when you selected a database which
>> is connected and stop the backend Python server because the request for
>> graphs for database level will fail and there is no response returned 
>> from
>> server.
>> It might be not reproducible to you If you set the refresh rate to
>> higher value (i.e. Preferences > Graphs) in preferences. Just set refresh
>> rate to 1 for all dashboard graphs and repeat the steps.
>>
>>>
>>>
>>> Thanks,
>>>
>>> Wenlin &Violet
>>>
>>>
>>>
>>> On Mon, Aug 7, 2017 at 2:30 PM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.com> wrote:
>>>
 Hi
 Review comments:

1.

For consistency, we use two spaces for indentation in CSS
files. Four spaces are used in _dashboard.scss file. The
configurations are defined in web/.editorconfig file.
2.

In,dashboard.css Can we return function object in return
instead of function class itself, this will eliminate the need of 
 creating
function object every time we use info and error?
3.

On Dashboard, I can see Postgres icon is misaligned compared to
other icons in Getting Started section. It is not related to
this patch. adjusting margin top will fix it.
4.

I tried to test out Error message displayed, but I encounter an
error(screenshot attached).
Steps to reproduce:
   - Open pgAdmin4 in browser
   - Connect to PostgreSQL Server, Keep dashboard tab open.
   - Navigate to the database which is connected.
>

Re: [pgAdmin4][patch] update the alert style in the sub-navigation

2017-08-10 Thread Violet Cheng
Here's the Redmine link

https://redmine.postgresql.org/issues/2644

On Thu, Aug 10, 2017 at 4:03 PM, Violet Cheng  wrote:

> Hi Surinder!
>
> Are you referring to the green message popup? If so, it also appears to be
> happening on master. We'll log a bug in our backlog and Redmine and
> prioritize it. We agree that it needs to be fixed, but don't think it's
> unrelated to this patch.
>
> Thanks!
> Violet & Sarah
>
> On Thu, Aug 10, 2017 at 2:32 PM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Please find attached screenshot.
>>
>> On Thu, Aug 10, 2017 at 11:30 AM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> Hi Sarah,
>>>
>>> We noticed that due to this patch, the alert style of "Database
>>> connected" message is changed.
>>> Can you please look into this?
>>>
>>> Thanks,
>>> Surinder
>>>
>>> On Wed, Aug 9, 2017 at 4:43 PM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.com> wrote:
>>>
 ​Hi,

 ​The updated patch looks good to me.

 On Wed, Aug 9, 2017 at 4:15 PM, Sarah McAlear 
 wrote:

> As discussed with Surinder, we have created a Redmine ticket for his
> 4th comment regarding the error message not showing up when the app can't
> be reached. This issue existed prior to this patch and should be
> prioritized.
>
> https://redmine.postgresql.org/issues/2640
>
> Thanks!
> Matt & Sarah
>
> On Wed, Aug 9, 2017 at 4:06 PM, Sarah McAlear 
> wrote:
>
>> Hi Surinder!
>>
>> I am not able to see anything different from what I see on Master
>> with or without the patch applied. I tried adjusting the preferences. I 
>> did
>> update the dashboard.js to instantiate a new object, great idea!
>>
>> Thanks,
>> Sarah
>>
>>
>>
>> On Wed, Aug 9, 2017 at 1:42 PM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> Hi Wenlin,
>>>
>>> On Tue, Aug 8, 2017 at 3:15 PM, Wenlin Zhang 
>>> wrote:
>>>
 Hi Surinder,

Thanks for your review.

We have changed the indentation for _dashboard.scss file and
 also removed the style about icon-postgres:before, like
 margin-top,etc, but we are not sure if it is perfectly aligned now, 
 you can
 add further change to it.

 As the second comment, I'm sorry I'm not sure what's the
 problem, could you please clarify it? Because we replace css with scss
 right now, dashboard.css doesn't exist. So maybe you are looking at 
 the css
 file that are complied by the scss?

>>> ​Sorry I​ typed 'dashboard.css' instead of 'dashboard.js'.
>>> ​In dashboard.js can we change `return DashboardAlert;` to `return
>>> new DashboardAlert();`
>>> and then we can remove the instances being created(var
>>> alertDashboard = new AlertDashboard();) from dashboard.js, and simply
>>> use `AlertDashboard.info('message')`.
>>>
>>>
 For the fourth comment, we tried the steps you suggested on
 master branch, the error is not shown either. So it should be an 
 existing
 issue. But if you want to see the error message, navigate to "Servers" 
 at
 the top of browser, then navigate back to postgresql server, then you 
 will
 see the error message.

>>> ​The error in console will appear when you selected a database which
>>> is connected and stop the backend Python server because the request for
>>> graphs for database level will fail and there is no response returned 
>>> from
>>> server.
>>> It might be not reproducible to you If you set the refresh rate to
>>> higher value (i.e. Preferences > Graphs) in preferences. Just set 
>>> refresh
>>> rate to 1 for all dashboard graphs and repeat the steps.
>>>


 Thanks,

 Wenlin &Violet



 On Mon, Aug 7, 2017 at 2:30 PM, Surinder Kumar <
 surinder.ku...@enterprisedb.com> wrote:

> Hi
> Review comments:
>
>1.
>
>For consistency, we use two spaces for indentation in CSS
>files. Four spaces are used in _dashboard.scss file. The
>configurations are defined in web/.editorconfig file.
>2.
>
>In,dashboard.css Can we return function object in return
>instead of function class itself, this will eliminate the need of 
> creating
>function object every time we use info and error?
>3.
>
>On Dashboard, I can see Postgres icon is misaligned compared
>to other icons in Getting Started section. It is not related
>to this patch. adjusting margin top will fix it.
>4.
>
>I tried to test out

Is there a CLI for creating the object tree?

2017-08-10 Thread Tim Uckun
Hi.

I want to write a tool for PG and I would like to recreate the object tree
in pgadmin on the disk.  For example.

db
   schemas
   public
  tables
  some_table.sql
I looked at the source code and it seems like you guys read the definitions
from the DB and then apply a template. This is great and I could rewrite
all that in my own code but I was hoping it would be possible to avoid all
that work.   Do you guys know of any type of CLI or a library that would
help me accomplish this? I also thought of just calling out to pg_dump and
then writing a parser for that file but again I would rather take a
shortcut if one exists.

Thanks.


[pgadmin4][patch] Fix the style of the alert when connecting/disconnecting from a server

2017-08-10 Thread Sarah McAlear
Hi Hackers!

This patch applies to RM 2644 .
It fixes the styling of the checkmark area.

Thanks!
Sarah
diff --git a/web/pgadmin/static/scss/_alertify.overrides.scss 
b/web/pgadmin/static/scss/_alertify.overrides.scss
index 8344956b..2c1e1d49 100644
--- a/web/pgadmin/static/scss/_alertify.overrides.scss
+++ b/web/pgadmin/static/scss/_alertify.overrides.scss
@@ -176,6 +176,6 @@ button.pg-alertify-button {
   @extend .ajs-text-smoothing;
 }
 
-.media-body {
+.ajs-message > .media > .media-body.media-middle {
   display: table-row;
 }


Re: [pgAdmin4][RM2586] Cleanup feature test

2017-08-10 Thread Harshal Dhumal
Hi,

Please find attached updated patch. In this patch I have removed unused
imports.
Regarding ajax calls on slow network/computer I have already taken care of
those.
Before sending first version of patch I tested this on Murtuza's machine
and also
tested on slow machine as well.


Thanks,
Harshal

-- 
*Harshal Dhumal*
*Sr. Software Engineer*

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

On Mon, Aug 7, 2017 at 8:55 AM, Sarah McAlear  wrote:

> Hi Harshal!
>
> There are a few files in which there are leftover imports that are not
> needed anymore:
>
>- PGDataypeFeatureTest
>- PgadminPage
>- CheckForXssFeatureTest
>- CheckDebuggerForXssFeatureTest
>
>
> We also noticed that there were quite a few time.sleep functions that
> were removed. This seems great overall, but we think that some of them were
> in place because of varying network and computer speeds. So for example, in
> QueryToolFeatureTest, there is an ajax call that was followed by a
> time.sleep to ensure that it had finished executing before continuing to
> execute. Removing this may reintroduce some flakiness. If there are no
> issues with flakiness after this patch, it seems like a great idea. We ran
> the tests a few times and didn't notice any flakiness, but we're unsure if
> it will be a problem on a different system.
>
> Thanks!
> Wenlin & Sarah
>
>
> On Wed, Aug 2, 2017 at 9:32 PM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find attached patch to improve feature test execution time.
>> Now on my machine overall execution time is cut down to 280 seconds from
>> 400+ seconds
>>
>> Changes:
>>
>> 1. Removed fixed python time.sleeps where ever possible.
>> 2. Removed connect to server test cases.
>> 3. Query tool test cases:
>>  i. Merged 3 test cases On demand result on scroll, grid select all
>> and column select all.
>>  ii. Merged 3 test cases Explain query, Explain query with verbose
>> and Explain query with cost.
>>  iii. Merged 3 test cases Explain analyze query, Explain analyze with
>> buffers and Explain analyze with timing.
>> 4. Improved debugger XSS test case execution time.
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
diff --git a/web/pgadmin/feature_tests/connect_to_server_feature_test.py b/web/pgadmin/feature_tests/connect_to_server_feature_test.py
deleted file mode 100644
index 97e96f2..000
--- a/web/pgadmin/feature_tests/connect_to_server_feature_test.py
+++ /dev/null
@@ -1,84 +0,0 @@
-##
-#
-# pgAdmin 4 - PostgreSQL Tools
-#
-# Copyright (C) 2013 - 2017, The pgAdmin Development Team
-# This software is released under the PostgreSQL Licence
-#
-##
-
-import time
-from selenium.webdriver import ActionChains
-
-import config as app_config
-from regression.feature_utils.base_feature_test import BaseFeatureTest
-from regression.python_test_utils import test_utils
-
-
-class ConnectsToServerFeatureTest(BaseFeatureTest):
-"""
-Tests that a database connection can be created from the UI
-"""
-scenarios = [
-("Test database connection", dict())
-]
-
-def before(self):
-connection = test_utils.get_db_connection(self.server['db'],
-  self.server['username'],
-  self.server['db_password'],
-  self.server['host'],
-  self.server['port'],
-  self.server['sslmode'])
-test_utils.drop_database(connection, "acceptance_test_db")
-test_utils.create_database(self.server, "acceptance_test_db")
-test_utils.create_table(self.server, "acceptance_test_db", "test_table")
-
-def runTest(self):
-"""This function tests that a database connection can be created from
-the UI"""
-self.assertEqual(app_config.APP_NAME, self.page.driver.title)
-self.page.wait_for_spinner_to_disappear()
-
-self._connects_to_server()
-self._tables_node_expandable()
-
-def after(self):
-self.page.remove_server(self.server)
-
-connection = test_utils.get_db_connection(self.server['db'],
-  self.server['username'],
-  self.server['db_password'],
-  self.server['host'],
-  self.server['port'],
-  self.server['sslmode'])
-test_utils.drop_database(connection, "acceptance_test_db")
-
-def _connects_to_se

Re: [pgAdmin4][patch] update the alert style in the sub-navigation

2017-08-10 Thread Sarah McAlear
Hi!

We fixed that issue and created a new patch

.

Thanks!

On Thu, Aug 10, 2017 at 4:10 PM, Violet Cheng  wrote:

> Here's the Redmine link
>
> https://redmine.postgresql.org/issues/2644
>
> On Thu, Aug 10, 2017 at 4:03 PM, Violet Cheng  wrote:
>
>> Hi Surinder!
>>
>> Are you referring to the green message popup? If so, it also appears to
>> be happening on master. We'll log a bug in our backlog and Redmine and
>> prioritize it. We agree that it needs to be fixed, but don't think it's
>> unrelated to this patch.
>>
>> Thanks!
>> Violet & Sarah
>>
>> On Thu, Aug 10, 2017 at 2:32 PM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> Please find attached screenshot.
>>>
>>> On Thu, Aug 10, 2017 at 11:30 AM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.com> wrote:
>>>
 Hi Sarah,

 We noticed that due to this patch, the alert style of "Database
 connected" message is changed.
 Can you please look into this?

 Thanks,
 Surinder

 On Wed, Aug 9, 2017 at 4:43 PM, Surinder Kumar <
 surinder.ku...@enterprisedb.com> wrote:

> ​Hi,
>
> ​The updated patch looks good to me.
>
> On Wed, Aug 9, 2017 at 4:15 PM, Sarah McAlear 
> wrote:
>
>> As discussed with Surinder, we have created a Redmine ticket for his
>> 4th comment regarding the error message not showing up when the app can't
>> be reached. This issue existed prior to this patch and should be
>> prioritized.
>>
>> https://redmine.postgresql.org/issues/2640
>>
>> Thanks!
>> Matt & Sarah
>>
>> On Wed, Aug 9, 2017 at 4:06 PM, Sarah McAlear 
>> wrote:
>>
>>> Hi Surinder!
>>>
>>> I am not able to see anything different from what I see on Master
>>> with or without the patch applied. I tried adjusting the preferences. I 
>>> did
>>> update the dashboard.js to instantiate a new object, great idea!
>>>
>>> Thanks,
>>> Sarah
>>>
>>>
>>>
>>> On Wed, Aug 9, 2017 at 1:42 PM, Surinder Kumar <
>>> surinder.ku...@enterprisedb.com> wrote:
>>>
 Hi Wenlin,

 On Tue, Aug 8, 2017 at 3:15 PM, Wenlin Zhang 
 wrote:

> Hi Surinder,
>
>Thanks for your review.
>
>We have changed the indentation for _dashboard.scss file and
> also removed the style about icon-postgres:before, like
> margin-top,etc, but we are not sure if it is perfectly aligned now, 
> you can
> add further change to it.
>
> As the second comment, I'm sorry I'm not sure what's the
> problem, could you please clarify it? Because we replace css with scss
> right now, dashboard.css doesn't exist. So maybe you are looking at 
> the css
> file that are complied by the scss?
>
 ​Sorry I​ typed 'dashboard.css' instead of 'dashboard.js'.
 ​In dashboard.js can we change `return DashboardAlert;` to `return
 new DashboardAlert();`
 and then we can remove the instances being created(var
 alertDashboard = new AlertDashboard();) from dashboard.js, and simply
 use `AlertDashboard.info('message')`.


> For the fourth comment, we tried the steps you suggested on
> master branch, the error is not shown either. So it should be an 
> existing
> issue. But if you want to see the error message, navigate to 
> "Servers" at
> the top of browser, then navigate back to postgresql server, then you 
> will
> see the error message.
>
 ​The error in console will appear when you selected a database
 which is connected and stop the backend Python server because the 
 request
 for graphs for database level will fail and there is no response 
 returned
 from server.
 It might be not reproducible to you If you set the refresh rate to
 higher value (i.e. Preferences > Graphs) in preferences. Just set 
 refresh
 rate to 1 for all dashboard graphs and repeat the steps.

>
>
> Thanks,
>
> Wenlin &Violet
>
>
>
> On Mon, Aug 7, 2017 at 2:30 PM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi
>> Review comments:
>>
>>1.
>>
>>For consistency, we use two spaces for indentation in CSS
>>files. Four spaces are used in _dashboard.scss file. The
>>configurations are defined in web/.editorconfig file.
>>2.
>>
>>In,dashboard.css Can we return function object in return
>>instead of function class itself, this will eliminate 

Re: [pgAdmin4][RM2586] Cleanup feature test

2017-08-10 Thread Sarah McAlear
Great job on cleaning up the whole feature test suite! This is really going
to benefit all of the developers a lot.

Thanks,
Matt & Sarah

On Fri, Aug 11, 2017 at 12:24 PM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached updated patch. In this patch I have removed unused
> imports.
> Regarding ajax calls on slow network/computer I have already taken care of
> those.
> Before sending first version of patch I tested this on Murtuza's machine
> and also
> tested on slow machine as well.
>
>
> Thanks,
> Harshal
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Aug 7, 2017 at 8:55 AM, Sarah McAlear  wrote:
>
>> Hi Harshal!
>>
>> There are a few files in which there are leftover imports that are not
>> needed anymore:
>>
>>- PGDataypeFeatureTest
>>- PgadminPage
>>- CheckForXssFeatureTest
>>- CheckDebuggerForXssFeatureTest
>>
>>
>> We also noticed that there were quite a few time.sleep functions that
>> were removed. This seems great overall, but we think that some of them were
>> in place because of varying network and computer speeds. So for example, in
>> QueryToolFeatureTest, there is an ajax call that was followed by a
>> time.sleep to ensure that it had finished executing before continuing to
>> execute. Removing this may reintroduce some flakiness. If there are no
>> issues with flakiness after this patch, it seems like a great idea. We ran
>> the tests a few times and didn't notice any flakiness, but we're unsure if
>> it will be a problem on a different system.
>>
>> Thanks!
>> Wenlin & Sarah
>>
>>
>> On Wed, Aug 2, 2017 at 9:32 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find attached patch to improve feature test execution time.
>>> Now on my machine overall execution time is cut down to 280 seconds from
>>> 400+ seconds
>>>
>>> Changes:
>>>
>>> 1. Removed fixed python time.sleeps where ever possible.
>>> 2. Removed connect to server test cases.
>>> 3. Query tool test cases:
>>>  i. Merged 3 test cases On demand result on scroll, grid select all
>>> and column select all.
>>>  ii. Merged 3 test cases Explain query, Explain query with verbose
>>> and Explain query with cost.
>>>  iii. Merged 3 test cases Explain analyze query, Explain
>>> analyze with buffers and Explain analyze with timing.
>>> 4. Improved debugger XSS test case execution time.
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>


Re: [pgAdmin4][patch] extract generate_url function from node.js and collection.js

2017-08-10 Thread Ashesh Vashi
On Thu, Aug 10, 2017 at 1:15 PM, Violet Cheng  wrote:

> Hi hackers,
>
> We tried to extract and refactor generate_url function in node.js and
> collection.js. Please see the patch attached.
>
Khushboo,

Please review this one.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company



*http://www.linkedin.com/in/asheshvashi*


>
> Thanks,
> Violet & Sarah
>


Re: [pgadmin4][patch] Fix the style of the alert when connecting/disconnecting from a server

2017-08-10 Thread Surinder Kumar
Hi

I reviewed this patch and it looks good to me.

Thanks,
Surinder

On Fri, Aug 11, 2017 at 9:13 AM, Sarah McAlear  wrote:

> Hi Hackers!
>
> This patch applies to RM 2644 .
> It fixes the styling of the checkmark area.
>
> Thanks!
> Sarah
>
>
>