Re: [pgAdmin4][RM#3073] Allow user to schedule without End date from UI

[Murtuza Zabuawala]

Hi Dave,

I have found the issue, it was in Backform control itself :)
Issue: We were passing today's date value as minDate option in pgAgent
schedule schema while rendering the control, So when server was sending
previous datetime value it was not displaying it causing sync problem.

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


On Tue, Feb 27, 2018 at 8:39 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> ​Thanks Dave, I'll look into it.
> ​
>
>
> On Tue, Feb 27, 2018 at 8:37 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Feb 27, 2018 at 12:03 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> I'm not able to re-produce the issue, Could you hard refresh and try
>>> again?
>>>
>>
>> I still see it.
>>
>>
>>>
>>> If possible could you please provide exact steps?
>>>
>>
>> Attached is a dump of my test pgagent schema.
>>
>> 1) Load the schema dump into the maintenance database
>> 2) Connect pgAdmin
>> 3) Browse to pgAgent Jobs
>> 4) Right-click Properties, and select the Schedules tab
>> 5) Open the subnode for sched1
>>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Tue, Feb 27, 2018 at 5:30 PM, Dave Page  wrote:
>>>
 Sorry - here it is.

 On Tue, Feb 27, 2018 at 11:59 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Could you please send screenshot?
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Tue, Feb 27, 2018 at 5:24 PM, Dave Page  wrote:
>
>> Hi
>>
>> Still not quite right - see the attached screenshot which is the
>> result of simply viewing the properties of an existing job. Note that the
>> start time is shown in the grid but not the subnode control.
>>
>> Thanks.
>>
>> On Tue, Feb 27, 2018 at 8:26 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> As validation related patch was committed with RM#3148 [ Sorry about
>>> that I forgot to checkout :) ]
>>> PFA patch to fix the issues you mentioned, I have also removed extra
>>> error message from sub node collection control and made it optional via
>>> flag.
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Mon, Feb 26, 2018 at 10:14 PM, Dave Page 
>>> wrote:
>>>
 Hi

 On Mon, Feb 26, 2018 at 2:46 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue where user was not able to create
> pgAgent job from UI without entering End date in schedule section.
>

  Whilst this does resolve the validation issue, there are still a
 couple of other related problems, as can be seen in the attached
 screenshots:

 - The Start date/time in the subnode control doesn't seem to be
 properly synchronised with the value in the grid.

 - If you leave the End date/time blank (but maybe click into it
 first), the grid will show "Invalid date".

 It's possible there are other oddities as well - please check
 carefully for anything else.

 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
>>
>
>


 --
 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
>>
>
>
diff --git 
a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
 
b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
index a88f9d0..1cfa38c 100644
--- 
a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
+++ 
b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
@@ -279,7 +279,7 @@ define('pgadmin.node.pga_schedule', [
   control: 'datetimepicker', cell: 

Re: RM3079 fix for wrong sql datetime/time related datatypes

[Harshal Dhumal]

Hi,

On Tue, Feb 27, 2018 at 8:54 PM, Dave Page  wrote:

> Hi
>
> On Tue, Feb 27, 2018 at 2:36 PM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find patch to fix wrong sql issue for time related type.
>>
>> Steps to reproduce:
>>
>> Alter any time/datetime array related data type, it generates sql with
>> addition array bracket
>>
>> [image: Inline image 1]
>>
>
> This seems to be missing the test case that you noted should be included
> in the original bug report!
>

Please find updated patch with test cases.


Thanks,

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
index 54215d4..a2a8f17 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_add.py
@@ -24,7 +24,7 @@ from regression.python_test_utils import test_utils as utils
 class ColumnAddTestCase(BaseTestGenerator):
 """This class will add new column under table node."""
 scenarios = [
-('Add table Node URL', dict(url='/browser/column/obj/'))
+('Add column Node URL', dict(url='/browser/column/obj/'))
 ]
 
 def setUp(self):
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_delete.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_delete.py
index 28fdc7a..1496ea8 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_delete.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_delete.py
@@ -24,7 +24,7 @@ from . import utils as columns_utils
 class ColumnDeleteTestCase(BaseTestGenerator):
 """This class will delete column under table node."""
 scenarios = [
-('Delete table Node URL', dict(url='/browser/column/obj/'))
+('Delete column Node URL', dict(url='/browser/column/obj/'))
 ]
 
 def setUp(self):
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py
new file mode 100644
index 000..3cdc292
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py
@@ -0,0 +1,183 @@
+##
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##
+
+import json
+import uuid
+
+from pgadmin.browser.server_groups.servers.databases.schemas.tables.tests \
+import utils as tables_utils
+from pgadmin.browser.server_groups.servers.databases.schemas.tests import \
+utils as schema_utils
+from pgadmin.browser.server_groups.servers.databases.tests import utils as \
+database_utils
+from pgadmin.utils.route import BaseTestGenerator
+from regression import parent_node_dict
+from regression.python_test_utils import test_utils as utils
+from . import utils as columns_utils
+
+try:
+from urllib.parse import urlencode
+except ImportError as e:
+from urllib import urlencode
+
+
+class ColumnMsqlTestCase(BaseTestGenerator):
+"""This class will test msql route of column with various combinations."""
+scenarios = [
+('msql column change timestamp array length',
+ dict(
+ url='/browser/column/msql/',
+ data_type='timestamp(3) with time zone[]',
+ new_len=6,
+ expected_res='ALTER TABLE {schema}.{table}\nALTER COLUMN '
+  '{column} TYPE timestamp({len}) with time zone [];'
+ )),
+('msql column change timestamp length',
+ dict(
+ url='/browser/column/msql/',
+ data_type='timestamp(4) with time zone',
+ new_len=7,
+ expected_res='ALTER TABLE {schema}.{table}\nALTER COLUMN '
+  '{column} TYPE timestamp({len}) with time zone ;'
+ )),
+('msql column change numeric array precision',
+ dict(
+ url='/browser/column/msql/',
+ data_type='numeric(5,2)[]',
+ old_len=5,
+ new_precision=4,
+ expected_res='ALTER TABLE {schema}.{table}\nALTER COLUMN '
+  '{column} TYPE numeric ({len}, {precision})[];'
+ )),
+('msql column change numeric 

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Khushboo Vashi]

On Mon, Feb 26, 2018 at 10:02 PM, Dave Page  wrote:

> Argh, I ran some tests, but didn't spot any lost messages in the tests I
> ran. I'll revert the patch.
>
> Khushboo;
>
> Please look at the following:
>
> - Fix the patch so it doesn't drop messages.
>
Fixed.
By default, the notice attribute of the connection object of psycopg 2 only
stores 50 notices. Once it reaches to 50 it starts from 1 again.
To fix this I have changed the notice attribute from list to deque to
append more messages. Currently I have kept the maximum limit at a time of
the notice attribute is 10 (in a single poll).

> - Add regression tests to make sure it doesn't break in the future. This
> may require creating one or more functions the spew out a whole lot of
> notices, and then running a couple of queries and checking the output.
>
Added. With this regression test, the current code is failing which has
been taken care in this patch.

> - Check the messages panel on the history tab. I just noticed it seems to
> only be showing an even smaller subset of the messages.
>
Tested and no issues found.

>
>
Thanks.
>
> On Mon, Feb 26, 2018 at 4:23 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Sent bit early,
>>
>> You can run 'VACUUM FULL VERBOSE' in query tool and verify the populated
>> messages (pgAdmin3 vs. pgAdmin4).
>>
>>
>> On Mon, Feb 26, 2018 at 9:48 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Khushboo/Dave,
>>>
>>> With given commit, I'm again seeing the issue raised in
>>> https://redmine.postgresql.org/issues/1523 :(
>>>
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Mon, Feb 26, 2018 at 7:49 PM, Dave Page  wrote:
>>>
 Ensure we pick up the messages from the current query and not a
 previous one. Fixes #3094

 Branch
 --
 master

 Details
 ---
 https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdif
 f;h=08b3ccc01a4d57e8ea3657f8882a53dcd1b99386
 Author: Khushboo Vashi 

 Modified Files
 --
 web/pgadmin/utils/driver/abstract.py  |  1 +
 web/pgadmin/utils/driver/psycopg2/__init__.py | 64
 +--
 2 files changed, 21 insertions(+), 44 deletions(-)


>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/feature_tests/keyboard_shortcut_test.py b/web/pgadmin/feature_tests/keyboard_shortcut_test.py
index b83457c..f751211 100644
--- a/web/pgadmin/feature_tests/keyboard_shortcut_test.py
+++ b/web/pgadmin/feature_tests/keyboard_shortcut_test.py
@@ -62,7 +62,9 @@ class KeyboardShortcutFeatureTest(BaseFeatureTest):
 ).key_down(
 key_combo[2]
 ).key_up(
-Keys.ALT
+key_combo[0]
+).key_up(
+key_combo[1]
 ).perform()
 
 print("Executing shortcut: " + self.new_shortcuts[s]['locator'] + "...", file=sys.stderr, end="")
diff --git a/web/pgadmin/tools/sqleditor/tests/test_poll_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_poll_query_tool.py
new file mode 100644
index 000..61655e7
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/tests/test_poll_query_tool.py
@@ -0,0 +1,109 @@
+##
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##
+
+import json
+
+from pgadmin.browser.server_groups.servers.databases.tests import utils as \
+database_utils
+from pgadmin.utils.route import BaseTestGenerator
+from regression import parent_node_dict
+from regression.python_test_utils import test_utils as utils
+
+
+class TestPollQueryTool(BaseTestGenerator):
+""" This class will test the query tool polling. """
+scenarios = [
+('When query tool polling returns messages with result data-set',
+ dict(
+ sql="""
+DROP TABLE IF EXISTS test_for_notices;
+
+DO $$
+BEGIN
+RAISE NOTICE 'Hello, world!';
+END $$;
+
+SELECT 'CHECKING POLLING';
+""",
+ expected_message='NOTICE:  table "test_for_notices" ' +
+  """does not exist, skipping
+NOTICE:  Hello, world!
+""",
+ expected_result='CHECKING POLLING'
+ )),
+('When query tool polling returns a large subset of messages'
+ ' with result data-set',
+ dict(
+ sql="""
+DO $$
+BEGIN
+FOR i in 1..1000 LOOP
+RAISE NOTICE 'Count is %', i;
+END LOOP;
+END $$;
+
+SELECT 'CHECKING POLLING FOR LONG MESSAGES';
+""",
+ expected_message="\n".join(["NOTICE:  Coun

Feature #3061

[Neethu Mariya Joy]

Hi,

I am Neethu Mariya Joy, an undergraduate sophomore pursuing BE(Hons) in
Computer Engineering from BITS Pilani, India.

I would like to work on the feature #3061, "Dashboard Customisation".
Kindly provide suggestions for the same.

Sincerely,
Neethu Mariya Joy
GitHub  | Linkedin

Re: RM3079 fix for wrong sql datetime/time related datatypes

[Joao De Almeida Pereira]

Hello Harshal,

I tried the example you showed and it works. Also passed the patch through
our CI Pipeline and everything is good.

Thanks
Joao

On Wed, Feb 28, 2018 at 6:25 AM Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> On Tue, Feb 27, 2018 at 8:54 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Feb 27, 2018 at 2:36 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find patch to fix wrong sql issue for time related type.
>>>
>>> Steps to reproduce:
>>>
>>> Alter any time/datetime array related data type, it generates sql with
>>> addition array bracket
>>>
>>> [image: Inline image 1]
>>>
>>
>> This seems to be missing the test case that you noted should be included
>> in the original bug report!
>>
>
> Please find updated patch with test cases.
>
>
> Thanks,
>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: RM3079 fix for wrong sql datetime/time related datatypes

[Harshal Dhumal]

Great, thanks!

On Feb 28, 2018 8:15 PM, "Joao De Almeida Pereira" <
jdealmeidapere...@pivotal.io> wrote:

> Hello Harshal,
>
> I tried the example you showed and it works. Also passed the patch through
> our CI Pipeline and everything is good.
>
> Thanks
> Joao
>
> On Wed, Feb 28, 2018 at 6:25 AM Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> On Tue, Feb 27, 2018 at 8:54 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Tue, Feb 27, 2018 at 2:36 PM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi,

 Please find patch to fix wrong sql issue for time related type.

 Steps to reproduce:

 Alter any time/datetime array related data type, it generates sql with
 addition array bracket

 [image: Inline image 1]

>>>
>>> This seems to be missing the test case that you noted should be included
>>> in the original bug report!
>>>
>>
>> Please find updated patch with test cases.
>>
>>
>> Thanks,
>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>

Re: [pgAdmin4][RM#3073] Allow user to schedule without End date from UI

[Joao De Almeida Pereira]

Hello Murtuza,
I do not have the pgAgent installed so it was a little it hard to test this.
After looking into the code I think we are missing some testing coverage
around the pga_job Javascript part so that we can catch these problems.
Even some code extraction can be done around the pg_jobset change

Nevertheless I passed the patch through our CI and it is all green

Thanks
Joao

On Wed, Feb 28, 2018 at 5:36 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> I have found the issue, it was in Backform control itself :)
> Issue: We were passing today's date value as minDate option in pgAgent
> schedule schema while rendering the control, So when server was sending
> previous datetime value it was not displaying it causing sync problem.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Tue, Feb 27, 2018 at 8:39 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> ​Thanks Dave, I'll look into it.
>> ​
>>
>>
>> On Tue, Feb 27, 2018 at 8:37 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Tue, Feb 27, 2018 at 12:03 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 I'm not able to re-produce the issue, Could you hard refresh and try
 again?

>>>
>>> I still see it.
>>>
>>>

 If possible could you please provide exact steps?

>>>
>>> Attached is a dump of my test pgagent schema.
>>>
>>> 1) Load the schema dump into the maintenance database
>>> 2) Connect pgAdmin
>>> 3) Browse to pgAgent Jobs
>>> 4) Right-click Properties, and select the Schedules tab
>>> 5) Open the subnode for sched1
>>>
>>>

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


 On Tue, Feb 27, 2018 at 5:30 PM, Dave Page  wrote:

> Sorry - here it is.
>
> On Tue, Feb 27, 2018 at 11:59 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Could you please send screenshot?
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Tue, Feb 27, 2018 at 5:24 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> Still not quite right - see the attached screenshot which is the
>>> result of simply viewing the properties of an existing job. Note that 
>>> the
>>> start time is shown in the grid but not the subnode control.
>>>
>>> Thanks.
>>>
>>> On Tue, Feb 27, 2018 at 8:26 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 As validation related patch was committed with RM#3148 [ Sorry
 about that I forgot to checkout :) ]
 PFA patch to fix the issues you mentioned, I have also removed
 extra error message from sub node collection control and made it 
 optional
 via flag.



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


 On Mon, Feb 26, 2018 at 10:14 PM, Dave Page 
 wrote:

> Hi
>
> On Mon, Feb 26, 2018 at 2:46 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to fix the issue where user was not able to create
>> pgAgent job from UI without entering End date in schedule section.
>>
>
>  Whilst this does resolve the validation issue, there are still a
> couple of other related problems, as can be seen in the attached
> screenshots:
>
> - The Start date/time in the subnode control doesn't seem to be
> properly synchronised with the value in the grid.
>
> - If you leave the End date/time blank (but maybe click into it
> first), the grid will show "Invalid date".
>
> It's possible there are other oddities as well - please check
> carefully for anything else.
>
> 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
>>>
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


>>>
>>>

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Joao De Almeida Pereira]

Hello Khushboo,
After reviewing the patch I have the gut feeling that we do not have enough
test coverage on this issue, specially due to the intricate while loop and
conditions around the polling.
I think that this deserve Unit tests around it, When I say Unit Test I am
not talking about executing queries against the database, but do some
stubbing of the database so that we can control the flow that we want.

It is a temptation to try to always do a Feature Test to test what we want
because it is "easier" to write and ultimately it is what users see, but
while 1 Feature Test runs we can run 200 Unit Tests that give us much more
confidence that the code is doing what we expect it to do.

This being said, I run the tests on the CI Pipeline and all tests pass.
Running pycodestyle fails due to some line sizes on the
psycopg2/__init__py. I believe that it is not what you changed, but since
you were changing the file it can be fixed it is just:

pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long (81 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long (91 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long (81 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long (91 >
79 characters)
4   E501 line too long (81 > 79 characters)


Thanks
Joao


On Wed, Feb 28, 2018 at 6:49 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Mon, Feb 26, 2018 at 10:02 PM, Dave Page  wrote:
>
>> Argh, I ran some tests, but didn't spot any lost messages in the tests I
>> ran. I'll revert the patch.
>>
>> Khushboo;
>>
>> Please look at the following:
>>
>> - Fix the patch so it doesn't drop messages.
>>
> Fixed.
> By default, the notice attribute of the connection object of psycopg 2
> only stores 50 notices. Once it reaches to 50 it starts from 1 again.
> To fix this I have changed the notice attribute from list to deque to
> append more messages. Currently I have kept the maximum limit at a time of
> the notice attribute is 10 (in a single poll).
>
>> - Add regression tests to make sure it doesn't break in the future. This
>> may require creating one or more functions the spew out a whole lot of
>> notices, and then running a couple of queries and checking the output.
>>
> Added. With this regression test, the current code is failing which has
> been taken care in this patch.
>
>> - Check the messages panel on the history tab. I just noticed it seems to
>> only be showing an even smaller subset of the messages.
>>
> Tested and no issues found.
>
>>
>>
> Thanks.
>>
>> On Mon, Feb 26, 2018 at 4:23 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Sent bit early,
>>>
>>> You can run 'VACUUM FULL VERBOSE' in query tool and verify the
>>> populated messages (pgAdmin3 vs. pgAdmin4).
>>>
>>>
>>> On Mon, Feb 26, 2018 at 9:48 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Khushboo/Dave,

 With given commit, I'm again seeing the issue raised in
 https://redmine.postgresql.org/issues/1523 :(




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


 On Mon, Feb 26, 2018 at 7:49 PM, Dave Page  wrote:

> Ensure we pick up the messages from the current query and not a
> previous one. Fixes #3094
>
> Branch
> --
> master
>
> Details
> ---
>
> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=08b3ccc01a4d57e8ea3657f8882a53dcd1b99386
> Author: Khushboo Vashi 
>
> Modified Files
> --
> web/pgadmin/utils/driver/abstract.py  |  1 +
> web/pgadmin/utils/driver/psycopg2/__init__.py | 64
> +--
> 2 files changed, 21 insertions(+), 44 deletions(-)
>
>

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

Re: Feature #3061

[Joao De Almeida Pereira]

Hello Neethu
Welcome to the pgadmin-hackers, nice to see you here.

In order to start this story, I believe that we need to talk with some
users to understand what does this story mean. At this point the issue is
to broad and I personally do not understand what customization means.

But feel free to start having ideas on what does customization mean, and we
can create some prototypes and talk with users to see what their reaction
is to them.

Thanks
Joao

On Wed, Feb 28, 2018 at 7:59 AM Neethu Mariya Joy 
wrote:

> Hi,
>
> I am Neethu Mariya Joy, an undergraduate sophomore pursuing BE(Hons) in
> Computer Engineering from BITS Pilani, India.
>
> I would like to work on the feature #3061, "Dashboard Customisation".
> Kindly provide suggestions for the same.
>
> Sincerely,
> Neethu Mariya Joy
> GitHub  | Linkedin
> 
>

[pgAdmin4][RM#3161] Fix PEP-8 issues

[Murtuza Zabuawala]

Hi,

PFA patch to fix the PEP8 issues in Casts, Event triggers, Extensions and
Languages module.

pycodestyle --config=.pycodestyle
pgadmin/browser/server_groups/servers/databases/languages/
pycodestyle --config=.pycodestyle
pgadmin/browser/server_groups/servers/databases/extensions/
pycodestyle --config=.pycodestyle
pgadmin/browser/server_groups/servers/databases/event_triggers/
pycodestyle --config=.pycodestyle
pgadmin/browser/server_groups/servers/databases/casts/

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git 
a/web/pgadmin/browser/server_groups/servers/databases/casts/__init__.py 
b/web/pgadmin/browser/server_groups/servers/databases/casts/__init__.py
index e9ef8ac..2ea178e 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/casts/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/casts/__init__.py
@@ -26,6 +26,7 @@ from pgadmin.utils import IS_PY2
 if not IS_PY2:
 unicode = str
 
+
 class CastModule(CollectionNodeModule):
 """
  class CastModule(CollectionNodeModule)
@@ -96,8 +97,9 @@ class CastView(PGChildNodeView):
 class CastView(PGChildNodeView)
 
 A view class for cast node derived from PGChildNodeView. This class is
-responsible for all the stuff related to view like 
create/update/delete cast,
-showing properties of cast node, showing sql in sql pane.
+responsible for all the stuff related to view like
+create/update/delete cast, showing properties of cast node,
+showing sql in sql pane.
 
 Methods:
 ---
@@ -110,11 +112,12 @@ class CastView(PGChildNodeView):
 manager,conn & template_path properties to self
 
 * list()
-  - This function is used to list all the cast nodes within that 
collection.
+  - This function is used to list all the cast nodes within that
+  collection.
 
 * nodes()
-  - This function will used to create all the child node within that 
collection.
-Here it will create all the cast nodes.
+  - This function will used to create all the child node within that
+  collection. Here it will create all the cast nodes.
 
 * properties(gid, sid, did, rg_id)
   - This function will show the properties of the selected cast node
@@ -135,14 +138,16 @@ class CastView(PGChildNodeView):
   - This function will generate sql from model data
 
 * sql(gid, sid, did, rg_id):
-  - This function will generate sql to show in sql pane for the selected 
cast node.
+  - This function will generate sql to show in sql pane for the selected
+  cast node.
 
 * get_type():
-  - This function will fetch all the types for source and target types 
select control.
+  - This function will fetch all the types for source and target types
+  select control.
 
 * get_functions():
-  - This function will fetch associated functions list depending on 
selected source
-and target types while creating a new cast node.
+  - This function will fetch associated functions list depending on
+  selected source and target types while creating a new cast node.
 """
 
 node_type = blueprint.node_type
@@ -171,8 +176,14 @@ class CastView(PGChildNodeView):
 'stats': [{'get': 'statistics'}],
 'dependency': [{'get': 'dependencies'}],
 'dependent': [{'get': 'dependents'}],
-'get_type': [{'get': 'get_src_and_trg_type'}, {'get': 
'get_src_and_trg_type'}],
-'get_functions': [{'post': 'get_functions'}, {'post': 'get_functions'}]
+'get_type': [
+{'get': 'get_src_and_trg_type'},
+{'get': 'get_src_and_trg_type'}
+],
+'get_functions': [
+{'post': 'get_functions'},
+{'post': 'get_functions'}
+]
 })
 
 def _init_(self, **kwargs):
@@ -192,7 +203,9 @@ class CastView(PGChildNodeView):
 def wrap(*args, **kwargs):
 # Here args[0] will hold self & kwargs will hold gid,sid,did
 self = args[0]
-self.manager = 
get_driver(PG_DEFAULT_DRIVER).connection_manager(kwargs['sid'])
+self.manager = get_driver(
+PG_DEFAULT_DRIVER
+).connection_manager(kwargs['sid'])
 self.conn = self.manager.connection(did=kwargs['did'])
 # Set template path for the SQL scripts
 self.template_path = 'cast/sql/#{0}#'.format(self.manager.version)
@@ -225,7 +238,8 @@ class CastView(PGChildNodeView):
 return internal_server_error(errormsg=res)
 
 for row in res['rows']:
-row['castcontext'] = True if row['castcontext'] == 'IMPLICIT' else 
False
+row['castcontext'] = True if row['castcontext'] == 'IMPLICIT' \
+else False
 
 return ajax_response(
 response=res['rows'],
@@ -235,8 +249,8 @@ class CastView(PGChildNodeView):
 @check_precondition
 def nodes(self,

Re: [pgAdmin4][RM#3161] Fix PEP-8 issues

[Joao De Almeida Pereira]

Hello Murtuza,

I passed the patch through our CI and all test are green. Also run the
codestyle in all those folders and everything is clean.

575 issues to go

Thanks
Joao

On Wed, Feb 28, 2018 at 1:05 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the PEP8 issues in Casts, Event triggers, Extensions and
> Languages module.
>
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/languages/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/extensions/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/event_triggers/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/casts/
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: Feature #3061

[Murtuza Zabuawala]

Hello,

First of all I'm assuming that you have some degree of knowledge about
Python, HTML, JS & Git.

We have had added similar feature recently.
Check: https://redmine.postgresql.org/issues/2951 for more details.
Code location of Dashboard module in git repository:
../pgadmin4/web/pgadmin/dashboard

There are 3 requirments in the ticket,
1) disable/enable some widgets (widget - Server Sessions | Transactions per
second | Tuples in etc)
We have done this for all the graphs in one shot, now user can
enable/disable all the graphs via preferences dialog.
You can refer the commit

as a reference if you want to add this functionality for each
individual graphs.

2) add custom/user specify widgets
- First of all, You need to provide an option to Create new dashboard
dialog, Update existing dashboard dialog option and also to delete those
existing custom dashboards on dashboard page.
- You have to create a separate table in sqlite3 database to store the data
for that user defined dashboards like Dashboard name, SQL etc.
- You also have to provide option to use to choose what type of graph user
wants (Line chart, Pie chart, Table etc)
- Size & location of the graph/table.

3) add user role with access only to dashboard
This needs further discussion with community members if we really need this
or not.

You can refer README for more information regarding how to run pgAdmin4.

Let us know if you need any help.


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


On Wed, Feb 28, 2018 at 6:29 PM, Neethu Mariya Joy <
neethumariya...@gmail.com> wrote:

> Hi,
>
> I am Neethu Mariya Joy, an undergraduate sophomore pursuing BE(Hons) in
> Computer Engineering from BITS Pilani, India.
>
> I would like to work on the feature #3061, "Dashboard Customisation".
> Kindly provide suggestions for the same.
>
> Sincerely,
> Neethu Mariya Joy
> GitHub  | Linkedin
> 
>

Re: [pgAdmin4][RM#3161] Fix PEP-8 issues

[Murtuza Zabuawala]

Thank you Joao for reviewing.

On Thu, Mar 1, 2018 at 3:10 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Murtuza,
>
> I passed the patch through our CI and all test are green. Also run the
> codestyle in all those folders and everything is clean.
>
> 575 issues to go
>
> Thanks
> Joao
>
> On Wed, Feb 28, 2018 at 1:05 PM Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to fix the PEP8 issues in Casts, Event triggers, Extensions and
>> Languages module.
>>
>> pycodestyle --config=.pycodestyle pgadmin/browser/server_groups/
>> servers/databases/languages/
>> pycodestyle --config=.pycodestyle pgadmin/browser/server_groups/
>> servers/databases/extensions/
>> pycodestyle --config=.pycodestyle pgadmin/browser/server_groups/
>> servers/databases/event_triggers/
>> pycodestyle --config=.pycodestyle pgadmin/browser/server_groups/
>> servers/databases/casts/
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Khushboo Vashi]

Hi Joao,

Thanks for reviewing.

On Wed, Feb 28, 2018 at 8:55 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Khushboo,
> After reviewing the patch I have the gut feeling that we do not have
> enough test coverage on this issue, specially due to the intricate while
> loop and conditions around the polling.
> I think that this deserve Unit tests around it, When I say Unit Test I am
> not talking about executing queries against the database, but do some
> stubbing of the database so that we can control the flow that we want.
>
You are right. It needs more unit testing. I have checked below scenarios:
1. Returns 2 notices with data output
2. Returns 1000 notices with data output
3. No notices with data output

By running above, I have checked, each time returned notices are accurate,
no old notices are getting appended, it does not affect with the amount of
messages (few, none or more).  Also, with the updated patch, I have made
sure that all these queries run with the single transaction id (same
connection).

So, please let me know if you think I can add more things to this.

>
>
It is a temptation to try to always do a Feature Test to test what we want
> because it is "easier" to write and ultimately it is what users see, but
> while 1 Feature Test runs we can run 200 Unit Tests that give us much more
> confidence that the code is doing what we expect it to do.
>
> Right, so added regression tests instead of feature tests.

This being said, I run the tests on the CI Pipeline and all tests pass.
> Running pycodestyle fails due to some line sizes on the
> psycopg2/__init__py. I believe that it is not what you changed, but since
> you were changing the file it can be fixed it is just:
>
> pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long (81
> > 79 characters)
> pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long (91
> > 79 characters)
> pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long (81
> > 79 characters)
> pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long (91
> > 79 characters)
> 4   E501 line too long (81 > 79 characters)
>
> Fixed. Thanks for pointing out.

>
> Thanks
> Joao
>
>
> On Wed, Feb 28, 2018 at 6:49 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> On Mon, Feb 26, 2018 at 10:02 PM, Dave Page  wrote:
>>
>>> Argh, I ran some tests, but didn't spot any lost messages in the tests I
>>> ran. I'll revert the patch.
>>>
>>> Khushboo;
>>>
>>> Please look at the following:
>>>
>>> - Fix the patch so it doesn't drop messages.
>>>
>> Fixed.
>> By default, the notice attribute of the connection object of psycopg 2
>> only stores 50 notices. Once it reaches to 50 it starts from 1 again.
>> To fix this I have changed the notice attribute from list to deque to
>> append more messages. Currently I have kept the maximum limit at a time of
>> the notice attribute is 10 (in a single poll).
>>
>>> - Add regression tests to make sure it doesn't break in the future. This
>>> may require creating one or more functions the spew out a whole lot of
>>> notices, and then running a couple of queries and checking the output.
>>>
>> Added. With this regression test, the current code is failing which has
>> been taken care in this patch.
>>
>>> - Check the messages panel on the history tab. I just noticed it seems
>>> to only be showing an even smaller subset of the messages.
>>>
>> Tested and no issues found.
>>
>>>
>>>
>> Thanks.
>>>
>>> On Mon, Feb 26, 2018 at 4:23 PM, Murtuza Zabuawala >> enterprisedb.com> wrote:
>>>
 Sent bit early,

 You can run 'VACUUM FULL VERBOSE' in query tool and verify the
 populated messages (pgAdmin3 vs. pgAdmin4).


 On Mon, Feb 26, 2018 at 9:48 PM, Murtuza Zabuawala >>> enterprisedb.com> wrote:

> Hi Khushboo/Dave,
>
> With given commit, I'm again seeing the issue raised in
> https://redmine.postgresql.org/issues/1523 :(
>
>
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Mon, Feb 26, 2018 at 7:49 PM, Dave Page  wrote:
>
>> Ensure we pick up the messages from the current query and not a
>> previous one. Fixes #3094
>>
>> Branch
>> --
>> master
>>
>> Details
>> ---
>> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=
>> 08b3ccc01a4d57e8ea3657f8882a53dcd1b99386
>> Author: Khushboo Vashi 
>>
>> Modified Files
>> --
>> web/pgadmin/utils/driver/abstract.py  |  1 +
>> web/pgadmin/utils/driver/psycopg2/__init__.py | 64
>> +--
>> 2 files changed, 21 insertions(+), 44 deletions(-)
>>
>>
>

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

[pgAdmin4][Patch]: RM #3135 - [Web based] Syntax error displayed when user try to insert data on table where primray key is in captial letters and table contains OIDS

[Khushboo Vashi]

Hi,

Please find the attached patch to fix RM # 3135 - [Web based] Syntax error
displayed when user try to insert data on table where primray key is in
captial letters and table contains OIDS

Thanks,
Khushboo
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
index a4ed872..7e3f8a0 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
@@ -6,5 +6,5 @@ INSERT INTO {{ conn|qtIdent(nsp_name, object_name) }} (
 {% for col in data_to_be_saved %}
 {% if not loop.first %}, {% endif %}%({{ col }})s::{{ data_type[col] }}{% endfor %}
 )
-{% if pk_names %} returning {{pk_names}}{% endif %}
+{% if pk_names and not has_oids %} returning {{pk_names}}{% endif %}
 {% if has_oids %} returning oid{% endif %};