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

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM1802] ERD Tool (Beta)
Date: 2021-01-21 04:47:42
Message-ID: CAM9w-_nr1mJdnnGj3UBWNNcJCCcFunOzZTu+_q+PeMWixyUnVA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

On Wed, Jan 20, 2021 at 9:20 PM Dave Page <dpage(at)pgadmin(dot)org> 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 <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Thanks, patch applied.
>>
>> On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> 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 <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Thanks, patch applied.
>>>>
>>>> On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> 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 <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Thanks, patch applied.
>>>>>>
>>>>>> On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> 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 <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Thanks, patch applied.
>>>>>>>>
>>>>>>>> On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> 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 <
>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> 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 <
>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi <
>>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>>
>>>>>>>>>>>> Can you please review it?
>>>>>>>>>>>>
>>>>>>>>>>>> On it.
>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <
>>>>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> 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*
>>>>>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> *Thanks & Regards*
>>>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>>>>
>>>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>> Aditya Toshniwal
>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>> <http://edbpostgres.com>
>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Aditya Toshniwal
>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>> <http://edbpostgres.com>
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *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*
<http://edbpostgres.com>
"Don't Complain about Heat, Plant a TREE"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Nikhil Mohite 2021-01-21 06:48:30 [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Previous Message Dave Page 2021-01-20 15:52:37 pgAdmin 4 commit: Fix menu text (the Query tool option doesn't open a d