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

From: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
To: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Date: 2021-02-03 08:51:40
Message-ID: CAOBg0AMA7tYj7zgOO0A4NVCPedq9ef9bWrg819ACFoTDgkhu-Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>
>

Attachment Content-Type Size
RM_6143_v2.patch application/octet-stream 47.9 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2021-02-03 10:30:54 [pgAdmin][patch] Use unique DB name in ERD API test cases
Previous Message Akshay Joshi 2021-02-03 07:29:56 Re: [pgAdmin][RM5091] Option to hide Statistics, Dependencies, Dependants tab