[pgAdmin4][RM#3014] Fix validation issues while creating new sequence

2018-02-21 Thread Murtuza Zabuawala
Hi,

PFA patches,
1) To fix the validation issue while creating new sequence
RM#3014

2) Fix PEP-8 issues in sequence module
pycodestyle --config=.pycodestyle
./pgadmin/browser/server_groups/servers/databases/schemas/sequences/

3) To fix the regression test for Tablespace when running with Python3.x
RM#3138

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git 
a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/sql/default/create.sql
 
b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/sql/default/create.sql
index 0444a0d..e58507a 100644
--- 
a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/sql/default/create.sql
+++ 
b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/sql/default/create.sql
@@ -1,18 +1,16 @@
-{% if data %}
-CREATE SEQUENCE {{ conn|qtIdent(data.schema) }}.{{ conn|qtIdent(data.name) }}
-{% if data.cycled and data.cycled == True %}
-CYCLE
-{% endif %}
-{% if data.increment is defined %}
-INCREMENT {{data.increment}}
-{% endif %}{% if data.start is defined %}
-START {{data.start}}
-{% elif data.current_value is defined %}
-START {{data.current_value}}
-{% endif %}{% if data.minimum is defined %}
-MINVALUE {{data.minimum}}
-{% endif %}{% if data.maximum is defined %}
-MAXVALUE {{data.maximum}}
-{% endif %}{% if data.cache is defined %}
+CREATE SEQUENCE {{ conn|qtIdent(data.schema, data.name) }}{% if data.increment 
is defined and data.cycled %}
+
+CYCLE{% endif %}{% if data.increment is defined and data.increment is 
number %}
+
+INCREMENT {{data.increment}}{% endif %}{% if data.start is defined and 
data.start is number %}
+
+START {{data.start}}{% elif data.current_value is defined and 
data.current_value is number %}
+
+START {{data.current_value}}{% endif %}{% if data.minimum is defined and 
data.minimum is number %}
+
+MINVALUE {{data.minimum}}{% endif %}{% if data.maximum is defined and 
data.maximum is number %}
+
+MAXVALUE {{data.maximum}}{% endif %}{% if data.cache is defined and 
data.cache is number %}
+
 CACHE {{data.cache}}{% endif %};
-{% endif %}
+
diff --git 
a/web/pgadmin/browser/server_groups/servers/tablespaces/tests/test_backend_supported.py
 
b/web/pgadmin/browser/server_groups/servers/tablespaces/tests/test_backend_supported.py
index 3aa43e2..9530f8f 100644
--- 
a/web/pgadmin/browser/server_groups/servers/tablespaces/tests/test_backend_supported.py
+++ 
b/web/pgadmin/browser/server_groups/servers/tablespaces/tests/test_backend_supported.py
@@ -6,8 +6,12 @@
 # This software is released under the PostgreSQL Licence
 #
 ##
+import sys
 
-from mock import MagicMock
+if sys.version_info < (3, 3):
+from mock import MagicMock
+else:
+from unittest.mock import MagicMock
 
 from pgadmin.browser.server_groups.servers.tablespaces import TablespaceModule
 from pgadmin.utils.route import BaseTestGenerator
@@ -43,4 +47,6 @@ class BackendSupportedTestCase(BaseTestGenerator):
 manager = MagicMock()
 manager.sversion = self.manager['sversion']
 manager.server_type = self.manager['server_type']
-self.assertEquals(self.expected_result, 
module.BackendSupported(manager))
+self.assertEquals(
+self.expected_result, module.BackendSupported(manager)
+)
diff --git 
a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/__init__.py
 
b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/__init__.py
index 25722d6..f49c2d7 100644
--- 
a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/__init__.py
+++ 
b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/__init__.py
@@ -11,7 +11,6 @@
 
 import simplejson as json
 from functools import wraps
-
 import pgadmin.browser.server_groups.servers.databases as database
 from flask import render_template, make_response, request, jsonify
 from flask_babel import gettext as _
@@ -151,7 +150,9 @@ class SequenceView(PGChildNodeView):
 else:
 self.conn = self.manager.connection()
 
-self.template_path = 
'sequence/sql/#{0}#'.format(self.manager.version)
+self.template_path = 'sequence/sql/#{0}#'.format(
+self.manager.version
+)
 self.acl = ['r', 'w', 'U']
 self.qtIdent = driver.qtIdent
 
@@ -162,7 +163,8 @@ class SequenceView(PGChildNodeView):
 @check_precondition(action='list')
 def list(self, gid, sid, did, scid):
 """
-This function is used to list all the sequence nodes within the 
collection.
+This function is used to list all the sequence nodes within the
+collection.
 
 Args:
   gid: Ser

Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

2018-02-21 Thread Harshal Dhumal
Hi,

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

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

On Tue, Feb 20, 2018 at 10:34 PM, Dave Page  wrote:

> Hi
>
> On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find attached patch to enable keyboard navigation in dialog.
>>
>> To allow navigation from one tab pane (bootstrap tab pane) to another one
>> I have added two new shortcut preferences *Dialog tab previous *(
>> shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
>> and forward tab navigation.
>>
>> Also all dialog controls (within same tab pane) can be navigated using
>> TAB key.
>>
>
>

> This seems unreliable to me - for example, it keeps getting stuck on the
> connection tab on the server properties dialog.
>
>

> Also, can we use the same wording as for the tabbed panel navigation
> please? E.g. Next/Previous instead of Forward/Back.
>

I have fixed all of above issues. Please find updated patch.

Thanks,


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/docs/en_US/keyboard_shortcuts.rst b/docs/en_US/keyboard_shortcuts.rst
index 1142975..7699798 100644
--- a/docs/en_US/keyboard_shortcuts.rst
+++ b/docs/en_US/keyboard_shortcuts.rst
@@ -41,6 +41,21 @@ When using main browser window, the following keyboard shortcuts are available:
 | Alt+Shift+G   | Direct debugging   |
 +---++
 
+
+**Dialog tab shortcuts**
+
+When any dialog which has bootstrap tabs (nav tabs) below shortcuts are
+available to navigate within them:
+
++---++
+| Shortcut for all platform | Function   |
++===++
+| Control+Shift+[   | Dialog tab backward|
++---++
+| Control+Shift+]   | Dialog tab forward |
++---++
+
+
 **SQL Editors**
 
 When using the syntax-highlighting SQL editors, the following shortcuts are available:
diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py
index 1d0374a..d827a49 100644
--- a/web/pgadmin/browser/__init__.py
+++ b/web/pgadmin/browser/__init__.py
@@ -430,6 +430,36 @@ class BrowserModule(PgAdminModule):
 fields=fields
 )
 
+self.preference.register(
+'keyboard_shortcuts',
+'dialog_tab_forward',
+gettext('Dialog tab forward'),
+'keyboardshortcut',
+{
+'alt': False,
+'shift': True,
+'control': True,
+'key': {'key_code': 93, 'char': ']'}
+},
+category_label=gettext('Keyboard shortcuts'),
+fields=fields
+)
+
+self.preference.register(
+'keyboard_shortcuts',
+'dialog_tab_backward',
+gettext('Dialog tab backward'),
+'keyboardshortcut',
+{
+'alt': False,
+'shift': True,
+'control': True,
+'key': {'key_code': 91, 'char': '['}
+},
+category_label=gettext('Keyboard shortcuts'),
+fields=fields
+)
+
 def get_exposed_url_endpoints(self):
 """
 Returns:
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 13af4ea..cf739bb 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -1951,7 +1951,25 @@ define('pgadmin.browser', [
   brace_matching: pgBrowser.utils.braceMatching,
   indent_with_tabs: pgBrowser.utils.is_indent_with_tabs,
 },
+find_and_set_focus: function(container) {
+  if (container.length == 0) {
+return;
+  }
+  setTimeout(function() {
+var first_el = container
+  .find('button.fa-plus:first');
+
+if (first_el.length == 0) {
+  first_el = container.find('.pgadmin-controls:first>input,.CodeMirror-scroll');
+}
 
+if(first_el.length > 0) {
+  first_el[0].focus();
+} else {
+  container[0].focus();
+}
+  }, 500);
+},
   });
 
 /* Remove paste event mapping from CodeMirror's emacsy KeyMap binding
diff --git a/web/pgadmin/browser/static/js/keyboard.js b/web/pgadmin/browser/static/js/keyboard.js
index 95b77db..982ef48 100644
--- a/web/pgadmin/browser/static/js/keyboa

Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool

2018-02-21 Thread Joao De Almeida Pereira
Hello Murtuza,
After reviewing the code I have some suggestions:
 - We should split the PEP and the features into different patches, or else
it becomes very hard to separate Feature code from Code Style change
 - The function: `getTextRepresentaionShortcut` has a typo
 - As a personal point I have a hard time reading multiple declarations of
variables in javascript under a single `var`, I prefer 1 `let` per variable
 - I think we should use cameCase for our variables in Javascript code
 - What is the reason behind the move of the key shortcuts back into the
SQLEditorController? This look like something that could be extracted from
SQLEditor file into its own
 - Looks like we are missing some test coverage on the new implemented parts
 - The getKeyboardShortcuts function is retrieving information from
window.top and window.opener I think that we can come up with a better
pattern then this. Relying on window information doesn't look very good.
That was the pattern for JQuery that the new frameworks are trying to avoid
because polluting the global scope is almost always a Code Smell.


I see there was some refactoring on the backend with the creation of
RegisterQueryToolPreferences and it is looking good, hope that we can do
this in more places specially in the front-end.

Using the patch that you sent I passed it through our CI and everything is
Passing.

Thanks
Joao

On Tue, Feb 20, 2018 at 1:13 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch for adding accessibility features in query tool.
> RM#2900
>
> Please review.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>


[pgadmin4] Priorities of features

2018-02-21 Thread Joao De Almeida Pereira
Hi Hackers,

After the last month or something that I came back to this project I
noticed that we put a lot of effort in making all these shortcuts
configurable.
But I do not understand the driver for this push.

Do we have people complaining about the lack of shortcuts?
Are people complaining about the Key Combinations that we selected?
Do we have a clear idea of what the people that use pgAdmin want?

I would love to understand the driver for this. There is already a lot of
code done for this, I know, but in my mind there are much more pressing
issues in our code and feature wise that we need to address like:
- Code maintainability
- Continuous Integration
- Continuous delivery, more frequent releases in a automated way
- What happens when you have 500 tables
- What happens when you have 500 partitions in tables and so on.
- We need to understand what the Users from pgAdmin need

I would love to hear from the community to understand if we are creating a
product that is easy to use for them and that brings them value. Also we
need to grow the developer community for pgAdmin and that only happens when
the application is maintainable and uses patterns that people understand
easily without having to do hours and hours of code archeology to add a
feature that is a bit more involved.

Thanks
Joao


Re: [pgAdmin4][Patch]: RM #3077 - ERROR: invalid byte sequence for encoding "LATIN1":0x00

2018-02-21 Thread Joao De Almeida Pereira
Hi
The patch looks good, do we have any example error that we can test in
order to ensure the problem no longer shows up in the future?
If they could be Unit Tests instead of Feature tests that would be awesome,
because Feature tests are much more expensive then Unit.

Thanks
Joao

On Wed, Feb 21, 2018 at 2:33 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix RM #3077 : ERROR: invalid byte
> sequence for encoding "LATIN1":0x00
>
> Thanks,
> Khushboo
>


Re: [pgAdmin4][RM#3014] Fix validation issues while creating new sequence

2018-02-21 Thread Joao De Almeida Pereira
Hi Murtuza,
Looks good.
Was there any reason for the change in position of the if statements in the
sql file. Makes the output more readable?
Looks like we need some test to ensure that the Sequence issue does not
happen again. Maybe a Unit Test is good enough, to ensure that we are
creating a good SQL

Thanks
Joao

On Wed, Feb 21, 2018 at 3:13 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patches,
> 1) To fix the validation issue while creating new sequence
> RM#3014
>
> 2) Fix PEP-8 issues in sequence module
> pycodestyle --config=.pycodestyle
> ./pgadmin/browser/server_groups/servers/databases/schemas/sequences/
>
> 3) To fix the regression test for Tablespace when running with Python3.x
> RM#3138
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>


Re: [pgadmin4] Priorities of features

2018-02-21 Thread Dave Page
Hi

On Wed, Feb 21, 2018 at 3:42 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
>
> After the last month or something that I came back to this project I
> noticed that we put a lot of effort in making all these shortcuts
> configurable.
> But I do not understand the driver for this push.
>
> Do we have people complaining about the lack of shortcuts?
> Are people complaining about the Key Combinations that we selected?
> Do we have a clear idea of what the people that use pgAdmin want?
>
> I would love to understand the driver for this. There is already a lot of
> code done for this, I know, but in my mind there are much more pressing
> issues in our code and feature wise that we need to address like:
> - Code maintainability
> - Continuous Integration
> - Continuous delivery, more frequent releases in a automated way
> - What happens when you have 500 tables
> - What happens when you have 500 partitions in tables and so on.
> - We need to understand what the Users from pgAdmin need
>
> I would love to hear from the community to understand if we are creating a
> product that is easy to use for them and that brings them value. Also we
> need to grow the developer community for pgAdmin and that only happens when
> the application is maintainable and uses patterns that people understand
> easily without having to do hours and hours of code archeology to add a
> feature that is a bit more involved.
>

We (EDB) have legal obligations in some countries to ensure software we
supply is accessible to all users - not to mention that it's just good
practice to do so. We have users that need this so that's where we've been
focussing some of our efforts for a few sprints.

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

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


Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

2018-02-21 Thread Joao De Almeida Pereira
Hello Harshal,

I passed the patch through our CI and all the tests passed. The changes do
not break previous behavior but because there are no tests on the new
feature  we could not be sure it was really working. So we did some manual
testing and sometimes it doesn't work, like it gets stuck in a place and
you need to press the shortcut again in order for it to move.

Codewise I have some suggestions:
 - dialogTabNavigator looks a nice candidate for a class with its own file.
This way we can test the behavior
 - There is no difference between a variable called e and a variable error
so for sake of clarity I would love to see variable names that we can
easily read
 - We are also using 2 different types of variable naming camelCase and
snake_case, if we could use only camelCase on Javascript it would make the
code more uniform
 - I noticed that there are some linting issues in the Javascript code

Summing it up I believe that despite the feature not working 100% of the
time, looks like the code is almost there but needs some refactoring to
make it more readable, instead of comments we could have function calls
that more clearly state what we are looking for something like
DialogTabNavigator.isLastTabOfChild
​
​


Thanks
Joao

On Wed, Feb 21, 2018 at 4:32 AM Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Tue, Feb 20, 2018 at 10:34 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find attached patch to enable keyboard navigation in dialog.
>>>
>>> To allow navigation from one tab pane (bootstrap tab pane) to another one
>>> I have added two new shortcut preferences *Dialog tab previous *(
>>> shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
>>> and forward tab navigation.
>>>
>>> Also all dialog controls (within same tab pane) can be navigated using
>>> TAB key.
>>>
>>
>>
>
>> This seems unreliable to me - for example, it keeps getting stuck on the
>> connection tab on the server properties dialog.
>>
>>
>
>> Also, can we use the same wording as for the tabbed panel navigation
>> please? E.g. Next/Previous instead of Forward/Back.
>>
>
> I have fixed all of above issues. Please find updated patch.
>
> Thanks,
>
>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>


Re: [pgAdmin4][RM#3014] Fix validation issues while creating new sequence

2018-02-21 Thread Dave Page
On Wed, Feb 21, 2018 at 3:54 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Murtuza,
> Looks good.
> Was there any reason for the change in position of the if statements in
> the sql file. Makes the output more readable?
> Looks like we need some test to ensure that the Sequence issue does not
> happen again. Maybe a Unit Test is good enough, to ensure that we are
> creating a good SQL
>

I think we need to come up with a framework or standard method for testing
the generated SQL and reverse engineered SQL for all objects. That's been
bugging me for a while - the problem is that for just a single object type
there may be thousands of possible combinations of options to test to
ensure complete coverage.

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

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


Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

2018-02-21 Thread Dave Page
On Wed, Feb 21, 2018 at 4:22 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Harshal,
>
> I passed the patch through our CI and all the tests passed. The changes do
> not break previous behavior but because there are no tests on the new
> feature  we could not be sure it was really working. So we did some manual
> testing and sometimes it doesn't work, like it gets stuck in a place and
> you need to press the shortcut again in order for it to move.
>

Was that with the updated patch? It sounds like the issue I saw with the
original one.


>
> Codewise I have some suggestions:
>  - dialogTabNavigator looks a nice candidate for a class with its own
> file. This way we can test the behavior
>  - There is no difference between a variable called e and a variable error
> so for sake of clarity I would love to see variable names that we can
> easily read
>  - We are also using 2 different types of variable naming camelCase and
> snake_case, if we could use only camelCase on Javascript it would make the
> code more uniform
>  - I noticed that there are some linting issues in the Javascript code
>
> Summing it up I believe that despite the feature not working 100% of the
> time, looks like the code is almost there but needs some refactoring to
> make it more readable, instead of comments we could have function calls
> that more clearly state what we are looking for something like
> DialogTabNavigator.isLastTabOfChild
> ​
> ​
>
>
> Thanks
> Joao
>
> On Wed, Feb 21, 2018 at 4:32 AM Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Tue, Feb 20, 2018 at 10:34 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find attached patch to enable keyboard navigation in dialog.

 To allow navigation from one tab pane (bootstrap tab pane) to another
 one
 I have added two new shortcut preferences *Dialog tab previous *(
 shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
 and forward tab navigation.

 Also all dialog controls (within same tab pane) can be navigated using
 TAB key.

>>>
>>>
>>
>>> This seems unreliable to me - for example, it keeps getting stuck on the
>>> connection tab on the server properties dialog.
>>>
>>>
>>
>>> Also, can we use the same wording as for the tabbed panel navigation
>>> please? E.g. Next/Previous instead of Forward/Back.
>>>
>>
>> I have fixed all of above issues. Please find updated patch.
>>
>> Thanks,
>>
>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>


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

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


pgAdmin 4 commit: Don't depend on standards_conforming_strings being en

2018-02-21 Thread Dave Page
Don't depend on standards_conforming_strings being enabled. Fixes #3077

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=b49d625c2d0243df6c5ad7d11470cfd0b9ada867
Author: Khushboo Vashi 

Modified Files
--
.../schemas/tables/templates/trigger/sql/10_plus/properties.sql | 2 +-
.../schemas/tables/templates/trigger/sql/default/properties.sql | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)



Re: [pgAdmin4][Patch]: RM #3077 - ERROR: invalid byte sequence for encoding "LATIN1":0x00

2018-02-21 Thread Dave Page
Thanks, patch applied.

On Wed, Feb 21, 2018 at 7:33 AM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix RM #3077 : ERROR: invalid byte
> sequence for encoding "LATIN1":0x00
>
> Thanks,
> Khushboo
>



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

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


Re: [pgAdmin4][Patch]: RM #3077 - ERROR: invalid byte sequence for encoding "LATIN1":0x00

2018-02-21 Thread Dave Page
On Wed, Feb 21, 2018 at 3:45 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi
> The patch looks good, do we have any example error that we can test in
> order to ensure the problem no longer shows up in the future?
> If they could be Unit Tests instead of Feature tests that would be
> awesome, because Feature tests are much more expensive then Unit.
>

I think we need to consider a couple of options here:

1) Make sure that many/all of our tests use non-ASCII names.

2) Consider adding a mechanism to allow running the regression tests with
overridden GUC values. For example, a parameter in the test config file
that gives a set of GUCs to change upon connection.

The downside of 2 of course, is that it adds a huge amount of possible
environment combinations to test.

Thoughts?

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

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


Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

2018-02-21 Thread Joao De Almeida Pereira
Yep I installed the V2 file

On Wed, Feb 21, 2018 at 11:31 AM Dave Page  wrote:

> On Wed, Feb 21, 2018 at 4:22 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Harshal,
>>
>> I passed the patch through our CI and all the tests passed. The changes
>> do not break previous behavior but because there are no tests on the new
>> feature  we could not be sure it was really working. So we did some manual
>> testing and sometimes it doesn't work, like it gets stuck in a place and
>> you need to press the shortcut again in order for it to move.
>>
>
> Was that with the updated patch? It sounds like the issue I saw with the
> original one.
>
>
>>
>> Codewise I have some suggestions:
>>  - dialogTabNavigator looks a nice candidate for a class with its own
>> file. This way we can test the behavior
>>  - There is no difference between a variable called e and a variable
>> error so for sake of clarity I would love to see variable names that we
>> can easily read
>>  - We are also using 2 different types of variable naming camelCase and
>> snake_case, if we could use only camelCase on Javascript it would make the
>> code more uniform
>>  - I noticed that there are some linting issues in the Javascript code
>>
>> Summing it up I believe that despite the feature not working 100% of the
>> time, looks like the code is almost there but needs some refactoring to
>> make it more readable, instead of comments we could have function calls
>> that more clearly state what we are looking for something like
>> DialogTabNavigator.isLastTabOfChild
>> ​
>> ​
>>
>>
>> Thanks
>> Joao
>>
>> On Wed, Feb 21, 2018 at 4:32 AM Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Tue, Feb 20, 2018 at 10:34 PM, Dave Page  wrote:
>>>
 Hi

 On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
 harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch to enable keyboard navigation in dialog.
>
> To allow navigation from one tab pane (bootstrap tab pane) to another
> one
> I have added two new shortcut preferences *Dialog tab previous *(
> shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
> and forward tab navigation.
>
> Also all dialog controls (within same tab pane) can be navigated using
> TAB key.
>


>>>
 This seems unreliable to me - for example, it keeps getting stuck on
 the connection tab on the server properties dialog.


>>>
 Also, can we use the same wording as for the tabbed panel navigation
 please? E.g. Next/Previous instead of Forward/Back.

>>>
>>> I have fixed all of above issues. Please find updated patch.
>>>
>>> Thanks,
>>>
>>>

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

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

>>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


pgAdmin 4 commit: Fix validation of sequence parameters. Fixes #3014

2018-02-21 Thread Dave Page
Fix validation of sequence parameters. Fixes #3014

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f8771d55852eb66872319dcd6fbb94e397347ea0
Author: Murtuza Zabuawala 

Modified Files
--
.../templates/sequence/sql/default/create.sql  | 32 ++
1 file changed, 15 insertions(+), 17 deletions(-)



pgAdmin 4 commit: Fix tablespace tests for Python 3.x. Fixes #3138

2018-02-21 Thread Dave Page
Fix tablespace tests for Python 3.x. Fixes #3138

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=51cc04b5b0c10d8ad693ee036009d0a6323e0f48
Author: Murtuza Zabuawala 

Modified Files
--
.../servers/tablespaces/tests/test_backend_supported.py| 10 --
1 file changed, 8 insertions(+), 2 deletions(-)



pgAdmin 4 commit: PEP8 cleanups for the sequences module.

2018-02-21 Thread Dave Page
PEP8 cleanups for the sequences module.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=55875f0cfa7b19cdd4831150e8232e53df6f9ef3
Author: Murtuza Zabuawala 

Modified Files
--
.../databases/schemas/sequences/__init__.py| 105 +++--
.../databases/schemas/sequences/tests/__init__.py  |   5 +-
2 files changed, 78 insertions(+), 32 deletions(-)



Re: [pgAdmin4][RM#3014] Fix validation issues while creating new sequence

2018-02-21 Thread Dave Page
Thanks, applied.

On Wed, Feb 21, 2018 at 8:13 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patches,
> 1) To fix the validation issue while creating new sequence
> RM#3014
>
> 2) Fix PEP-8 issues in sequence module
> pycodestyle --config=.pycodestyle ./pgadmin/browser/server_
> groups/servers/databases/schemas/sequences/
>
> 3) To fix the regression test for Tablespace when running with Python3.x
> RM#3138
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>


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

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


Re: [pgAdmin4][RM#3014] Fix validation issues while creating new sequence

2018-02-21 Thread Murtuza Zabuawala
Hi Joao,

​Thank you for your time in reviewing the patch.​


On Wed, Feb 21, 2018 at 9:24 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Murtuza,
> Looks good.
> Was there any reason for the change in position of the if statements in
> the sql file. Makes the output more readable?
>
​Yes​


> Looks like we need some test to ensure that the Sequence issue does not
> happen again. Maybe a Unit Test is good enough, to ensure that we are
> creating a good SQL
>
​I'll add a test case for the same.​


>
> Thanks
> Joao
>
> On Wed, Feb 21, 2018 at 3:13 AM Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patches,
>> 1) To fix the validation issue while creating new sequence
>> RM#3014
>>
>> 2) Fix PEP-8 issues in sequence module
>> pycodestyle --config=.pycodestyle ./pgadmin/browser/server_
>> groups/servers/databases/schemas/sequences/
>>
>> 3) To fix the regression test for Tablespace when running with Python3.x
>> RM#3138
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>


Re: Regarding PEP-8 checker

2018-02-21 Thread Dave Page
On Tue, Feb 20, 2018 at 6:28 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi All,
>
> Earlier we have fixed PEP-8 issues in below modules,
> pycodestyle --config=.pycodestyle ./pgadmin/tools/
> pycodestyle --config=.pycodestyle ./pgadmin/utils/
> pycodestyle --config=.pycodestyle ./pgadmin/misc/
> pycodestyle --config=.pycodestyle ./pgadmin/about/
> pycodestyle --config=.pycodestyle ./pgadmin/dashboard/
> pycodestyle --config=.pycodestyle ./pgadmin/feature_tests/
> pycodestyle --config=.pycodestyle ./regression/
> pycodestyle --config=.pycodestyle ./pgadmin/setup/
> pycodestyle --config=.pycodestyle ./pgadmin/settings/
> pycodestyle --config=.pycodestyle ./pgadmin/redirects/
> pycodestyle --config=.pycodestyle ./pgadmin/preferences/
> pycodestyle --config=.pycodestyle ./pgadmin/model/
> pycodestyle --config=.pycodestyle ./pgadmin/help/
>
>
> But today when I ran PEP-8 checker on above modules I see many PEP-8
> errors :(
>
> It's a humble request to you to run PEP-8 checker on your changes (If you
> have made changes in any of above mentioned modules), so that we don't
> waste our efforts in the same modules again and again.
>

+.

I would also ask that everyone on the EDB team also take a few minutes to
cleanup 3 or 4 modules per day. Once we have everything in a clean state,
we'll add a PEP8 codestyle check to the Jenkins tests to help keep it clean.

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

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


Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool

2018-02-21 Thread Murtuza Zabuawala
Hi Joao,

Thank you for your time in reviewing the patch.


On Wed, Feb 21, 2018 at 9:11 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Murtuza,
> After reviewing the code I have some suggestions:
>  - We should split the PEP and the features into different patches, or
> else it becomes very hard to separate Feature code from Code Style change
>
​I'll keep that in mind next time onwards.

>  - The function: `getTextRepresentaionShortcut` has a typo
>
​I'll fix​

​that.​

>  - As a personal point I have a hard time reading multiple declarations of
> variables in javascript under a single `var`, I prefer 1 `let` per variable
>
 - I think we should use cameCase for our variables in Javascript code
>  - What is the reason behind the move of the key shortcuts back into the
> SQLEditorController? This look like something that could be extracted from
> SQLEditor file into its own
>
I'll try to move that code.​


>  - Looks like we are missing some test coverage on the new implemented
> parts
>
​I didn't added any new code as such, I just moved out preferences code out
from main file.

>  - The getKeyboardShortcuts function is retrieving information from
> window.top and window.opener I think that we can come up with a better
> pattern then this. Relying on window information doesn't look very good.
> That was the pattern for JQuery that the new frameworks are trying to avoid
> because polluting the global scope is almost always a Code Smell.
>
​What do you suggest on this?​


>
>
> I see there was some refactoring on the backend with the creation of
> RegisterQueryToolPreferences and it is looking good, hope that we can do
> this in more places specially in the front-end.
>
> Using the patch that you sent I passed it through our CI and everything is
> Passing.
>
> Thanks
> Joao
>
> On Tue, Feb 20, 2018 at 1:13 PM Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch for adding accessibility features in query tool.
>> RM#2900
>>
>> Please review.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>


Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

2018-02-21 Thread Harshal Dhumal
Hi,

On Wed, Feb 21, 2018 at 10:59 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Yep I installed the V2 file
>
> On Wed, Feb 21, 2018 at 11:31 AM Dave Page  wrote:
>
>> On Wed, Feb 21, 2018 at 4:22 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Harshal,
>>>
>>> I passed the patch through our CI and all the tests passed. The changes
>>> do not break previous behavior but because there are no tests on the new
>>> feature  we could not be sure it was really working. So we did some manual
>>> testing and sometimes it doesn't work, like it gets stuck in a place and
>>> you need to press the shortcut again in order for it to move.
>>>
>>
>> It stuck because I have to wait until next tab is completely visible
(fade in effect is completed).
The fade in or fade out transition duration is 150 ms (set by bootstrap).
So I can not set focus back to tab pane
until fade in or fade out transition is completed. May be one improvement I
can do is to reduce wait time to
something 200 ms (currently it's 500 ms).

Note that the original issue reported by Dave is already fixed in updated
patch.


> Was that with the updated patch? It sounds like the issue I saw with the
>> original one.
>>
>>
>>>
>>> Codewise I have some suggestions:
>>>  - dialogTabNavigator looks a nice candidate for a class with its own
>>> file. This way we can test the behavior
>>>
>> Ok I'll move dialogTabNavigator to new file and will add test cases.

>  - There is no difference between a variable called e and a variable error
>>> so for sake of clarity I would love to see variable names that we can
>>> easily read
>>>  - We are also using 2 different types of variable naming camelCase and
>>> snake_case, if we could use only camelCase on Javascript it would make the
>>> code more uniform
>>>
>> Ok I'll do this.


>  - I noticed that there are some linting issues in the Javascript code
>>>
>> I just found that linter was disabled for this file by adding comment  /*
eslint-disable */ at first line. (not sure why we did that)


>>> Summing it up I believe that despite the feature not working 100% of the
>>> time, looks like the code is almost there but needs some refactoring to
>>> make it more readable, instead of comments we could have function calls
>>> that more clearly state what we are looking for something like
>>> DialogTabNavigator.isLastTabOfChild
>>> ​
>>> ​
>>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Wed, Feb 21, 2018 at 4:32 AM Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi,

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

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

 On Tue, Feb 20, 2018 at 10:34 PM, Dave Page  wrote:

> Hi
>
> On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find attached patch to enable keyboard navigation in dialog.
>>
>> To allow navigation from one tab pane (bootstrap tab pane) to another
>> one
>> I have added two new shortcut preferences *Dialog tab previous *(
>> shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
>> and forward tab navigation.
>>
>> Also all dialog controls (within same tab pane) can be navigated
>> using TAB key.
>>
>
>

> This seems unreliable to me - for example, it keeps getting stuck on
> the connection tab on the server properties dialog.
>
>

> Also, can we use the same wording as for the tabbed panel navigation
> please? E.g. Next/Previous instead of Forward/Back.
>

 I have fixed all of above issues. Please find updated patch.

 Thanks,


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>


Re: [pgAdmin4][RM#2900] Adding accessibility features in query tool

2018-02-21 Thread Joao De Almeida Pereira
Hello Murtuza,
I created a small patch on the way I believe the function should work. Not
100% sure why we are piggybacking on the window.top, window.opener and so
on to give us information. Nowadays I do not think that loading the JS
files again costs that much in terms of time.

The issue I came across was that cache_preferences/get_preference behave in
a very strange way.

Example:
If the cache is populated no problem it returns a value and it is fine
If the cache is not populated it sets a timeout that will check in 1 second
if is the cache is populated if it is not then does nothing, if it is then
returns the value.
The problem is that it returns the value to no place.

This caching implementation works until this point because for some good
fortune we never call get_preference before we have the cache full.
Is my assessment correct?

Nevertheless if the caching was using promises we could avoid the problem
above.

Thanks
Joao

On Wed, Feb 21, 2018 at 12:50 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Joao,
>
> Thank you for your time in reviewing the patch.
>
>
> On Wed, Feb 21, 2018 at 9:11 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Murtuza,
>> After reviewing the code I have some suggestions:
>>  - We should split the PEP and the features into different patches, or
>> else it becomes very hard to separate Feature code from Code Style change
>>
> ​I'll keep that in mind next time onwards.
>
>>  - The function: `getTextRepresentaionShortcut` has a typo
>>
> ​I'll fix​
>
> ​that.​
>
>>  - As a personal point I have a hard time reading multiple declarations
>> of variables in javascript under a single `var`, I prefer 1 `let` per
>> variable
>>
>  - I think we should use cameCase for our variables in Javascript code
>>  - What is the reason behind the move of the key shortcuts back into the
>> SQLEditorController? This look like something that could be extracted from
>> SQLEditor file into its own
>>
> I'll try to move that code.​
>
>
>>  - Looks like we are missing some test coverage on the new implemented
>> parts
>>
> ​I didn't added any new code as such, I just moved out preferences code
> out from main file.
>
>>  - The getKeyboardShortcuts function is retrieving information from
>> window.top and window.opener I think that we can come up with a better
>> pattern then this. Relying on window information doesn't look very good.
>> That was the pattern for JQuery that the new frameworks are trying to avoid
>> because polluting the global scope is almost always a Code Smell.
>>
> ​What do you suggest on this?​
>
>
>>
>>
>> I see there was some refactoring on the backend with the creation of
>> RegisterQueryToolPreferences and it is looking good, hope that we can do
>> this in more places specially in the front-end.
>>
>> Using the patch that you sent I passed it through our CI and everything
>> is Passing.
>>
>> Thanks
>> Joao
>>
>> On Tue, Feb 20, 2018 at 1:13 PM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch for adding accessibility features in query tool.
>>> RM#2900
>>>
>>> Please review.
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>


possible-solution.patch
Description: Binary data


[pgadmin4] PEP-8 on pgadmin/tools

2018-02-21 Thread Joao De Almeida Pereira
Hi hackers,
Attached, you can find the PEP-8 the pgadmin/tools fixes.

Thanks
Joao


pep8-on-pgadmin-tools.diff
Description: Binary data


Re: [pgAdmin4][Patch]: RM #3077 - ERROR: invalid byte sequence for encoding "LATIN1":0x00

2018-02-21 Thread Khushboo Vashi
On Wed, Feb 21, 2018 at 10:51 PM, Dave Page  wrote:

>
>
> On Wed, Feb 21, 2018 at 3:45 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi
>> The patch looks good, do we have any example error that we can test in
>> order to ensure the problem no longer shows up in the future?
>> If they could be Unit Tests instead of Feature tests that would be
>> awesome, because Feature tests are much more expensive then Unit.
>>
>
> I think we need to consider a couple of options here:
>
> 1) Make sure that many/all of our tests use non-ASCII names.
>
> +1

> 2) Consider adding a mechanism to allow running the regression tests with
> overridden GUC values. For example, a parameter in the test config file
> that gives a set of GUCs to change upon connection.
>
> The downside of 2 of course, is that it adds a huge amount of possible
> environment combinations to test.
>
> We should implement this as it would be very helpful in reducing the
testing efforts.
But as you said there will be large set of combinations, so first we need
to come up with the possible combinations which we would like to include
first.

> Thoughts?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


How to decrypt stored psql server password from pgadmin4.db?

2018-02-21 Thread Viper 12XN
Hi,

long story short, I need to recover a psql server password stored in
pgadmin4.db. I can see the encrypted password, but have no idea how to
decrypt it. It also seems salted. Please help.

Looking forward to hearing from you.

P.S Resetting the password in psql is not an option. I really
need to know the current password.


[pgAdmin4][Patch]: Remove webpack plugin hardSourceWebpackPlugin from the production environment

2018-02-21 Thread Khushboo Vashi
Hi,

Please find the attached patch to remove webpack plugin
hardSourceWebpackPlugin from the production environment.

Thanks,
Khushboo
diff --git a/web/webpack.config.js b/web/webpack.config.js
index b989a4c..84f8466 100644
--- a/web/webpack.config.js
+++ b/web/webpack.config.js
@@ -324,7 +324,6 @@ module.exports = {
 uglifyPlugin,
 optimizeAssetsPlugin,
 definePlugin,
-hardSourceWebpackPlugin,
 sourceMapDevToolPlugin,
   ]: [
 extractSass,