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]>
Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
Date: Tue, 6 Oct 2020 14:21:47 +0530
Message-ID: <CAOBg0AOXBxobGxs07j9OO71p9v3gM3YYdw5yRJOkMry++DVcaA@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDfn8cm8YiK8e1rUWRZ+jU7AAr1krdwQb-giDg-B3ATzsQ@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>

Hi Akshay,

I have updated the following points as per suggestions:
1. Updated connection success message. (Added database name in the success
message.)
2. Resolve issue of the mouse pointer and dropdown will show below the
connection string only.
3. Added loader for both new connections and load existing connections.
4. Removed async: false for update connection.


Regards,
Nikhil Mohite.


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


Attachments:

  [application/octet-stream] RM_3794_V6.patch (6.0K, 3-RM_3794_V6.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..a674f2c 100644
--- a/web/pgadmin/static/js/sqleditor/new_connection_dialog.js
+++ b/web/pgadmin/static/js/sqleditor/new_connection_dialog.js
@@ -230,6 +230,8 @@ 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);
               }
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..600a644 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,15 +2172,21 @@ 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');
                 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 {
@@ -2526,6 +2545,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]
  Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
  In-Reply-To: <CAOBg0AOXBxobGxs07j9OO71p9v3gM3YYdw5yRJOkMry++DVcaA@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