Query Tool JS Minimal Refactoring

Yosry Muhammad <[email protected]>
Newsgroups gmane.comp.db.postgresql.pgadmin.devel
Message-ID <CAFSMqn8DnxHf7LPLmQOX+8Z6Tivbe59f9MqtYKU2W_JE3T61SA@mail.gmail.com>
Hi hackers,

Please find attached a patch with minimal refactoring of:
web/pgadmin/tools/sqleditor/static/js/sqleditor.js

This includes merging 2 redundant functions into one and renaming some
functions to have more expressive and consistent names.

Please review !
Thanks.
-- 
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/
sqleditorjs_refactor.patch (text/x-patch, 8.8 KB)
diff --git a/web/pgadmin/static/js/sqleditor/query_tool_actions.js b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
index 18e15ecbe..34b6827ad 100644
--- a/web/pgadmin/static/js/sqleditor/query_tool_actions.js
+++ b/web/pgadmin/static/js/sqleditor/query_tool_actions.js
@@ -39,13 +39,8 @@ let queryToolActions = {
   },
 
   executeQuery: function (sqlEditorController) {
-    if(sqlEditorController.is_query_tool) {
-      this._clearMessageTab();
-      sqlEditorController.execute();
-    } else {
-      this._clearMessageTab();
-      sqlEditorController.execute_data_query();
-    }
+    this._clearMessageTab();
+    sqlEditorController.check_data_changes_to_execute_query();
   },
 
   explainAnalyze: function (sqlEditorController) {
@@ -60,7 +55,7 @@ let queryToolActions = {
       settings: this._settings(),
     };
     this._clearMessageTab();
-    sqlEditorController.execute(explainObject);
+    sqlEditorController.check_data_changes_to_execute_query(explainObject);
   },
 
   explain: function (sqlEditorController) {
@@ -76,7 +71,7 @@ let queryToolActions = {
       settings: this._settings(),
     };
     this._clearMessageTab();
-    sqlEditorController.execute(explainObject);
+    sqlEditorController.check_data_changes_to_execute_query(explainObject);
   },
 
   download: function (sqlEditorController) {
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 4f387a5ce..669791190 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -2224,7 +2224,7 @@ define('tools.querytool', [
             cm.className += ' bg-gray-lighter opacity-5 hide-cursor-workaround';
           }
           self.disable_tool_buttons(true);
-          self.execute_data_query();
+          self.check_data_changes_to_execute_query();
         }
       },
 
@@ -2265,9 +2265,8 @@ define('tools.querytool', [
         self.on('pgadmin-sqleditor:unindent_selected_code', self._unindent_selected_code, self);
       },
 
-      // Checks if there is any dirty data in the grid before
-      // it executes the sql query in View Data mode
-      execute_data_query: function() {
+      // Checks if there is any dirty data in the grid before executing a query
+      check_data_changes_to_execute_query: function(explain_prefix=null, shouldReconnect=false) {
         var self = this;
 
         // Check if the data grid has any changes before running query
@@ -2279,8 +2278,13 @@ define('tools.querytool', [
           alertify.confirm(gettext('Unsaved changes'),
             gettext('The data has been modified, but not saved. Are you sure you wish to discard the changes?'),
             function() {
-              // Do nothing as user do not want to save, just continue
-              self._run_query();
+              // The user does not want to save, just continue
+              if(self.is_query_tool) {
+                self._execute_sql_query(explain_prefix, shouldReconnect);
+              }
+              else {
+                self._execute_view_data_query();
+              }
             },
             function() {
               // Stop, User wants to save
@@ -2291,12 +2295,17 @@ define('tools.querytool', [
             cancel: gettext('No'),
           });
         } else {
-          self._run_query();
+          if(self.is_query_tool) {
+            self._execute_sql_query(explain_prefix, shouldReconnect);
+          }
+          else {
+            self._execute_view_data_query();
+          }
         }
       },
 
-      // This function makes the ajax call to execute the sql query in View Data mode
-      _run_query: function() {
+      // Makes the ajax call to execute the sql query in View Data mode
+      _execute_view_data_query: function() {
         var self = this,
           url = url_for('sqleditor.view_data_start', {
             'trans_id': self.transId,
@@ -2373,12 +2382,36 @@ define('tools.querytool', [
           .fail(function(e) {
             self.trigger('pgadmin-sqleditor:loading-icon:hide');
             let msg = httpErrorHandler.handleQueryToolAjaxError(
-              pgAdmin, self, e, '_run_query', [], true
+              pgAdmin, self, e, '_execute_view_data_query', [], true
             );
             self.update_msg_history(false, msg);
           });
       },
 
+      // Executes sql query in the editor in Query Tool mode
+      _execute_sql_query: function(explain_prefix, shouldReconnect) {
+        var self = this, sql = '';
+
+        self.has_more_rows = false;
+        self.fetching_rows = false;
+
+        if (!_.isUndefined(self.special_sql)) {
+          sql = self.special_sql;
+        } else {
+          /* If code is selected in the code mirror then execute
+           * the selected part else execute the complete code.
+           */
+          var selected_code = self.gridView.query_tool_obj.getSelection();
+          if (selected_code.length > 0)
+            sql = selected_code;
+          else
+            sql = self.gridView.query_tool_obj.getValue();
+        }
+
+        const executeQuery = new ExecuteQuery.ExecuteQuery(this, pgAdmin.Browser.UserManagement);
+        executeQuery.execute(sql, explain_prefix, shouldReconnect);
+      },
+
       // This is a wrapper to call_render function
       // We need this because we have separated columns route & result route
       // We need to combine both result here in wrapper before rendering grid
@@ -3713,60 +3746,6 @@ define('tools.querytool', [
         }
       },
 
-      // Checks if there is any dirty data in the grid before
-      // it executes the sql query in Query Tool mode
-      execute: function(explain_prefix, shouldReconnect=false) {
-        var self = this;
-
-        // Check if the data grid has any changes before running query
-        if (self.can_edit && _.has(self, 'data_store') &&
-          (_.size(self.data_store.added) ||
-            _.size(self.data_store.updated) ||
-            _.size(self.data_store.deleted))
-        ) {
-          alertify.confirm(gettext('Unsaved changes'),
-            gettext('The data has been modified, but not saved. Are you sure you wish to discard the changes?'),
-            function() {
-              // Do nothing as user do not want to save, just continue
-              self._execute_sql_query(explain_prefix, shouldReconnect);
-            },
-            function() {
-              // Stop, User wants to save
-              return true;
-            }
-          ).set('labels', {
-            ok: gettext('Yes'),
-            cancel: gettext('No'),
-          });
-        } else {
-          self._execute_sql_query(explain_prefix, shouldReconnect);
-        }
-      },
-
-      // Executes sql query in the editor in Query Tool mode
-      _execute_sql_query: function(explain_prefix, shouldReconnect) {
-        var self = this, sql = '';
-
-        self.has_more_rows = false;
-        self.fetching_rows = false;
-
-        if (!_.isUndefined(self.special_sql)) {
-          sql = self.special_sql;
-        } else {
-          /* If code is selected in the code mirror then execute
-           * the selected part else execute the complete code.
-           */
-          var selected_code = self.gridView.query_tool_obj.getSelection();
-          if (selected_code.length > 0)
-            sql = selected_code;
-          else
-            sql = self.gridView.query_tool_obj.getValue();
-        }
-
-        const executeQuery = new ExecuteQuery.ExecuteQuery(this, pgAdmin.Browser.UserManagement);
-        executeQuery.execute(sql, explain_prefix, shouldReconnect);
-      },
-
       /* This function is used to highlight the error line and
        * underlining for the error word.
        */
diff --git a/web/regression/javascript/sqleditor/query_tool_actions_spec.js b/web/regression/javascript/sqleditor/query_tool_actions_spec.js
index e5e486572..23cea7dc1 100644
--- a/web/regression/javascript/sqleditor/query_tool_actions_spec.js
+++ b/web/regression/javascript/sqleditor/query_tool_actions_spec.js
@@ -39,10 +39,10 @@ describe('queryToolActions', () => {
         sqlEditorController.is_query_tool = false;
       });
 
-      it('it calls the execute_data_query function on the sqlEditorController', () => {
+      it('it calls the check_data_changes_to_execute_query function on the sqlEditorController', () => {
         queryToolActions.executeQuery(sqlEditorController);
 
-        expect(sqlEditorController.execute_data_query).toHaveBeenCalled();
+        expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalled();
       });
     });
   });
@@ -622,7 +622,7 @@ describe('queryToolActions', () => {
       table_name: 'iAmATable',
       is_query_tool: true,
       execute: jasmine.createSpy('execute'),
-      execute_data_query: jasmine.createSpy('execute_data_query'),
+      check_data_changes_to_execute_query: jasmine.createSpy('check_data_changes_to_execute_query'),
     };
   }
 });
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.