Re: [patch][pgAdmin] RM3893 pgadmin4 Reassign/Drop Owned

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch][pgAdmin] RM3893 pgadmin4 Reassign/Drop Owned
Date: 2021-07-07 05:14:30
Message-ID: CANxoLDeSuVKdFQRXdVwTa4s1QT_YMUihK3An30KvVFsdzg6Kzg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, the patch applied.

On Tue, Jul 6, 2021 at 9:47 PM Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
wrote:

> Hi Akshay,
>
> Please find the updated patch.
>
> On Tue, Jul 6, 2021 at 8:07 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
> wrote:
>
>> Hi Rahul
>>
>> On Tue, Jul 6, 2021 at 7:13 PM Rahul Shirsat <
>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Akshay,
>>>
>>> As discussed in the review meeting, following are the review comments
>>> fixation:
>>>
>>>
>>> - Label change - "Role operation" to "Reassign/Drop Own"
>>>
>>> IIRC, we have finalized "Operation" then from where
>> "Reassign/Drop Own" comes from?
>>
>
> My Bad. I just missed out on updating the feedback list at the end. Now
> it is corrected.
>
>>
>>> - Alert msg changes-
>>> - Change the alert message to "Reassign owned executed
>>> successfully!" & "Drop owned executed successfully!" respectively.
>>> - Change the "Drop with" option to "Cascade" and provide the
>>> "Yes/No" option.
>>>
>>> It should be "Cascade?".
>>
> This has been corrected.
>
>>
>>> - Include the SQL tab.
>>>
>>> Wrong SQL's are generated. I have two users "user1" and
>> "user2". I selected "user2" but all the queries created for "user1"
>>
>> [image: Screenshot 2021-07-06 at 8.00.49 PM.png]
>>
> I have fixed this issue.
>
> Also, added test cases for SQL tab.
>>>
>>> On Mon, Jun 28, 2021 at 6:55 PM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Thanks, the patch applied.
>>>>
>>>> On Mon, Jun 28, 2021 at 2:00 PM Rahul Shirsat <
>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Akshay,
>>>>>
>>>>> On Thu, Jun 24, 2021 at 2:32 PM Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Rahul
>>>>>>
>>>>>> Following are the review comments:
>>>>>>
>>>>>> - The context menu should be renamed to 'Reassign/Drop Owned...'
>>>>>> instead of 'Reassign/Drop role'.
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> - Remove " role" from the dialog header. It should be
>>>>>> "Reassign/Drop Owned - <role name>"
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> - We have only two operations "Reassign and Drop", so instead of
>>>>>> using Radio modern can we use switch control? Though radio modern also
>>>>>> looks good.
>>>>>>
>>>>>> @Dave what do you think?
>>>>>>
>>>>>> Keeping the radio modern as discussed.
>>>>>
>>>>>>
>>>>>> - By default, Reassign operation should be selected.
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> - After successfully reassign/drop we should show
>>>>>> aleritfy notifier "Reassign owned successfully" or "Drop
>>>>>> owned successfully".
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> -
>>>>>> - "Roles dropped successfully!" string should be modified to
>>>>>> "Drop owned successfully!".
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> - Add validation if the database or reassign object is not
>>>>>> selected, show them on GUI.
>>>>>>
>>>>>> Done
>>>>>
>>>>>>
>>>>>> - Connection to the database should be released(disconnected)
>>>>>> after the execution of the query.
>>>>>>
>>>>>> This has been taken care of.
>>>>>
>>>>>>
>>>>>> - From Postgres 9.5 onwards we have "*CURRENT_USER | SESSION_USER*"
>>>>>> please check that as well. From Postgres 14 *CURRENT_ROLE* is
>>>>>> also there.
>>>>>>
>>>>>> All of the above 3 options added. CURRENT_ROLE will be visible on UI
>>>>> when connected to postgres >= v14. Also added test cases for the same.
>>>>>
>>>>>>
>>>>>> - DROP OWNED query have *[ CASCADE | RESTRICT ] *which was not
>>>>>> there even in pgadmin III, add support this would be good. Create two
>>>>>> switch control for cascade and restrict and depending on that create the
>>>>>> SQL query.
>>>>>>
>>>>>> These options too are added now as a switch case.
>>>>>
>>>>>>
>>>>>> - Documentation and screenshot update required after the above
>>>>>> changes. The "Reassign/Drop Own" string should be replaced with
>>>>>> "Reassign/Drop Owned" in the documentation.
>>>>>>
>>>>>> Updated the documentations as well.
>>>>>
>>>>> *Sorry for previously adding this option* - I have removed the SQL
>>>>> tab as REASSIGN/DROP OWNED are single line and not necessarily required to
>>>>> be viewed by the user (referring to Delete Object or Drop Cascade options
>>>>> of other pg_catalog objects).
>>>>>
>>>>>>
>>>>>> On Mon, Jun 21, 2021 at 4:37 PM Rahul Shirsat <
>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>>
>>>>>>> Please find the patch which includes the reassign/drop own feature.
>>>>>>>
>>>>>>> This also includes documentation of the feature.
>>>>>>>
>>>>>>> --
>>>>>>> *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.
>>>>>
>>>>
>>>>
>>>> --
>>>> *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.
>>>
>>
>>
>> --
>> *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.
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-07-07 05:26:08 Re: [pgAdmin][RM-6569]: [Housekeeping][React] Port catalog objects to react
Previous Message Akshay Joshi 2021-07-07 05:14:16 pgAdmin 4 commit: 1) Modified some labels and controls.