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

Hi Akshay,

As discussed in the review meeting, following are the review comments
fixation:

- Label change - "Role operation" to "Reassign/Drop Own"
- 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.
- Include the SQL tab.

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.