public inbox for [email protected]  
help / color / mirror / Atom feed
[pgAdmin][RM-6143]: Shared server entries not getting deleted.
12+ messages / 3 participants
[nested] [flat]

* [pgAdmin][RM-6143]: Shared server entries not getting deleted.
@ 2021-01-21 06:48 Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Nikhil Mohite @ 2021-01-21 06:48 UTC (permalink / raw)
  To: pgadmin-hackers

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


Attachments:

  [application/octet-stream] RM_6143.patch (870B, 3-RM_6143.patch)
  download | inline diff:
diff --git a/web/pgadmin/tools/user_management/__init__.py b/web/pgadmin/tools/user_management/__init__.py
index ce280a3d..649c50db 100644
--- a/web/pgadmin/tools/user_management/__init__.py
+++ b/web/pgadmin/tools/user_management/__init__.py
@@ -28,7 +28,7 @@ from pgadmin.utils.constants import MIMETYPE_APP_JS, INTERNAL,\
     SUPPORTED_AUTH_SOURCES, KERBEROS
 from pgadmin.utils.validation_utils import validate_email
 from pgadmin.model import db, Role, User, UserPreference, Server, \
-    ServerGroup, Process, Setting
+    ServerGroup, Process, Setting, SharedServer
 
 # set template path for sql scripts
 MODULE_NAME = 'user_management'
@@ -347,6 +347,8 @@ def delete(uid):
 
         Process.query.filter_by(user_id=uid).delete()
 
+        SharedServer.query.filter_by(user_id=uid).delete()
+
         # Finally delete user
         db.session.delete(usr)
 


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
@ 2021-01-21 09:22 ` Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Akshay Joshi @ 2021-01-21 09:22 UTC (permalink / raw)
  To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers

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*


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
@ 2021-01-21 09:43   ` Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Dave Page @ 2021-01-21 09:43 UTC (permalink / raw)
  To: Akshay Joshi <[email protected]>; +Cc: Nikhil Mohite <[email protected]>; pgadmin-hackers

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.

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

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


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
@ 2021-01-21 09:54     ` Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Akshay Joshi @ 2021-01-21 09:54 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Nikhil Mohite <[email protected]>; pgadmin-hackers

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.
>
> 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).
>
> 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*


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
@ 2021-01-21 10:33       ` Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Nikhil Mohite @ 2021-01-21 10:33 UTC (permalink / raw)
  To: Akshay Joshi <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers

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.

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

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


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
@ 2021-01-21 10:36         ` Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Dave Page @ 2021-01-21 10:36 UTC (permalink / raw)
  To: Nikhil Mohite <[email protected]>; +Cc: Akshay Joshi <[email protected]>; pgadmin-hackers

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


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
@ 2021-02-01 07:51           ` Nikhil Mohite <[email protected]>
  2021-02-01 09:22             ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Nikhil Mohite @ 2021-02-01 07:51 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Akshay Joshi <[email protected]>; pgadmin-hackers

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

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


Attachments:

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

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
@ 2021-02-01 09:22             ` Dave Page <[email protected]>
  2021-02-03 08:51               ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Dave Page @ 2021-02-01 09:22 UTC (permalink / raw)
  To: Nikhil Mohite <[email protected]>; +Cc: Akshay Joshi <[email protected]>; pgadmin-hackers

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

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-01 09:22             ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
@ 2021-02-03 08:51               ` Nikhil Mohite <[email protected]>
  2021-02-04 06:03                 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Nikhil Mohite @ 2021-02-03 08:51 UTC (permalink / raw)
  To: 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 <[email protected]> wrote:

> 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

  [application/octet-stream] RM_6143_v2.patch (47.9K, 4-RM_6143_v2.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/__init__.py b/web/pgadmin/browser/server_groups/__init__.py
index 476a4e66..56177263 100644
--- a/web/pgadmin/browser/server_groups/__init__.py
+++ b/web/pgadmin/browser/server_groups/__init__.py
@@ -177,6 +177,17 @@ class ServerGroupView(NodeView):
         # if server group id is 1 we won't delete it.
         sg = groups.first()
 
+        shared_servers = Server.query.filter_by(servergroup_id=sg.id,
+                                                shared=True).all()
+        if shared_servers:
+            return make_json_response(
+                status=417,
+                success=0,
+                errormsg=gettext(
+                    'The specified server group cannot be deleted.'
+                )
+            )
+
         if sg.id == gid:
             return make_json_response(
                 status=417,
diff --git a/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js b/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
index de1d0f21..4f282da8 100644
--- a/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
+++ b/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
@@ -100,352 +100,353 @@ export default function newConnectionDialogModel(response, sgid, sid, handler, c
       server_name: server_name,
       database_name: database_name,
     },
-    schema: [{
-      id: 'server',
-      name: 'server',
-      label: gettext('Server'),
-      type: 'text',
-      editable: true,
-      disabled: false,
-      select2: {
-        allowClear: false,
-        width: 'style',
-        templateResult: formatNode,
-        templateSelection: formatNode,
-      },
-      events: {
-        'focus select': 'clearInvalid',
-        'keydown :input': 'processTab',
-        'select2:select': 'onSelect',
-        'select2:selecting': 'beforeSelect',
-        'select2:clear': 'onChange',
-      },
-      transform: function(data) {
-        let group_template_options = [];
-        for (let key in data) {
-          if (data.hasOwnProperty(key)) {
-            group_template_options.push({'group': key, 'optval': data[key]});
-          }
-        }
-        return group_template_options;
-      },
-      control: Backform.Select2Control.extend({
-        template: _.template([
-          '<% if(label == false) {} else {%>',
-          '  <label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
-          '<% }%>',
-          '<div class="<%=controlsClassName%>">',
-          ' <select class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>"',
-          '  name="<%=name%>" value="<%-value%>" <%=disabled ? "disabled" : ""%>',
-          '  <%=required ? "required" : ""%><%= select2.multiple ? " multiple>" : ">" %>',
-          '  <%=select2.first_empty ? " <option></option>" : ""%>',
-          '  <% for (var i=0; i < options.length; i++) {%>',
-          '   <% if (options[i].group) { %>',
-          '     <% var group = options[i].group; %>',
-          '     <% if (options[i].optval) { %> <% var option_length = options[i].optval.length; %>',
-          '      <optgroup label="<%=group%>">',
-          '      <% for (var subindex=0; subindex < option_length; subindex++) {%>',
-          '        <% var option = options[i].optval[subindex]; %>',
-          '        <option ',
-          '        <% if (option.image) { %> data-image=<%=option.image%> <%}%>',
-          '        <% if (option.connected) { %> data-connected=connected <%}%>',
-          '        value=<%- formatter.fromRaw(option.value) %>',
-          '        <% if (option.selected) {%>selected="selected"<%} else {%>',
-          '        <% if (!select2.multiple && option.value === rawValue) {%>selected="selected"<%}%>',
-          '        <% if (select2.multiple && rawValue && rawValue.indexOf(option.value) != -1){%>selected="selected" data-index="rawValue.indexOf(option.value)"<%}%>',
-          '        <%}%>',
-          '        <%= disabled ? "disabled" : ""%>><%-option.label%></option>',
-          '      <%}%>',
-          '      </optgroup>',
-          '     <%}%>',
-          '   <%} else {%>',
-          '     <% var option = options[i]; %>',
-          '     <option ',
-          '     <% if (option.image) { %> data-image=<%=option.image%> <%}%>',
-          '     <% if (option.connected) { %> data-connected=connected <%}%>',
-          '     value=<%- formatter.fromRaw(option.value) %>',
-          '     <% if (option.selected) {%>selected="selected"<%} else {%>',
-          '     <% if (!select2.multiple && option.value === rawValue) {%>selected="selected"<%}%>',
-          '     <% if (select2.multiple && rawValue && rawValue.indexOf(option.value) != -1){%>selected="selected" data-index="rawValue.indexOf(option.value)"<%}%>',
-          '     <%}%>',
-          '     <%= disabled ? "disabled" : ""%>><%-option.label%></option>',
-          '   <%}%>',
-          '  <%}%>',
-          ' </select>',
-          ' <% if (helpMessage && helpMessage.length) { %>',
-          ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
-          ' <% } %>',
-          '</div>',
-        ].join('\n')),
-        beforeSelect: function() {
-          var selVal = arguments[0].params.args.data.id;
-
-          if(this.field.get('connect') && this.$el.find('option[value="'+selVal+'"]').attr('data-connected') !== 'connected') {
-            this.field.get('connect').apply(this, [selVal, this.changeIcon.bind(this)]);
-          } else {
-            $(this.$sel).trigger('change');
-            setTimeout(function(){ this.onChange.apply(this); }.bind(this), 200);
+    schema: [
+      {
+        id: 'server',
+        name: 'server',
+        label: gettext('Server'),
+        type: 'text',
+        editable: true,
+        disabled: false,
+        select2: {
+          allowClear: false,
+          width: 'style',
+          templateResult: formatNode,
+          templateSelection: formatNode,
+        },
+        events: {
+          'focus select': 'clearInvalid',
+          'keydown :input': 'processTab',
+          'select2:select': 'onSelect',
+          'select2:selecting': 'beforeSelect',
+          'select2:clear': 'onChange',
+        },
+        transform: function(data) {
+          let group_template_options = [];
+          for (let key in data) {
+            if (data.hasOwnProperty(key)) {
+              group_template_options.push({'group': key, 'optval': data[key]});
+            }
           }
+          return group_template_options;
         },
-        changeIcon: function(data) {
-          let span = this.$el.find('.select2-selection .select2-selection__rendered span.wcTabIcon'),
-            selSpan = this.$el.find('option:selected');
+        control: Backform.Select2Control.extend({
+          template: _.template([
+            '<% if(label == false) {} else {%>',
+            '  <label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
+            '<% }%>',
+            '<div class="<%=controlsClassName%>">',
+            ' <select class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>"',
+            '  name="<%=name%>" value="<%-value%>" <%=disabled ? "disabled" : ""%>',
+            '  <%=required ? "required" : ""%><%= select2.multiple ? " multiple>" : ">" %>',
+            '  <%=select2.first_empty ? " <option></option>" : ""%>',
+            '  <% for (var i=0; i < options.length; i++) {%>',
+            '   <% if (options[i].group) { %>',
+            '     <% var group = options[i].group; %>',
+            '     <% if (options[i].optval) { %> <% var option_length = options[i].optval.length; %>',
+            '      <optgroup label="<%=group%>">',
+            '      <% for (var subindex=0; subindex < option_length; subindex++) {%>',
+            '        <% var option = options[i].optval[subindex]; %>',
+            '        <option ',
+            '        <% if (option.image) { %> data-image=<%=option.image%> <%}%>',
+            '        <% if (option.connected) { %> data-connected=connected <%}%>',
+            '        value=<%- formatter.fromRaw(option.value) %>',
+            '        <% if (option.selected) {%>selected="selected"<%} else {%>',
+            '        <% if (!select2.multiple && option.value === rawValue) {%>selected="selected"<%}%>',
+            '        <% if (select2.multiple && rawValue && rawValue.indexOf(option.value) != -1){%>selected="selected" data-index="rawValue.indexOf(option.value)"<%}%>',
+            '        <%}%>',
+            '        <%= disabled ? "disabled" : ""%>><%-option.label%></option>',
+            '      <%}%>',
+            '      </optgroup>',
+            '     <%}%>',
+            '   <%} else {%>',
+            '     <% var option = options[i]; %>',
+            '     <option ',
+            '     <% if (option.image) { %> data-image=<%=option.image%> <%}%>',
+            '     <% if (option.connected) { %> data-connected=connected <%}%>',
+            '     value=<%- formatter.fromRaw(option.value) %>',
+            '     <% if (option.selected) {%>selected="selected"<%} else {%>',
+            '     <% if (!select2.multiple && option.value === rawValue) {%>selected="selected"<%}%>',
+            '     <% if (select2.multiple && rawValue && rawValue.indexOf(option.value) != -1){%>selected="selected" data-index="rawValue.indexOf(option.value)"<%}%>',
+            '     <%}%>',
+            '     <%= disabled ? "disabled" : ""%>><%-option.label%></option>',
+            '   <%}%>',
+            '  <%}%>',
+            ' </select>',
+            ' <% if (helpMessage && helpMessage.length) { %>',
+            ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
+            ' <% } %>',
+            '</div>',
+          ].join('\n')),
+          beforeSelect: function() {
+            var selVal = arguments[0].params.args.data.id;
 
-          if (span.hasClass('icon-server-not-connected') || span.hasClass('icon-shared-server-not-connected')) {
-            let icon = (data.icon) ? data.icon : 'icon-pg';
-            span.removeClass('icon-server-not-connected');
-            span.addClass(icon);
-            span.attr('data-connected', 'connected');
+            if(this.field.get('connect') && this.$el.find('option[value="'+selVal+'"]').attr('data-connected') !== 'connected') {
+              this.field.get('connect').apply(this, [selVal, this.changeIcon.bind(this)]);
+            } else {
+              $(this.$sel).trigger('change');
+              setTimeout(function(){ this.onChange.apply(this); }.bind(this), 200);
+            }
+          },
+          changeIcon: function(data) {
+            let span = this.$el.find('.select2-selection .select2-selection__rendered span.wcTabIcon'),
+              selSpan = this.$el.find('option:selected');
 
-            selSpan.data().image = icon;
-            selSpan.attr('data-connected', 'connected');
-            alertify.connectServer().destroy();
-            this.onChange.apply(this);
-          }
-          else if (span.hasClass('icon-database-not-connected')) {
-            let icon = (data.icon) ? data.icon : 'pg-icon-database';
+            if (span.hasClass('icon-server-not-connected') || span.hasClass('icon-shared-server-not-connected')) {
+              let icon = (data.icon) ? data.icon : 'icon-pg';
+              span.removeClass('icon-server-not-connected');
+              span.addClass(icon);
+              span.attr('data-connected', 'connected');
 
-            span.removeClass('icon-database-not-connected');
-            span.addClass(icon);
-            span.attr('data-connected', 'connected');
+              selSpan.data().image = icon;
+              selSpan.attr('data-connected', 'connected');
+              alertify.connectServer().destroy();
+              this.onChange.apply(this);
+            }
+            else if (span.hasClass('icon-database-not-connected')) {
+              let icon = (data.icon) ? data.icon : 'pg-icon-database';
 
-            selSpan.removeClass('icon-database-not-connected');
-            selSpan.data().image = icon;
-            selSpan.attr('data-connected', 'connected');
-            alertify.connectServer().destroy();
-            this.onChange.apply(this);
-          }
-        },
-        connect: function(self) {
-          let local_self = self;
+              span.removeClass('icon-database-not-connected');
+              span.addClass(icon);
+              span.attr('data-connected', 'connected');
 
-          if(alertify.connectServer) {
-            delete alertify.connectServer;
-          }
+              selSpan.removeClass('icon-database-not-connected');
+              selSpan.data().image = icon;
+              selSpan.attr('data-connected', 'connected');
+              alertify.connectServer().destroy();
+              this.onChange.apply(this);
+            }
+          },
+          connect: function(self) {
+            let local_self = self;
 
-          alertify.dialog('connectServer', function factory() {
-            return {
-              main: function(
-                title, message, server_id, submit_password=true, connect_server=null,
-              ) {
-                this.set('title', title);
-                this.message = message;
-                this.server_id = server_id;
-                this.submit_password = submit_password;
-                this.connect_server = connect_server;
-              },
-              setup:function() {
-                return {
-                  buttons:[{
-                    text: gettext('Cancel'), className: 'btn btn-secondary fa fa-times pg-alertify-button',
-                    key: 27,
-                  },{
-                    text: gettext('OK'), key: 13, className: 'btn btn-primary fa fa-check pg-alertify-button',
-                  }],
-                  focus: {element: '#password', select: true},
-                  options: {
-                    modal: 0, resizable: false, maximizable: false, pinnable: false,
-                  },
-                };
-              },
-              build:function() {
-              },
-              prepare:function() {
-                this.setContent(this.message);
-              },
-              callback: function(closeEvent) {
-                var loadingDiv = $('#show_filter_progress');
-                if (closeEvent.button.text == gettext('OK')) {
-                  if(this.submit_password) {
-                    var _url = url_for('sqleditor.connect_server', {'sid': this.server_id});
+            if(alertify.connectServer) {
+              delete alertify.connectServer;
+            }
 
-                    loadingDiv.removeClass('d-none');
-                    $.ajax({
-                      type: 'POST',
-                      timeout: 30000,
-                      url: _url,
-                      data: $('#frmPassword').serialize(),
-                    })
-                      .done(function(res) {
+            alertify.dialog('connectServer', function factory() {
+              return {
+                main: function(
+                  title, message, server_id, submit_password=true, connect_server=null,
+                ) {
+                  this.set('title', title);
+                  this.message = message;
+                  this.server_id = server_id;
+                  this.submit_password = submit_password;
+                  this.connect_server = connect_server;
+                },
+                setup:function() {
+                  return {
+                    buttons:[{
+                      text: gettext('Cancel'), className: 'btn btn-secondary fa fa-times pg-alertify-button',
+                      key: 27,
+                    },{
+                      text: gettext('OK'), key: 13, className: 'btn btn-primary fa fa-check pg-alertify-button',
+                    }],
+                    focus: {element: '#password', select: true},
+                    options: {
+                      modal: 0, resizable: false, maximizable: false, pinnable: false,
+                    },
+                  };
+                },
+                build:function() {
+                },
+                prepare:function() {
+                  this.setContent(this.message);
+                },
+                callback: function(closeEvent) {
+                  var loadingDiv = $('#show_filter_progress');
+                  if (closeEvent.button.text == gettext('OK')) {
+                    if(this.submit_password) {
+                      var _url = url_for('sqleditor.connect_server', {'sid': this.server_id});
 
-                        local_self.model.attributes.database = null;
-                        local_self.changeIcon(res.data);
-                        local_self.model.attributes.user = null;
-                        local_self.model.attributes.role = null;
-                        Backform.Select2Control.prototype.onChange.apply(local_self, arguments);
-                        Object.keys(response.server_list).forEach(key => {
-                          response.server_list[key].forEach(option => {
-                            if (option.value == local_self.getValueFromDOM()) {
-                              response.server_name = option.label;
-                            }
+                      loadingDiv.removeClass('d-none');
+                      $.ajax({
+                        type: 'POST',
+                        timeout: 30000,
+                        url: _url,
+                        data: $('#frmPassword').serialize(),
+                      })
+                        .done(function(res) {
+
+                          local_self.model.attributes.database = null;
+                          local_self.changeIcon(res.data);
+                          local_self.model.attributes.user = null;
+                          local_self.model.attributes.role = null;
+                          Backform.Select2Control.prototype.onChange.apply(local_self, arguments);
+                          Object.keys(response.server_list).forEach(key => {
+                            response.server_list[key].forEach(option => {
+                              if (option.value == local_self.getValueFromDOM()) {
+                                response.server_name = option.label;
+                              }
+                            });
                           });
-                        });
 
+                          loadingDiv.addClass('d-none');
+                          alertify.connectServer().destroy();
+                        })
+                        .fail(function(xhr) {
+                          loadingDiv.addClass('d-none');
+                          alertify.connectServer().destroy();
+                          alertify.connectServer('Connect to server', xhr.responseJSON.result, local_self.getValueFromDOM());
+                        });
+                    } else {
+                      if(Object.keys(this.connect_server).length > 0) {
+                        this.connect_server['password'] = $('#password').val();
+                        this.connect_server['is_selected'] = false;
+                        handler.gridView.on_change_connection(this.connect_server, conn_self, false);
+                        loadingDiv = $('#fetching_data');
                         loadingDiv.addClass('d-none');
-                        alertify.connectServer().destroy();
-                      })
-                      .fail(function(xhr) {
+                      } else {
+                        response.password = $('#password').val();
                         loadingDiv.addClass('d-none');
-                        alertify.connectServer().destroy();
-                        alertify.connectServer('Connect to server', xhr.responseJSON.result, local_self.getValueFromDOM());
-                      });
-                  } else {
-                    if(Object.keys(this.connect_server).length > 0) {
-                      this.connect_server['password'] = $('#password').val();
-                      this.connect_server['is_selected'] = false;
-                      handler.gridView.on_change_connection(this.connect_server, conn_self, false);
-                      loadingDiv = $('#fetching_data');
-                      loadingDiv.addClass('d-none');
-                    } else {
-                      response.password = $('#password').val();
-                      loadingDiv.addClass('d-none');
+                      }
                     }
-                  }
-                } else {
-                  local_self.model.attributes.database = null;
-                  local_self.model.attributes.user = null;
-                  local_self.model.attributes.role = null;
-                  Backform.Select2Control.prototype.onChange.apply(local_self, arguments);
-                  loadingDiv.addClass('d-none');
-                  alertify.connectServer().destroy();
+                  } else {
+                    local_self.model.attributes.database = null;
+                    local_self.model.attributes.user = null;
+                    local_self.model.attributes.role = null;
+                    Backform.Select2Control.prototype.onChange.apply(local_self, arguments);
+                    loadingDiv.addClass('d-none');
+                    alertify.connectServer().destroy();
 
-                }
-                closeEvent.close = true;
-              },
-            };
-          });
-        },
-        render: function() {
-          let self = this;
-          self.connect(self);
-          Object.keys(response.server_list).forEach(key => {
-            response.server_list[key].forEach(option => {
-              if (option.value == parseInt(sid)) {
-                response.server_name = option.label;
-              }
+                  }
+                  closeEvent.close = true;
+                },
+              };
             });
-          });
-          var transform = self.field.get('transform') || self.defaults.transform;
-          if (transform && _.isFunction(transform)) {
-            self.field.set('options', transform.bind(self, response.server_list));
-          } else {
-            self.field.set('options', response.server_list);
-          }
-          return Backform.Select2Control.prototype.render.apply(self, arguments);
-        },
-        onChange: function() {
-          this.model.attributes.database = null;
-          this.model.attributes.user = null;
-          let self = this;
-          self.connect(self);
-
-          let url = url_for('sqleditor.connect_server', {
-            'sid': self.getValueFromDOM(),
-            'usr': self.model.attributes.user,
-          });
-          var loadingDiv = $('#show_filter_progress');
-          loadingDiv.removeClass('d-none');
-          $.ajax({
-            url: url,
-            type: 'POST',
-            headers: {
-              'Cache-Control' : 'no-cache',
-            },
-          }).done(function () {
-            Backform.Select2Control.prototype.onChange.apply(self, arguments);
+          },
+          render: function() {
+            let self = this;
+            self.connect(self);
             Object.keys(response.server_list).forEach(key => {
               response.server_list[key].forEach(option => {
-                if (option.value == self.getValueFromDOM()) {
+                if (option.value == parseInt(sid)) {
                   response.server_name = option.label;
                 }
               });
             });
-            loadingDiv.addClass('d-none');
-          }).fail(function(xhr){
-            loadingDiv.addClass('d-none');
-            alertify.connectServer('Connect to server', xhr.responseJSON.result, self.getValueFromDOM());
-          });
-
-        },
-      }),
-    },
-    {
-      id: 'database',
-      name: 'database',
-      label: gettext('Database'),
-      type: 'text',
-      editable: true,
-      disabled: function(m) {
-        let self_local = this;
-        if (!_.isUndefined(m.get('server')) && !_.isNull(m.get('server'))
-            && m.get('server') !== '') {
-          setTimeout(function() {
-            if(self_local.options.length) {
-              m.set('database', self_local.options[0].value);
+            var transform = self.field.get('transform') || self.defaults.transform;
+            if (transform && _.isFunction(transform)) {
+              self.field.set('options', transform.bind(self, response.server_list));
+            } else {
+              self.field.set('options', response.server_list);
             }
-          }, 10);
-          return false;
-        }
+            return Backform.Select2Control.prototype.render.apply(self, arguments);
+          },
+          onChange: function() {
+            this.model.attributes.database = null;
+            this.model.attributes.user = null;
+            let self = this;
+            self.connect(self);
 
-        return true;
-      },
-      deps: ['server'],
-      url: 'sqleditor.get_new_connection_database',
-      select2: {
-        allowClear: false,
-        width: '100%',
-        first_empty: true,
-        select_first: false,
-      },
-      extraClasses:['new-connection-dialog-style'],
-      control: NewConnectionSelect2Control,
-      transform: function(data) {
-        response.database_list = data;
-        return data;
-      },
-    },
-    {
-      id: 'user',
-      name: 'user',
-      label: gettext('User'),
-      type: 'text',
-      editable: true,
-      deps: ['server'],
-      select2: {
-        allowClear: false,
-        width: '100%',
+            let url = url_for('sqleditor.connect_server', {
+              'sid': self.getValueFromDOM(),
+              'usr': self.model.attributes.user,
+            });
+            var loadingDiv = $('#show_filter_progress');
+            loadingDiv.removeClass('d-none');
+            $.ajax({
+              url: url,
+              type: 'POST',
+              headers: {
+                'Cache-Control' : 'no-cache',
+              },
+            }).done(function () {
+              Backform.Select2Control.prototype.onChange.apply(self, arguments);
+              Object.keys(response.server_list).forEach(key => {
+                response.server_list[key].forEach(option => {
+                  if (option.value == self.getValueFromDOM()) {
+                    response.server_name = option.label;
+                  }
+                });
+              });
+              loadingDiv.addClass('d-none');
+            }).fail(function(xhr){
+              loadingDiv.addClass('d-none');
+              alertify.connectServer('Connect to server', xhr.responseJSON.result, self.getValueFromDOM());
+            });
+
+          },
+        }),
       },
-      control: NewConnectionSelect2Control,
-      url: 'sqleditor.get_new_connection_user',
-      disabled: function(m) {
-        let self_local = this;
-        if (!_.isUndefined(m.get('server')) && !_.isNull(m.get('server'))
+      {
+        id: 'database',
+        name: 'database',
+        label: gettext('Database'),
+        type: 'text',
+        editable: true,
+        disabled: function(m) {
+          let self_local = this;
+          if (!_.isUndefined(m.get('server')) && !_.isNull(m.get('server'))
             && m.get('server') !== '') {
-          setTimeout(function() {
-            if(self_local.options.length) {
-              m.set('user', self_local.options[0].value);
-            }
-          }, 10);
-          return false;
-        }
-        return true;
+            setTimeout(function() {
+              if(self_local.options.length) {
+                m.set('database', self_local.options[0].value);
+              }
+            }, 10);
+            return false;
+          }
+
+          return true;
+        },
+        deps: ['server'],
+        url: 'sqleditor.get_new_connection_database',
+        select2: {
+          allowClear: false,
+          width: '100%',
+          first_empty: true,
+          select_first: false,
+        },
+        extraClasses:['new-connection-dialog-style'],
+        control: NewConnectionSelect2Control,
+        transform: function(data) {
+          response.database_list = data;
+          return data;
+        },
       },
-    },{
-      id: 'role',
-      name: 'role',
-      label: gettext('Role'),
-      type: 'text',
-      editable: true,
-      deps: ['server'],
-      select2: {
-        allowClear: false,
-        width: '100%',
-        first_empty: true,
+      {
+        id: 'user',
+        name: 'user',
+        label: gettext('User'),
+        type: 'text',
+        editable: true,
+        deps: ['server'],
+        select2: {
+          allowClear: false,
+          width: '100%',
+        },
+        control: NewConnectionSelect2Control,
+        url: 'sqleditor.get_new_connection_user',
+        disabled: function(m) {
+          let self_local = this;
+          if (!_.isUndefined(m.get('server')) && !_.isNull(m.get('server'))
+            && m.get('server') !== '') {
+            setTimeout(function() {
+              if(self_local.options.length) {
+                m.set('user', self_local.options[0].value);
+              }
+            }, 10);
+            return false;
+          }
+          return true;
+        },
+      },{
+        id: 'role',
+        name: 'role',
+        label: gettext('Role'),
+        type: 'text',
+        editable: true,
+        deps: ['server'],
+        select2: {
+          allowClear: false,
+          width: '100%',
+          first_empty: true,
+        },
+        control: NewConnectionSelect2Control,
+        url: 'sqleditor.get_new_connection_role',
+        disabled: false,
       },
-      control: NewConnectionSelect2Control,
-      url: 'sqleditor.get_new_connection_role',
-      disabled: false,
-    },
     ],
     validate: function() {
       let msg = null;
diff --git a/web/pgadmin/tools/user_management/__init__.py b/web/pgadmin/tools/user_management/__init__.py
index ce280a3d..f202f898 100644
--- a/web/pgadmin/tools/user_management/__init__.py
+++ b/web/pgadmin/tools/user_management/__init__.py
@@ -28,7 +28,7 @@ from pgadmin.utils.constants import MIMETYPE_APP_JS, INTERNAL,\
     SUPPORTED_AUTH_SOURCES, KERBEROS
 from pgadmin.utils.validation_utils import validate_email
 from pgadmin.model import db, Role, User, UserPreference, Server, \
-    ServerGroup, Process, Setting
+    ServerGroup, Process, Setting, roles_users, SharedServer
 
 # set template path for sql scripts
 MODULE_NAME = 'user_management'
@@ -78,7 +78,9 @@ class UserManagementModule(PgAdminModule):
             'user_management.update_user', 'user_management.delete_user',
             'user_management.create_user', 'user_management.users',
             'user_management.user', current_app.login_manager.login_view,
-            'user_management.auth_sources', 'user_management.auth_sources'
+            'user_management.auth_sources', 'user_management.auth_sources',
+            'user_management.shared_servers', 'user_management.admin_users',
+            'user_management.change_owner',
         ]
 
 
@@ -336,6 +338,8 @@ def delete(uid):
         abort(404)
 
     try:
+        server_groups = ServerGroup.query.filter_by(user_id=uid).all()
+        sg = [server_group.id for server_group in server_groups]
 
         Setting.query.filter_by(user_id=uid).delete()
 
@@ -346,6 +350,11 @@ def delete(uid):
         ServerGroup.query.filter_by(user_id=uid).delete()
 
         Process.query.filter_by(user_id=uid).delete()
+        # Delete Shared servers for current user.
+        SharedServer.query.filter_by(user_id=uid).delete()
+
+        SharedServer.query.filter(SharedServer.servergroup_id.in_(sg)).delete(
+            synchronize_session=False)
 
         # Finally delete user
         db.session.delete(usr)
@@ -361,6 +370,174 @@ def delete(uid):
         return internal_server_error(errormsg=str(e))
 
 
[email protected]('/change_owner', methods=['POST'], endpoint='change_owner')
+@roles_required('Administrator')
+def change_owner():
+    """
+
+    Returns:
+
+    """
+
+    data = request.form if request.form else json.loads(
+        request.data, encoding='utf-8'
+    )
+    try:
+        old_user_servers = Server.query.filter_by(shared=True, user_id=data[
+            'old_owner']).all()
+        server_group_ids = [server.servergroup_id for server in
+                            old_user_servers]
+        server_groups = ServerGroup.query.filter(
+            ServerGroup.id.in_(server_group_ids)).all()
+
+        new_owner_sg = ServerGroup.query.filter_by(
+            user_id=data['new_owner']).all()
+        old_owner_sg = ServerGroup.query.filter_by(
+            user_id=data['old_owner']).all()
+        sg_data = {sg.name: sg.id for sg in new_owner_sg}
+        old_sg_data = {sg.id: sg.name for sg in old_owner_sg}
+
+        deleted_sg = []
+        # Change server user.
+        for server in old_user_servers:
+            if old_sg_data[server.servergroup_id] in sg_data:
+                sh_servers = SharedServer.query.filter_by(
+                    servergroup_id=server.servergroup_id).all()
+                for sh in sh_servers:
+                    sh.servergroup_id = sg_data[
+                        old_sg_data[server.servergroup_id]]
+                # Update Server user and server group to prevent deleting
+                # shared server associated with deleting user.
+                Server.query.filter_by(
+                    servergroup_id=server.servergroup_id, shared=True,
+                    user_id=data['old_owner']
+                ).update(
+                    {
+                        'servergroup_id': sg_data[old_sg_data[
+                            server.servergroup_id]],
+                        'user_id': data['new_owner']
+                    }
+                )
+                ServerGroup.query.filter_by(id=server.servergroup_id).delete()
+                deleted_sg.append(server.servergroup_id)
+            else:
+                server.user_id = data['new_owner']
+
+        # Change server group user.
+        for server_group in server_groups:
+            if server_group.id not in deleted_sg:
+                server_group.user_id = data['new_owner']
+
+        db.session.commit()
+        # Delete old owner records.
+        delete(data['old_owner'])
+
+        return make_json_response(
+            success=1,
+            info=_("Owner changed successfully."),
+            data={}
+        )
+    except Exception as e:
+        return internal_server_error(
+            errormsg='Unable to update shared server owner')
+
+
[email protected](
+    '/shared_servers/<int:uid>', methods=['GET'], endpoint='shared_servers'
+)
+@roles_required('Administrator')
+def get_shared_servers(uid):
+    """
+
+    Args:
+      uid:
+
+    Returns:
+
+    """
+    usr = User.query.get(uid)
+
+    if not usr:
+        abort(404)
+
+    try:
+        shared_servers_count = 0
+        admin_role = Role.query.filter_by(name='Administrator')[0]
+        # Check user has admin role.
+        for role in usr.roles:
+            if role.id == admin_role.id:
+                # get all server created by user.
+                servers = Server.query.filter_by(user_id=usr.id).all()
+                for server in servers:
+                    if server.shared:
+                        shared_servers_count += 1
+                break
+
+        if shared_servers_count:
+            return make_json_response(
+                success=1,
+                info=_(
+                    "{0} Shared servers are associated with this user."
+                    "".format(shared_servers_count)
+                ),
+                data={
+                    'shared_servers': shared_servers_count
+                }
+            )
+
+        return make_json_response(
+            success=1,
+            info=_("No shared servers found"),
+            data={'shared_servers': 0}
+        )
+    except Exception as e:
+        return internal_server_error(errormsg=str(e))
+
+
+# @blueprint.route(
+#     '/admin_users', methods=['GET'], endpoint='admin_users'
+# )
[email protected](
+    '/admin_users/<int:uid>', methods=['GET'], endpoint='admin_users'
+)
+@roles_required('Administrator')
+def admin_users(uid=None):
+    """
+
+    Args:
+      uid:
+
+    Returns:
+
+    """
+    admin_role = Role.query.filter_by(name='Administrator')[0]
+
+    admin_users = db.session.query(roles_users).filter_by(
+        role_id=admin_role.id).all()
+
+    if uid:
+        admin_users = [user[0] for user in admin_users if user[0] != uid]
+    else:
+        admin_users = [user[0] for user in admin_users]
+
+    admin_list = User.query.filter(User.id.in_(admin_users)).all()
+
+    user_list = [{'value': admin.id, 'label': admin.username} for admin in
+                 admin_list]
+
+    return make_json_response(
+        success=1,
+        info=_("No shared servers found"),
+        data={
+            'status': 'success',
+            'msg': 'Admin user list',
+            'result': {
+                'data': user_list,
+            }
+        }
+    )
+
+
 @blueprint.route('/user/<int:uid>', methods=['PUT'], endpoint='update_user')
 @roles_required('Administrator')
 def update(uid):
diff --git a/web/pgadmin/tools/user_management/static/js/user_management.js b/web/pgadmin/tools/user_management/static/js/user_management.js
index c3aca881..531c8c59 100644
--- a/web/pgadmin/tools/user_management/static/js/user_management.js
+++ b/web/pgadmin/tools/user_management/static/js/user_management.js
@@ -604,7 +604,223 @@ define([
         }),
         gridSchema = Backform.generateGridColumnsFromModel(
           null, UserModel, 'edit'),
+
+
         deleteUserCell = Backgrid.Extension.DeleteCell.extend({
+          changeOwnership: function(res, uid) {
+            let self = this;
+
+            let ownershipSelect2Control = Backform.Select2Control.extend({
+              fetchData: function(){
+                let self = this;
+                let url = self.field.get('url');
+
+                url = url_for(url, {'uid': uid});
+
+                $.ajax({
+                  url: url,
+                  headers: {
+                    'Cache-Control' : 'no-cache',
+                  },
+                }).done(function (res) {
+                  var transform = self.field.get('transform');
+                  if(res.data.status){
+                    let data = res.data.result.data;
+
+                    if (transform && _.isFunction(transform)) {
+                      self.field.set('options', transform.bind(self, data));
+                    } else {
+                      self.field.set('options', data);
+                    }
+                  } else {
+                    if (transform && _.isFunction(transform)) {
+                      self.field.set('options', transform.bind(self, []));
+                    } else {
+                      self.field.set('options', []);
+                    }
+                  }
+                  Backform.Select2Control.prototype.render.apply(self, arguments);
+                }).fail(function(e){
+                  let msg = '';
+                  if(e.status == 404) {
+                    msg = 'Unable to find url.';
+                  } else {
+                    msg = e.responseJSON.errormsg;
+                  }
+                  alertify.error(msg);
+                });
+              },
+              render: function() {
+                this.fetchData();
+                return Backform.Select2Control.prototype.render.apply(this, arguments);
+              },
+              onChange: function() {
+                Backform.Select2Control.prototype.onChange.apply(this, arguments);
+              },
+            });
+
+            let ownershipModel = pgBrowser.DataModel.extend({
+              schema: [
+                {
+                  id: 'note_text_ch_owner',
+                  control: Backform.NoteControl,
+                  text: 'Select the user that will take ownership of the shared servers created by <b>' + self.model.get('username') + '</b>. <b>' + res['data'].shared_servers + '</b> shared servers are currently owned by this user.',
+                  group: gettext('General'),
+                },
+                {
+                  id: 'user',
+                  name: 'user',
+                  label: gettext('User'),
+                  type: 'text',
+                  editable: true,
+                  select2: {
+                    allowClear: true,
+                    width: '100%',
+                    first_empty: true,
+                  },
+                  control: ownershipSelect2Control,
+                  url: 'user_management.admin_users',
+                  helpMessage: gettext('Note: If no user is selected, the shared servers will be deleted.'),
+                }],
+            });
+            // Change shared server ownership before deleting the admin user
+            if (!alertify.changeOwnershipDialog) {
+              alertify.dialog('changeOwnershipDialog', function factory() {
+                let $container = $('<div class=\'change-ownership\'></div>');
+                return {
+                  main: function(message) {
+                    this.msg = message;
+                  },
+                  build: function() {
+                    this.elements.content.appendChild($container.get(0));
+                    alertify.pgDialogBuild.apply(this);
+                  },
+                  setup: function(){
+                    return {
+                      buttons: [
+                        {
+                          text: gettext('Cancel'),
+                          key: 27,
+                          className: 'btn btn-secondary fa fa-times pg-alertify-button',
+                          'data-btn-name': 'cancel',
+                        }, {
+                          text: gettext('OK'),
+                          key: 13,
+                          className: 'btn btn-primary fa fa-check pg-alertify-button',
+                          'data-btn-name': 'ok',
+                        },
+                      ],
+                      // Set options for dialog
+                      options: {
+                        title: 'Change ownership',
+                        //disable both padding and overflow control.
+                        padding: !1,
+                        overflow: !1,
+                        model: 0,
+                        resizable: true,
+                        maximizable: false,
+                        pinnable: false,
+                        closableByDimmer: false,
+                        modal: false,
+                        autoReset: false,
+                        closable: true,
+                      },
+                    };
+                  },
+                  prepare: function() {
+                    let self = this;
+                    $container.html('');
+
+                    self.ownershipModel = new ownershipModel();
+                    let fields = pgBackform.generateViewSchema(null, self.ownershipModel, 'create', null, null, true, null);
+
+                    let view = this.view = new pgBackform.Dialog({
+                      el: '<div></div>',
+                      model: self.ownershipModel,
+                      schema: fields,
+                    });
+                    //Render change ownership dialog.
+                    $container.append(view.render().$el[0]);
+                  },
+                  callback: function(e) {
+                    if(e.button['data-btn-name'] === 'ok') {
+                      e.cancel = true; // Do not close dialog
+                      let ownershipModel = this.ownershipModel.toJSON();
+                      if (ownershipModel.user == '' || ownershipModel.user == undefined) {
+                        alertify.confirm(
+                          gettext('Delete user?'),
+                          gettext('The shared servers owned by <b>'+ self.model.get('username') +'</b> will be deleted. Do you wish to continue?'),
+                          function() {
+
+                            self.model.destroy({
+                              wait: true,
+                              success: function() {
+                                alertify.success(gettext('User deleted.'));
+                                alertify.changeOwnershipDialog().destroy();
+                                alertify.UserManagement().destroy();
+                              },
+                              error: function() {
+                                alertify.error(
+                                  gettext('Error during deleting user.')
+                                );
+                              },
+                            });
+                            alertify.changeOwnershipDialog().destroy();
+                          },
+                          function() {
+                            return true;
+                          }
+                        );
+                      } else {
+                        self.changeOwner(ownershipModel.user, uid);
+                      }
+                    } else {
+                      alertify.changeOwnershipDialog().destroy();
+                    }
+                  },
+                };
+              });
+            }
+            alertify.changeOwnershipDialog('Change ownership').resizeTo(pgBrowser.stdW.md, pgBrowser.stdH.md);
+          },
+          changeOwner: function(user_id, old_user) {
+            $.ajax({
+              url: url_for('user_management.change_owner'),
+              method: 'POST',
+              data:{'new_owner': user_id, 'old_owner': old_user},
+            })
+              .done(function(res) {
+                alertify.changeOwnershipDialog().destroy();
+                alertify.UserManagement().destroy();
+                alertify.success(gettext(res.info));
+              })
+              .fail(function() {
+                alertify.error(gettext('Unable to change owner.'));
+              });
+          },
+          deleteUser: function() {
+            let self = this;
+            alertify.confirm(
+              gettext('Delete user?'),
+              gettext('Are you sure you wish to delete this user?'),
+              function() {
+                self.model.destroy({
+                  wait: true,
+                  success: function() {
+                    alertify.success(gettext('User deleted.'));
+                  },
+                  error: function() {
+                    alertify.error(
+                      gettext('Error during deleting user.')
+                    );
+                  },
+                });
+              },
+              function() {
+                return true;
+              }
+            );
+          },
           deleteRow: function(e) {
             var self = this;
             e.preventDefault();
@@ -629,26 +845,27 @@ define([
               if (self.model.isNew()) {
                 self.model.destroy();
               } else {
-                alertify.confirm(
-                  gettext('Delete user?'),
-                  gettext('Are you sure you wish to delete this user?'),
-                  function() {
-                    self.model.destroy({
-                      wait: true,
-                      success: function() {
-                        alertify.success(gettext('User deleted.'));
-                      },
-                      error: function() {
-                        alertify.error(
-                          gettext('Error during deleting user.')
-                        );
-                      },
+                if(self.model.get('role') == 1){
+                  $.ajax({
+                    url: url_for('user_management.shared_servers', {'uid': self.model.get('id'),
+                    }),
+                    method: 'GET',
+                    async: false,
+                  })
+                    .done(function(res) {
+                      if(res['data'].shared_servers > 0) {
+                        self.changeOwnership(res, self.model.get('id'));
+                      } else {
+                        self.deleteUser();
+                      }
+                    })
+                    .fail(function() {
+                      self.deleteUser();
                     });
-                  },
-                  function() {
-                    return true;
-                  }
-                );
+                } else {
+                  self.deleteUser();
+                }
+
               }
             } else {
               alertify.alert(


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-01 09:22             ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-03 08:51               ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
@ 2021-02-04 06:03                 ` Akshay Joshi <[email protected]>
  2021-02-22 07:21                   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Akshay Joshi @ 2021-02-04 06:03 UTC (permalink / raw)
  To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers

Thanks, patch applied.

On Wed, Feb 3, 2021 at 2:22 PM Nikhil Mohite <[email protected]>
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 <[email protected]> wrote:
>
>> 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
>>
>>

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

*Mobile: +91 976-788-8246*


Attachments:

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

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-01 09:22             ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-03 08:51               ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-04 06:03                 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
@ 2021-02-22 07:21                   ` Nikhil Mohite <[email protected]>
  2021-02-22 09:32                     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 12+ messages in thread

From: Nikhil Mohite @ 2021-02-22 07:21 UTC (permalink / raw)
  To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers

Hi Akshay/ Team,

Please find an attached updated patch for RM-6143, Added missing updated
shared server owner name in the *"sharedserver"* table while the user
changing the ownership of the shared server.

On Thu, Feb 4, 2021 at 11:34 AM Akshay Joshi <[email protected]>
wrote:

> Thanks, patch applied.
>
> On Wed, Feb 3, 2021 at 2:22 PM Nikhil Mohite <
> [email protected]> 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 <[email protected]> wrote:
>>
>>> 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
>>>
>>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>


Attachments:

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

  [application/octet-stream] RM_6143_v3.patch (1.7K, 4-RM_6143_v3.patch)
  download | inline diff:
diff --git a/web/pgadmin/tools/user_management/__init__.py b/web/pgadmin/tools/user_management/__init__.py
index 15a1b673..ebfba540 100644
--- a/web/pgadmin/tools/user_management/__init__.py
+++ b/web/pgadmin/tools/user_management/__init__.py
@@ -383,6 +383,7 @@ def change_owner():
         request.data, encoding='utf-8'
     )
     try:
+        new_user = User.query.get(data['new_owner'])
         old_user_servers = Server.query.filter_by(shared=True, user_id=data[
             'old_owner']).all()
         server_group_ids = [server.servergroup_id for server in
@@ -400,12 +401,15 @@ def change_owner():
         deleted_sg = []
         # Change server user.
         for server in old_user_servers:
+            sh_servers = SharedServer.query.filter_by(
+                servergroup_id=server.servergroup_id).all()
+
             if old_sg_data[server.servergroup_id] in sg_data:
-                sh_servers = SharedServer.query.filter_by(
-                    servergroup_id=server.servergroup_id).all()
+
                 for sh in sh_servers:
                     sh.servergroup_id = sg_data[
                         old_sg_data[server.servergroup_id]]
+                    sh.server_owner = new_user.username
                 # Update Server user and server group to prevent deleting
                 # shared server associated with deleting user.
                 Server.query.filter_by(
@@ -422,6 +426,8 @@ def change_owner():
                 deleted_sg.append(server.servergroup_id)
             else:
                 server.user_id = data['new_owner']
+                for sh in sh_servers:
+                    sh.server_owner = new_user.username
 
         # Change server group user.
         for server_group in server_groups:


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
  2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 09:22 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 09:43   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-01-21 09:54     ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-01-21 10:33       ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-01-21 10:36         ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-01 07:51           ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-01 09:22             ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Dave Page <[email protected]>
  2021-02-03 08:51               ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
  2021-02-04 06:03                 ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Akshay Joshi <[email protected]>
  2021-02-22 07:21                   ` Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
@ 2021-02-22 09:32                     ` Akshay Joshi <[email protected]>
  0 siblings, 0 replies; 12+ messages in thread

From: Akshay Joshi @ 2021-02-22 09:32 UTC (permalink / raw)
  To: Nikhil Mohite <[email protected]>; +Cc: pgadmin-hackers

Thanks, patch applied.

On Mon, Feb 22, 2021 at 12:51 PM Nikhil Mohite <
[email protected]> wrote:

> Hi Akshay/ Team,
>
> Please find an attached updated patch for RM-6143, Added missing updated
> shared server owner name in the *"sharedserver"* table while the user
> changing the ownership of the shared server.
>
> On Thu, Feb 4, 2021 at 11:34 AM Akshay Joshi <
> [email protected]> wrote:
>
>> Thanks, patch applied.
>>
>> On Wed, Feb 3, 2021 at 2:22 PM Nikhil Mohite <
>> [email protected]> 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 <[email protected]> wrote:
>>>
>>>> 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
>>>>
>>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Principal Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>

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

*Mobile: +91 976-788-8246*


Attachments:

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

^ permalink  raw  reply  [nested|flat] 12+ messages in thread


end of thread, other threads:[~2021-02-22 09:32 UTC | newest]

Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 06:48 [pgAdmin][RM-6143]: Shared server entries not getting deleted. Nikhil Mohite <[email protected]>
2021-01-21 09:22 ` Akshay Joshi <[email protected]>
2021-01-21 09:43   ` Dave Page <[email protected]>
2021-01-21 09:54     ` Akshay Joshi <[email protected]>
2021-01-21 10:33       ` Nikhil Mohite <[email protected]>
2021-01-21 10:36         ` Dave Page <[email protected]>
2021-02-01 07:51           ` Nikhil Mohite <[email protected]>
2021-02-01 09:22             ` Dave Page <[email protected]>
2021-02-03 08:51               ` Nikhil Mohite <[email protected]>
2021-02-04 06:03                 ` Akshay Joshi <[email protected]>
2021-02-22 07:21                   ` Nikhil Mohite <[email protected]>
2021-02-22 09:32                     ` Akshay Joshi <[email protected]>

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