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

Hi Dave,

I found one issue with the selecting previous selected node which I was
fixed and attached the updated patch.
Please let me know if you find any specific scenario which is not covered.

While testing the patch, please clear your sqlite entry from the setting
table as we have changed the structure.

Thanks,
Khushboo

On Thu, Oct 4, 2018 at 8:07 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

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