Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.

From: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.
Date: 2022-01-06 08:40:33
Message-ID: CAKtn9dN2s3gOTZEaL_NxOURTD6yQUybPfOS4G-TnZrMqn_Te9Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Hackers,

Please find the updated patch v3, which includes the fixes of -

1. Split the restore options / backup options into 2 tabs
2. Restore icon changes
3. feature tests issues
4. restore help link broken
5. Comment section not visible
6. only data for table - should be yes and disabled

On Tue, Dec 28, 2021 at 1:50 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Thanks, the patch applied.
>
> On Sun, Dec 26, 2021 at 4:18 PM Rahul Shirsat <
> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>
>> Hi Aditya/Akshay,
>>
>> Please find the comments below.
>>
>> On Wed, Dec 22, 2021 at 1:57 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Rahul,
>>>
>>> Everything looks good except:
>>>
>>> 1. Remove unnecessary whitespaces
>>>
>>>
>>> adityatoshniwal(at)LAPTOP381PNIN pgadmin4_copy % git apply
>>> ~/Downloads/RM7018.patch
>>>
>>> /Users/adityatoshniwal/Downloads/RM7018.patch:14070: trailing whitespace.
>>>
>>> constructor(getRestoreSectionSchema, getRestoreTypeObjSchema,
>>> getRestoreSaveOptSchema,
>>>
>>> /Users/adityatoshniwal/Downloads/RM7018.patch:15563: trailing whitespace.
>>>
>>> import RestoreSchema, {getRestoreSaveOptSchema,
>>> getRestoreQueryOptionSchema, getRestoreDisableOptionSchema,
>>>
>>> /Users/adityatoshniwal/Downloads/RM7018.patch:15620: new blank line at
>>> EOF.
>>>
>>> +
>>>
>>> warning: 3 lines add whitespace errors.
>>>
>> Done
>>
>>>
>>> 2. Use lodash, and not underscore.
>>>
>>> + 'sources/gettext', 'sources/url_for', 'underscore',
>>> 'pgadmin.browser', 'sources/utils',
>>>
>> It's removed now.
>>
>>> 3. Use gettext
>>> panel.title(`Restore (${pgBrowser.Nodes[data._type].label}:
>>> ${data.label})`);
>>>
>> It's fixed now.
>>
>>> 4. Dialog size is huge. Reduce the dialog size.
>>>
>> As per discussion on the call, I have reduced the height of the dialog
>> (both for restore & backup)
>>
>>> 5. Mouseover on header should change cursor to "move"
>>>
>> It's fixed now.
>>
>>>
>>> [image: image.png]
>>>
>>> 6. If I remove the bin path from preferences, the "utility not found"
>>> error comes after I click on restore. It should come as soon as I hit the
>>> menu. Same problem with backup.
>>>
>> Fixed for both restore & backup utility.
>>
>>> 7. In restore.js the below code is repeated. You should get the treeInfo
>>> only once the dialog opens.
>>> const selectedNode = pgBrowser.tree.selected();
>>> var selectedTreeNode = pgBrowser.tree.findNodeByDomElement(selectedNode);
>>> const treeInfo = pgBrowser.tree.getTreeNodeHierarchy(selectedTreeNode);
>>>
>> Refactored the code, it's removed now.
>>
>>> 8. Minor spacing fixes between arrow and text.
>>> ()=>getRestoreQueryOptionSchema({nodeInfo: treeNodeInfo}),
>>> ()=> getRestoreDisableOptionSchema({nodeInfo: treeNodeInfo}),
>>> ()=> getRestoreMiscellaneousSchema({nodeInfo: treeNodeInfo}),
>>>
>> Have taken care of this.
>>
>>>
>>> On Wed, Dec 22, 2021 at 11:35 AM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Aditya
>>>>
>>>> Can you please review it?
>>>>
>>>> On Tue, Dec 21, 2021 at 4:52 PM Rahul Shirsat <
>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please find the attached patch, which ports the restore dialog to
>>>>> react.
>>>>>
>>>>> --
>>>>> *Rahul Shirsat*
>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> *Rahul Shirsat*
>> Senior Software Engineer | EnterpriseDB Corporation.
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>

--
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.

Attachment Content-Type Size
RM7018_v3.patch application/octet-stream 11.8 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-01-06 11:17:25 pgAdmin 4 commit: Fixed review comments:
Previous Message Akshay Joshi 2022-01-06 07:39:01 pgAdmin 4 commit: Fixed a schema diff issue where generated script adds