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

From: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(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-06 16:16:40
Message-ID: CAKtn9dN8qBVJm-Nx-x=ouFUogpb4tjbuk_+O2rfhYU+32pybSg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

Attachment Content-Type Size
RM3893_v4.patch application/octet-stream 347.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Nikhil Mohite 2021-07-07 04:55:45 Re: [pgAdmin][RM-6569]: [Housekeeping][React] Port catalog objects to react
Previous Message Dave Page 2021-07-06 15:40:52 Re: Bug #6337