Subject: Re: Re : [BUG #4943]

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 9 Dec 2019 15:24:58 +0530

++pgadmin-hackers

Hi Karan,

It is good to know that you're contributing.
I would suggest {server name}/{db name} as name instead of did is better
for UX. You can get the db name from conn object and server name using the
sid (refer - web/pgadmin/browser/server_groups/servers/__init__.py)

On Mon, Dec 9, 2019 at 3:11 PM Karan Takalkar <[email-masked]> wrote:

> Hi,
>
> I have been working on #4943 ("Database connected" success message itself
> is not enough) added by you on pgAdmin4 issues.
> I want to know what details of database should be included in success
> message.(i have added 'did')
>
> The success message can be modified by altering json response in the
> connect method in file
> PGADMIN_SRC/web/pgadmin/browser/server_groups/servers/databases/__init__.py.
> please have a look at the screenshots attached.
>
> original:
> info=_( "Database connected.")
> new:
> info=_("Postgres version/{0} Database connected.".format(did))
>
> Regards,
> Karan
>

-- 
Thanks and Regards,
Aditya Toshniwal
Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"

---

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 9 Dec 2019 20:35:03 +0530

Hi Karan,

Kindly add a hyphen between the message and names, create a patch to
pgAdmin hackers(check cc).
Kindly also run the test cases and pep8 before sending.

On Mon, Dec 9, 2019, 20:17 Karan Takalkar <[email-masked]> wrote:

> I have updated the success message.
> Please check the screenshots attached.
> Should i make a patch?
>
> Regards,
> Karan
>
> On Mon, 9 Dec, 2019, 3:25 PM Aditya Toshniwal, <
> [email-masked]> wrote:
>
>> ++pgadmin-hackers
>>
>> Hi Karan,
>>
>>> I have been working on #4943 ("Database connected" success message
>>> itself is not enough) added by you on pgAdmin4 issues.

---

From: Aditya Toshniwal <[email-masked]>
Date: Tue, 10 Dec 2019 18:34:26 +0530

[please use reply all to reply]

You can add more variables to the response along with info and use those in
the front end.

On Tue, Dec 10, 2019, 18:24 Karan Takalkar <[email-masked]> wrote:

> Hi
>
> I had been naively modifying jason response of connect function in
> databases __init__.py, later realized there are a lot of dependencies on it
> and most of regression tests use:
> if db_con["info"] == "Database connected.": .
>
> I am now modifying the Alertify.success script* directly responsible for
> the popup, i could append the database name in the message but am having
> trouble finding the *variable to supply server name.*
> The file and location of function is :
> *
> (web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
> line 523)
>
> the message should be:
> Alertify.success("(?server_name_variable?}+'/'+data.label+' - '+res.info")
>
> data.label contains database name
> res.info is the jason response coming from databases __init__.py connect
> function ; which is "Database connected."
>
> On Mon, Dec 9, 2019 at 8:35 PM Aditya Toshniwal <
>>>> I would suggest {server name}/{db name} as name instead of did is
>>>> better for UX. You can get the db name from conn object and server name
>>>> using the sid (refer
>>>> - web/pgadmin/browser/server_groups/servers/__init__.py)
>>>>
>>>>

---

From: Karan Takalkar <[email-masked]>
Date: Tue, 10 Dec 2019 18:50:48 +0530

Hi,

"add more variables to the response along with info and use those in the
front end"
i have already implemented that , but am still failing (7)  test cases
particularly in the databases/extensions tests(5).
i had run regression tests for browser node.(and all it's sub directories).
Please check the patch attached.

On Tue, Dec 10, 2019 at 6:34 PM Aditya Toshniwal <
[email-masked]> wrote:

> You can add more variables to the response along with info and use those
> in the front end.
>
diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
index 26fd5ccac..a07dcaf16 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
@@ -30,6 +30,7 @@ from pgadmin.utils.ajax import make_json_response, \
     make_response as ajax_response, internal_server_error, unauthorized
 from pgadmin.utils.driver import get_driver
 from pgadmin.tools.sqleditor.utils.query_history import QueryHistory
+from pgadmin.model import Server

 class DatabaseModule(CollectionNodeModule):
@@ -468,7 +469,9 @@ class DatabaseView(PGChildNodeView):
                 info=_("Database connected."),
                 data={
                     'icon': 'pg-icon-database',
-                    'connected': True
+                    'connected': True,
+                    'svr':  Server.query.filter_by(id=sid)[0].name,
+                    'db': conn.db
                 }
             )
 
diff --git a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
index ab0df6939..43029ce90 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
@@ -520,7 +520,7 @@ define('pgadmin.node.database', [
                 tree.addIcon(item, {icon: data.icon});
               }
 
-              Alertify.success(res.info);
+              Alertify.success(res.data.svr+'/'+res.data.db+' - '+res.info);
               obj.trigger('connected', obj, item, data);
               pgBrowser.Events.trigger(
                 'pgadmin:database:connected', item, data

---

From: Aditya Toshniwal <[email-masked]>
Date: Wed, 11 Dec 2019 15:02:33 +0530

Hi Karan,

I've updated your patch a bit. Kindly test and share the logs if test cases
fail.
Test cases seems to be working fine for me. Kindly also change disconnected
messages.

On Tue, Dec 10, 2019 at 6:51 PM Karan Takalkar <[email-masked]>
wrote:

>>> res.info is the jason response coming from databases __init__.py
>>> connect function ; which is "Database connected."
>>>
>>>>>>> I want to know what details of database should be included in
>>>>>>> success message.(i have added 'did')
>>>>>>>

---

From: Karan Takalkar <[email-masked]>
Date: Wed, 11 Dec 2019 17:19:02 +0530

Hi,

I have updated the disconnect messages.
Databases/extension tests are failing , i have attached the log below.
They are also failing if i undo the modifications.
i do not know if this a  problem with my setup or i maybe not following a
convention.
Please check the updated patch.

Regards,
Karan

On Wed, Dec 11, 2019 at 3:03 PM Aditya Toshniwal <
[email-masked]> wrote:

> Hi Karan,
>
> I've updated your patch a bit. Kindly test and share the logs if test
> cases fail.
> Test cases seems to be working fine for me. Kindly also change
> disconnected messages.
>
>>>> I am now modifying the Alertify.success script* directly responsible
>>>> for the popup, i could append the database name in the message but am
>>>> having trouble finding the *variable to supply server name.*
>>>>>>>> The success message can be modified by altering json response in
>>>>>>>> the connect method in file
diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
index 26fd5ccac..905401fa7 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
@@ -30,7 +30,7 @@ from pgadmin.utils.ajax import make_json_response, \
     make_response as ajax_response, internal_server_error, unauthorized
 from pgadmin.utils.driver import get_driver
 from pgadmin.tools.sqleditor.utils.query_history import QueryHistory
-
+from pgadmin.model import Server
 
 class DatabaseModule(CollectionNodeModule):
     NODE_TYPE = 'database'
@@ -468,7 +468,9 @@ class DatabaseView(PGChildNodeView):
                 info=_("Database connected."),
                 data={
                     'icon': 'pg-icon-database',
-                    'connected': True
+                    'connected': True,
+                    'info_prefix': '{0}/{1}'.
+                    format(Server.query.filter_by(id=sid)[0].name, conn.db)
                 }
             )
 
@@ -478,7 +480,7 @@ class DatabaseView(PGChildNodeView):
         # Release Connection
         from pgadmin.utils.driver import get_driver
         manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(sid)
-
+        conn = manager.connection(did=did, auto_reconnect=True)
         status = manager.release(did=did)
 
         if not status:
@@ -489,7 +491,9 @@ class DatabaseView(PGChildNodeView):
                 info=_("Database disconnected."),
                 data={
                     'icon': 'icon-database-not-connected',
-                    'connected': False
+                    'connected': False,
+                    'info_prefix': '{0}/{1}'.
+                    format(Server.query.filter_by(id=sid)[0].name, conn.db)
                 }
             )
 
diff --git a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
index ab0df6939..838977016 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/static/js/database.js
@@ -200,6 +200,9 @@ define('pgadmin.node.database', [
                 .done(function(res) {
                   if (res.success == 1) {
                     var prv_i = t.parent(i);
+                    if(res.data.info_prefix) {
+                      res.info = `${res.data.info_prefix} - ${res.info}`;
+                    }
                     Alertify.success(res.info);
                     t.removeIcon(i);
                     data.connected = false;
@@ -519,6 +522,9 @@ define('pgadmin.node.database', [
                 data.icon = res.data.icon;
                 tree.addIcon(item, {icon: data.icon});
               }
+              if(res.data.info_prefix) {
+                res.info = `${res.data.info_prefix} - ${res.info}`;
+              }
 
               Alertify.success(res.info);
               obj.trigger('connected', obj, item, data);

---

From: Aditya Toshniwal <[email-masked]>
Date: Wed, 11 Dec 2019 17:41:03 +0530

Hi,

I've updated your patch further to escape the info prefix so that it
doesn't break if db name is something like - "<script>alert(1)</script>".
Fixed a pep8 issue.
Otherwise, the patch looks good to me and test cases works fine.

On Wed, Dec 11, 2019 at 5:19 PM Karan Takalkar <[email-masked]>
wrote:

>>>> You can add more variables to the response along with info and use
>>>> those in the front end.
>>>>
>>>>>>>>> I have been working on #4943 ("Database connected" success
>>>>>>>>> message itself is not enough) added by you on pgAdmin4 issues.

---

From: Akshay Joshi <[email-masked]>
Date: Thu, 12 Dec 2019 11:13:07 +0530

Thanks, patch applied.

On Wed, Dec 11, 2019 at 5:41 PM Aditya Toshniwal <
[email-masked]> wrote:

>>>> "add more variables to the response along with info and use those in
>>>> the front end"

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*