public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aditya Toshniwal <[email protected]>
To: Karan Takalkar <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Re : [BUG #4943]
Date: Wed, 11 Dec 2019 17:41:03 +0530
Message-ID: <CAM9w-_k5AKhwGM7OT74DwVYUJrq1vLv+-jdqtnBTDLO1MEWw2Q@mail.gmail.com> (raw)
In-Reply-To: <CADiZPH7CaEG+vgrNvs4XfO3eTRV3NAs_dZc-fWSor=Aq9PgkLg@mail.gmail.com>
References: <CADiZPH7qT7PVSrwduvL8Sa6N2W27NkP4EcsycmyygokYxXaFUQ@mail.gmail.com>
<CAM9w-_=KyZkcHFg9g1esMq7mSiAEYqaC1hWcFiqnj4W8MyPk0w@mail.gmail.com>
<CADiZPH5kx_J1h-yHBDT9nK+rewdeVE++5p=6e8XM+sF7T3s+MQ@mail.gmail.com>
<CAM9w-_=TTvNZcYBq4q=XJyZdsL9UQc3vNzVZ5an7ML8kuO+U6g@mail.gmail.com>
<CADiZPH4WL7+Y-Arbjd_66u0XDZcVYzUNr9+JiVK4O4Tue=OA0A@mail.gmail.com>
<CAM9w-_nw5E73CVL5TsHOC-NpoGrQjdgsbwNbX4Jw-iXs+N8W=Q@mail.gmail.com>
<CADiZPH4EPQ9EJ-ds87wu0gMgao=1bJxhGcgBwxV-HuHQcteJTA@mail.gmail.com>
<CAM9w-_nzFUPTC5NBDXiyO7m+d5QVhJjwNCCFg=4gTJE4e9vfcA@mail.gmail.com>
<CADiZPH7CaEG+vgrNvs4XfO3eTRV3NAs_dZc-fWSor=Aq9PgkLg@mail.gmail.com>
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 protected]>
wrote:
> 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 protected]> 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.
>>
>> On Tue, Dec 10, 2019 at 6:51 PM Karan Takalkar <[email protected]>
>> wrote:
>>
>>> 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 protected]> wrote:
>>>
>>>> [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 protected]>
>>>> 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 <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> 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 protected]>
>>>>>> 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 protected]> wrote:
>>>>>>>
>>>>>>>> ++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 protected]> 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"
>>>>>>>>
>>>>>>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Sr. Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
--
Thanks and Regards,
Aditya Toshniwal
Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Attachments:
[application/octet-stream] BUG_4943_updated_v3.patch (3.0K, 3-BUG_4943_updated_v3.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py
index 26fd5ccac..c4cb71a11 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,
+ 'info_prefix': '{0}/{1}'.
+ format(Server.query.filter_by(id=sid)[0].name, conn.db)
}
)
@@ -478,7 +481,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 +492,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..12e519ec4 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 = `${_.escape(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 = `${_.escape(res.data.info_prefix)} - ${res.info}`;
+ }
Alertify.success(res.info);
obj.trigger('connected', obj, item, data);
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Re : [BUG #4943]
In-Reply-To: <CAM9w-_k5AKhwGM7OT74DwVYUJrq1vLv+-jdqtnBTDLO1MEWw2Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox