Re: [pgadmin-hackers] PATCH: added "Collation" & "Catalog Objects" nodes in pgAdmin4

[Murtuza Zabuawala]

Hi,

PFA updated patch with fix for below mentioned issues except issue no.8
(That issue occurs when you create any database object for other schema 
rather then selected schema in tree view, once object is created it will 
gets associated within current tree schema rather then schema mentioned 
while creating that object, So it is yet to be fixed in our existing 
infrastructure.)



Regards,
Murtuza

On Tuesday 19 January 2016 10:31 AM, Murtuza Zabuawala wrote:

Thanks Neel.

I'll work on below mentioned issues & send new patch.

Regards,
Murtuza

On Monday 18 January 2016 10:12 PM, Neel Patel wrote:

Hi Murtuza,

Please find below review comments for the collation node.

1. "Owner" field should be changed from text control to 
Backform.NodeListByNameControl.


2. Remove "Use Slony" option, we will implement it as separate module.

3. Use 2 space indentation instead of 4 space in javascript file.

4. In some of the sql file, 'qtIdent' and 'qtLiteral' is not used. 
Please check all the SQL files and make sure we use the same.
e.g. In update.sql  -- Change  '{{ data.description }}'   to {{ 
data.description|qtLiteral }}


5. While creating the new collation, there is a spelling  mistake in 
"Definition" tab text. Currently it is displayed as "Defination" 
which is wrong.


6. Wrong SQL is getting generated when we create the new collation 
from "Copy collation" as below. Please correct the generated SQL.


CREATE COLLATION public.my_collation FROM pg_catalog.\;

7. If we "Edit" the existing created collation with collation Name 
then below wrong SQL is getting generated. In below SQL - if user 
change the name of the collation then "... RENAME TO ..." query 
should be first executed then other modified parameters should be 
applied.
We also noticed that when we change only collation name then also for 
the unchanged parameters, queries are getting generated which should 
be corrected.  Query should be generated only for modified parameters.


Below is the wrong SQL.

ALTER COLLATION my_schema.my_collation_1_up
OWNER TO ;
COMMENT ON COLLATION my_schema.my_collation_1_up
IS 'testing comment';
ALTER COLLATION my_schema.my_collation_1_up
RENAME TO my_collation_1;
ALTER COLLATION my_schema.my_collation_1 SET SCHEMA None;

8.  When we create the new collation with below parameters then we 
are getting "*IndexError: list index out of range*" error at python side.

 Use below parameter to reproduce the error.

 CREATE COLLATION public.collation_23
 (
LOCALE = 'en_AG.utf8'
 );
ALTER COLLATION public.collation_23
OWNER TO postgres;
COMMENT ON COLLATION public.collation_23
IS 'My comment';
9. As per Dave's comment, change the copyright year to 2016 and some 
of the spelling mistakes.


10. In "validate" function in collation.js, multiple time 
"this.get('')" is used. Instead of using  multiple time, 
we can assign in one variable

  and use that variable in all the places.

Do let us know for any comments/issues.

Thanks,
Neel Patel

On Wed, Jan 6, 2016 at 6:10 PM, Dave Page > wrote:


On Wed, Jan 6, 2016 at 12:18 PM, Murtuza Zabuawala
> wrote:
> Hi,
>
> Please find patch to add "Collation" & "Catalog Objects" nodes
in pgAdmin4.
>
> Please note that this patch is mainly for "Collation" &
"Catalog Objects"
> nodes, Schema/Catalog node is not yet complete as we are yet to
implement
> privileges control for the same.

Thanks - a couple of quick comments:

- Please ensure the copyright notices are updated for 2016.

- The text:

Below SQL will 

Should be:

The SQL below will 

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

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


--
Sent via pgadmin-hackers mailing list
(pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers






diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py
new file mode 100644
index 000..e8876d6
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py
@@ -0,0 +1,446 @@
+##
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2016, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##
+import json
+from flask import render_template, make_response, request, jsonify
+from flask.ext.babel import gettext
+from pgadmin.utils.ajax import 

Re: [pgadmin-hackers] PATCH: added "Collation" & "Catalog Objects" nodes in pgAdmin4

[Neel Patel]

Hi Murtuza,

Please find below review comments for the collation node.

1. "Owner" field should be changed from text control to
Backform.NodeListByNameControl.

2. Remove "Use Slony" option, we will implement it as separate module.

3. Use 2 space indentation instead of 4 space in javascript file.

4. In some of the sql file, 'qtIdent' and 'qtLiteral' is not used. Please
check all the SQL files and make sure we use the same.
e.g. In update.sql  -- Change  '{{ data.description }}'   to {{
data.description|qtLiteral }}

5. While creating the new collation, there is a spelling  mistake in
"Definition" tab text. Currently it is displayed as "Defination" which is
wrong.

6. Wrong SQL is getting generated when we create the new collation from
"Copy collation" as below. Please correct the generated SQL.

CREATE COLLATION public.my_collation FROM pg_catalog.\;

7. If we "Edit" the existing created collation with collation Name then
below wrong SQL is getting generated. In below SQL - if user change the
name of the collation then "... RENAME TO ..." query should be first
executed then other modified parameters should be applied.
We also noticed that when we change only collation name then also for the
unchanged parameters, queries are getting generated which should be
corrected.  Query should be generated only for modified parameters.

Below is the wrong SQL.

ALTER COLLATION my_schema.my_collation_1_up
OWNER TO ;
COMMENT ON COLLATION my_schema.my_collation_1_up
IS 'testing comment';
ALTER COLLATION my_schema.my_collation_1_up
RENAME TO my_collation_1;
ALTER COLLATION my_schema.my_collation_1 SET SCHEMA None;

8.  When we create the new collation with below parameters then we are
getting "*IndexError: list index out of range*" error at python side.
 Use below parameter to reproduce the error.

 CREATE COLLATION public.collation_23
 (
  LOCALE = 'en_AG.utf8'
 );
ALTER COLLATION public.collation_23
OWNER TO postgres;
COMMENT ON COLLATION public.collation_23
IS 'My comment';

9. As per Dave's comment, change the copyright year to 2016 and some of the
spelling mistakes.

10. In "validate" function in collation.js, multiple time
"this.get('')" is used. Instead of using  multiple time, we
can assign in one variable
  and use that variable in all the places.

Do let us know for any comments/issues.

Thanks,
Neel Patel

On Wed, Jan 6, 2016 at 6:10 PM, Dave Page  wrote:

> On Wed, Jan 6, 2016 at 12:18 PM, Murtuza Zabuawala
>  wrote:
> > Hi,
> >
> > Please find patch to add "Collation" & "Catalog Objects" nodes in
> pgAdmin4.
> >
> > Please note that this patch is mainly for "Collation" & "Catalog Objects"
> > nodes, Schema/Catalog node is not yet complete as we are yet to implement
> > privileges control for the same.
>
> Thanks - a couple of quick comments:
>
> - Please ensure the copyright notices are updated for 2016.
>
> - The text:
>
> Below SQL will 
>
> Should be:
>
> The SQL below will 
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>

Re: [pgadmin-hackers] PATCH: added "Collation" & "Catalog Objects" nodes in pgAdmin4

[Murtuza Zabuawala]

Thanks Neel.

I'll work on below mentioned issues & send new patch.

Regards,
Murtuza

On Monday 18 January 2016 10:12 PM, Neel Patel wrote:

Hi Murtuza,

Please find below review comments for the collation node.

1. "Owner" field should be changed from text control to 
Backform.NodeListByNameControl.


2. Remove "Use Slony" option, we will implement it as separate module.

3. Use 2 space indentation instead of 4 space in javascript file.

4. In some of the sql file, 'qtIdent' and 'qtLiteral' is not used. 
Please check all the SQL files and make sure we use the same.
e.g. In update.sql  -- Change  '{{ data.description }}'   to {{ 
data.description|qtLiteral }}


5. While creating the new collation, there is a spelling  mistake in 
"Definition" tab text. Currently it is displayed as "Defination" which 
is wrong.


6. Wrong SQL is getting generated when we create the new collation 
from "Copy collation" as below. Please correct the generated SQL.


CREATE COLLATION public.my_collation FROM pg_catalog.\;

7. If we "Edit" the existing created collation with collation Name 
then below wrong SQL is getting generated. In below SQL - if user 
change the name of the collation then "... RENAME TO ..." query should 
be first executed then other modified parameters should be applied.
We also noticed that when we change only collation name then also for 
the unchanged parameters, queries are getting generated which should 
be corrected.  Query should be generated only for modified parameters.


Below is the wrong SQL.

ALTER COLLATION my_schema.my_collation_1_up
OWNER TO ;
COMMENT ON COLLATION my_schema.my_collation_1_up
IS 'testing comment';
ALTER COLLATION my_schema.my_collation_1_up
RENAME TO my_collation_1;
ALTER COLLATION my_schema.my_collation_1 SET SCHEMA None;

8.  When we create the new collation with below parameters then we are 
getting "*IndexError: list index out of range*" error at python side.

 Use below parameter to reproduce the error.

 CREATE COLLATION public.collation_23
 (
LOCALE = 'en_AG.utf8'
 );
ALTER COLLATION public.collation_23
OWNER TO postgres;
COMMENT ON COLLATION public.collation_23
IS 'My comment';
9. As per Dave's comment, change the copyright year to 2016 and some 
of the spelling mistakes.


10. In "validate" function in collation.js, multiple time 
"this.get('')" is used. Instead of using  multiple time, 
we can assign in one variable

  and use that variable in all the places.

Do let us know for any comments/issues.

Thanks,
Neel Patel

On Wed, Jan 6, 2016 at 6:10 PM, Dave Page > wrote:


On Wed, Jan 6, 2016 at 12:18 PM, Murtuza Zabuawala
> wrote:
> Hi,
>
> Please find patch to add "Collation" & "Catalog Objects" nodes
in pgAdmin4.
>
> Please note that this patch is mainly for "Collation" & "Catalog
Objects"
> nodes, Schema/Catalog node is not yet complete as we are yet to
implement
> privileges control for the same.

Thanks - a couple of quick comments:

- Please ensure the copyright notices are updated for 2016.

- The text:

Below SQL will 

Should be:

The SQL below will 

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

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


--
Sent via pgadmin-hackers mailing list
(pgadmin-hackers@postgresql.org
)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] PATCH: added "Collation" & "Catalog Objects" nodes in pgAdmin4

[Dave Page]

On Wed, Jan 6, 2016 at 12:18 PM, Murtuza Zabuawala
 wrote:
> Hi,
>
> Please find patch to add "Collation" & "Catalog Objects" nodes in pgAdmin4.
>
> Please note that this patch is mainly for "Collation" & "Catalog Objects"
> nodes, Schema/Catalog node is not yet complete as we are yet to implement
> privileges control for the same.

Thanks - a couple of quick comments:

- Please ensure the copyright notices are updated for 2016.

- The text:

Below SQL will 

Should be:

The SQL below will 

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

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


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers