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

Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Mon, Oct 1, 2018 at 7:14 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

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