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

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][PATCH] To fix the issue with Node rename
Date: 2017-06-06 08:50:26
Message-ID: CA+OCxoxAGNGf0FMJQd6MoL_4S1PA74WP2Su0mD4=q9pkZ-hoiw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, applied.

On Wed, May 17, 2017 at 10:53 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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.
>> [image: 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.
>>
>> [image: 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(dot)dhumal(at)enterprisedb(dot)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(at)pgadmin(dot)org> wrote:
>>>
>>>> OK, that needs fixing then...
>>>>
>>>>
>>>> On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <
>>>> harshal(dot)dhumal(at)enterprisedb(dot)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(at)pgadmin(dot)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(dot)zabuawala(at)enterprisedb(dot)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(at)pgadmin(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)dhumal(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage(at)pgadmin(dot)org
>>>>>>>>>>>>>> > wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Ashesh, can you review/commit this please? Thanks.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Mon, Apr 24, 2017 at 6:17 AM, Murtuza Zabuawala <
>>>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> PFA minor patch to fix the issue where node rename is not
>>>>>>>>>>>>>>>> working properly after 7dd9efd8
>>>>>>>>>>>>>>>> <https://redmine.postgresql.org/projects/pgadmin4/repository/revisions/7dd9efd811c7845d9dc985b66f8d33497f2f4bfa> 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(at)postgresql(dot)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(at)postgresql(dot)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

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-06-06 08:52:47 pgAdmin 4 commit: Enable dialogue help buttons on Language and Foreign
Previous Message Dave Page 2017-06-06 08:50:08 pgAdmin 4 commit: Properly refresh the parent node when renaming childr