public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Nikhil Mohite <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
Date: Mon, 22 Feb 2021 15:02:02 +0530
Message-ID: <CANxoLDeemd+UdZ5yhA_Geth6eKaP8BXJ=saOeqcW4d0ojw6YRw@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0ANTT=SeeG8yMaVUK_5UcO3fCGAae5KdzXAKL2GP3_kGeg@mail.gmail.com>
References: <CAOBg0APfo=oZvAotP9dpDf8_CveOnP9EpwsV=zn1uYPWgqPZ-Q@mail.gmail.com>
<CANxoLDfTspipwkMcJnNrnOWvguOMeHqNestMJ1dBqLGoowTeqQ@mail.gmail.com>
<CA+OCxowLS7AuXSmrfZ+PkOiVAiXU0RFPiJo8qz1b97yXG48mvA@mail.gmail.com>
<CANxoLDfN2AehHnBPDLPO1vR+yw-16T8dcRx04--UX-69UmCzqQ@mail.gmail.com>
<CAOBg0AM3ycNtnMwJ2Rk1Ty-qCW4dCsq11Zz6Eaai3bRWDb3LMg@mail.gmail.com>
<CA+OCxozsMp+p62THjyi=UX9j-4ZmsDz75OLYQPsPFWzQ6cEXQA@mail.gmail.com>
<CAOBg0ANns7UBMCXkuYQ-Rzsz4rtgQTCLOPke=ZMb77hkRq2fLg@mail.gmail.com>
<CA+OCxoygL=vjY73=sRd9c_aKcqfTTFw4OUJsGtD=0qJAZCTpXg@mail.gmail.com>
<CAOBg0AMA7tYj7zgOO0A4NVCPedq9ef9bWrg819ACFoTDgkhu-Q@mail.gmail.com>
<CANxoLDe20GoJZjjcEsuACtGAAx5CaH2gEnC4N+V8M19UagfTVw@mail.gmail.com>
<CAOBg0ANTT=SeeG8yMaVUK_5UcO3fCGAae5KdzXAKL2GP3_kGeg@mail.gmail.com>
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
view thread (12+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin][RM-6143]: Shared server entries not getting deleted.
In-Reply-To: <CANxoLDeemd+UdZ5yhA_Geth6eKaP8BXJ=saOeqcW4d0ojw6YRw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox