public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nikhil Mohite <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Murtuza Zabuawala <[email protected]>
Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
Date: Wed, 7 Oct 2020 12:11:41 +0530
Message-ID: <CAOBg0AMU_yTSrVU6Z+Q0Je4PuE3ncALdDA3Q=QUjQeXiWuHS6w@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDcZYnhT0xw5SLs8VCS60LPFLW_OU+NEzcNs747uhv_a=A@mail.gmail.com>
References: <CAOBg0AM=b7hfJjTJAXu=wGkKHEsV_zoZpSO-QU6Z-G9p2wVF0w@mail.gmail.com>
	<CANxoLDdUj_RKgQWFE6F7noEJbDz+P_3-njnn2mf10TSa0V9wdw@mail.gmail.com>
	<CAOBg0AOHJT4wUC3AXjHV2aorF8_OpP3erah8yhyi2eKwn6F_Sg@mail.gmail.com>
	<CANxoLDca1AtBKrnW8rshWqsd_iwHEfXV2PeqCnUQnoEonfs52g@mail.gmail.com>
	<CAOBg0APx0+twWJah7RaE9ZmDsBXUnbagikR7NyG3fUD21GYd8Q@mail.gmail.com>
	<CANxoLDdkvhUE3pohPPgyyz2tYwYej+VAkxUE-e09JBxd3Aeh0w@mail.gmail.com>
	<CAOBg0ANsr6eE_4+6ApKYhj5m5N+ChbQgj_yWgufh0KqiZrovsw@mail.gmail.com>
	<CANxoLDfn8cm8YiK8e1rUWRZ+jU7AAr1krdwQb-giDg-B3ATzsQ@mail.gmail.com>
	<CAKKotZRSjJTKzcappNGZ0yJiuiwxBfBxoex+6fXF3RzdbEKDBg@mail.gmail.com>
	<CA+OCxoyfycZAT5Wc6L72HCXKrPTgkj9jcR4FgO6TKZN5KvZ_Ow@mail.gmail.com>
	<CANxoLDcZYnhT0xw5SLs8VCS60LPFLW_OU+NEzcNs747uhv_a=A@mail.gmail.com>

Hi Akshay,

I checked the implementation and found 2 locations which I missed in the
last patch to remove async: False.
I have removed all occurrences of async: False now also added missing
loader in required places.

PFA updated the patch for the same.

Regards,
Nikhil Mohite.

On Tue, Oct 6, 2020 at 6:19 PM Akshay Joshi <[email protected]>
wrote:

> Hi Nikhil
>
> Please verify and remove async = false wherever possible.
>
> On Tue, Oct 6, 2020 at 5:24 PM Dave Page <[email protected]>
> wrote:
>
>>
>>
>> On Tue, Oct 6, 2020 at 12:51 PM Murtuza Zabuawala <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> We have used aysnc=False in most ajax calls with this feature, It is
>>> causing UI hang in case of slow server response.
>>> You can try adding a time.sleep() call at the python side response and
>>> check the UI hang, I think we should avoid sync calls as much as possible.
>>>
>>
>> I consider a sync ajax call to be a bug.
>>
>>
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> *EDB*
>>> *POWER TO POSTGRES*
>>> https://www.edbpostgres.com
>>>
>>>
>>> On Thu, Oct 1, 2020 at 1:31 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Thanks, patch applied.
>>>>
>>>> On Thu, Oct 1, 2020 at 10:42 AM Nikhil Mohite <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Akshay,
>>>>>
>>>>> I have resolved the sonarQube issues, PFA updated patch for the same.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Nikhil Mohite.
>>>>>
>>>>>
>>>>> On Tue, Sep 29, 2020 at 11:31 AM Akshay Joshi <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Nikhil
>>>>>>
>>>>>> Your patch introduces 1 new Bug and 13 new code smells, please fix
>>>>>> those and resend the patch.
>>>>>>
>>>>>> On Mon, Sep 28, 2020 at 7:31 PM Nikhil Mohite <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Akshay,
>>>>>>>
>>>>>>> I have resolved code conflict issues and sonarqube issues.
>>>>>>> PFA updated patch.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Nikhil Mohite.
>>>>>>>
>>>>>>> On Mon, Sep 28, 2020 at 5:58 PM Akshay Joshi <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi Nikhil
>>>>>>>>
>>>>>>>> The patch is not applying, rebase, and send it again. Please check
>>>>>>>> your code should not create any new SonarQube issues.
>>>>>>>>
>>>>>>>> On Mon, Sep 28, 2020 at 11:20 AM Nikhil Mohite <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi Akshay,
>>>>>>>>>
>>>>>>>>> I have resolved all the review comments and also updated the test
>>>>>>>>> cases as per the new implementation.
>>>>>>>>>
>>>>>>>>> PFA updated patch.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Mon, Sep 21, 2020 at 5:24 PM Akshay Joshi <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Nikhil
>>>>>>>>>>
>>>>>>>>>> Following are the initial review comments:
>>>>>>>>>>
>>>>>>>>>>    - Open View/Edit data on any table and click on the same
>>>>>>>>>>    database connection and then click on the Execute button. Got
>>>>>>>>>>    "get_primary_keys() takes 1 positional argument but 2 were given" error.
>>>>>>>>>>    - In my opinion, we should hide the option to change the
>>>>>>>>>>    database connection for View/Edit Data.
>>>>>>>>>>    - If the user clicks on the same database connection multiple
>>>>>>>>>>    times then no need to change the backend connection and transaction id. Add
>>>>>>>>>>    validation at the backend, no action required in this case.
>>>>>>>>>>    - The role option is missing from the "connect to server"
>>>>>>>>>>    dialog.
>>>>>>>>>>    - The Password field should not be there on the "connect to
>>>>>>>>>>    server" dialog. Sometimes we saved the password so asking a password every
>>>>>>>>>>    time is not correct. Check the pgAdmin 3 behavior.
>>>>>>>>>>
>>>>>>>>>> Code review still remains.
>>>>>>>>>>
>>>>>>>>>> On Thu, Sep 17, 2020 at 4:15 PM Nikhil Mohite <
>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Team,
>>>>>>>>>>>
>>>>>>>>>>> Regarding RM-3794 <https://redmine.postgresql.org/issues/3794;
>>>>>>>>>>> allow the user to change the database connection from an open query tool:
>>>>>>>>>>> I have implemented the feature and also added documentation for
>>>>>>>>>>> it.
>>>>>>>>>>>
>>>>>>>>>>> PFA patch.
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Thanks & Regards,*
>>>>>>>>>>> *Nikhil Mohite*
>>>>>>>>>>> *Software Engineer.*
>>>>>>>>>>> *EDB Postgres* <https://www.enterprisedb.com/;
>>>>>>>>>>> *Mob.No: +91-7798364578.*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *Thanks & Regards*
>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>>
>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> Dave Page
>> VP & Chief Architect, Database Infrastructure
>> EDB: http://www.enterprisedb.com
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Sr. Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>


Attachments:

  [application/octet-stream] RM_3794_V7.patch (11.1K, 3-RM_3794_V7.patch)
  download | inline diff:
diff --git a/web/pgadmin/static/js/sqleditor/new_connection_dialog.js b/web/pgadmin/static/js/sqleditor/new_connection_dialog.js
index 3fcd374..13267ab 100644
--- a/web/pgadmin/static/js/sqleditor/new_connection_dialog.js
+++ b/web/pgadmin/static/js/sqleditor/new_connection_dialog.js
@@ -95,7 +95,7 @@ let NewConnectionDialog = {
                 closableByDimmer: false,
                 modal: false,
                 autoReset: false,
-                closable: true,
+                closable: false,
               },
             };
           },
@@ -152,14 +152,18 @@ let NewConnectionDialog = {
                 self.statusBar.removeClass('d-none');
                 $(self.statusBar.find('.alert-text')).html(msg);
                 // Disable Okay button
-                self.__internal.buttons[2].element.disabled = true;
+                if(self.__internal){
+                  self.__internal.buttons[2].element.disabled = true;
+                }
               });
 
               view.listenTo(view.model, 'pgadmin-session:valid', function() {
                 self.statusBar.addClass('d-none');
                 $(self.statusBar.find('.alert-text')).html('');
                 // Enable Okay button
-                self.__internal.buttons[2].element.disabled = false;
+                if(self.__internal) {
+                  self.__internal.buttons[2].element.disabled = false;
+                }
               });
             });
 
@@ -230,15 +234,18 @@ let NewConnectionDialog = {
                   'user': newConnCollectionModel['user'],
                   'role': newConnCollectionModel['role'],
                   'password': response.password,
+                  'server_name': response.server_name,
+                  'database_name': selected_database_name,
                 };
                 handler.gridView.on_change_connection(connection_details, self);
               }
             } else {
-              self.close();
+              Alertify.newConnectionDialog().destroy();
             }
           },
         };
       });
+
       setTimeout(function(){
         Alertify.newConnectionDialog('Connect to server.').resizeTo(pgAdmin.Browser.stdW.md,pgAdmin.Browser.stdH.md);
       }, 500);
diff --git a/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js b/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
index e262d0e..9e03051 100644
--- a/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
+++ b/web/pgadmin/static/js/sqleditor/new_connection_dialog_model.js
@@ -31,7 +31,6 @@ export default function newConnectionDialogModel(response, sgid, sid) {
       });
 
       $.ajax({
-        async: false,
         url: url,
         headers: {
           'Cache-Control' : 'no-cache',
@@ -52,8 +51,8 @@ export default function newConnectionDialogModel(response, sgid, sid) {
           } else {
             self.field.set('options', []);
           }
-          //alertify.error(res.data.msg);
         }
+        Backform.Select2Control.prototype.render.apply(self, arguments);
       }).fail(function(e){
         let msg = '';
         if(e.status == 404) {
@@ -131,7 +130,8 @@ export default function newConnectionDialogModel(response, sgid, sid) {
                   if (closeEvent.button.text == gettext('OK')) {
                     if(this.submit_password) {
                       var _url = url_for('sqleditor.connect_server', {'sid': this.server_id});
-
+                      var loadingDiv = $('#show_filter_progress');
+                      loadingDiv.removeClass('d-none');
                       $.ajax({
                         type: 'POST',
                         timeout: 30000,
@@ -148,8 +148,10 @@ export default function newConnectionDialogModel(response, sgid, sid) {
                               response.server_name = obj.name;
                             }
                           });
+                          loadingDiv.addClass('d-none');
                         })
                         .fail(function(xhr) {
+                          loadingDiv.addClass('d-none');
                           alertify.connectServer('Connect to server', xhr.responseJSON.result, local_self.getValueFromDOM());
                         });
                     } else {
@@ -182,8 +184,9 @@ export default function newConnectionDialogModel(response, sgid, sid) {
             'sid': self.getValueFromDOM(),
             'usr': self.model.attributes.user,
           });
+          var loadingDiv = $('#show_filter_progress');
+          loadingDiv.removeClass('d-none');
           $.ajax({
-            async: false,
             url: url,
             type: 'POST',
             headers: {
@@ -196,7 +199,9 @@ export default function newConnectionDialogModel(response, sgid, sid) {
                 response.server_name = obj.name;
               }
             });
+            loadingDiv.addClass('d-none');
           }).fail(function(xhr){
+            loadingDiv.addClass('d-none');
             alertify.connectServer('Connect to server', xhr.responseJSON.result, self.getValueFromDOM());
           });
 
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index 4970027..feedc63 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -418,11 +418,13 @@
                 </i>
             </div>
             <div class="connection-info btn-group mr-1" role="group" aria-label="">
-                <div class="editor-title" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"
-                     style="background-color: {% if fgcolor %}{{ bgcolor or '#FFFFFF' }}{% endif %}; color: {% if fgcolor %}{{ fgcolor }}{% endif %};">&nbsp;
+                <div class="connection-data" data-toggle="dropdown">
+                    <div class="editor-title" aria-haspopup="true" aria-expanded="false"
+                         style="background-color: {% if fgcolor %}{{ bgcolor or '#FFFFFF' }}{% endif %}; color: {% if fgcolor %}{{ fgcolor }}{% endif %};">&nbsp;
+                    </div>
+                    <span class="conn-info-dd dropdown-toggle dropdown-toggle-split"
+                    aria-haspopup="true" aria-expanded="false"></span>
                 </div>
-                <span class="conn-info-dd dropdown-toggle dropdown-toggle-split"
-                data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"></span>
                 <ul class="dropdown-menu" id="connections-list">
                 </ul>
             </div>
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 4ef4b8f..cb1f8a2 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -182,7 +182,7 @@ define('tools.querytool', [
         });
       } else {
         $('.conn-info-dd').hide();
-        $('.editor-title').css({pointerEvents: 'none'});
+        $('.connection-data').css({pointerEvents: 'none', cursor: 'arrow'});
       }
     },
 
@@ -2122,8 +2122,20 @@ define('tools.querytool', [
       let title = this.$el.find('.editor-title').html();
       if(connection_details['title'] != title) {
         var self = this;
+        var loadingDiv = null;
+        var msgDiv = null;
+        if(ref){
+          loadingDiv = $('#show_filter_progress');
+          loadingDiv.removeClass('d-none');
+          msgDiv = loadingDiv.find('.sql-editor-busy-text');
+          msgDiv.text('Connecting to database...');
+        } else{
+          loadingDiv = $('#fetching_data');
+          loadingDiv.removeClass('d-none');
+          msgDiv = loadingDiv.find('.sql-editor-busy-text');
+        }
+
         $.ajax({
-          async: false,
           url: url_for('datagrid.update_query_tool_connection', {
             'trans_id': self.transId,
             'sgid': connection_details['server_group'],
@@ -2148,7 +2160,8 @@ define('tools.querytool', [
               };
               self.set_editor_title(self.handler.url_params.title);
               self.handler.setTitle(self.handler.url_params.title);
-              alertify.success('connected successfully');
+              let success_msg = connection_details['server_name'] + '/' + connection_details['database_name']+ '- Database connected';
+              alertify.success(success_msg);
               if(ref){
                 let connection_data = {
                   'server_group': self.handler.url_params.sgid,
@@ -2159,39 +2172,27 @@ define('tools.querytool', [
                   'role': connection_details['role'],
                   'password': connection_details['password'],
                   'is_allow_new_connection': true,
+                  'database_name': connection_details['database_name'],
+                  'server_name': connection_details['server_name'],
                 };
                 self.connection_list.unshift(connection_data);
                 self.render_connection(self.connection_list);
+                loadingDiv.addClass('d-none');
+                alertify.newConnectionDialog().destroy();
                 ref.close();
+              } else {
+                loadingDiv.addClass('d-none');
               }
             }
             return true;
           })
           .fail(function(xhr) {
+            loadingDiv.addClass('d-none');
             if(xhr.status == 428) {
               alertify.connectServer('Connect to server', xhr.responseJSON.result, connection_details['server'], false);
             } else {
               alertify.error(xhr.responseJSON['errormsg']);
             }
-            /*let url = url_for('sqleditor.connect_server_with_user', {
-              'sid': newConnCollectionModel['server'],
-              'usr': newConnCollectionModel['user']
-            });
-            $.ajax({
-              async: false,
-              url: url,
-              headers: {
-                'Cache-Control' : 'no-cache',
-              },
-            }).done(function () {
-              Backform.Select2Control.prototype.onChange.apply(self, arguments);
-              response.server_list.forEach(function(obj){
-                if(obj.id==self.model.changed.server) {
-                  response.server_name = obj.name;
-                }
-              });
-            }).fail(function(xhr){});*/
-
           });
       }
     },
@@ -2526,6 +2527,8 @@ define('tools.querytool', [
             'role': null,
             'title': _.unescape(url_params.title),
             'is_allow_new_connection': false,
+            'database_name': url_params.title.split('/')[0],
+            'server_name': url_params.title.split('@')[1],
           };
           self.gridView.connection_list.unshift(connection_data);
           self.gridView.render_connection(self.gridView.connection_list);
diff --git a/web/pgadmin/tools/sqleditor/static/scss/_sqleditor.scss b/web/pgadmin/tools/sqleditor/static/scss/_sqleditor.scss
index 53f2449..7fc576a 100644
--- a/web/pgadmin/tools/sqleditor/static/scss/_sqleditor.scss
+++ b/web/pgadmin/tools/sqleditor/static/scss/_sqleditor.scss
@@ -43,6 +43,12 @@
   cursor: pointer;
 }
 
+.connection-data {
+  display: inherit;
+  cursor: pointer;
+  width: auto;
+}
+
 
 #editor-panel {
   z-index: 0;


view thread (18+ 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], [email protected]
  Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
  In-Reply-To: <CAOBg0AMU_yTSrVU6Z+Q0Je4PuE3ncALdDA3Q=QUjQeXiWuHS6w@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