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

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

Hi,

Please find the rebased patch from the latest pull.

On Mon, Jan 25, 2021 at 5:12 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> 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 <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>>
>> On Thu, Jan 21, 2021 at 3:08 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> 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.
>>>>
>>>
>>> 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 <
>>>>> 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"
>>>>
>>>
>>>
>>> --
>>> 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"
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
> <http://edbpostgres.com>
> "Don't Complain about Heat, Plant a TREE"
>

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

Attachment Content-Type Size
RM1802.fixes_v2.patch application/octet-stream 68.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Rahul Shirsat 2021-01-25 11:58:59 [pgAdmin] RM5871 Show username in 'Connect to Server' pop up when server is created using service & user.
Previous Message Aditya Toshniwal 2021-01-25 11:42:45 Re: [pgAdmin][RM1802] ERD Tool (Beta)