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

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Khushboo Vashi <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Aditya Toshniwal <[email protected]>
Subject: Re: [pgAdmin][RM1802] ERD Tool (Beta)
Date: Tue, 12 Jan 2021 12:18:11 +0530
Message-ID: <CAFOhELdL0-M_itS4z-qNKUq6=RieWqc9UPos=oJTJFjyLrYQQw@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELd4JNuX82Vw3ZNdiKHHwBgu+rTdm9RfwY-qNjGkH47taQ@mail.gmail.com>
References: <CAM9w-_kowrenfKpiRLYWEpHJOOT2k9zRg6KCVpA0FR4Q=CA4hw@mail.gmail.com>
	<CANxoLDda-CPNuU76FM1kOTdhNpRW170fr3FFQnaWxOpG=aeu2A@mail.gmail.com>
	<CAFOhELd4JNuX82Vw3ZNdiKHHwBgu+rTdm9RfwY-qNjGkH47taQ@mail.gmail.com>

Hi Aditya,

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


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

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

be great help as we all are new to React.


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

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


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

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


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


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

*Suggestion:*

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

*Observations:*

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


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

Thanks,
Khushboo


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

>
>
> On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi <
> [email protected]> wrote:
>
>> Hi Khushboo,
>>
>> Can you please review it?
>>
>> On it.
>
>> On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <
>> [email protected]> 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*
>>
>


Attachments:

  [image/png] erd_column_editing.png (105.7K, 3-erd_column_editing.png)
  download | view image

  [image/png] erd_console_errors.png (461.5K, 4-erd_console_errors.png)
  download | view image

view thread (21+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin][RM1802] ERD Tool (Beta)
  In-Reply-To: <CAFOhELdL0-M_itS4z-qNKUq6=RieWqc9UPos=oJTJFjyLrYQQw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox