Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d42ZY-0001LH-Ub for pgadmin-hackers@arkaria.postgresql.org; Fri, 28 Apr 2017 09:52:21 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d42ZY-0006OU-Ej for pgadmin-hackers@arkaria.postgresql.org; Fri, 28 Apr 2017 09:52:20 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d42ZX-0006OI-JE for pgadmin-hackers@postgresql.org; Fri, 28 Apr 2017 09:52:19 +0000 Received: from mail-it0-x229.google.com ([2607:f8b0:4001:c0b::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d42ZU-0000So-5R for pgadmin-hackers@postgresql.org; Fri, 28 Apr 2017 09:52:18 +0000 Received: by mail-it0-x229.google.com with SMTP id c123so8134607ith.1 for ; Fri, 28 Apr 2017 02:52:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ZPdth8rZCLhDdHH/cC55zsd9FAMUhieGp8lOUm9ZE4Y=; b=Git4RmLfSry8mWnqbQ7AmIUzDPtQMvs6Ycmy/RPWGN+yYmLlCAzW/vMgZcS7nbsqcr YkfUnNsgJ4ebQs+KZAm1t4K6hNUcbrTqNHHMT5W8gKcaeB71u6tc6QCrqZyemBdtQOVE gVDEqVoDgoDqldQn8IOrjEGGRltmB9cifSQ3pIbVXosr8Kw3Ol9H+wnKrygd2gtP8Jl5 NVG0ugxPnI7KukOesa3YaBtZK+jQGyPwe4ASrQP6OLZ1NVr9CMpPVX9/3CqpKEJ+84vH c1gvRyng+zWp9VsMDrMRVki8KcoZRozfICLuARyzkQoyNvqjFpeFoYz0/ZTnon3qwwkX EZvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ZPdth8rZCLhDdHH/cC55zsd9FAMUhieGp8lOUm9ZE4Y=; b=cyryeYg1FPlgjXpT2me4PgFdQ2L8LthlJDY33OvlrXzQ4/k2SqU8aRuMiBFYYK7/nL s9otAFIlqjPcH1UvGUNqUIdFOR4o3LFZkkZkdliaWdxCnmtfq3TQLChpZCmBVWMR9eOn N3PAAksM52kuYYultNi1jYi+AZ8ADz5qw7ybHsmSVQErTOebuJh/zH75llQImK1M5tbG XYE+Z280HXYFMdKuRLaKKt8rADkeX15rsBorIjGj810enPOLhglLO6CEdSlO8agSwBhs mjD/BxjzZIqFT3xLUzyfysGgSWHlTxTH9R8bYxLRrN2Y7CHLUL/8Y3INs5z27A1mMc1N JNaw== X-Gm-Message-State: AN3rC/7g5/HFaHYh0Y6iJtanwXdflvfk/MAw4fs/nY1zl//b1Yw7zzxu YNUA4Aztf2MhI4V8jIm1FVu1htGJOFuP X-Received: by 10.36.103.9 with SMTP id u9mr8621515itc.44.1493373134999; Fri, 28 Apr 2017 02:52:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.178.76 with HTTP; Fri, 28 Apr 2017 02:51:54 -0700 (PDT) In-Reply-To: References: From: Ashesh Vashi Date: Fri, 28 Apr 2017 15:21:54 +0530 Message-ID: Subject: Re: Pains and thoughts about refactoring the Tree Menu using React To: Joao Pedro De Almeida Pereira Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a114ac8fe3ac9ae054e370736 X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a114ac8fe3ac9ae054e370736 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sat, Apr 22, 2017 at 12:17 AM, Joao Pedro De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hi Hackers, > > After a conversation with Dave we believe that we need to provide more > context on our pains and what we propose as a first step for implementing > the Tree Menu using React. > > The pain: > > Here is a quick description of what we think we understand and where we > got stuck while doing our refactor. > > If you look at the above image, both the NodeJS and CollectionJS classes > are templates. Our goal was to extract those from being templates so that > we could test them. We started with the generate_url function. The issue = we > ran into was that the generate_url function is inherited by CollectionJS > from NodeJS but then over writes some of the functionality that it > inherits. In order to have one function that our react component would > delegate to, we have to consolidate these two class methods. This turned > out to be a non-trivial refactor due to the lack of tests/documentation. > > Our proposal: > > > This diagram represents what we believe can be our first approach to > updating the tree. We can create a very basic tree that only does tree > stuff (Open Node, Close Nodes) and then delegates to other adapters to > execute all the business logic functionality (Generate URLs to get the > node, Right Click menu, =E2=80=A6) > We will loose the current extendible architecture of by creating fix number of adapters, instead - we should generates events like right now, let the modules/extensions decide - what to do. There are many operations are dependents on the tree events. e.g. - Before open When server node is being opened, 'beforeopen' event checks if server is connected, or not. If not try to connect it, otherwise it does not let it open it - Added Dependent modules libraries are loaded, when a particular type of node is added. i.e. When the first database node is added in the tree, it loads all the schema level, and other javascript modules. - Selected When any of the node is added, it shows properties of the selected, change the dashboard context (if necessary). Current tree generates many events. beforeload, beforeappend, added, appended, loaded, init, beforeselect, selected, beforefocus, focused, focus, blurred, beforetoggle, beforeopen, openned, toggled, etc. We generate 'pgadmin-browser:tree' events on a pgBrowser.Events objects to be used by different modules. At the moment, 'pgadmin-browser:tree:selected' is widely used by different modules. i.e. SQL, dependents, dependencies, properties, Dashboard, etc. If you want to see the complete list of events generated, you can use the following patch. *diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js* *index 5b1c3a7..575c78d 100644* *--- a/web/pgadmin/browser/static/js/datamodel.js* *+++ b/web/pgadmin/browser/static/js/datamodel.js* *@@ -1149,5 +1149,10 @@ function(_, pgAdmin, $, Backbone) {* * pgBrowser.Events =3D _.extend({}, Backbone.Events);* *+ pgBrowser.Events.on(* *+ 'pgadmin-browser:tree', function() {* *+ console.log(arguments[0]);* *+ });* *+* * return pgBrowser;* * });* Apart from them, we also need to understand, how the tree traversal will work. How would you traverse through a particular node, and expand all its parent= ? > Asks: > > - > > Are there any more operation that currently are being triggered by the > tree? If so we need to add them to the Adapter List. > > Please see my above comments. > > - > > Because we lack the context and knowledge of the current tree > implementation, we need your help to extract these methods from the pl= aces > they are currently in. We believe that a good place to start would be > writing tests for and implementing the generation of URLs used to fetc= h the > child nodes. > > Attached you can find an example of the JavaScript tests that we have bee= n > writing that can be applied to this extracted method. > You may want to start from here: https://www.pgadmin.org/docs4/dev/code_snippets.html#nodeview Every tree node is inherited from this class, you can start looking at the DatabaseView, and DatabaseModule class. -- Thanks, Ashesh > > Thanks > > Rob, Oliver & Joao > > --001a114ac8fe3ac9ae054e370736 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On S= at, Apr 22, 2017 at 12:17 AM, Joao Pedro De Almeida Pereira <jd= ealmeidapereira@pivotal.io> wrote:

Hi Hackers,


After a conversation with Dave we believe that we need to provide = more context on our pains and what we propose as a first step for implement= ing the Tree Menu using React.


The pain:

Here is a quick description o= f what we think we understand and where we got stuck while doing our refact= or.


If you look at the above image, both the NodeJS and CollectionJS c= lasses are templates. Our goal was to extract those from being templates so= that we could test them. We started with the generate_url function. The is= sue we ran into was that the generate_url function is inherited by Collecti= onJS from NodeJS but then over writes some of the functionality that it inh= erits. In order to have one function that our react component would delegat= e to, we have to consolidate these two class methods. This turned out to be= a non-trivial refactor due to the lack of tests/documentation.

<= br>

Our proposal:


This diagram repres= ents what we believe can be our first approach to updating the tree. We can= create a very basic tree that only does tree stuff (Open Node, Close Nodes= ) and then delegates to other adapters to execute all the business logic fu= nctionality (Generate URLs to get the node, Right Click menu, =E2=80=A6)

We will loose the current extendible= architecture of by creating fix number of adapters, instead - we should ge= nerates events like right now, let the modules/extensions decide - what to = do.

There are many operations are dependents on th= e tree events.
e.g.
- Before open
When server= node is being opened, 'beforeopen' event checks if server is conne= cted, or not.
If not try to connect it, otherwise it does not let= it open it

- Added
Dependent modules li= braries are loaded, when a particular type of node is added.
i.e.= When the first database node is added in the tree, it loads all the schema= level, and other javascript modules.

- Selected
When any of the node is added, it shows properties of the selected= , change the dashboard context (if necessary).

Cur= rent tree generates many events.
beforeload, beforeappend, a= dded, appended, loaded, init, beforeselect, selected, beforefocus, focused,= focus, blurred,=C2=A0beforetoggle,=C2=A0beforeopen, openned, toggled, etc.=

We generate 'pgadmin-browser:tree' = events on a pgBrowser.Events objects to be used by different modules.
=
At the moment, 'pgadmin-browser:tree:selected' is widely used = by different modules.
i.e. SQL, dependents, dependencies, propert= ies, Dashboard, etc.

If you want to see the comple= te list of events generated, you can use the following patch.
d= iff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browse= r/static/js/datamodel.js
index 5b1c3a7..575c78d 100644
<= div>--- a/web/pgad= min/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/d= atamodel.js
@@ -1149,5 +1149,10 @@ function(_, pgAdmin, $, Backbone) {

=C2=A0 =C2=A0 =C2=A0pgBrowser.Events =3D _.extend({}, Backbone.Ev= ents);

+ =C2=A0 =C2=A0pgBrowser.Events.on(
= + =C2=A0 =C2=A0 = =C2=A0'pgadmin-browser:tree', function() {
= + =C2=A0 =C2=A0 =C2= =A0 =C2=A0console.log(arguments[0]);
+ =C2=A0 =C2=A0 =C2=A0});
+
= =C2=A0 =C2=A0 =C2=A0return pgBrowser;
=C2=A0});=C2=A0

Apart from them, we also need to understand, how th= e tree traversal will work.
How would you traverse through a part= icular node, and expand all its parent?


Asks:

  • Are = there any more operation that currently are being triggered by the tree? If= so we need to add them to the Adapter List.

Please see my above comments.=C2=A0
    =

  • Because we lack the context and knowledge o= f the current tree implementation, we need your help to extract these metho= ds from the places they are currently in. We believe that a good place to s= tart would be writing tests for and implementing the generation of URLs use= d to fetch the child nodes.

Attached you can find an example of the JavaScript tests that we = have been writing that can be applied to this extracted method.

<= /span>

Every tree node is inherited from this class, you can start looking= at the DatabaseView, and DatabaseModule class.

--= Thanks, Ashesh

<= div>
Thanks

= Rob, Oliver & Joao



--001a114ac8fe3ac9ae054e370736--