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

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(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-05-16 12:36:43
Message-ID: CAFiP3vx6R38=jVJQP_QwPEBwJdTdLn8jM9sEqywpc3UiG1yfLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

Attachment Content-Type Size
fix_update_node_v4.patch text/x-patch 10.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Joao Pedro De Almeida Pereira 2017-05-16 13:37:23 Re: [pgAdmin4][PATCH] To fix the issue with Node rename
Previous Message Murtuza Zabuawala 2017-05-16 10:05:32 [pgAdmin4][PATCH] To fix the issue in function node