Обсуждение: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

Поиск
Список
Период
Сортировка

[pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355

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

Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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

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

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Ashesh Vashi
Дата:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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

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

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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

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


Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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

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



Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
Ashesh, can you get this in today please?

Thanks.

On Fri, May 12, 2017 at 7:07 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Ashesh Vashi
Дата:
Sure.
I am reviewing it.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company


http://www.linkedin.com/in/asheshvashi


On Fri, May 12, 2017 at 3:09 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you get this in today please?

Thanks.

On Fri, May 12, 2017 at 7:07 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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

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




Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Harshal Dhumal
Дата:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




--
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] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




--
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] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






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

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

Вложения

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
So is the last patch considered good?

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Harshal Dhumal
Дата:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
OK, that needs fixing then...

On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Harshal Dhumal
Дата:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Joao Pedro De Almeida Pereira
Дата:
Hello Hackers,

We looked into this patch and it looks like it touches part of the tree menu am I correct?
In our previous conversations with Dave, he mentioned that he wanted to replace the tree menu with a more stable version of it. In our perspective, every time that we have the opportunity extract functions that are relevant to the tree, we should do it and also wrap them with tests so that in the future, when the community starts the work of upgrading the tree we could have some confidence that our changes are not breaking any functionality.
Also removing as much code from template files as possible should also be a goal that we aim for, because that code becomes testable.

Thanks
Joao

On Tue, May 16, 2017 at 8:36 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






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



--
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] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Harshal,

I found below minor issues,

1) When node is already expanded and you rename it, it closes and do not expand again.
For example let's say if I have table node open and if rename my server then it close everything up to server.

2) Create table t1 and then select "Properties Tab", Now rename the table t1 to t2, the Properties panel do not get refreshed with new data.
Inline image 1

3) Drop & Disconnect options gets removed from context menu after we rename the database.

Steps: 
- Create a database "test_1"
- Right click on database "test_1" & check the context menu, you will see that Drop & Disconnect options are present.
- Rename  database "test_1" to "test_2"
- Right click on database "test_2" & check the context menu, you will see that Drop & Disconnect options are missing now.

 We have to refresh parent (server node) to get those options back.

Inline image 2

Rest of the functionality looks good to me.

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


On Tue, May 16, 2017 at 6:06 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi,

PFA updated patch for the same, we have also fixed the database context menu issue which was due to improper node information sent from python code.

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


On Wed, May 17, 2017 at 12:28 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

I found below minor issues,

1) When node is already expanded and you rename it, it closes and do not expand again.
For example let's say if I have table node open and if rename my server then it close everything up to server.

2) Create table t1 and then select "Properties Tab", Now rename the table t1 to t2, the Properties panel do not get refreshed with new data.
Inline image 1

3) Drop & Disconnect options gets removed from context menu after we rename the database.

Steps: 
- Create a database "test_1"
- Right click on database "test_1" & check the context menu, you will see that Drop & Disconnect options are present.
- Rename  database "test_1" to "test_2"
- Right click on database "test_2" & check the context menu, you will see that Drop & Disconnect options are missing now.

 We have to refresh parent (server node) to get those options back.

Inline image 2

Rest of the functionality looks good to me.

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


On Tue, May 16, 2017 at 6:06 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Murtuza Zabuawala
Дата:
Hi Joao,

Yes, this patch is related to browser tree issue, In this patch we have fixed some issues with 'onUpdateTreeNode' function to handle some corner cases for server & server-group nodes, Current code for 'onAddTreeNode', 'onUpdateTreeNode', 'onRefreshTreeNode' functions for browser tree is coupled with their respective inner function calls and recursive in nature due to aciTree API implementation for making function calls in orderly manner.

@Ashesh,
Any thoughts on this?

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


On Tue, May 16, 2017 at 7:07 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Hackers,

We looked into this patch and it looks like it touches part of the tree menu am I correct?
In our previous conversations with Dave, he mentioned that he wanted to replace the tree menu with a more stable version of it. In our perspective, every time that we have the opportunity extract functions that are relevant to the tree, we should do it and also wrap them with tests so that in the future, when the community starts the work of upgrading the tree we could have some confidence that our changes are not breaking any functionality.
Also removing as much code from template files as possible should also be a goal that we aim for, because that code becomes testable.

Thanks
Joao

On Tue, May 16, 2017 at 8:36 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






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



--
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] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:


On Wed, May 17, 2017 at 11:41 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Joao,

Yes, this patch is related to browser tree issue, In this patch we have fixed some issues with 'onUpdateTreeNode' function to handle some corner cases for server & server-group nodes, Current code for 'onAddTreeNode', 'onUpdateTreeNode', 'onRefreshTreeNode' functions for browser tree is coupled with their respective inner function calls and recursive in nature due to aciTree API implementation for making function calls in orderly manner.

@Ashesh,
Any thoughts on this?

I'm obviously not Ashesh, but in general, I agree with what Joao suggests - treeview related code should be refactored into testable modules, that are independent of the tree (for the most part) whenever it makes sense to do so, and tests added to aid future replacement of aciTree/Backbone. That said, if it's not feasible in a given case, then we should go ahead and fix the existing code. 

In this case, I'm leaning towards the view that this code is too tightly coupled with aciTree to be worth changing more than necessary. What do you guys think?

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

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

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Joao Pedro De Almeida Pereira
Дата:
Hello Hackers,

@Dave, @Murtuza
We do understand that the changes are tightly coupled with the tree, but if we want to be able to upgrade our tree menu in the future, we need to start decoupling functionality and the underlaying tree.
Another step that we can take is to change our variable naming convention because variables like _d or this.i do not communicate their purpose, making it hard to read.


Thanks
Joao & Shruti

On Wed, May 17, 2017 at 11:05 AM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, May 17, 2017 at 11:41 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Joao,

Yes, this patch is related to browser tree issue, In this patch we have fixed some issues with 'onUpdateTreeNode' function to handle some corner cases for server & server-group nodes, Current code for 'onAddTreeNode', 'onUpdateTreeNode', 'onRefreshTreeNode' functions for browser tree is coupled with their respective inner function calls and recursive in nature due to aciTree API implementation for making function calls in orderly manner.

@Ashesh,
Any thoughts on this?

I'm obviously not Ashesh, but in general, I agree with what Joao suggests - treeview related code should be refactored into testable modules, that are independent of the tree (for the most part) whenever it makes sense to do so, and tests added to aid future replacement of aciTree/Backbone. That said, if it's not feasible in a given case, then we should go ahead and fix the existing code. 

In this case, I'm leaning towards the view that this code is too tightly coupled with aciTree to be worth changing more than necessary. What do you guys think?

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

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

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Ashesh Vashi
Дата:
On Wed, May 17, 2017 at 8:35 PM, Dave Page <dpage@pgadmin.org> wrote:


On Wed, May 17, 2017 at 11:41 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Joao,

Yes, this patch is related to browser tree issue, In this patch we have fixed some issues with 'onUpdateTreeNode' function to handle some corner cases for server & server-group nodes, Current code for 'onAddTreeNode', 'onUpdateTreeNode', 'onRefreshTreeNode' functions for browser tree is coupled with their respective inner function calls and recursive in nature due to aciTree API implementation for making function calls in orderly manner.

@Ashesh,
Any thoughts on this?

I'm obviously not Ashesh, but in general, I agree with what Joao suggests - treeview related code should be refactored into testable modules, that are independent of the tree (for the most part) whenever it makes sense to do so, and tests added to aid future replacement of aciTree/Backbone. That said, if it's not feasible in a given case, then we should go ahead and fix the existing code. 
Agree with you.
We should go ahead, and check-in the code as of now. 

In this case, I'm leaning towards the view that this code is too tightly coupled with aciTree to be worth changing more than necessary. What do you guys think?
Joao,

In general, I agree with the suggestion, but - in this case, it is too tightly coupled with aciTree.
As we were considering rewriting the browser tree in the POC proposed by you, we will add necessary tests with that patch.

-- Thanks, Ashesh

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

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

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

От
Dave Page
Дата:
Thanks, applied.

On Wed, May 17, 2017 at 10:53 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA updated patch for the same, we have also fixed the database context menu issue which was due to improper node information sent from python code.

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


On Wed, May 17, 2017 at 12:28 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

I found below minor issues,

1) When node is already expanded and you rename it, it closes and do not expand again.
For example let's say if I have table node open and if rename my server then it close everything up to server.

2) Create table t1 and then select "Properties Tab", Now rename the table t1 to t2, the Properties panel do not get refreshed with new data.
Inline image 1

3) Drop & Disconnect options gets removed from context menu after we rename the database.

Steps: 
- Create a database "test_1"
- Right click on database "test_1" & check the context menu, you will see that Drop & Disconnect options are present.
- Rename  database "test_1" to "test_2"
- Right click on database "test_2" & check the context menu, you will see that Drop & Disconnect options are missing now.

 We have to refresh parent (server node) to get those options back.

Inline image 2

Rest of the functionality looks good to me.

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


On Tue, May 16, 2017 at 6:06 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi,

Here is updated patch to fix issue with node rename. In this patch I have fixed issue which I mentioned earlier in this mail thread.

@Murtuza, as you know all other scenarios, can you please test this patch once.

Thanks,

 

-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 2:46 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, that needs fixing then...


On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Dave,


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Mon, May 15, 2017 at 1:43 PM, Dave Page <dpage@pgadmin.org> wrote:
So is the last patch considered good?
Except one thing, when user renames server group (note that server group should never be expanded before renaming) and then expands it by clicking + icon then it's old name gets restored.

Step:
1. Load application in browser.
2. Do not expand any of the server group.
3. Rename Server group (new name gets updated in tree).
4. Now expand Server group (old name gets restored in tree).

Note that this happens only when Server groups was never expanded before.

 

On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

Yes, It is existing one only, We did not touch on any part of sorting algorithm in this patch.

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


On Mon, May 15, 2017 at 1:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh is out this week. As long as new nodes are sorted with the same algorithm as existing ones, that's fine.

On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Harshal,

We are using https://github.com/javve/natural-sort for sorting nodes which is implemented by Ashesh.

@Ashesh,
Any suggestion on this? 

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


On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:
Hi Murtuza,

Currently nodes are sorted in case sensitive manner it should be case insensitive.



See current Server group order is A, Servers, a1, a​2. It should be A, a1, a2, Servers.
Similarly check sorting order for server and database nodes


-- 
Harshal Dhumal
Sr. Software Engineer

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

On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

Please find updated patch as discussed.

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


On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

As discussed please find updated patch removing hardcoded check for server & server-group node.

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


On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Ashesh,

PFA updated patch for the issue.

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


On Wed, Apr 26, 2017 at 10:29 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please? Thanks.

On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

PFA minor patch to fix the issue where node rename is not working properly after 7dd9efd8 commit .
RM#2355
We should remove the existing node, and then insert at right place instead of refreshing the parent.
Because - that will select the parent node, and not that node, and also - it adds overhead of refreshing the whole parent node.

Please send the patch as per our discussion.

-- Thanks, Ashesh

--
Regards,
Murtuza Zabuawala
EnterpriseDB: 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




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






--
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
Вложения