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