Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Date: 2021-02-04 06:03:48
Message-ID: CANxoLDe20GoJZjjcEsuACtGAAx5CaH2gEnC4N+V8M19UagfTVw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, patch applied.

On Wed, Feb 3, 2021 at 2:22 PM Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
wrote:

> Hi Team,
>
> Please find the updated patch for RM-6143.
>
>
> Regards,
> Nikhil Mohite.
>
> On Mon, Feb 1, 2021 at 2:52 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Mon, Feb 1, 2021 at 7:51 AM Nikhil Mohite <
>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave/ Team,
>>>
>>> As per the suggestions I have created a sample UI for change ownership
>>> of shared servers before deleting the user(only for admin user).
>>> [image: shared_server_change_ownsership.png]
>>> The user list will contain all admin users. (This alert will get display
>>> if the admin user has created any shared servers.)
>>>
>>
>> The UI is fine, but the wording needs work. A couple of general hints: An
>> entire sense would never be in parentheses, and there should always be a
>> space following a full stop.
>>
>> "Select the user that will take ownership of the shared servers created
>> by <user name>. <num servers> shared servers are currently owned by this
>> user."
>>
>> "Note: If no user is selected, the shared servers will be deleted."
>>
>> I'd also suggest that if the user does not select a new owner, a message
>> box should ask for confirmation before continuing:
>>
>> "The shared servers owned by <user name> will be deleted. Do you wish to
>> continue?".
>>
>>
>>>
>>> Any suggestions or if I missed anything please let me know.
>>>
>>> Regards,
>>> Nikhil Mohite
>>>
>>> On Thu, Jan 21, 2021 at 4:07 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Jan 21, 2021 at 10:33 AM Nikhil Mohite <
>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Dave,
>>>>>
>>>>> On Thu, Jan 21, 2021 at 3:24 PM Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Reverted the commit.
>>>>>>
>>>>>> On Thu, Jan 21, 2021 at 3:13 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> This seems like a very bad idea. What if the user that has left was
>>>>>>> the user that setup 50 connections used by everyone else?
>>>>>>>
>>>>>>> Deleting those shared entries is (I would guess) most likely *not*
>>>>>>> what the majority of users would want, and the current behaviour is
>>>>>>> definitely safest.
>>>>>>>
>>>>>> In the current implementation when the admin user gets deleted all "*Server
>>>>> groups*" created by that user are getting deleted, so if that admin
>>>>> has created any *Shared server* other users are not able to access it
>>>>> as its server group is not present in the database.
>>>>>
>>>>
>>>> That seems bad. I would suggest we only delete the group if there are
>>>> no shared servers left in it that would become orphaned. Of course, in that
>>>> case we'll also need to reassign ownership of the group.
>>>>
>>>>
>>>>>
>>>>>>> We should make this optional; i.e. ask the use if they want shared
>>>>>>> servers created by the user to be deleted. If they say no, they should be
>>>>>>> reassigned to another user; either the admin that's deleting the user, or
>>>>>>> their choice of user (a little more complex of course, but more flexible).
>>>>>>>
>>>>>> In the shared server table, we are creating entries per user, for
>>>>> deletion of non-admin user we can delete the shared server tables entries
>>>>> as it will not affect any other users. (because only admin users can mark
>>>>> the server as shared.)
>>>>> In case of admin user deletion, will add an extra check as suggested.
>>>>>
>>>>
>>>> Sounds good - thanks!
>>>>
>>>>
>>>>>
>>>>>>>
>>>>>>
>>>>>>> Please revert this, until the deletion is made optional.
>>>>>>>
>>>>>>> On Thu, Jan 21, 2021 at 9:23 AM Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Thanks, patch applied.
>>>>>>>>
>>>>>>>> On Thu, Jan 21, 2021 at 12:18 PM Nikhil Mohite <
>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Team,
>>>>>>>>>
>>>>>>>>> Please find the attached patch for RM-6143
>>>>>>>>> <https://redmine.postgresql.org/issues/6143>: Shared server
>>>>>>>>> entries not getting deleted.
>>>>>>>>> Added code to delete shared server entries if the admin deletes
>>>>>>>>> the user from user management.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Thanks & Regards,*
>>>>>>>>> *Nikhil Mohite*
>>>>>>>>> *Software Engineer.*
>>>>>>>>> *EDB Postgres* <https://www.enterprisedb.com/>
>>>>>>>>> *Mob.No: +91-7798364578.*
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Dave Page
>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>> Twitter: @pgsnake
>>>>>>>
>>>>>>> EDB: http://www.enterprisedb.com
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>> Regards,
>>>>> Nikhil Mohite.
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EDB: http://www.enterprisedb.com
>>>>
>>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: http://www.enterprisedb.com
>>
>>

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

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-02-04 06:15:56 pgAdmin 4 commit: Fixed an issue where the focus is not properly set on
Previous Message Akshay Joshi 2021-02-04 06:02:59 pgAdmin 4 commit: Fixed an issue where shared server entries not gettin