[pgAdmin4][RM#3014] Fix validation issues while creating new sequence
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)
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
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
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
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
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
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)
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
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)
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
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
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
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)
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
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
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.
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
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
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
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
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)
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
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
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
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?
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
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,