On Thu, Jan 21, 2021 at 4:48 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:
> Hi Dave,
>
> On Wed, Jan 20, 2021 at 9:20 PM Dave Page <dp...@pgadmin.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?
>
>> On Mon, Jan 18, 2021 at 11:45 AM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Thanks, patch applied.
>>>
>>> On Mon, Jan 18, 2021 at 5:08 PM Aditya Toshniwal <
>>> aditya.toshni...@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.jo...@enterprisedb.com> wrote:
>>>>
>>>>> Thanks, patch applied.
>>>>>
>>>>> On Mon, Jan 18, 2021 at 2:58 PM Aditya Toshniwal <
>>>>> aditya.toshni...@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.jo...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Thanks, patch applied.
>>>>>>>
>>>>>>> On Mon, Jan 18, 2021 at 10:34 AM Aditya Toshniwal <
>>>>>>> aditya.toshni...@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.jo...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>>>>> Thanks, patch applied.
>>>>>>>>>
>>>>>>>>> On Fri, Jan 15, 2021 at 7:01 PM Aditya Toshniwal <
>>>>>>>>> aditya.toshni...@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.va...@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.va...@enterprisedb.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Dec 25, 2020 at 4:34 PM Akshay Joshi <
>>>>>>>>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can you please review it?
>>>>>>>>>>>>>
>>>>>>>>>>>>> On it.
>>>>>>>>>>>>
>>>>>>>>>>>>> On Fri, Dec 25, 2020 at 3:31 PM Aditya Toshniwal <
>>>>>>>>>>>>> aditya.toshni...@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*
>>>>>>>>>>>>>> <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
