Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview

Hi

On Mon, Oct 1, 2018 at 1:53 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Dave
>
>
> On Mon, Oct 1, 2018 at 4:02 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Mon, Oct 1, 2018 at 11:06 AM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Tue, Sep 25, 2018 at 6:26 PM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Sep 25, 2018 at 6:17 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Sep 25, 2018 at 3:00 AM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> On Tue, Sep 25, 2018 at 12:15 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>> On Mon, Sep 24, 2018 at 2:05 AM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find the attached updated patch.
>>>>>>>>
>>>>>>>> Feature Details:
>>>>>>>> - The current tree state as well as the previous will be stored in
>>>>>>>> the sqlite database.
>>>>>>>> - The time interval to store the tree state is configurable via
>>>>>>>> preferences and the default is 30 secs.
>>>>>>>> -1 can be used to stop the tree saving functionality,
>>>>>>>> - Jasmine test cases are included.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Khushboo
>>>>>>>>
>>>>>>>
>>>>>>> This is looking good, however there are a couple of cases where I
>>>>>>> think it's not quite working as I would expect:
>>>>>>>
>>>>>>> 1) As soon as the user opens pgAdmin, the tree state should be
>>>>>>> restored.
>>>>>>>
>>>>>>> As per our initial discussion, we have decided that once the user
>>>>>> connects / expands the server, then we will restore that server state.
>>>>>>
>>>>>> *"If the user has switched network that may cause a whole bunch of
>>>>>> connection failures after some period of time. Perhaps we should only
>>>>>> restore when opening a particular server." *
>>>>>>
>>>>>
>>>>> Yeah, I remember that now. I guess it just seems less than ideal - but
>>>>> I'm not sure how we could open everything by default without risking
>>>>> connection failures. Let's stick with the original plan (ie. what you've
>>>>> implemented).
>>>>>
>>>> Okay.
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> 2) If a database or server is disconnected, I would expect it's state
>>>>>>> to be stored. When it is explicitly re-connected by the user, the state
>>>>>>> should be restored.
>>>>>>>
>>>>>>> This is already implemented and working. Am I missing any particular
>>>>>> scenario?
>>>>>>
>>>>>
>>>>> I tried disconnecting a server and database, and when I re-opened it,
>>>>> the state wasn't restored. Do we explicitly save the state on disconnect,
>>>>> or was it that I needed to wait up to 30 seconds (or should have waited 30
>>>>> seconds before disconnecting)?
>>>>>
>>>> I have explicitly saved the state on the server disconnection. So, on
>>>> the reconnection of the server, the state should be restored.
>>>> But this logic is only applicable on server not on the databases. So,
>>>> once you disconnect the database the state will not be restored.
>>>> Should I include databases also ?
>>>>
>>>>
>>>>>
>>>>>
>>>>>> I think that covers the different cases I'm imagining.
>>>>>>>
>>>>>>
>>>>> I wrote that to try to convince myself I hadn't forgotten anything - I
>>>>> think I had though. When the state is restored on any given branch of the
>>>>> tree, the previously selected node should be re-selected. That doesn't seem
>>>>> to be happening at the moment.
>>>>>
>>>>> This means even if I have opened multiple servers/databases, the
>>> selected item will remain only one.
>>> Is this correct?
>>>
>>
>> The selected item will be the previously selected item from whatever was
>> just restored.
>>
>>
> Previously when the last selected node was not considered, I stored the
> data in the database as below:
>
> {
>
> 1:
>
> [
>
>
> “server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16400”,
>
>
> "server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16394,coll-column/16394”,
>
>
> "server_group/1,server/1,coll-database/1,database/12669,coll-schema/12669,schema/2200,coll-table/2200,table/16397,coll-column/16397”
>
> ]
>
> }
>
>
>
> Here key is the server_id i.e. 1 and all the paths are stored as above
>
>
> Now, we are adding previously selected item as well as the database
> restore option, so I am thinking of changing the data storage structure as
> below.
>
>
>
> {
>
> 1:
>
> {
>
> 'paths':
>
> [
>
>
> “server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16400”,
>
>
> "server_group/1,server/1,coll-database/1,database/16393,coll-schema/16393,schema/2200,coll-table/2200,table/16394,coll-column/16394”,
>
>
> "server_group/1,server/1,coll-database/1,database/12669,coll-schema/12669,schema/2200,coll-table/2200,table/16397,coll-column/16397”
>
> ],
>
> 'selected' :
>
> {
>
> "server/1": "coll-database/1",
>
> "database/16393": "table/16400",
>
> "database/12669": "coll-column/16397"
>
> },
>
> 'connection_status':
>
> {
>
> "database/16393": 1,
>
> "database/12669": 0
>
> }
>
>
> }
>
> }
>
> Please review it and let me know your suggestion.
>

Looks reasonable to me.

>
> Thanks,
> Khushboo
>
>
>
>> Will do.
>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>>>
>>>>>>> Thanks, Dave.
>>>>>>>
>>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>>> --
>>>>>>> Dave Page
>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>> Twitter: @pgsnake
>>>>>>>
>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company