Hi Rahul
On Tue, Jul 6, 2021 at 7:13 PM Rahul Shirsat <rahul.shir...@enterprisedb.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?
>
> - 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?".
>
> - 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]
> Also, added test cases for SQL tab.
>
> On Mon, Jun 28, 2021 at 6:55 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Thanks, the patch applied.
>>
>> On Mon, Jun 28, 2021 at 2:00 PM Rahul Shirsat <
>> rahul.shir...@enterprisedb.com> wrote:
>>
>>> Hi Akshay,
>>>
>>> On Thu, Jun 24, 2021 at 2:32 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.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.shir...@enterprisedb.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*
