Subject: [pgAdmin][RM1802] ERD Tool (Beta)

From: Aditya Toshniwal <[email-masked]>
Date: Fri, 25 Dec 2020 15:31:00 +0530

Hi Hackers,

Attached patch introduces ERD Tool(Beta) to pgAdmin. Below are the details:
1) Create a diagram from scratch or generate for an existing DB.
2) Generate "Create" DDL from the diagram.
3) Save the diagram and resume it later.
4) Supports basic table fields, one-to-many relationships, many-to-many
relationships, adding notes.
5) Test cases added with 75-80% test coverage.

Please review.

-- 
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
<http://edbpostgres.com>
"Don't Complain about Heat, Plant a TREE"

---

From: Akshay Joshi <[email-masked]>
Date: Fri, 25 Dec 2020 16:33:50 +0530

Hi Khushboo,

Can you please review it?

-- 
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

---

From: Khushboo Vashi <[email-masked]>
Date: Mon, 28 Dec 2020 10:53:37 +0530

On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi <[email-masked]>
wrote:

> On it.

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 12 Jan 2021 12:18:11 +0530

Hi Aditya,

The functionalities and the code looks good to me, however some of the
comments as below:

   - Correct the comments at some places (3 occurrences found  in
/erd/__init__.py)  which mention Schema diff instead of ERD.

Some comments in the JS/JSX file regarding components/functions (For
example, IconButton (forwardRef), Bodywidget etc.) would

be great help as we all are new to React.

   - Remove the unused imports (for ex bad_request) in /erd/__init__.py
   - Remove commented code

# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False

   - TableNode.jsx, below two lines can be combined.

import { PortModelAlignment, DefaultNodeModel } from
'@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';

   - onImageClick function in BodyWidget.jsx is no use I think, so it
should be removed.

   - I got some console errors while adding/editing tables. Refer to
the attached screenshot.
   - In the column Edit Mode, while deleting the primary key, it gives
the error, which does not go away with any further modifications.
   - While generating the SQL, if the server is disconnected, a proper
error message should be thrown, right now some server side error is
coming.
   - Please remove ... from the menu title (New ERD Project(Beta)...)
as it is not opening a dialog.
   - For large data sets, generate ERD hangs.
   - Opening the ERD panel in a new window is not working, it opens in
the same tab even if you have set the Preference "Open in new browser
tab" to True.
   - No shortcut is provided to open the ERD Tool.
   - SonarQube fixes required.
   -

*Suggestion:*

While removal of the FK link, If any of the table is selected, it is
being deleted with FK link.
Either we should warn the user OR make 2 different buttons for FK
removal and table removal as the user may be confused if the selected
table is also removed with the FK.

*Observations:*

Lodash has been used in this module in place of Underscore, though the
dependency is already introduced by another module,
but we have mentioned it in the package.json file, which is somewhat not
convincing to me.
Lodash is more advanced than Underscore but we should pick anyone as it
will be easy to manage.

Table dialog code is duplicate of the table node, as it was difficult to
extend it because it was attached to the tree.
So, we need to keep in mind that while implementing React in pgAdmin, the
nodes should be properly detached from the tree itself, so we can reuse it.

Thanks,
Khushboo

On Mon, Dec 28, 2020 at 10:53 AM Khushboo Vashi <
[email-masked]> wrote:

>
>
> On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi <
> [email-masked]> wrote:
>
>> Hi Khushboo,
>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Fri, 15 Jan 2021 19:00:37 +0530

Hi,

I've fixed the issues. You can find the comments inline.
I've also added PropTypes for the components for increased validation.

On Tue, Jan 12, 2021 at 12:18 PM Khushboo Vashi <
[email-masked]> wrote:

> Hi Aditya,
>
> The functionalities and the code looks good to me, however some of the comments as below:
>
>
>    - Correct the comments at some places (3 occurrences found  in /erd/__init__.py)  which mention Schema diff instead of ERD.
>
> Some comments in the JS/JSX file regarding components/functions (For example, IconButton (forwardRef), Bodywidget etc.) would
>
> Done. Added comments to the components.

> Removed.

> Removed.

> Done.

>
>    - onImageClick function in BodyWidget.jsx is no use I think, so it should be removed.
>
> I wanted to keep the code as it will be used in future. Anyway, I've
removed the code.

>
>    - I got some console errors while adding/editing tables. Refer to the attached screenshot.
>
> I tried but I didn't get any. Looking at the screenshot, the error is from
the underlying library. Can't do much in this.

>
>    - In the column Edit Mode, while deleting the primary key, it gives the error, which does not go away with any further modifications.
>
> Fixed.

>
>    - While generating the SQL, if the server is disconnected, a proper error message should be thrown, right now some server side error is coming.
>
> It will show connection lost error now. Fixed.

>
>    - Please remove ... from the menu title (New ERD Project(Beta)...) as it is not opening a dialog.
>
> Done.

> It shows the spinner and waits for the response to come from the back end.
I've used the existing table fetching code which is used at other places.
I'll create an RM to improve the back end code for fetching the tables data
which will help the schema diff tool as well.

>
>    - Opening the ERD panel in a new window is not working, it opens in the same tab even if you have set the Preference "Open in new browser tab" to True.
>
> Fixed. Added the setting in "Tab settings".

> A shortcut is helpful if we are using it frequently. ERD tool won't be
used that frequently. We already have a limited number of keys available
for shortcuts. I think we should roll out without shortcut for now. If
there is a user demand for it then we can think of adding it.

> Fixed.

>
>    -
>
> *Suggestion:*
>
> While removal of the FK link, If any of the table is selected, it is being deleted with FK link.
> Either we should warn the user OR make 2 different buttons for FK removal and table removal as the user may be confused if the selected table is also removed with the FK.
>
> I've added a confirmation dialog which will show the number of tables and
links selected. This way user will know what he has selected before
deleting.

> TL;DR; we cannot.
lodash is a peer dependency for react-diagrams (and some existing modules
in pgAdmin) so it will come to package.json without choice. We cannot
remove underscore because it is a dependency of backbone. Underscore is
outdated, and I cannot migrate the complete pgAdmin code. So, I decided to
go with 100/0 method. All the new codes will use lodash only as we'll phase
out underscore with time. Just like jQuery vs ReactJS.

> Yes. I agree. We need to separate out data source from UI going forward with
React.

---

From: Akshay Joshi <[email-masked]>
Date: Sat, 16 Jan 2021 17:07:53 +0530

Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <
[email-masked]> wrote:

>> I tried but I didn't get any. Looking at the screenshot, the error is
> from the underlying library. Can't do much in this.
>
>>
>> It shows the spinner and waits for the response to come from the back
> end. I've used the existing table fetching code which is used at other
> places. I'll create an RM to improve the back end code for fetching the
> tables data which will help the schema diff tool as well.
>
>>
> outdated, and I cannot migrate the complete pgAdmin code. So, I decided
> to go with 100/0 method. All the new codes will use lodash only as we'll
> phase out underscore with time. Just like jQuery vs ReactJS.
>
>>
>>
>>>>> 4) Supports basic table fields, one-to-many relationships,
>>>>> many-to-many relationships, adding notes.

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 18 Jan 2021 10:33:32 +0530

Hi Akshay,

I forgot to remove few of the dependencies which are not required as of now
(may be in future). Attached patch removes those dependencies from
package.json.

On Sat, Jan 16, 2021 at 5:08 PM Akshay Joshi <[email-masked]>
wrote:

>>> I've added a confirmation dialog which will show the number of tables
>> and links selected. This way user will know what he has selected before
>> deleting.
>>
>>> *Observations:*
>>>
>>> So, we need to keep in mind that while implementing React in pgAdmin,
>>> the nodes should be properly detached from the tree itself, so we can reuse
>>> it.
>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 18 Jan 2021 13:09:50 +0530

Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <
[email-masked]> wrote:

> Hi Akshay,
>
> I forgot to remove few of the dependencies which are not required as of
> now (may be in future). Attached patch removes those dependencies from
> package.json.
>
> On Sat, Jan 16, 2021 at 5:08 PM Akshay Joshi <
>>> used that frequently. We already have a limited number of keys
>>> available for shortcuts. I think we should roll out without shortcut for
>>> now. If there is a user demand for it then we can think of adding it.
>>>
>>>>
>>>> but we have mentioned it in the package.json file, which is somewhat
>>>> not convincing to me.
>>> lodash is a peer dependency for react-diagrams (and some existing
>>> modules in pgAdmin) so it will come to package.json without choice. We
>>> cannot remove underscore because it is a dependency of backbone. Underscore
>>> is outdated, and I cannot migrate the complete pgAdmin code. So, I
>>> decided to go with 100/0 method. All the new codes will use lodash only as
>>> we'll phase out underscore with time. Just like jQuery vs ReactJS.
>>>
>>>>
>>>>
>>>> Table dialog code is duplicate of the table node, as it was difficult
>>>> to extend it because it was attached to the tree.
>>>> Yes. I agree. We need to separate out data source from UI going forward
>>>  with React.
>>>
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 18 Jan 2021 14:57:53 +0530

Hi,

The jasmine test cases are working fine on my local machine. The test cases
are successful on jenkins other than on linux, not sure why.
I have made some fixes by looking at the log. Please review and try.

On Mon, Jan 18, 2021 at 1:10 PM Akshay Joshi <[email-masked]>
wrote:

>>>>> Lodash is more advanced than Underscore but we should pick anyone as
>>>>> it will be easy to manage.
>>>>>
>>>>> TL;DR; we cannot.

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 18 Jan 2021 16:40:39 +0530

Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <
[email-masked]> wrote:

> Hi,
>
> The jasmine test cases are working fine on my local machine. The test
> cases are successful on jenkins other than on linux, not sure why.
> On Mon, Jan 18, 2021 at 1:10 PM Akshay Joshi <
>>>>>> A shortcut is helpful if we are using it frequently. ERD tool won't
>>>>> be used that frequently. We already have a limited number of keys
>>>>>> Lodash has been used in this module in place of Underscore, though
>>>>>> the dependency is already introduced by another module,
>>>>>> Yes. I agree. We need to separate out data source from UI going
>>>>> forward with React.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 18 Jan 2021 17:07:42 +0530

OK, So the changes have worked. But still failing at one more place.
Attached the patch fixes it.

On Mon, Jan 18, 2021 at 4:40 PM Akshay Joshi <[email-masked]>
wrote:

>>>>>>> I tried but I didn't get any. Looking at the screenshot, the error
>>>>>> is from the underlying library. Can't do much in this.
>>>>>>
>>>>>>>
>>>>>>> It shows the spinner and waits for the response to come from the
>>>>>> back end. I've used the existing table fetching code which is used at other
>>>>>>> I've added a confirmation dialog which will show the number of
>>>>>> tables and links selected. This way user will know what he has selected
>>>>>> before deleting.
>>>>>>
>>>>>>> *Observations:*
>>>>>>>
>>>>>>> Table dialog code is duplicate of the table node, as it was
>>>>>>> difficult to extend it because it was attached to the tree.
>>>>>>> So, we need to keep in mind that while implementing React in
>>>>>>> pgAdmin, the nodes should be properly detached from the tree itself, so we
>>>>>>> can reuse it.
>>>>>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 18 Jan 2021 17:15:11 +0530

Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <
[email-masked]> wrote:

> On Mon, Jan 18, 2021 at 4:40 PM Akshay Joshi <
>>>>> I forgot to remove few of the dependencies which are not required as
>>>>> of now (may be in future). Attached patch removes those dependencies from
>>>>> package.json.
>>>>>
>>>>>>>> I wanted to keep the code as it will be used in future. Anyway,
>>>>>>> I've removed the code.
>>>>>>>
>>>>>>>>
>>>>>>>> but we have mentioned it in the package.json file, which is
>>>>>>>> somewhat not convincing to me.
>>>>>>>> Lodash is more advanced than Underscore but we should pick anyone
>>>>>>>> as it will be easy to manage.
>>>>>>>>
>>>>>>>> TL;DR; we cannot.

---

From: Dave Page <[email-masked]>
Date: Wed, 20 Jan 2021 15:50:13 +0000

Hi

Where's the Save Image button gone? I know Aditya was removing it whilst
working on other things, but it's still required for phase 1 release.

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[email-masked]>
wrote:

>>>>>>>>> A shortcut is helpful if we are using it frequently. ERD tool
>>>>>>>> won't be used that frequently. We already have a limited number of
>>>>>>>> keys available for shortcuts. I think we should roll out without shortcut
>>>>>>>> for now. If there is a user demand for it then we can think of adding it.
>>>>>>>>
>>>>>>>>>

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

---

From: Aditya Toshniwal <[email-masked]>
Date: Thu, 21 Jan 2021 10:17:42 +0530

Hi Dave,

> working on other things, but it's still required for phase 1 release.
It was not working 100% right. :(
So I've removed it for the time being. I'm still working on it.

>
> On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <
>>>>>>>>>> I tried but I didn't get any. Looking at the screenshot, the
>>>>>>>>> error is from the underlying library. Can't do much in this.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> won't be used that frequently. We already have a limited number
>>>>>>>>> of keys available for shortcuts. I think we should roll out without
>>>>>>>>> shortcut for now. If there is a user demand for it then we can think of
>>>>>>>>> adding it.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Lodash has been used in this module in place of Underscore,
>>>>>>>>>> though the dependency is already introduced by another module,
>>>>>>>>> is outdated, and I cannot migrate the complete pgAdmin code. So,
>>>>>>>>> I decided to go with 100/0 method. All the new codes will use lodash only
>>>>>>>>> as we'll phase out underscore with time. Just like jQuery vs ReactJS.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>

---

From: Dave Page <[email-masked]>
Date: Thu, 21 Jan 2021 09:38:46 +0000

> So I've removed it for the time being. I'm still working on it.

OK, so that work will be completed in time for the build next week?

>>>>>>>> I forgot to remove few of the dependencies which are not required
>>>>>>>> as of now (may be in future). Attached patch removes those dependencies
>>>>>>>> from package.json.
>>>>>>>>
>>>>>>>>>>> Lodash is more advanced than Underscore but we should pick
>>>>>>>>>>> anyone as it will be easy to manage.
>>>>>>>>>>>
>>>>>>>>>>> TL;DR; we cannot.

---

From: Aditya Toshniwal <[email-masked]>
Date: Thu, 21 Jan 2021 16:32:51 +0530

Hi,

> OK, so that work will be completed in time for the build next week?
I'm trying my best to make it available before release. I'm struggling to
make it work perfectly.

>>>>>>> The jasmine test cases are working fine on my local machine. The
>>>>>>> test cases are successful on jenkins other than on linux, not sure why.
>>>>>>>>>>>> It shows the spinner and waits for the response to come from
>>>>>>>>>>> the back end. I've used the existing table fetching code which is used at
>>>>>>>>>>> other places. I'll create an RM to improve the back end code for fetching
>>>>>>>>>>> the tables data which will help the schema diff tool as well.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> lodash is a peer dependency for react-diagrams (and some
>>>>>>>>>>> existing modules in pgAdmin) so it will come to package.json without
>>>>>>>>>>> choice. We cannot remove underscore because it is a dependency of backbone.
>>>>>>>>>>> Underscore is outdated, and I cannot migrate the complete pgAdmin code. So,

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 25 Jan 2021 17:12:45 +0530

Hi Hackers,

Attached is the patch to fix below issues in ERD:

   1. After opening an existing project, the first table is already
   selected but edit, clone, delete buttons are disable. Fixed.
   2. ERD project title gets changed when 2 ERD projects are open & anyone
   of it edited. Fixed.
   3. Closing ERD tab, does not ask for confirmation pop up. Added.
   4. Shortcut for 'Show more/Fewer details' is missing. Added.
   5. Deleting primary key does not delete associated links. Fixed.
   6. Long table & schema name are getting out of box. Fixed.
   7. Long table name in notes pop-up need re-alignment. Fixed.
   8. Same table name present in ERD/canvas is allowed in Add Table dialogue.
   Added validation in the dialog.
   9. Download image option is added, but it is not perfect yet. Image
   icons (table, schema, etc.) are not showing up.
   10. Rename panel option should be disabled by default. It should be
   enabled for the tools which implement rename functionality.
   11. The Toolbar is not visible in Safari for the ERD tool. Fixed.

Please review.

On Thu, Jan 21, 2021 at 4:32 PM Aditya Toshniwal <
[email-masked]> wrote:

>>>> Where's the Save Image button gone? I know Aditya was removing it
>>>> whilst working on other things, but it's still required for phase 1 release.
>>>>
>>>>>>>>>>>>> I wanted to keep the code as it will be used in future.
>>>>>>>>>>>> Anyway, I've removed the code.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>> won't be used that frequently. We already have a limited
>>>>>>>>>>>> number of keys available for shortcuts. I think we should roll out without
>>>>>>>>>>>>> Yes. I agree. We need to separate out data source from UI
>>>>>>>>>>>> going forward with React.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>
>>>>>>>>>>>>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 25 Jan 2021 17:17:40 +0530

Hi,

Please find the rebased patch from the latest pull.

On Mon, Jan 25, 2021 at 5:12 PM Aditya Toshniwal <
[email-masked]> wrote:

>    2. ERD project title gets changed when 2 ERD projects are open &
>    anyone of it edited. Fixed.
>    8. Same table name present in ERD/canvas is allowed in Add Table
>    dialogue. Added validation in the dialog.
>>>>>>>>>>> I forgot to remove few of the dependencies which are not
>>>>>>>>>>> required as of now (may be in future). Attached patch removes those
>>>>>>>>>>> dependencies from package.json.
>>>>>>>>>>>
>>>>>>>>>>>>>> I've added a confirmation dialog which will show the number
>>>>>>>>>>>>> of tables and links selected. This way user will know what he has selected
>>>>>>>>>>>>> before deleting.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Observations:*
>>>>>>>>>>>>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 25 Jan 2021 17:33:54 +0530

Thanks, patch applied.

On Mon, Jan 25, 2021 at 5:18 PM Aditya Toshniwal <
[email-masked]> wrote:

>>> I'm trying my best to make it available before release. I'm struggling
>>> to make it work perfectly.
>>>
>>>>
>>>>
>>>>>
>>>>>>>>>>>>>>> A shortcut is helpful if we are using it frequently. ERD
>>>>>>>>>>>>>> tool won't be used that frequently. We already have a
>>>>>>>>>>>>>> limited number of keys available for shortcuts. I think we should roll out
>>>>>>>>>>>>>> without shortcut for now. If there is a user demand for it then we can
>>>>>>>>>>>>>> think of adding it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 4) Supports basic table fields, one-to-many
>>>>>>>>>>>>>>>>>> relationships, many-to-many relationships, adding notes.
>>>>>>>>>>>>>>>>>> pgAdmin hacker | Sr. Software Engineer |
>>>>>>>>>>>>>>>>>> *edbpostgres.com* <http://edbpostgres.com>

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 25 Jan 2021 19:38:43 +0530

Hi,

The attached patch fixes an issue created by existing table name check.
Please review.

On Mon, Jan 25, 2021 at 5:34 PM Akshay Joshi <[email-masked]>
wrote:

>>>>>>>>>>>>>>>> I tried but I didn't get any. Looking at the screenshot,
>>>>>>>>>>>>>>> the error is from the underlying library. Can't do much in this.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It shows the spinner and waits for the response to come
>>>>>>>>>>>>>>> from the back end. I've used the existing table fetching code which is used
>>>>>>>>>>>>>>> at other places. I'll create an RM to improve the back end code for
>>>>>>>>>>>>>>> fetching the tables data which will help the schema diff tool as well.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So, we need to keep in mind that while implementing React
>>>>>>>>>>>>>>>> in pgAdmin, the nodes should be properly detached from the tree itself, so
>>>>>>>>>>>>>>>> we can reuse it.
>>>>>>>>>>>>>>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 25 Jan 2021 19:47:03 +0530

Thanks, patch applied.

On Mon, Jan 25, 2021 at 7:39 PM Aditya Toshniwal <
[email-masked]> wrote:

> On Mon, Jan 25, 2021 at 5:34 PM Akshay Joshi <
>>>>>>>>>>>> The jasmine test cases are working fine on my local machine.
>>>>>>>>>>>> The test cases are successful on jenkins other than on linux, not sure why.
>>>>>>>>>>>>>>>>> I've added a confirmation dialog which will show the
>>>>>>>>>>>>>>>> number of tables and links selected. This way user will know what he has
>>>>>>>>>>>>>>>> selected before deleting.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> *Observations:*
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Lodash has been used in this module in place of
>>>>>>>>>>>>>>>>> Underscore, though the dependency is already introduced by another module,
>>>>>>>>>>>>>>>>> but we have mentioned it in the package.json file, which
>>>>>>>>>>>>>>>>> is somewhat not convincing to me.
>>>>>>>>>>>>>>>>> Table dialog code is duplicate of the table node, as it
>>>>>>>>>>>>>>>>> was difficult to extend it because it was attached to the tree.