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

On Thu, Oct 4, 2018 at 7:52 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Thu, Oct 4, 2018 at 1:12 PM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>
> This seems to behave quite strangely - only sporadically saving the
> treeview state, and then not always (maybe ever?) restoring the selected
> node. When I watch the Python console, even with the save interval set to 1
> second, I very rarely see the state being saved. For example:
>
> 2018-10-04 15:14:01,341: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
> 15:14:01] "GET /browser/database/children/1/1/44243 HTTP/1.1" 200 -
> 2018-10-04 15:14:01,355: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
> 15:14:01] "POST /settings/save_tree_state/ HTTP/1.1" 200 -
> 2018-10-04 15:14:02,357: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
> 15:14:02] "POST /settings/save_tree_state/ HTTP/1.1" 200 -
> 2018-10-04 15:14:23,459: INFO werkzeug: 127.0.0.1 - - [04/Oct/2018
> 15:14:23] "GET /preferences/ HTTP/1.1" 200 -
>
> You can see there it called it twice, then 21 seconds later I opened the
> preferences panel to check what the interval was set to (it was still 1
> second). Even as I type this some minutes later, it still hasn't called it
> again. I think it's essential that works reliably, as it's so unreliable to
> do it on Window close.
>
>
I have checked the previous state and the current state and if both are
same then I skip the save call.
Is this the case or even if you change the tree view state it is not saving?

>
>> 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
>>>
>>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>