From 5887fb38153e94307dfc8be47085353b9b9acf81 Mon Sep 17 00:00:00 2001 From: Yosry Muhammad Date: Fri, 23 Aug 2019 16:15:43 +0100 Subject: [PATCH] Some refactoring of sqleditor.js. --- .../static/js/sqleditor/execute_query.js | 8 +- .../static/js/sqleditor/query_tool_actions.js | 13 +-- .../tools/sqleditor/static/js/sqleditor.js | 107 +++++++----------- .../sqleditor/execute_query_spec.js | 6 +- .../sqleditor/query_tool_actions_spec.js | 25 ++-- 5 files changed, 66 insertions(+), 93 deletions(-) diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js index 26da69148..b8baa410d 100644 --- a/web/pgadmin/static/js/sqleditor/execute_query.js +++ b/web/pgadmin/static/js/sqleditor/execute_query.js @@ -224,22 +224,22 @@ class ExecuteQuery { } if (this.userManagement.isPgaLoginRequired(httpMessage.response)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.userManagement.pgaLogin(); } if (httpErrorHandler.httpResponseRequiresNewTransaction(httpMessage.response)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.initTransaction(); } if (this.wasDatabaseConnectionLost(httpMessage)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.handle_connection_lost(false, httpMessage); } if(this.isCryptKeyMissing(httpMessage)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.handle_cryptkey_missing(); return; } 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 0f54aa5cc..4433019c3 100644 --- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js @@ -2272,7 +2272,7 @@ define('tools.querytool', [ } self.disable_tool_buttons(true); pgBrowser.Events.on('pgadmin:query_tool:connected:'+ transId,()=>{ - self.execute_data_query(); + self.check_data_changes_to_execute_query(); }); } }, @@ -2314,9 +2314,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 @@ -2328,8 +2327,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 @@ -2340,12 +2344,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, @@ -2422,12 +2431,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 @@ -3760,60 +3793,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/execute_query_spec.js b/web/regression/javascript/sqleditor/execute_query_spec.js index 809b938b5..9963997f6 100644 --- a/web/regression/javascript/sqleditor/execute_query_spec.js +++ b/web/regression/javascript/sqleditor/execute_query_spec.js @@ -1449,7 +1449,7 @@ describe('ExecuteQuery', () => { it('should save the state', () => { setTimeout(() => { expect(sqlEditorMock.saveState).toHaveBeenCalledWith( - 'execute', + 'check_data_changes_to_execute_query', [''] ); }, 0); @@ -1574,7 +1574,7 @@ describe('ExecuteQuery', () => { it('should save the state', () => { setTimeout(() => { expect(sqlEditorMock.saveState).toHaveBeenCalledWith( - 'execute', + 'check_data_changes_to_execute_query', [''] ); }, 0); @@ -1604,7 +1604,7 @@ describe('ExecuteQuery', () => { it('saves state', () => { setTimeout(() => { expect(sqlEditorMock.saveState).toHaveBeenCalledWith( - 'execute', + 'check_data_changes_to_execute_query', [''] ); }, 0); diff --git a/web/regression/javascript/sqleditor/query_tool_actions_spec.js b/web/regression/javascript/sqleditor/query_tool_actions_spec.js index e5e486572..487bbb4eb 100644 --- a/web/regression/javascript/sqleditor/query_tool_actions_spec.js +++ b/web/regression/javascript/sqleditor/query_tool_actions_spec.js @@ -30,7 +30,7 @@ describe('queryToolActions', () => { it('calls the execute function on the sqlEditorController', () => { queryToolActions.executeQuery(sqlEditorController); - expect(sqlEditorController.execute).toHaveBeenCalled(); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalled(); }); }); describe('when the command is being run from the view data view', () => { @@ -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(); }); }); }); @@ -74,7 +74,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -100,7 +100,7 @@ describe('queryToolActions', () => { summary: true, settings: true, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -128,7 +128,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -156,7 +156,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -184,7 +184,7 @@ describe('queryToolActions', () => { settings: true, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); }); @@ -213,7 +213,7 @@ describe('queryToolActions', () => { summary: false, settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -239,7 +239,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -264,7 +264,7 @@ describe('queryToolActions', () => { summary: false, settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); }); @@ -621,8 +621,7 @@ describe('queryToolActions', () => { trigger: jasmine.createSpy('trigger'), 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'), }; } });