[pgAdmin][RM1802] ERD Tool (Beta)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[pgAdmin][RM1802] ERD Tool (Beta)

Aditya Toshniwal
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
"Don't Complain about Heat, Plant a TREE"

RM1802.patch (687K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Hi Khushboo,

Can you please review it?

On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Khushboo Vashi


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Khushboo Vashi
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 <[hidden email]> wrote:


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246


erd_column_editing.png (144K) Download Attachment
erd_console_errors.png (631K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

RM1802_v2.patch (690K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

RM1802.removedeps.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

RM1801.jasmine.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

RM1801.jasmine_v2.patch (894 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Dave Page-7
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"
Reply | Threaded
Open this post in threaded view
|

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

Dave Page-7


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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?
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
Hi,


On Thu, Jan 21, 2021 at 3:08 PM Dave Page <[hidden email]> wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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'm trying my best to make it available before release. I'm struggling to make it work perfectly.
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"
Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> wrote:
Hi,


On Thu, Jan 21, 2021 at 3:08 PM Dave Page <[hidden email]> wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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'm trying my best to make it available before release. I'm struggling to make it work perfectly.
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com



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


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

RM1802.fixes_v1.patch (93K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
Hi,

Please find the rebased patch from the latest pull.

On Mon, Jan 25, 2021 at 5:12 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Hi,


On Thu, Jan 21, 2021 at 3:08 PM Dave Page <[hidden email]> wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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'm trying my best to make it available before release. I'm struggling to make it work perfectly.
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com



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


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


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

RM1802.fixes_v2.patch (93K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Akshay Joshi
Thanks, patch applied.

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

Please find the rebased patch from the latest pull.

On Mon, Jan 25, 2021 at 5:12 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Hi,


On Thu, Jan 21, 2021 at 3:08 PM Dave Page <[hidden email]> wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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'm trying my best to make it available before release. I'm struggling to make it work perfectly.
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com



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


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


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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

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

Aditya Toshniwal
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 <[hidden email]> wrote:
Thanks, patch applied.

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

Please find the rebased patch from the latest pull.

On Mon, Jan 25, 2021 at 5:12 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Hi,


On Thu, Jan 21, 2021 at 3:08 PM Dave Page <[hidden email]> wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <[hidden email]> wrote:
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.
It was not working 100% right. :(
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'm trying my best to make it available before release. I'm struggling to make it work perfectly.
 

On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <[hidden email]> 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.
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 <[hidden email]> wrote:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <[hidden email]> 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 <[hidden email]> wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <[hidden email]> wrote:
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 <[hidden email]> 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
be great help as we all are new to React. 
Done. Added comments to the components.
  • Remove the unused imports (for ex bad_request) in /erd/__init__.py
Removed. 
  • Remove commented code
# req_args = request.args
# if ('recreate' in req_args and
#     req_args['recreate'] == '1'):
#     connect = False
Removed. 
  • TableNode.jsx, below two lines can be combined.
import { PortModelAlignment, DefaultNodeModel } from '@projectstorm/react-diagrams';
import { PortWidget } from '@projectstorm/react-diagrams';
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. 
  • For large data sets, generate ERD hangs.
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".
  • No shortcut is provided to open the ERD Tool.
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.
  • SonarQube fixes required.
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.
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.
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.


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.
Yes. I agree. We need to separate out data source from UI going forward with React.

Thanks,
Khushboo


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


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

Can you please review it?

On it. 
On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <[hidden email]> wrote:
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
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

EDB: http://www.enterprisedb.com



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


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

EDB: http://www.enterprisedb.com



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


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


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


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

RM1802.fixes_v3.patch (6K) Download Attachment
12