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

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Dave Page <[email protected]>
To: Nikhil Mohite <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Date: Mon, 1 Feb 2021 09:22:12 +0000
Message-ID: <CA+OCxoygL=vjY73=sRd9c_aKcqfTTFw4OUJsGtD=0qJAZCTpXg@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0ANns7UBMCXkuYQ-Rzsz4rtgQTCLOPke=ZMb77hkRq2fLg@mail.gmail.com>
References: <CAOBg0APfo=oZvAotP9dpDf8_CveOnP9EpwsV=zn1uYPWgqPZ-Q@mail.gmail.com>
	<CANxoLDfTspipwkMcJnNrnOWvguOMeHqNestMJ1dBqLGoowTeqQ@mail.gmail.com>
	<CA+OCxowLS7AuXSmrfZ+PkOiVAiXU0RFPiJo8qz1b97yXG48mvA@mail.gmail.com>
	<CANxoLDfN2AehHnBPDLPO1vR+yw-16T8dcRx04--UX-69UmCzqQ@mail.gmail.com>
	<CAOBg0AM3ycNtnMwJ2Rk1Ty-qCW4dCsq11Zz6Eaai3bRWDb3LMg@mail.gmail.com>
	<CA+OCxozsMp+p62THjyi=UX9j-4ZmsDz75OLYQPsPFWzQ6cEXQA@mail.gmail.com>
	<CAOBg0ANns7UBMCXkuYQ-Rzsz4rtgQTCLOPke=ZMb77hkRq2fLg@mail.gmail.com>

Hi

On Mon, Feb 1, 2021 at 7:51 AM Nikhil Mohite <[email protected]>
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 <[email protected]> wrote:
>
>> Hi
>>
>> On Thu, Jan 21, 2021 at 10:33 AM Nikhil Mohite <
>> [email protected]> wrote:
>>
>>> Hi Dave,
>>>
>>> On Thu, Jan 21, 2021 at 3:24 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Reverted the commit.
>>>>
>>>> On Thu, Jan 21, 2021 at 3:13 PM Dave Page <[email protected]> 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 <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Thanks, patch applied.
>>>>>>
>>>>>> On Thu, Jan 21, 2021 at 12:18 PM Nikhil Mohite <
>>>>>> [email protected]> 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


Attachments:

  [image/png] shared_server_change_ownsership.png (90.0K, 3-shared_server_change_ownsership.png)
  download | view image

view thread (12+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  In-Reply-To: <CA+OCxoygL=vjY73=sRd9c_aKcqfTTFw4OUJsGtD=0qJAZCTpXg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox