Re: Re : [BUG #4943]
Aditya Toshniwal <[email protected]> Wed, 11 Dec 2019 17:41:03 +0530
| Newsgroups | gmane.comp.db.postgresql.pgadmin.devel |
|---|---|
| Message-ID | <CAM9w-_k5AKhwGM7OT74DwVYUJrq1vLv+-jdqtnBTDLO1MEWw2Q@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"
BUG_4943_updated_v3.patch
(application/octet-stream, 3 KB)
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);