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: Joao Pedro De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, 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-05-17 15:05:11
Message-ID: CA+OCxoxPcMjCgZJGRz0EJ5Ljyo6ttZiwr9d+r3PRDZh7AmSCBA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Wed, May 17, 2017 at 11:41 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-05-17 15:16:57 Re: [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid
Previous Message Dave Page 2017-05-17 14:58:03 Re: [pgAdmin4][PATCH] To fix the issue in server group node