Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Dave Page]

Thanks, both applied!

On Wed, Nov 22, 2017 at 5:30 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> ​Hi Dave,
>
> On Tue, Nov 21, 2017 at 10:53 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> PFA updated patch.
>>>
>>> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>>>
 HI

 On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with custom tristate boolean editor for SlickGrid to
> make it compatible with Qt runtime.
> I have tested it web mode & also modified the feature test accordingly.
>
> Also thanks to Neel for testing the patch with latest runtime code.
>

 Cool - so a few thoughts...

 - Can we make the null symbol a question mark?

>>> ​instead of question mark we will make square
>>> gray color
>>> ​.​
>>>
>>
>> Sorry - I just realised this is not good from an accessibility
>> perspective as users may not be able to distinguish between the white and
>> gray in-fill. I've made it include a ? as well.
>>
> ​
> I have enhanced the the visibility of '?'
> little bit
> ​more​
> , attaching the patch if you prefer it that way. ​
>
>
>
>>
>>
>>>
 - I think the feature tests should be enhanced to ensure we verify the
 clickthrough sequence of the new control. I don't think we need a new test
 - maybe just an enhancement to the existing editor test?

>>> ​Done​
>>>
>>>
>>
>> Thanks - patch applied.
>>
>>
>>>
 - With a table of the definition shown below, if I add a row with a
 default value for the ID, and false, true, null and hit save, then
 immediately (without refreshing) try to change the first boolean value to
 true and hit save, then I get the following error:

 UPDATE public.bools SET
 b1 = %(b1)s::boolean WHERE
 ;
 2017-11-21 10:34:57,378: ERROR pgadmin:
 Failed to execute query (execute_void) for the server #1 - DB:postgres
 (Query-id: 4249364):
 Error Message:ERROR:  syntax error at or near ";"
 LINE 3: ;


 Table:

 CREATE TABLE public.bools
 (
 id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
 b1 boolean,
 b2 boolean,
 b3 boolean,
 CONSTRAINT bools_pkey PRIMARY KEY (id)
 )

 ​This issue is not related to given editor changes.
>>> I have opened the separate ticket for the issue
>>> https://redmine.postgresql.org/issues/2886
>>>
>>
>> OK, thanks.
>>
> ​Also attached patch for RM#2886, the issue was due to incorrect
> conditional logic, As per the current implementation the newly added row
> should gets disabled if it is saved with default primary key value until
> gird reloaded with actual PK for updation but due to
>  incorrect condition
> ​ it was enable​ in this case.
>
>>
>>
>> --
>> 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][Patch]: To fix issues in Boolean editor

[Murtuza Zabuawala]

​Hi Dave,

On Tue, Nov 21, 2017 at 10:53 PM, Dave Page  wrote:

> Hi
>
> On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch.
>>
>> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>>
>>> HI
>>>
>>> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 PFA updated patch with custom tristate boolean editor for SlickGrid to
 make it compatible with Qt runtime.
 I have tested it web mode & also modified the feature test accordingly.

 Also thanks to Neel for testing the patch with latest runtime code.

>>>
>>> Cool - so a few thoughts...
>>>
>>> - Can we make the null symbol a question mark?
>>>
>> ​instead of question mark we will make square
>> gray color
>> ​.​
>>
>
> Sorry - I just realised this is not good from an accessibility perspective
> as users may not be able to distinguish between the white and gray in-fill.
> I've made it include a ? as well.
>
​
I have enhanced the the visibility of '?'
little bit
​more​
, attaching the patch if you prefer it that way. ​



>
>
>>
>>> - I think the feature tests should be enhanced to ensure we verify the
>>> clickthrough sequence of the new control. I don't think we need a new test
>>> - maybe just an enhancement to the existing editor test?
>>>
>> ​Done​
>>
>>
>
> Thanks - patch applied.
>
>
>>
>>> - With a table of the definition shown below, if I add a row with a
>>> default value for the ID, and false, true, null and hit save, then
>>> immediately (without refreshing) try to change the first boolean value to
>>> true and hit save, then I get the following error:
>>>
>>> UPDATE public.bools SET
>>> b1 = %(b1)s::boolean WHERE
>>> ;
>>> 2017-11-21 10:34:57,378: ERROR pgadmin:
>>> Failed to execute query (execute_void) for the server #1 - DB:postgres
>>> (Query-id: 4249364):
>>> Error Message:ERROR:  syntax error at or near ";"
>>> LINE 3: ;
>>>
>>>
>>> Table:
>>>
>>> CREATE TABLE public.bools
>>> (
>>> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
>>> b1 boolean,
>>> b2 boolean,
>>> b3 boolean,
>>> CONSTRAINT bools_pkey PRIMARY KEY (id)
>>> )
>>>
>>> ​This issue is not related to given editor changes.
>> I have opened the separate ticket for the issue
>> https://redmine.postgresql.org/issues/2886
>>
>
> OK, thanks.
>
​Also attached patch for RM#2886, the issue was due to incorrect
conditional logic, As per the current implementation the newly added row
should gets disabled if it is saved with default primary key value until
gird reloaded with actual PK for updation but due to
 incorrect condition
​ it was enable​ in this case.

>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/web/pgadmin/static/css/bootstrap.overrides.css 
b/web/pgadmin/static/css/bootstrap.overrides.css
index fcc030d..b894021 100755
--- a/web/pgadmin/static/css/bootstrap.overrides.css
+++ b/web/pgadmin/static/css/bootstrap.overrides.css
@@ -1404,20 +1404,21 @@ body {
 .multi-checkbox .check {
   display: inline-block;
   vertical-align: top;
-  width: 15px;
-  height: 15px;
+  width: 16px;
+  height: 16px;
   border: 1px solid #333;
   margin: 3px;
   text-align: center;
-  line-height: 15px;
+  line-height: 16px;
 }
 
+.multi-checkbox .check.checked,
 .multi-checkbox .check.unchecked {
   background: #fff;
 }
 
 .multi-checkbox .check.partial {
-  background: #cc;
+  background: #e8e8e8;
 }
 
 .multi-checkbox .check.checked:after {
@@ -1425,5 +1426,5 @@ body {
 }
 
 .multi-checkbox .check.partial:after {
-  content: "\fe56";
+  content: "\003F";
 }
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js 
b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 9a0c3de..ba0ce67 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -668,12 +668,14 @@ define('tools.querytool', [
 }
   }
 }
+
 // Disable rows having default values
 if (!_.isUndefined(self.handler.rows_to_disable) &&
-  _.indexOf(self.handler.rows_to_disable, i) !== -1
-) {
+self.handler.rows_to_disable.length > 0 &&
+_.indexOf(self.handler.rows_to_disable, i) !== -1) {
   cssClass += ' disabled_row';
 }
+
 return {'cssClasses': cssClass};
   };
 
@@ -825,16 +827,17 @@ define('tools.querytool', [
 // so that cell edit is enabled for that row.
 var grid = args.grid,
   row_data = grid.getDataItem(args.row),
-  is_primary_key = _.all(
-_.values(
-  _.pick(
-row_data, self.primary_keys
-  )
-),
-function (val) {
-  return val != undefined
-   

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Dave Page]

Hi

On Tue, Nov 21, 2017 at 1:17 PM, Murtuza Zabuawala  wrote:

> Hi Dave,
>
> PFA updated patch.
>
> On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:
>
>> HI
>>
>> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> PFA updated patch with custom tristate boolean editor for SlickGrid to
>>> make it compatible with Qt runtime.
>>> I have tested it web mode & also modified the feature test accordingly.
>>>
>>> Also thanks to Neel for testing the patch with latest runtime code.
>>>
>>
>> Cool - so a few thoughts...
>>
>> - Can we make the null symbol a question mark?
>>
> ​instead of question mark we will make square
> gray color
> ​.​
>

Sorry - I just realised this is not good from an accessibility perspective
as users may not be able to distinguish between the white and gray in-fill.
I've made it include a ? as well.


>
>> - I think the feature tests should be enhanced to ensure we verify the
>> clickthrough sequence of the new control. I don't think we need a new test
>> - maybe just an enhancement to the existing editor test?
>>
> ​Done​
>
>

Thanks - patch applied.


>
>> - With a table of the definition shown below, if I add a row with a
>> default value for the ID, and false, true, null and hit save, then
>> immediately (without refreshing) try to change the first boolean value to
>> true and hit save, then I get the following error:
>>
>> UPDATE public.bools SET
>> b1 = %(b1)s::boolean WHERE
>> ;
>> 2017-11-21 10:34:57,378: ERROR pgadmin:
>> Failed to execute query (execute_void) for the server #1 - DB:postgres
>> (Query-id: 4249364):
>> Error Message:ERROR:  syntax error at or near ";"
>> LINE 3: ;
>>
>>
>> Table:
>>
>> CREATE TABLE public.bools
>> (
>> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
>> b1 boolean,
>> b2 boolean,
>> b3 boolean,
>> CONSTRAINT bools_pkey PRIMARY KEY (id)
>> )
>>
>> ​This issue is not related to given editor changes.
> I have opened the separate ticket for the issue
> https://redmine.postgresql.org/issues/2886
>

OK, thanks.


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

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

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Murtuza Zabuawala]

Hi Dave,

PFA updated patch.

On Tue, Nov 21, 2017 at 4:16 PM, Dave Page  wrote:

> HI
>
> On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch with custom tristate boolean editor for SlickGrid to
>> make it compatible with Qt runtime.
>> I have tested it web mode & also modified the feature test accordingly.
>>
>> Also thanks to Neel for testing the patch with latest runtime code.
>>
>
> Cool - so a few thoughts...
>
> - Can we make the null symbol a question mark?
>
​instead of question mark we will make square
gray color
​.​

>
> - I think the feature tests should be enhanced to ensure we verify the
> clickthrough sequence of the new control. I don't think we need a new test
> - maybe just an enhancement to the existing editor test?
>
​Done​


>
> - With a table of the definition shown below, if I add a row with a
> default value for the ID, and false, true, null and hit save, then
> immediately (without refreshing) try to change the first boolean value to
> true and hit save, then I get the following error:
>
> UPDATE public.bools SET
> b1 = %(b1)s::boolean WHERE
> ;
> 2017-11-21 10:34:57,378: ERROR pgadmin:
> Failed to execute query (execute_void) for the server #1 - DB:postgres
> (Query-id: 4249364):
> Error Message:ERROR:  syntax error at or near ";"
> LINE 3: ;
>
>
> Table:
>
> CREATE TABLE public.bools
> (
> id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
> b1 boolean,
> b2 boolean,
> b3 boolean,
> CONSTRAINT bools_pkey PRIMARY KEY (id)
> )
>
> ​This issue is not related to given editor changes.
I have opened the separate ticket for the issue
https://redmine.postgresql.org/issues/2886

> 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/feature_tests/test_data.json 
b/web/pgadmin/feature_tests/test_data.json
index ac00b0b..6c53cc8 100644
--- a/web/pgadmin/feature_tests/test_data.json
+++ b/web/pgadmin/feature_tests/test_data.json
@@ -13,13 +13,14 @@
   "10": ["[61,62]", "[61,62]", "json"],
   "11": ["", "true", "bool"],
   "12": ["", "[null]", "bool"],
-  "13": ["", "[null]", "text[]"],
-  "14": ["{}", "{}", "text[]"],
-  "15": ["{data,,'',\"\",\\'\\',\\\"\\\"}", "{data,[null],,,'',\"\"}", 
"text[]"],
-  "16": ["{}", "{}", "int[]"],
-  "17": ["{123,,456}", "{123,[null],456}", "int[]"],
-  "18": ["", "[null]", "boolean[]"],
-  "19": ["{false,,true}", "{false,[null],true}", "boolean[]"]
+  "13": ["", "false", "bool"],
+  "14": ["", "[null]", "text[]"],
+  "15": ["{}", "{}", "text[]"],
+  "16": ["{data,,'',\"\",\\'\\',\\\"\\\"}", "{data,[null],,,'',\"\"}", 
"text[]"],
+  "17": ["{}", "{}", "int[]"],
+  "18": ["{123,,456}", "{123,[null],456}", "int[]"],
+  "19": ["", "[null]", "boolean[]"],
+  "20": ["{false,,true}", "{false,[null],true}", "boolean[]"]
 }
   }
 }
diff --git a/web/pgadmin/feature_tests/view_data_dml_queries.py 
b/web/pgadmin/feature_tests/view_data_dml_queries.py
index 153d796..e5b0c09 100644
--- a/web/pgadmin/feature_tests/view_data_dml_queries.py
+++ b/web/pgadmin/feature_tests/view_data_dml_queries.py
@@ -65,8 +65,9 @@ CREATE TABLE public.defaults
 text_null4 text COLLATE pg_catalog."default",
 json_defaults json DEFAULT '[51, 52]'::json,
 json_null json,
-boolean_defaults boolean DEFAULT true,
+boolean_true boolean DEFAULT true,
 boolean_null boolean,
+boolean_false boolean,
 text_arr text[],
 text_arr_empty text[],
 text_arr_null text[],
@@ -194,12 +195,17 @@ CREATE TABLE public.defaults
 self.page.find_by_xpath("//*[contains(@class, 'pg_text_editor')]"
 "//button[contains(@class, 
'fa-save')]").click()
 else:
+# Boolean editor test for to True click
 if data[1] == 'true':
-checkbox_el = cell_el.find_element_by_xpath(".//input")
+checkbox_el = 
cell_el.find_element_by_xpath(".//*[contains(@class, 'multi-checkbox')]")
 checkbox_el.click()
-
ActionChains(self.driver).move_to_element(checkbox_el).double_click(
-checkbox_el
-).perform()
+# Boolean editor test for to False click
+elif data[1] == 'false':
+checkbox_el = 
cell_el.find_element_by_xpath(".//*[contains(@class, 'multi-checkbox')]")
+# Sets true
+checkbox_el.click()
+# Sets false
+ActionChains(self.driver).click(checkbox_el).perform()
 
 def _tables_node_expandable(self):
 self.page.toggle_open_tree_item(self.server['name'])
diff --git a/web/pgadmin/static/css/bootstrap.overrides.css 
b/web/pgadmin/static/css/bootstrap.overrides.css
index 

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Dave Page]

HI

On Tue, Nov 21, 2017 at 6:16 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch with custom tristate boolean editor for SlickGrid to
> make it compatible with Qt runtime.
> I have tested it web mode & also modified the feature test accordingly.
>
> Also thanks to Neel for testing the patch with latest runtime code.
>

Cool - so a few thoughts...

- Can we make the null symbol a question mark?

- I think the feature tests should be enhanced to ensure we verify the
clickthrough sequence of the new control. I don't think we need a new test
- maybe just an enhancement to the existing editor test?

- With a table of the definition shown below, if I add a row with a default
value for the ID, and false, true, null and hit save, then immediately
(without refreshing) try to change the first boolean value to true and hit
save, then I get the following error:

UPDATE public.bools SET
b1 = %(b1)s::boolean WHERE
;
2017-11-21 10:34:57,378: ERROR pgadmin:
Failed to execute query (execute_void) for the server #1 - DB:postgres
(Query-id: 4249364):
Error Message:ERROR:  syntax error at or near ";"
LINE 3: ;


Table:

CREATE TABLE public.bools
(
id integer NOT NULL DEFAULT nextval('bools_id_seq'::regclass),
b1 boolean,
b2 boolean,
b3 boolean,
CONSTRAINT bools_pkey PRIMARY KEY (id)
)

Thanks!

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

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

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Dave Page]

On Thu, Nov 16, 2017 at 4:30 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Thu, Nov 16, 2017 at 8:59 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Nov 16, 2017 at 2:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to fix the issue in boolean editor & also corrected feature
>>> test.
>>> RM#2848
>>>
>>
>> Testing in webkit on Mac (Qt 5.8), the problem does not appear to be
>> solved for me. I have run make bundle, and restarted the runtime multiple
>> times.
>>
> ​I tested in Chrome and it was working as expected, Have you tested patch
> with browser?​
>
>

It works fine in Chrome... but then, it was never broken there for me!

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

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

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Murtuza Zabuawala]

Hi Dave,

On Thu, Nov 16, 2017 at 8:59 PM, Dave Page  wrote:

> Hi
>
> On Thu, Nov 16, 2017 at 2:04 PM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to fix the issue in boolean editor & also corrected feature
>> test.
>> RM#2848
>>
>
> Testing in webkit on Mac (Qt 5.8), the problem does not appear to be
> solved for me. I have run make bundle, and restarted the runtime multiple
> times.
>
​I tested in Chrome and it was working as expected, Have you tested patch
with browser?​


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

Re: [pgAdmin4][Patch]: To fix issues in Boolean editor

[Dave Page]

Hi

On Thu, Nov 16, 2017 at 2:04 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue in boolean editor & also corrected feature test.
> RM#2848
>

Testing in webkit on Mac (Qt 5.8), the problem does not appear to be solved
for me. I have run make bundle, and restarted the runtime multiple times.


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

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