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

From Ashesh Vashi
Subject Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename
Date
Msg-id CAG7mmowJNw_1oDdTCz6yicT5rF2VsC4xp9fs13V45KdCxquKfw@mail.gmail.com
Whole thread Raw
In response to Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename  (Dave Page <dpage@pgadmin.org>)
List pgadmin-hackers
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

pgadmin-hackers by date:

Previous
From: Surinder Kumar
Date:
Subject: Re: [pgadmin-hackers] [pgAdmin4][Patch]: Load module's JS files onlywhen required
Next
From: Khushboo Vashi
Date:
Subject: Re: [pgadmin-hackers] [pgAdmin4][Patch]: Feature test for PGData-types in Query Tool