Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l2XkU-0005kF-2t for pgadmin-hackers@arkaria.postgresql.org; Thu, 21 Jan 2021 11:03:34 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l2XkT-0003Gs-0C for pgadmin-hackers@arkaria.postgresql.org; Thu, 21 Jan 2021 11:03:33 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l2XkS-0003Gl-N3 for pgadmin-hackers@lists.postgresql.org; Thu, 21 Jan 2021 11:03:32 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1l2XkP-0003gp-RJ for pgadmin-hackers@postgresql.org; Thu, 21 Jan 2021 11:03:32 +0000 Received: by mail-lf1-x12b.google.com with SMTP id m22so1928646lfg.5 for ; Thu, 21 Jan 2021 03:03:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FJVYoyNmZfsgs+pI01eDiiBrvpp90iUDf7Qv5PWT9JI=; b=R1FzYJ1dJsOnkJnMLq1aQHqR7M3xBj+0tGQb3YKevyBR9u6nvZRzkr/RLFmyjWpQyb EYPPyw7ZxiD7plNKIPu8eqikeuIvULNsdmhTDPQZUdc89J4Cf2Szg1u1+9Q9r+W0H1H3 WTwwmi3hgpsERyZOYK/fCRKQcuYYBvKtJSG1Xsi1u1FJiYnW+WHLDhfbCLcoGP3DoGVC 27x8/yDfNyUnE2XmCFXJ61SyEkHZrPu5s6TlGu+L8qKXrFpFLvBv8qoEtk75MPvL/pm2 QU//5SgKnbj7eeOyCd5tBnXDy3oImeTs9XVtlF7Uctrzg3pPVll4hUNGa34bfy4nqXLx nmRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FJVYoyNmZfsgs+pI01eDiiBrvpp90iUDf7Qv5PWT9JI=; b=BXhxjCS425NsfW0n1HJAPNJ4x+BM1DvQu58WalvY2KvORM930Pt86n6mpMMYm8DHsM UFoJTUurtY5J+vJBbbcq/W4yiGvNFEU5A1K6S0cfkLs/y/i6aWolNlrpx0U3FbCU6d9+ bx2aD/+pSU3JMsfqn0LuHxK0765ByJ4IOqweTjRKc8QBcIWpJqPUeroz8AOBKv4fR35S cx1VKyNoJfIqUDrpnwfg1E0Tdc3tuCHBhUeWEDlA9+NeBGry2RB8qvP5bwsAMcFeDkP1 RnkY3EkrxSqs0lTu3O6ZTn/V3QkImQuzO419GHl7xHwdLFEEKVLJ6I58ZFA6C7vMvnay hzdg== X-Gm-Message-State: AOAM531cvIU/2Dn+ngCBoW+jOjbYhRfv4eTpfJvPVKpI+a7uzzIbmWmz EQCJ0RqTZEyno0udxCjLDFCogg6eMv7R6n++BbljSa14WPpwW4U7WYpwRW6BF+1GPYpKtenAP98 uNpxtkbKggWLTeX1Ubr7dRS6EEyBzp5T4iuDoLfzII1iI7DnEi1JNjgg6NroZ0Qb/t/AoxIwBsA D/+jZNjmHbZ/9ZQ9jqkFJJsEnccR6FV7fTcSMzD+wqmVdViraV/UeXACgL3A== X-Google-Smtp-Source: ABdhPJwfOVddOA05P8xh+mkyqTIrE54UVsRVkDHuveNWIxvNUHvl9thm5KA+oU+DkCXi6uZErJDRpR7nyni7Ve+udFo= X-Received: by 2002:a05:6512:74e:: with SMTP id c14mr6507600lfs.529.1611227007991; Thu, 21 Jan 2021 03:03:27 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Aditya Toshniwal Date: Thu, 21 Jan 2021 16:32:51 +0530 Message-ID: Subject: Re: [pgAdmin][RM1802] ERD Tool (Beta) To: Dave Page Cc: Akshay Joshi , Khushboo Vashi , pgadmin-hackers Content-Type: multipart/alternative; boundary="00000000000076d7dc05b96705cf" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000076d7dc05b96705cf Content-Type: text/plain; charset="UTF-8" Hi, On Thu, Jan 21, 2021 at 3:08 PM Dave Page wrote: > > > On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal < > aditya.toshniwal@enterprisedb.com> wrote: > >> Hi Dave, >> >> On Wed, Jan 20, 2021 at 9:20 PM Dave Page 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.joshi@enterprisedb.com> wrote: >>> >>>> Thanks, patch applied. >>>> >>>> On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal < >>>> aditya.toshniwal@enterprisedb.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.joshi@enterprisedb.com> wrote: >>>>> >>>>>> Thanks, patch applied. >>>>>> >>>>>> On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal < >>>>>> aditya.toshniwal@enterprisedb.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.joshi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Thanks, patch applied. >>>>>>>> >>>>>>>> On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal < >>>>>>>> aditya.toshniwal@enterprisedb.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.joshi@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Thanks, patch applied. >>>>>>>>>> >>>>>>>>>> On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal < >>>>>>>>>> aditya.toshniwal@enterprisedb.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.vashi@enterprisedb.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.vashi@enterprisedb.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi < >>>>>>>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Khushboo, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Can you please review it? >>>>>>>>>>>>>> >>>>>>>>>>>>>> On it. >>>>>>>>>>>>> >>>>>>>>>>>>>> On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal < >>>>>>>>>>>>>> aditya.toshniwal@enterprisedb.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* >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> "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" --00000000000076d7dc05b96705cf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,


On Thu, Jan 21, 2021 at 3:08 PM = Dave Page <dpage@pgadmin.org>= ; wrote:


On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshn= iwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

On Wed, Jan 20, 20= 21 at 9:20 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

Where's= the Save Image button gone? I know Aditya was removing it whilst working o= n other things, but it's still required for phase 1 release.
It was not working 100% right. :(
So I've r= emoved it for the time being. I'm still working on it.=C2=A0<= /div>

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 struggli= ng to make it work perfectly.
=C2=A0
On Mon, J= an 18, 2021 at 11:45 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrot= e:
Thanks, patch applied.

On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal = <= aditya.toshniwal@enterprisedb.com> wrote:
OK, So the changes have worked. But still failing at o= ne more place.
Attached = the patch fixes it.

On Mon, Jan 18, 2021 at 4:40 PM Akshay Joshi <<= a href=3D"mailto:akshay.joshi@enterprisedb.com" target=3D"_blank">akshay.jo= shi@enterprisedb.com> wrote:
Thanks, patch applied.

On Mon, Jan 18, = 2021 at 2:58 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> = wrote:
Hi,

The jasmine test cases are working=C2=A0fine on my local m= achine. The test cases are successful on jenkins other than on linux, not s= ure why.
I have made som= e fixes by looking at the log. Please review and try.

On Mon, Jan 18, = 2021 at 1:10 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Than= ks, patch applied.

On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <= ;adi= tya.toshniwal@enterprisedb.com> wrote:
Hi Akshay,

I forgot to re= move few of the dependencies which are not required as of now (may be in fu= ture). Attached patch removes those dependencies from package.json.

On= Sat, Jan 16, 2021 at 5:08 PM Akshay Joshi <akshay.joshi@enterprisedb.com>= ; wrote:
Thanks, patch applied.

On Fri, Jan 15, 2021 at 7:01 PM Aditya Tosh= niwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

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

On Tue, Jan 12, 2021 at 1= 2:18 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi Aditya,
The functionalities and the code looks good to me, however so=
me 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 (F=
or 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.=C2=A0
    • Remove com=
mented code
    # req_args = =3D request.args
    # if ('recreate' in req_args and
    # =C2=A0 = =C2=A0 req_args['recreate'] =3D=3D '1'):
    # =C2=A0 =C2=A0= connect =3D False
    Removed= .=C2=A0
    • =
TableNode.jsx, below two lines can be combined.
    import { PortModelAlignment, DefaultNodeModel } from= '@projectstorm/react-diagrams';
    import { PortWidget } from '= ;@projectstorm/react-diagrams';
    Done.=C2=A0
      =

    • onImageClick function in BodyWidget.js=
x is no use I think, so it should be removed.
    <= /div>
    I wanted to keep the code as it will be used in future. = Anyway, I've removed the code.
    • I got some console er=
rors while adding/editing tables. Refer to the attached screenshot.<=
/li>
    I tried but I didn't get any. = Looking at the screenshot, the error is from the underlying library. Can= 9;t do much in this.=C2=A0
    • In the column Edit Mode, whil=
e deleting the primary key, it gives the error, which does not go away with=
 any further modifications.
    =
    Fixed.=C2=A0
    • =
While generating the SQL, if the server is=
 disconnected, a proper error message should be thrown, right now some serv=
er side error is coming.
    I= t will show connection lost error now. Fixed.=C2=A0
    • Plea= se remove ... from the menu title (New ERD Project(Beta)...) as it is not o= pening a dialog.
    Done.=C2=A0
    • 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 fetch= ing code which is used at other places. I'll create an RM to improve th= e back end code for fetching the tables data which will help the schema dif= f 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 i= t frequently. ERD tool won't be used that frequently.=C2=A0We alread= y 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.=C2=A0
    • =

    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 remov=
al and table removal as the user may be confused if the selected table is a=
lso removed with the FK.
    I've added= a confirmation dialog which will show the number of tables and links selec= ted. This way user will know what he has selected before deleting.
    Observations:
    Lodash has been used in this module in place of Un= derscore, though the dependency is already introduced by another module,but we have mentioned it in the package.json file, which is somewhat not c= onvincing to me.
    Lodash is more advanced than Underscore but we should p= ick anyone as it will be easy to manage.
    TL;DR; we cannot.
    lodash is a peer dependency for rea= ct-diagrams (and some existing modules in pgAdmin) so it will come to packa= ge.json without choice. We cannot remove underscore because it is a depende= ncy of backbone. Underscore is outdated, and I cannot migrate the complete = pgAdmin code.=C2=A0So, I decided to go with 100/0 method. All the new co= des 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 pgAd= min, 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=C2=A0with React.

    Thanks,
    Khushboo


    On Mon, Dec 28, 2020 at 10:53 AM Khush= boo Vashi <khushboo.vashi@enterprisedb.com> wrote:


    On Fri, Dec 25, 2020 at 4:= 34 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
    Hi Khushboo,

    Can you plea= se review it?

    On it.=C2=A0
    = On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <aditya.toshniwal@enterpris= edb.com> wrote:
    Hi Hac= kers,

    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 "Creat= e" DDL from the diagram.
    3) Save the diagram and resume it later.
    4) Supports basic table fields, one-to-many relation= ships, many-to-many relationships, adding notes.
    5) Test cases added with 75-80% test coverage.

    Please review.

    --
    Thanks,
    Ad= itya Toshniwal
    <= /font>
    pgAdmin hacker=C2=A0| = Sr. Software Engineer | edbpostgres.com
    "Don't Complain about Heat, Plant a TREE"


    --
    Thanks & Regards
    Akshay Joshi
    = pgAdmin Hacker | Principal Software Ar= chitect
    EDB Po= stgres
    Mobile: +91 976-788-8246

    <= /div>


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


    --
    Thank= s & Regards
    Akshay Joshi
    pgAdmin Hacker | Principal Softw= are Architect
    EDB Po= stgres
    Mobile: +91 976-788-8246



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


    --
    Thank= s & Regards
    Akshay Joshi
    pgAdmin Hacker | Principal Softw= are Architect
    EDB Po= stgres
    Mobile: +91 976-788-8246



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


    --
    Thank= s & Regards
    Akshay Joshi
    pgAdmin Hacker | Principal Softw= are Architect
    EDB Po= stgres
    Mobile: +91 976-788-8246



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


    --
    Thank= s & Regards
    Akshay Joshi
    pgAdmin Hacker | Principal Softw= are Architect
    EDB Po= stgres
    Mobile: +91 976-788-8246



    --


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


    --


    --
    Thanks,
    Aditya Toshniwal
    pgAdmin hacker=C2=A0| Sr. Softwa= re Engineer | edbpostgres.com<= /font>
    "Don't Complain about Heat, Plant a TREE&qu= ot;
    --00000000000076d7dc05b96705cf--