Subject: Pains and thoughts about refactoring the Tree Menu using React

From: Joao Pedro De Almeida Pereira <[email-masked]>
Date: Fri, 21 Apr 2017 14:47:46 -0400

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, …)

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.
   -

   Because we lack the context and knowledge of the current tree
   implementation, we need your help to extract these methods from the places
   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 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.

Thanks

Rob, Oliver & Joao

---

From: Joao Pedro De Almeida Pereira <[email-masked]>
Date: Fri, 21 Apr 2017 14:52:03 -0400

Hi Hackers,

The attachment was missing in the previous email, so there you go.

Note:
 We had to remove the extension of the file due to security reasons, just
add a .js

Thanks
Joao & Oliver

On Fri, Apr 21, 2017 at 2:47 PM, Joao Pedro De Almeida Pereira <
[email-masked]> wrote:

> Here is a quick description of what we think we understand and where we
> got stuck while doing our refactor.
>

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Ashesh Vashi <[email-masked]>
Date: Fri, 28 Apr 2017 15:21:54 +0530

> node, Right Click menu, …)
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 = _.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?

> Please see my above comments.

> 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

---

From: Joao Pedro De Almeida Pereira <[email-masked]>
Date: Mon, 1 May 2017 11:50:41 -0400

> How would you traverse through a particular node, and expand all its

 In the PoC that we sent with React Tree email
<https://www.postgresql.org/message-id/CAFS4TJavz7yp2yOiKApV5w3rpaAH1N5ahDdB92UR28pAjK=[email-masked]>
we
added a service that as a function called buildURL, that would map into the
GenerateURLAdaptor, and this function will create the URL to retrieve the
child nodes of a specific node. Once we have retrieved those child nodes
from the backend api, we would simply add the new nodes as children in the
tree.

Joao & Matt

On Fri, Apr 28, 2017 at 5:51 AM, Ashesh Vashi <[email-masked]
> wrote:

> 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.
>
> At the moment, 'pgadmin-browser:tree:selected' is widely used by
> different modules.
>>    Are there any more operation that currently are being triggered by
>>    the tree? If so we need to add them to the Adapter List.
>>
>> Attached you can find an example of the JavaScript tests that we have
>> been writing that can be applied to this extracted method.
>>