Re: PATCH: added "Collation" & "Catalog Objects" nodes in pgAdmin4

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('<column_name>')" 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 <dpage(at)pgadmin(dot)org
> <mailto:dpage(at)pgadmin(dot)org>> wrote:
>
> On Wed, Jan 6, 2016 at 12:18 PM, Murtuza Zabuawala
> <murtuza(dot)zabuawala(at)enterprisedb(dot)com
> <mailto:murtuza(dot)zabuawala(at)enterprisedb(dot)com>> 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(at)postgresql(dot)org
> <mailto:pgadmin-hackers(at)postgresql(dot)org>)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>