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

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 <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