public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin4][RM#3235] Code refactoring in Query tool 6+ messages / 3 participants [nested] [flat]
* [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-03 11:27 Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Murtuza Zabuawala @ 2018-04-03 11:27 UTC (permalink / raw) To: pgadmin-hackers Hi, PFA patch to extract the common code from query tool to handle ajax errors & connection handling, Also added unit tests around extracted code. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Attachments: [application/octet-stream] RM_3235.diff (35.2K, 3-RM_3235.diff) download | inline diff: diff --git a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js b/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js index e009611..f5bd2a9 100644 --- a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js +++ b/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js @@ -6,6 +6,7 @@ // This software is released under the PostgreSQL Licence // ////////////////////////////////////////////////////////////////////////// +import gettext from 'sources/gettext'; export function is_new_transaction_required(xhr) { /* If responseJSON is undefined then it could be object of @@ -21,3 +22,55 @@ export function is_new_transaction_required(xhr) { xhr.responseJSON.info && xhr.responseJSON.info === 'DATAGRID_TRANSACTION_REQUIRED'; } + +// Allow us to redirect to login dialog and if required then re-initiate the transaction +export function handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + stateParameters = stateParameters && stateParameters.length > 0 ? stateParameters : []; + if (pgAdmin.Browser.UserManagement.is_pga_login_required(exception)) { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + return pgAdmin.Browser.UserManagement.pga_login(); + } + + if(checkTransaction && is_new_transaction_required(exception)) { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + return handler.init_transaction(); + } +} + +// Allow us to handle the AJAX error from Query tool +export function handleQueryToolAjaxError( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + if (exception.readyState == 0) { + return gettext('Not connected to the server or the connection to the server has been closed.'); + } + + handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction + ); + + let msg = exception.responseText; + if (exception.responseJSON != undefined) { + if(exception.responseJSON.errormsg != undefined) { + msg = exception.responseJSON.errormsg; + } + + if(exception.status == 503 && exception.responseJSON.info != undefined && + exception.responseJSON.info == 'CONNECTION_LOST') { + setTimeout(function() { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + handler.handle_connection_lost(false, exception); + }); + } + } + + return msg; +} diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js index 923ccea..8bfffac 100644 --- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js @@ -482,12 +482,9 @@ define('tools.querytool', [ }); }, error:function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } - if(transaction.is_new_transaction_required(e)) { - return self.init_transaction(); - } + return transaction.handleLoginRequiredAndTransactionRequired( + pgAdmin, self, e, null, null, null + ); }, }); }.bind(ctx), @@ -1138,16 +1135,11 @@ define('tools.querytool', [ if (typeof cb == 'function') { cb(); } - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - pgAdmin.Browser.UserManagement.pga_login(); - } + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, null, null, false + ); + self.update_msg_history(false, msg); }, }); }, @@ -1931,18 +1923,12 @@ define('tools.querytool', [ } }) .fail(function(xhr) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(xhr)) { - pgAdmin.Browser.UserManagement.pga_login(); - } else { - if(xhr.responseJSON && - xhr.responseJSON.result) { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseJSON.result); - } else { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseText); - } - } + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, xhr, null, null, false + ); + alertify.dlgGetServerPass( + gettext('Connect to Server'), msg + ); }); }, /* This function is used to create instance of SQLEditorView, @@ -2001,22 +1987,13 @@ define('tools.querytool', [ self.init_events(); }, error: function(jqx) { - var msg = ''; + let msg = ''; self.init_events(); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(jqx)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } + msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, jqx, null, null, false + ); - /* Error from the server */ - if (jqx.status == 410 || jqx.status == 500) { - try { - var data = $.parseJSON(jqx.responseText); - msg = data.errormsg; - } catch (e) { - msg = jqx.responseText; - } - } pgBrowser.report_error( S(gettext('Error fetching SQL for script: %s.')).sprintf(msg).value() ); @@ -2185,37 +2162,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_run_query', []); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_run_query', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_run_query', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_run_query', null, true + ); self.update_msg_history(false, msg); }, }); @@ -2871,38 +2820,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save', [view, controller, save_as]); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_save', [view, controller, save_as]); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_save', [view, controller, save_as]); - self.handle_connection_lost(false, e); - }); - } - } - + let stateParams = [view, controller, save_as]; + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_save', stateParams, true + ); self.update_msg_history(false, msg); }, }); @@ -3048,13 +2969,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_select_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - alertify.error(errmsg); + let stateParams = [_e]; + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_select_file_handler', stateParams, false + ); + alertify.error(msg); // hide cursor $busy_icon_div.removeClass('show_progress'); }, @@ -3100,17 +3019,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - setTimeout( - function() { - alertify.error(errmsg); - }, 10 + let stateParams = [_e]; + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_save_file_handler', stateParams, false ); + alertify.error(msg); }, }); }, @@ -3206,36 +3119,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_show_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_show_filter', []); - return self.init_transaction(); - } - - var msg; - if (e.readyState == 0) { - msg = - gettext('Not connected to the server or the connection to the server has been closed.'); - } else { - msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_show_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - } + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_show_filter', null, true + ); setTimeout( function() { alertify.alert(gettext('Get Filter Error'), msg); @@ -3296,42 +3182,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_include_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_include_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter By Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_include_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter By Selection Error'), msg); - }, 10 + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_include_filter', null, true ); + alertify.alert(gettext('Filter By Selection Error'), msg); }, }); }, @@ -3387,42 +3241,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_exclude_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_exclude_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter Exclude Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_exclude_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter Exclude Selection Error'), msg); - }, 10 + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_exclude_filter', null, true ); + alertify.alert(gettext('Filter Exclude Selection Error'), msg); }, }); }, @@ -3457,42 +3279,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_remove_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_remove_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Remove Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_remove_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Remove Filter Error'), msg); - } + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_remove_filter', null, true ); + alertify.alert(gettext('Remove Filter Error'), msg); }, }); }, @@ -3532,42 +3322,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_apply_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_apply_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Apply Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_apply_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Apply Filter Error'), msg); - }, 10 + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_apply_filter', null, true ); + alertify.alert(gettext('Apply Filter Error'), msg); }, }); }, @@ -3687,42 +3445,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_set_limit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_set_limit', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Change limit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_set_limit', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Change limit Error'), msg); - }, 10 + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_set_limit', null, true ); + alertify.alert(gettext('Change limit Error'), msg); }, }); }, @@ -3846,23 +3572,9 @@ define('tools.querytool', [ error: function(e) { self.disable_tool_buttons(false); - if (e.readyState == 0) { - alertify.alert(gettext('Cancel Query Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_cancel_query', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined && - e.responseJSON.errormsg != undefined) - msg = e.responseJSON.errormsg; - + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_cancel_query', null, false + ); alertify.alert(gettext('Cancel Query Error'), msg); }, }); @@ -3907,38 +3619,10 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Rollback Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Rollback Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_rollback', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_rollback', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_rollback', []); - self.handle_connection_lost(false, e); - }); - } - } + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_rollback', null, true + ); alertify.alert(gettext('Auto Rollback Error'), msg); }, }); @@ -3968,38 +3652,9 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Commit Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Commit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_commit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_commit', []); - return self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_commit', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_commit', null, true + ); alertify.alert(gettext('Auto Commit Error'), msg); }, }); @@ -4036,21 +3691,10 @@ define('tools.querytool', [ } }, error: function(e) { - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_verbose', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_verbose', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting verbose option in explain.') + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_verbose', null, true ); - return; + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4086,19 +3730,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_costs', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_costs', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting costs option in explain.') + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_costs', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4134,19 +3769,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_buffers', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_buffers', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting buffers option in explain.') + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_buffers', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4181,19 +3807,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_timing', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_timing', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting timing option in explain.') + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_timing', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4295,21 +3912,11 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('get_preferences', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('get_preferences', []); - return self.init_transaction(); - } - - updateUI(); - alertify.alert( - gettext('Get Preferences error'), - gettext('Error occurred while getting query tool options.') + let msg = transaction.handleQueryToolAjaxError( + pgAdmin, self, e, 'get_preferences', null, true ); + updateUI(); + alertify.alert(gettext('Get Preferences error'), msg); }, }); }, diff --git a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js b/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js index 97d1bc5..2fb1f45 100644 --- a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js +++ b/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js @@ -7,7 +7,10 @@ // ////////////////////////////////////////////////////////////////////////// -import {is_new_transaction_required} from '../../../pgadmin/static/js/sqleditor/is_new_transaction_required'; +import { + is_new_transaction_required, + handleQueryToolAjaxError +} from '../../../pgadmin/static/js/sqleditor/is_new_transaction_required'; describe('#is_new_transaction_required', () => { describe('when status is not 404', () => { @@ -63,3 +66,126 @@ describe('#is_new_transaction_required', () => { }); }); }); + + +describe('#handleQueryToolAjaxError', () => { + let sqlEditorHandler, + exceptionSpy, stateToSave, + stateParameters, checkTransaction, UserManagementMock, + pgBrowserMock; + + beforeEach(() => { + stateToSave = 'testState'; + stateParameters = []; + checkTransaction = false; + sqlEditorHandler = jasmine.createSpyObj( + 'handler', ['init_transaction', 'save_state', 'handle_connection_lost'] + ); + exceptionSpy = { + readyState: 0, + status: 404, + data: { + info: 'CONNECTION_LOST', + }, + }; + pgBrowserMock = { + 'Browser': { + 'UserManagement': jasmine.createSpyObj('UserManagement', ['is_pga_login_required', 'pga_login']) + } + }; + }); + + describe('when ready state is 0', () => { + it('should return connection', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('Not connected to the server or the connection to the server has been closed.'); + }); + }); + + describe('when there is an ajax error due to login is required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 401; + exceptionSpy.data.info = 'PGADMIN_LOGIN_REQUIRED'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(true); + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(pgBrowserMock.Browser.UserManagement.pga_login).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error and new transaction initialization required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 404; + exceptionSpy.data.info = 'DATAGRID_TRANSACTION_REQUIRED'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = true; + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pga_login).not.toHaveBeenCalled(); + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.init_transaction).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error due to database connection has been lost', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 503; + exceptionSpy.responseJSON = { + 'info': 'CONNECTION_LOST' + }; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = false; + }); + + it('should save the current state and call connection lost handler', (done) => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pga_login).not.toHaveBeenCalled(); + setTimeout(() => { + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.handle_connection_lost).toHaveBeenCalledWith(false, exceptionSpy); + done(); + }, 0); + }); + }); + + describe('when there is an ajax error due to unknown reason', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 803; + exceptionSpy.responseText = 'ajax failed with unknown reason'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = false; + }); + + it('should return proper error message from ajax exception', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('ajax failed with unknown reason'); + }); + }); + +}); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-03 14:13 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Dave Page @ 2018-04-03 14:13 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers HI On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala < [email protected]> wrote: > Hi, > > PFA patch to extract the common code from query tool to handle ajax errors > & connection handling, Also added unit tests around extracted code. > Looks good to me, except, I wonder if we should rename is_new_transaction_required.js/is_new_transaction_required_spec.js to something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I like that though. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-03 15:38 Murtuza Zabuawala <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Murtuza Zabuawala @ 2018-04-03 15:38 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers Hi Dave, PFA updated patch, I've renamed it to query_tool_http_error_handler.js & query_tool_http_error_handler_spec.js respectively. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Tue, Apr 3, 2018 at 7:43 PM, Dave Page <[email protected]> wrote: > HI > > On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> Hi, >> >> PFA patch to extract the common code from query tool to handle ajax >> errors & connection handling, Also added unit tests around extracted code. >> > > Looks good to me, except, I wonder if we should rename > is_new_transaction_required.js/is_new_transaction_required_spec.js to > something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I > like that though. > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > Attachments: [application/octet-stream] RM_3235_v1.diff (43.6K, 3-RM_3235_v1.diff) download | inline diff: diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js index 9c36f28..8e201fc 100644 --- a/web/pgadmin/static/js/sqleditor/execute_query.js +++ b/web/pgadmin/static/js/sqleditor/execute_query.js @@ -11,7 +11,7 @@ import gettext from '../gettext'; import $ from 'jquery'; import url_for from '../url_for'; import axios from 'axios'; -import * as transaction from './is_new_transaction_required'; +import * as httpErrorHandler from './query_tool_http_error_handler'; class LoadingScreen { constructor(sqlEditor) { @@ -203,7 +203,7 @@ class ExecuteQuery { this.userManagement.pga_login(); } - if (transaction.is_new_transaction_required(httpMessage.response)) { + if (httpErrorHandler.is_new_transaction_required(httpMessage.response)) { this.sqlServerObject.save_state('execute', [this.explainPlan]); this.sqlServerObject.init_transaction(); } diff --git a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js b/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js deleted file mode 100644 index e009611..0000000 --- a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js +++ /dev/null @@ -1,23 +0,0 @@ -////////////////////////////////////////////////////////////////////////// -// -// pgAdmin 4 - PostgreSQL Tools -// -// Copyright (C) 2013 - 2018, The pgAdmin Development Team -// This software is released under the PostgreSQL Licence -// -////////////////////////////////////////////////////////////////////////// - -export function is_new_transaction_required(xhr) { - /* If responseJSON is undefined then it could be object of - * axios(Promise HTTP) response, so we should check accordingly. - */ - if (xhr.responseJSON === undefined && xhr.data !== undefined) { - return xhr.status === 404 && xhr.data && - xhr.data.info && - xhr.data.info === 'DATAGRID_TRANSACTION_REQUIRED'; - } - - return xhr.status === 404 && xhr.responseJSON && - xhr.responseJSON.info && - xhr.responseJSON.info === 'DATAGRID_TRANSACTION_REQUIRED'; -} diff --git a/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js b/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js new file mode 100644 index 0000000..f5bd2a9 --- /dev/null +++ b/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js @@ -0,0 +1,76 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2018, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// +import gettext from 'sources/gettext'; + +export function is_new_transaction_required(xhr) { + /* If responseJSON is undefined then it could be object of + * axios(Promise HTTP) response, so we should check accordingly. + */ + if (xhr.responseJSON === undefined && xhr.data !== undefined) { + return xhr.status === 404 && xhr.data && + xhr.data.info && + xhr.data.info === 'DATAGRID_TRANSACTION_REQUIRED'; + } + + return xhr.status === 404 && xhr.responseJSON && + xhr.responseJSON.info && + xhr.responseJSON.info === 'DATAGRID_TRANSACTION_REQUIRED'; +} + +// Allow us to redirect to login dialog and if required then re-initiate the transaction +export function handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + stateParameters = stateParameters && stateParameters.length > 0 ? stateParameters : []; + if (pgAdmin.Browser.UserManagement.is_pga_login_required(exception)) { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + return pgAdmin.Browser.UserManagement.pga_login(); + } + + if(checkTransaction && is_new_transaction_required(exception)) { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + return handler.init_transaction(); + } +} + +// Allow us to handle the AJAX error from Query tool +export function handleQueryToolAjaxError( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + if (exception.readyState == 0) { + return gettext('Not connected to the server or the connection to the server has been closed.'); + } + + handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction + ); + + let msg = exception.responseText; + if (exception.responseJSON != undefined) { + if(exception.responseJSON.errormsg != undefined) { + msg = exception.responseJSON.errormsg; + } + + if(exception.status == 503 && exception.responseJSON.info != undefined && + exception.responseJSON.info == 'CONNECTION_LOST') { + setTimeout(function() { + if (stateToSave) { + handler.save_state(stateToSave, stateParameters); + } + handler.handle_connection_lost(false, exception); + }); + } + } + + return msg; +} diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js index 0409a87..2c01075 100644 --- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js @@ -13,7 +13,7 @@ define('tools.querytool', [ 'sources/selection/set_staged_rows', 'sources/sqleditor_utils', 'sources/sqleditor/execute_query', - 'sources/sqleditor/is_new_transaction_required', + 'sources/sqleditor/query_tool_http_error_handler', 'sources/history/index.js', 'sources/../jsx/history/query_history', 'react', 'react-dom', @@ -33,7 +33,7 @@ define('tools.querytool', [ ], function( babelPollyfill, gettext, url_for, $, _, S, alertify, pgAdmin, Backbone, codemirror, pgExplain, GridSelector, ActiveCellCapture, clipboard, copyData, RangeSelectionHelper, handleQueryOutputKeyboardEvent, - XCellSelectionModel, setStagedRows, SqlEditorUtils, ExecuteQuery, transaction, + XCellSelectionModel, setStagedRows, SqlEditorUtils, ExecuteQuery, httpErrorHandler, HistoryBundle, queryHistory, React, ReactDOM, keyboardShortcuts, queryToolActions, Datagrid, modifyAnimation, calculateQueryRunTime, callRenderAfterPoll) { @@ -486,12 +486,9 @@ define('tools.querytool', [ }); }, error:function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } - if(transaction.is_new_transaction_required(e)) { - return self.init_transaction(); - } + return httpErrorHandler.handleLoginRequiredAndTransactionRequired( + pgAdmin, self, e, null, null, null + ); }, }); }.bind(ctx), @@ -1142,16 +1139,11 @@ define('tools.querytool', [ if (typeof cb == 'function') { cb(); } - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - pgAdmin.Browser.UserManagement.pga_login(); - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, null, null, false + ); + self.update_msg_history(false, msg); }, }); }, @@ -1935,18 +1927,12 @@ define('tools.querytool', [ } }) .fail(function(xhr) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(xhr)) { - pgAdmin.Browser.UserManagement.pga_login(); - } else { - if(xhr.responseJSON && - xhr.responseJSON.result) { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseJSON.result); - } else { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseText); - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, xhr, null, null, false + ); + alertify.dlgGetServerPass( + gettext('Connect to Server'), msg + ); }); }, /* This function is used to create instance of SQLEditorView, @@ -2005,22 +1991,13 @@ define('tools.querytool', [ self.init_events(); }, error: function(jqx) { - var msg = ''; + let msg = ''; self.init_events(); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(jqx)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } + msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, jqx, null, null, false + ); - /* Error from the server */ - if (jqx.status == 410 || jqx.status == 500) { - try { - var data = $.parseJSON(jqx.responseText); - msg = data.errormsg; - } catch (e) { - msg = jqx.responseText; - } - } pgBrowser.report_error( S(gettext('Error fetching SQL for script: %s.')).sprintf(msg).value() ); @@ -2189,37 +2166,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_run_query', []); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_run_query', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_run_query', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_run_query', null, true + ); self.update_msg_history(false, msg); }, }); @@ -2830,38 +2779,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save', [view, controller, save_as]); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_save', [view, controller, save_as]); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_save', [view, controller, save_as]); - self.handle_connection_lost(false, e); - }); - } - } - + let stateParams = [view, controller, save_as]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_save', stateParams, true + ); self.update_msg_history(false, msg); }, }); @@ -3007,13 +2928,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_select_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - alertify.error(errmsg); + let stateParams = [_e]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_select_file_handler', stateParams, false + ); + alertify.error(msg); // hide cursor $busy_icon_div.removeClass('show_progress'); }, @@ -3059,17 +2978,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - setTimeout( - function() { - alertify.error(errmsg); - }, 10 + let stateParams = [_e]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_save_file_handler', stateParams, false ); + alertify.error(msg); }, }); }, @@ -3165,36 +3078,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_show_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_show_filter', []); - return self.init_transaction(); - } - - var msg; - if (e.readyState == 0) { - msg = - gettext('Not connected to the server or the connection to the server has been closed.'); - } else { - msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_show_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_show_filter', null, true + ); setTimeout( function() { alertify.alert(gettext('Get Filter Error'), msg); @@ -3255,42 +3141,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_include_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_include_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter By Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_include_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter By Selection Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_include_filter', null, true ); + alertify.alert(gettext('Filter By Selection Error'), msg); }, }); }, @@ -3346,42 +3200,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_exclude_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_exclude_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter Exclude Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_exclude_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter Exclude Selection Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_exclude_filter', null, true ); + alertify.alert(gettext('Filter Exclude Selection Error'), msg); }, }); }, @@ -3416,42 +3238,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_remove_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_remove_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Remove Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_remove_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Remove Filter Error'), msg); - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_remove_filter', null, true ); + alertify.alert(gettext('Remove Filter Error'), msg); }, }); }, @@ -3491,42 +3281,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_apply_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_apply_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Apply Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_apply_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Apply Filter Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_apply_filter', null, true ); + alertify.alert(gettext('Apply Filter Error'), msg); }, }); }, @@ -3646,42 +3404,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_set_limit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_set_limit', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Change limit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_set_limit', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Change limit Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_set_limit', null, true ); + alertify.alert(gettext('Change limit Error'), msg); }, }); }, @@ -3805,23 +3531,9 @@ define('tools.querytool', [ error: function(e) { self.disable_tool_buttons(false); - if (e.readyState == 0) { - alertify.alert(gettext('Cancel Query Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_cancel_query', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined && - e.responseJSON.errormsg != undefined) - msg = e.responseJSON.errormsg; - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_cancel_query', null, false + ); alertify.alert(gettext('Cancel Query Error'), msg); }, }); @@ -3866,38 +3578,10 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Rollback Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Rollback Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_rollback', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_rollback', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_rollback', []); - self.handle_connection_lost(false, e); - }); - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_rollback', null, true + ); alertify.alert(gettext('Auto Rollback Error'), msg); }, }); @@ -3927,38 +3611,9 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Commit Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Commit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_commit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_commit', []); - return self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_commit', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_commit', null, true + ); alertify.alert(gettext('Auto Commit Error'), msg); }, }); @@ -3995,21 +3650,10 @@ define('tools.querytool', [ } }, error: function(e) { - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_verbose', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_verbose', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting verbose option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_verbose', null, true ); - return; + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4045,19 +3689,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_costs', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_costs', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting costs option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_costs', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4093,19 +3728,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_buffers', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_buffers', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting buffers option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_buffers', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4140,19 +3766,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_timing', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_timing', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting timing option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_timing', null, true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4254,21 +3871,11 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('get_preferences', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('get_preferences', []); - return self.init_transaction(); - } - - updateUI(); - alertify.alert( - gettext('Get Preferences error'), - gettext('Error occurred while getting query tool options.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, 'get_preferences', null, true ); + updateUI(); + alertify.alert(gettext('Get Preferences error'), msg); }, }); }, diff --git a/web/regression/javascript/sqleditor/execute_query_spec.js b/web/regression/javascript/sqleditor/execute_query_spec.js index 5f92dc5..f7176d1 100644 --- a/web/regression/javascript/sqleditor/execute_query_spec.js +++ b/web/regression/javascript/sqleditor/execute_query_spec.js @@ -8,7 +8,7 @@ ////////////////////////////////////////////////////////////////////////// import * as subject from 'sources/sqleditor/execute_query'; -import * as transaction from 'sources/sqleditor/is_new_transaction_required'; +import * as httpErrorHandler from 'sources/sqleditor/query_tool_http_error_handler'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import $ from 'jquery'; @@ -47,7 +47,7 @@ describe('ExecuteQuery', () => { sqlEditorMock.transId = 123; sqlEditorMock.rows_affected = 1000; executeQuery = new subject.ExecuteQuery(sqlEditorMock, userManagementMock); - isNewTransactionRequiredMock = spyOn(transaction, 'is_new_transaction_required'); + isNewTransactionRequiredMock = spyOn(httpErrorHandler, 'is_new_transaction_required'); }); afterEach(() => { diff --git a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js b/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js deleted file mode 100644 index 97d1bc5..0000000 --- a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js +++ /dev/null @@ -1,65 +0,0 @@ -////////////////////////////////////////////////////////////////////////// -// -// pgAdmin 4 - PostgreSQL Tools -// -// Copyright (C) 2013 - 2018, The pgAdmin Development Team -// This software is released under the PostgreSQL Licence -// -////////////////////////////////////////////////////////////////////////// - -import {is_new_transaction_required} from '../../../pgadmin/static/js/sqleditor/is_new_transaction_required'; - -describe('#is_new_transaction_required', () => { - describe('when status is not 404', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 300, - })).toBe(false); - }); - }); - - describe('when status is 404', () => { - describe('when data is not present', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - })).toBeFalsy(); - }); - }); - - describe('when data is present', () => { - describe('when info is not present inside data', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: {}, - })).toBeFalsy(); - }); - }); - - describe('when info is present inside data', () => { - describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: { - info: 'some information', - }, - })).toBe(false); - }); - }); - - describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: { - info: 'DATAGRID_TRANSACTION_REQUIRED', - }, - })).toBe(true); - }); - }); - }); - }); - }); -}); diff --git a/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js b/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js new file mode 100644 index 0000000..fd3f982 --- /dev/null +++ b/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js @@ -0,0 +1,191 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2018, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// + +import { + is_new_transaction_required, + handleQueryToolAjaxError +} from '../../../pgadmin/static/js/sqleditor/query_tool_http_error_handler'; + +describe('#is_new_transaction_required', () => { + describe('when status is not 404', () => { + it('should return false', () => { + expect(is_new_transaction_required({ + status: 300, + })).toBe(false); + }); + }); + + describe('when status is 404', () => { + describe('when data is not present', () => { + it('should return false', () => { + expect(is_new_transaction_required({ + status: 404, + })).toBeFalsy(); + }); + }); + + describe('when data is present', () => { + describe('when info is not present inside data', () => { + it('should return false', () => { + expect(is_new_transaction_required({ + status: 404, + data: {}, + })).toBeFalsy(); + }); + }); + + describe('when info is present inside data', () => { + describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => { + it('should return false', () => { + expect(is_new_transaction_required({ + status: 404, + data: { + info: 'some information', + }, + })).toBe(false); + }); + }); + + describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => { + it('should return false', () => { + expect(is_new_transaction_required({ + status: 404, + data: { + info: 'DATAGRID_TRANSACTION_REQUIRED', + }, + })).toBe(true); + }); + }); + }); + }); + }); +}); + + +describe('#handleQueryToolAjaxError', () => { + let sqlEditorHandler, + exceptionSpy, stateToSave, + stateParameters, checkTransaction, UserManagementMock, + pgBrowserMock; + + beforeEach(() => { + stateToSave = 'testState'; + stateParameters = []; + checkTransaction = false; + sqlEditorHandler = jasmine.createSpyObj( + 'handler', ['init_transaction', 'save_state', 'handle_connection_lost'] + ); + exceptionSpy = { + readyState: 0, + status: 404, + data: { + info: 'CONNECTION_LOST', + }, + }; + pgBrowserMock = { + 'Browser': { + 'UserManagement': jasmine.createSpyObj('UserManagement', ['is_pga_login_required', 'pga_login']) + } + }; + }); + + describe('when ready state is 0', () => { + it('should return connection', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('Not connected to the server or the connection to the server has been closed.'); + }); + }); + + describe('when there is an ajax error due to login is required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 401; + exceptionSpy.data.info = 'PGADMIN_LOGIN_REQUIRED'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(true); + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(pgBrowserMock.Browser.UserManagement.pga_login).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error and new transaction initialization required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 404; + exceptionSpy.data.info = 'DATAGRID_TRANSACTION_REQUIRED'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = true; + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pga_login).not.toHaveBeenCalled(); + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.init_transaction).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error due to database connection has been lost', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 503; + exceptionSpy.responseJSON = { + 'info': 'CONNECTION_LOST' + }; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = false; + }); + + it('should save the current state and call connection lost handler', (done) => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pga_login).not.toHaveBeenCalled(); + setTimeout(() => { + expect(sqlEditorHandler.save_state).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.handle_connection_lost).toHaveBeenCalledWith(false, exceptionSpy); + done(); + }, 0); + }); + }); + + describe('when there is an ajax error due to unknown reason', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 803; + exceptionSpy.responseText = 'ajax failed with unknown reason'; + pgBrowserMock.Browser.UserManagement.is_pga_login_required.and.returnValue(false); + checkTransaction = false; + }); + + it('should return proper error message from ajax exception', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('ajax failed with unknown reason'); + }); + }); + +}); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-03 18:56 Joao De Almeida Pereira <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Joao De Almeida Pereira @ 2018-04-03 18:56 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers Hi Murtuza It is really good to see other patches that are just refactoring code. We have some suggestions: - We are trying to use === instead of == because === ensure that the type is also checked ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness ) - Now that we are refactoring some code, maybe we should keep some consistency on the way we name functions and variables. We should use camelCase for names instead of snake_case. In general, if you see a function or variable name that doesn't fit the desired syntax or if a block of code seems confusing, it is better to refactor it. - By the name of the function is_new_transaction_required, it describes what the function represents but doesn't seem to explain the full scope of the function. What do you think about the name: httpResponseRequiresNewTransaction - The extraction doesn't look like it matches the code removed - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_timing', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_timing', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting timing option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_timing', null, true ); + alertify.alert(gettext('Explain options error'), msg); In this case we are only saving state if the following conditions are true: isNotConnectedToThePythonServer and httpResponseJSONIsPresent and connectionLostToPostgresDatabase and shouldSaveState That is not the case on the removed code. - The functions extracted when are called do not use all the parameters. This tells us that the function groups too much functionality that is not used in same cases. Maybe we should split this function into smaller functions that do each part. Thanks Victoria & Joao On Tue, Apr 3, 2018 at 11:38 AM Murtuza Zabuawala < [email protected]> wrote: > Hi Dave, > > PFA updated patch, I've renamed it to query_tool_http_error_handler.js > & query_tool_http_error_handler_spec.js respectively. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > On Tue, Apr 3, 2018 at 7:43 PM, Dave Page <[email protected]> wrote: > >> HI >> >> On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala < >> [email protected]> wrote: >> >>> Hi, >>> >>> PFA patch to extract the common code from query tool to handle ajax >>> errors & connection handling, Also added unit tests around extracted code. >>> >> >> Looks good to me, except, I wonder if we should rename >> is_new_transaction_required.js/is_new_transaction_required_spec.js to >> something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I >> like that though. >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-04 08:43 Murtuza Zabuawala <[email protected]> parent: Joao De Almeida Pereira <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Murtuza Zabuawala @ 2018-04-04 08:43 UTC (permalink / raw) To: Joao De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers Hi, Thank you Victoria & Joao for reviewing. PFA updated patch. On Wed, Apr 4, 2018 at 12:26 AM, Joao De Almeida Pereira < [email protected]> wrote: > Hi Murtuza > > It is really good to see other patches that are just refactoring code. > > We have some suggestions: > - We are trying to use === instead of == because === ensure that the type > is also checked (https://developer.mozilla.org/en-US/docs/Web/JavaScript/ > Equality_comparisons_and_sameness) > Done - Now that we are refactoring some code, maybe we should keep some > consistency on the way we name functions and variables. > We should use camelCase for names instead of snake_case. In general, if you > see a function or variable name that doesn't fit the desired syntax or if a > block of code seems confusing, it is better to refactor it. > Done, I have also changed other variables. BTW I'm using camelCase convention from last few patches :) - By the name of the function is_new_transaction_required, it describes > what the function represents but doesn't seem to explain the full scope of > the function. What do you think about the name: httpResponseRequiresNewT > ransaction > Done - The extraction doesn't look like it matches the code removed > > - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) > { > - self.save_state('_explain_timing', []); > - return pgAdmin.Browser.UserManagement.pga_login(); > - } > - > - if(transaction.is_new_transaction_required(e)) { > - self.save_state('_explain_timing', []); > - return self.init_transaction(); > - } > - > - alertify.alert(gettext('Explain options error'), > - gettext('Error occurred while setting timing option in > explain.') > + let msg = httpErrorHandler.handleQueryToolAjaxError( > + pgAdmin, self, e, '_explain_timing', null, true > ); > + alertify.alert(gettext('Explain options error'), msg); > In this case we are only saving state if the following conditions are > true: > isNotConnectedToThePythonServer and httpResponseJSONIsPresent and > connectionLostToPostgresDatabase and shouldSaveState > That is not the case on the removed code. > I think the *null* value got your attention b ut actually I have a check in *handleQueryToolAjaxError *which will make it array [] with respect to arguments for the state to save, Anyways I have also changed it to pass [] instead of null for better clarity. We have all those checks in our function so it check for those condition and save the state only if those returns True. - The functions extracted when are called do not use all the parameters. > This tells us that the function groups too much functionality that is not > used in same cases. Maybe we should split this function into smaller > functions that do each part. > We already had split up the function in two part. > > > Thanks > Victoria & Joao > > On Tue, Apr 3, 2018 at 11:38 AM Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> Hi Dave, >> >> PFA updated patch, I've renamed it to query_tool_http_error_handler.js >> & query_tool_http_error_handler_spec.js respectively. >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> On Tue, Apr 3, 2018 at 7:43 PM, Dave Page <[email protected]> wrote: >> >>> HI >>> >>> On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala <murtuza.zabuawala@ >>> enterprisedb.com> wrote: >>> >>>> Hi, >>>> >>>> PFA patch to extract the common code from query tool to handle ajax >>>> errors & connection handling, Also added unit tests around extracted code. >>>> >>> >>> Looks good to me, except, I wonder if we should rename >>> is_new_transaction_required.js/is_new_transaction_required_spec.js to >>> something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I >>> like that though. >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> Attachments: [application/octet-stream] RM_3235_v2.diff (56.3K, 3-RM_3235_v2.diff) download | inline diff: diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js index 9c36f28..333a3a2 100644 --- a/web/pgadmin/static/js/sqleditor/execute_query.js +++ b/web/pgadmin/static/js/sqleditor/execute_query.js @@ -11,7 +11,7 @@ import gettext from '../gettext'; import $ from 'jquery'; import url_for from '../url_for'; import axios from 'axios'; -import * as transaction from './is_new_transaction_required'; +import * as httpErrorHandler from './query_tool_http_error_handler'; class LoadingScreen { constructor(sqlEditor) { @@ -152,8 +152,8 @@ class ExecuteQuery { const errorData = error.response.data; - if (self.userManagement.is_pga_login_required(errorData)) { - return self.userManagement.pga_login(); + if (self.userManagement.isPgaLoginRequired(errorData)) { + return self.userManagement.pgaLogin(); } let msg = ExecuteQuery.extractErrorMessage(errorData); @@ -198,18 +198,18 @@ class ExecuteQuery { return; } - if (this.userManagement.is_pga_login_required(httpMessage.response)) { - this.sqlServerObject.save_state('execute', [this.explainPlan]); - this.userManagement.pga_login(); + if (this.userManagement.isPgaLoginRequired(httpMessage.response)) { + this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.userManagement.pgaLogin(); } - if (transaction.is_new_transaction_required(httpMessage.response)) { - this.sqlServerObject.save_state('execute', [this.explainPlan]); - this.sqlServerObject.init_transaction(); + if (httpErrorHandler.httpResponseRequiresNewTransaction(httpMessage.response)) { + this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.initTransaction(); } if (this.wasDatabaseConnectionLost(httpMessage)) { - this.sqlServerObject.save_state('execute', [this.explainPlan]); + this.sqlServerObject.saveState('execute', [this.explainPlan]); this.sqlServerObject.handle_connection_lost(false, httpMessage); } diff --git a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js b/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js deleted file mode 100644 index e009611..0000000 --- a/web/pgadmin/static/js/sqleditor/is_new_transaction_required.js +++ /dev/null @@ -1,23 +0,0 @@ -////////////////////////////////////////////////////////////////////////// -// -// pgAdmin 4 - PostgreSQL Tools -// -// Copyright (C) 2013 - 2018, The pgAdmin Development Team -// This software is released under the PostgreSQL Licence -// -////////////////////////////////////////////////////////////////////////// - -export function is_new_transaction_required(xhr) { - /* If responseJSON is undefined then it could be object of - * axios(Promise HTTP) response, so we should check accordingly. - */ - if (xhr.responseJSON === undefined && xhr.data !== undefined) { - return xhr.status === 404 && xhr.data && - xhr.data.info && - xhr.data.info === 'DATAGRID_TRANSACTION_REQUIRED'; - } - - return xhr.status === 404 && xhr.responseJSON && - xhr.responseJSON.info && - xhr.responseJSON.info === 'DATAGRID_TRANSACTION_REQUIRED'; -} diff --git a/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js b/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js new file mode 100644 index 0000000..ae224ed --- /dev/null +++ b/web/pgadmin/static/js/sqleditor/query_tool_http_error_handler.js @@ -0,0 +1,76 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2018, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// +import gettext from 'sources/gettext'; + +export function httpResponseRequiresNewTransaction(xhr) { + /* If responseJSON is undefined then it could be object of + * axios(Promise HTTP) response, so we should check accordingly. + */ + if (xhr.responseJSON === undefined && xhr.data !== undefined) { + return xhr.status === 404 && xhr.data && + xhr.data.info && + xhr.data.info === 'DATAGRID_TRANSACTION_REQUIRED'; + } + + return xhr.status === 404 && xhr.responseJSON && + xhr.responseJSON.info && + xhr.responseJSON.info === 'DATAGRID_TRANSACTION_REQUIRED'; +} + +// Allow us to redirect to login dialog and if required then re-initiate the transaction +export function handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + stateParameters = stateParameters && stateParameters.length > 0 ? stateParameters : []; + if (pgAdmin.Browser.UserManagement.isPgaLoginRequired(exception)) { + if (stateToSave) { + handler.saveState(stateToSave, stateParameters); + } + return pgAdmin.Browser.UserManagement.pgaLogin(); + } + + if(checkTransaction && httpResponseRequiresNewTransaction(exception)) { + if (stateToSave) { + handler.saveState(stateToSave, stateParameters); + } + return handler.initTransaction(); + } +} + +// Allow us to handle the AJAX error from Query tool +export function handleQueryToolAjaxError( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction +) { + if (exception.readyState === 0) { + return gettext('Not connected to the server or the connection to the server has been closed.'); + } + + handleLoginRequiredAndTransactionRequired( + pgAdmin, handler, exception, stateToSave, stateParameters, checkTransaction + ); + + let msg = exception.responseText; + if (exception.responseJSON !== undefined) { + if(exception.responseJSON.errormsg !== undefined) { + msg = exception.responseJSON.errormsg; + } + + if(exception.status === 503 && exception.responseJSON.info !== undefined && + exception.responseJSON.info == 'CONNECTION_LOST') { + setTimeout(function() { + if (stateToSave) { + handler.saveState(stateToSave, stateParameters); + } + handler.handle_connection_lost(false, exception); + }); + } + } + + return msg; +} diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js index 0409a87..60dacbb 100644 --- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js @@ -13,7 +13,7 @@ define('tools.querytool', [ 'sources/selection/set_staged_rows', 'sources/sqleditor_utils', 'sources/sqleditor/execute_query', - 'sources/sqleditor/is_new_transaction_required', + 'sources/sqleditor/query_tool_http_error_handler', 'sources/history/index.js', 'sources/../jsx/history/query_history', 'react', 'react-dom', @@ -33,7 +33,7 @@ define('tools.querytool', [ ], function( babelPollyfill, gettext, url_for, $, _, S, alertify, pgAdmin, Backbone, codemirror, pgExplain, GridSelector, ActiveCellCapture, clipboard, copyData, RangeSelectionHelper, handleQueryOutputKeyboardEvent, - XCellSelectionModel, setStagedRows, SqlEditorUtils, ExecuteQuery, transaction, + XCellSelectionModel, setStagedRows, SqlEditorUtils, ExecuteQuery, httpErrorHandler, HistoryBundle, queryHistory, React, ReactDOM, keyboardShortcuts, queryToolActions, Datagrid, modifyAnimation, calculateQueryRunTime, callRenderAfterPoll) { @@ -486,12 +486,9 @@ define('tools.querytool', [ }); }, error:function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } - if(transaction.is_new_transaction_required(e)) { - return self.init_transaction(); - } + return httpErrorHandler.handleLoginRequiredAndTransactionRequired( + pgAdmin, self, e, null, [], false + ); }, }); }.bind(ctx), @@ -1142,16 +1139,11 @@ define('tools.querytool', [ if (typeof cb == 'function') { cb(); } - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - pgAdmin.Browser.UserManagement.pga_login(); - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, null, [], false + ); + self.update_msg_history(false, msg); }, }); }, @@ -1815,10 +1807,10 @@ define('tools.querytool', [ self.warn_before_continue(); }); pgBrowser.Events.on('pgadmin:user:logged-in', function() { - self.init_transaction(); + self.initTransaction(); }); }, - save_state: function(fn, args) { + saveState: function(fn, args) { if (fn) { this.state = { 'fn': fn, @@ -1829,7 +1821,7 @@ define('tools.querytool', [ } }, - init_transaction: function() { + initTransaction: function() { var url_endpoint; if (this.is_query_tool) { url_endpoint = 'datagrid.initialize_query_tool'; @@ -1898,7 +1890,7 @@ define('tools.querytool', [ if ('fn' in self.state) { var fn = self.state['fn'], args = self.state['args']; - self.save_state(); + self.saveState(); if (args.indexOf('connect') == -1) { args.push('connect'); } @@ -1906,7 +1898,7 @@ define('tools.querytool', [ self[fn].apply(self, args); } }, function() { - self.save_state(); + self.saveState(); }) .set({ labels: { @@ -1925,28 +1917,22 @@ define('tools.querytool', [ if (res.success == 1) { alertify.success(res.info); if (create_transaction) { - self.init_transaction(); + self.initTransaction(); } else if ('fn' in self.state) { var fn = self.state['fn'], args = self.state['args']; - self.save_state(); + self.saveState(); self[fn].apply(self, args); } } }) .fail(function(xhr) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(xhr)) { - pgAdmin.Browser.UserManagement.pga_login(); - } else { - if(xhr.responseJSON && - xhr.responseJSON.result) { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseJSON.result); - } else { - alertify.dlgGetServerPass(gettext('Connect to Server'), - xhr.responseText); - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, xhr, null, [], false + ); + alertify.dlgGetServerPass( + gettext('Connect to Server'), msg + ); }); }, /* This function is used to create instance of SQLEditorView, @@ -2005,22 +1991,13 @@ define('tools.querytool', [ self.init_events(); }, error: function(jqx) { - var msg = ''; + let msg = ''; self.init_events(); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(jqx)) { - return pgAdmin.Browser.UserManagement.pga_login(); - } + msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, jqx, null, [], false + ); - /* Error from the server */ - if (jqx.status == 410 || jqx.status == 500) { - try { - var data = $.parseJSON(jqx.responseText); - msg = data.errormsg; - } catch (e) { - msg = jqx.responseText; - } - } pgBrowser.report_error( S(gettext('Error fetching SQL for script: %s.')).sprintf(msg).value() ); @@ -2189,37 +2166,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_run_query', []); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_run_query', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_run_query', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_run_query', [], true + ); self.update_msg_history(false, msg); }, }); @@ -2830,38 +2779,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (e.readyState == 0) { - self.update_msg_history(false, - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save', [view, controller, save_as]); - pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_save', [view, controller, save_as]); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_save', [view, controller, save_as]); - self.handle_connection_lost(false, e); - }); - } - } - + let stateParams = [view, controller, save_as]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_save', stateParams, true + ); self.update_msg_history(false, msg); }, }); @@ -3007,13 +2928,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_select_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - alertify.error(errmsg); + let stateParams = [_e]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_select_file_handler', stateParams, false + ); + alertify.error(msg); // hide cursor $busy_icon_div.removeClass('show_progress'); }, @@ -3059,17 +2978,11 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_save_file_handler', [_e]); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var errmsg = $.parseJSON(e.responseText).errormsg; - setTimeout( - function() { - alertify.error(errmsg); - }, 10 + let stateParams = [_e]; + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_save_file_handler', stateParams, false ); + alertify.error(msg); }, }); }, @@ -3165,36 +3078,9 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_show_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_show_filter', []); - return self.init_transaction(); - } - - var msg; - if (e.readyState == 0) { - msg = - gettext('Not connected to the server or the connection to the server has been closed.'); - } else { - msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_show_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_show_filter', [], true + ); setTimeout( function() { alertify.alert(gettext('Get Filter Error'), msg); @@ -3255,42 +3141,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_include_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_include_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter By Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_include_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter By Selection Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_include_filter', [], true ); + alertify.alert(gettext('Filter By Selection Error'), msg); }, }); }, @@ -3346,42 +3200,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_exclude_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_exclude_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Filter Exclude Selection Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_exclude_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Filter Exclude Selection Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_exclude_filter', [], true ); + alertify.alert(gettext('Filter Exclude Selection Error'), msg); }, }); }, @@ -3416,42 +3238,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_remove_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_remove_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Remove Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_remove_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Remove Filter Error'), msg); - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_remove_filter', [], true ); + alertify.alert(gettext('Remove Filter Error'), msg); }, }); }, @@ -3491,42 +3281,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_apply_filter', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_apply_filter', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Apply Filter Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_apply_filter', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Apply Filter Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_apply_filter', [], true ); + alertify.alert(gettext('Apply Filter Error'), msg); }, }); }, @@ -3646,42 +3404,10 @@ define('tools.querytool', [ }, error: function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_set_limit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_set_limit', []); - return self.init_transaction(); - } - - setTimeout( - function() { - if (e.readyState == 0) { - alertify.alert(gettext('Change limit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_set_limit', []); - self.handle_connection_lost(false, e); - }); - } - } - - alertify.alert(gettext('Change limit Error'), msg); - }, 10 + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_set_limit', [], true ); + alertify.alert(gettext('Change limit Error'), msg); }, }); }, @@ -3805,23 +3531,9 @@ define('tools.querytool', [ error: function(e) { self.disable_tool_buttons(false); - if (e.readyState == 0) { - alertify.alert(gettext('Cancel Query Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_cancel_query', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined && - e.responseJSON.errormsg != undefined) - msg = e.responseJSON.errormsg; - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_cancel_query', [], false + ); alertify.alert(gettext('Cancel Query Error'), msg); }, }); @@ -3866,38 +3578,10 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Rollback Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Rollback Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_rollback', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_rollback', []); - self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_rollback', []); - self.handle_connection_lost(false, e); - }); - } - } + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_rollback', [], true + ); alertify.alert(gettext('Auto Rollback Error'), msg); }, }); @@ -3927,38 +3611,9 @@ define('tools.querytool', [ alertify.alert(gettext('Auto Commit Error'), res.data.result); }, error: function(e) { - if (e.readyState == 0) { - alertify.alert(gettext('Auto Commit Error'), - gettext('Not connected to the server or the connection to the server has been closed.') - ); - return; - } - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_auto_commit', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_auto_commit', []); - return self.init_transaction(); - } - - var msg = e.responseText; - if (e.responseJSON != undefined) { - if(e.responseJSON.errormsg != undefined) { - msg = e.responseJSON.errormsg; - } - - if(e.status == 503 && e.responseJSON.info != undefined && - e.responseJSON.info == 'CONNECTION_LOST') { - setTimeout(function() { - self.save_state('_auto_commit', []); - self.handle_connection_lost(false, e); - }); - } - } - + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_auto_commit', [], true + ); alertify.alert(gettext('Auto Commit Error'), msg); }, }); @@ -3995,21 +3650,10 @@ define('tools.querytool', [ } }, error: function(e) { - - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_verbose', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_verbose', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting verbose option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_verbose', [], true ); - return; + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4045,19 +3689,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_costs', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_costs', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting costs option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_costs', [], true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4093,19 +3728,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_buffers', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_buffers', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting buffers option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_buffers', [], true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4140,19 +3766,10 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('_explain_timing', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('_explain_timing', []); - return self.init_transaction(); - } - - alertify.alert(gettext('Explain options error'), - gettext('Error occurred while setting timing option in explain.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, '_explain_timing', [], true ); + alertify.alert(gettext('Explain options error'), msg); }, }); }, @@ -4254,28 +3871,18 @@ define('tools.querytool', [ } }, error: function(e) { - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) { - self.save_state('get_preferences', []); - return pgAdmin.Browser.UserManagement.pga_login(); - } - - if(transaction.is_new_transaction_required(e)) { - self.save_state('get_preferences', []); - return self.init_transaction(); - } - - updateUI(); - alertify.alert( - gettext('Get Preferences error'), - gettext('Error occurred while getting query tool options.') + let msg = httpErrorHandler.handleQueryToolAjaxError( + pgAdmin, self, e, 'get_preferences', [], true ); + updateUI(); + alertify.alert(gettext('Get Preferences error'), msg); }, }); }, close: function() { var self = this; - pgBrowser.Events.off('pgadmin:user:logged-in', this.init_transaction); + pgBrowser.Events.off('pgadmin:user:logged-in', this.initTransaction); _.each(window.top.pgAdmin.Browser.docker.findPanels('frm_datagrid'), function(panel) { if (panel.isVisible()) { window.onbeforeunload = null; diff --git a/web/pgadmin/tools/user_management/static/js/user_management.js b/web/pgadmin/tools/user_management/static/js/user_management.js index 2627328..dcf65cb 100644 --- a/web/pgadmin/tools/user_management/static/js/user_management.js +++ b/web/pgadmin/tools/user_management/static/js/user_management.js @@ -129,7 +129,7 @@ define([ alertify.ChangePassword(title, url).resizeTo('75%', '70%'); }, - is_pga_login_required(xhr) { + isPgaLoginRequired(xhr) { /* If responseJSON is undefined then it could be object of * axios(Promise HTTP) response, so we should check accordingly. */ @@ -145,7 +145,7 @@ define([ }, // Callback to draw pgAdmin4 login dialog. - pga_login: function(url) { + pgaLogin: function(url) { var title = gettext('pgAdmin 4 login'); url = url || url_for('security.login'); if(!alertify.PgaLogin) { diff --git a/web/regression/javascript/sqleditor/execute_query_spec.js b/web/regression/javascript/sqleditor/execute_query_spec.js index 5f92dc5..06fefff 100644 --- a/web/regression/javascript/sqleditor/execute_query_spec.js +++ b/web/regression/javascript/sqleditor/execute_query_spec.js @@ -8,7 +8,7 @@ ////////////////////////////////////////////////////////////////////////// import * as subject from 'sources/sqleditor/execute_query'; -import * as transaction from 'sources/sqleditor/is_new_transaction_required'; +import * as httpErrorHandler from 'sources/sqleditor/query_tool_http_error_handler'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import $ from 'jquery'; @@ -27,8 +27,8 @@ describe('ExecuteQuery', () => { networkMock = new MockAdapter(axios); jasmine.addMatchers({jQuerytoHaveBeenCalledWith: jQuerytoHaveBeenCalledWith}); userManagementMock = jasmine.createSpyObj('UserManagement', [ - 'is_pga_login_required', - 'pga_login', + 'isPgaLoginRequired', + 'pgaLogin', ]); sqlEditorMock = jasmine.createSpyObj('SqlEditor', [ @@ -40,14 +40,14 @@ describe('ExecuteQuery', () => { 'update_msg_history', '_highlight_error', '_init_polling_flags', - 'save_state', - 'init_transaction', + 'saveState', + 'initTransaction', 'handle_connection_lost', ]); sqlEditorMock.transId = 123; sqlEditorMock.rows_affected = 1000; executeQuery = new subject.ExecuteQuery(sqlEditorMock, userManagementMock); - isNewTransactionRequiredMock = spyOn(transaction, 'is_new_transaction_required'); + isNewTransactionRequiredMock = spyOn(httpErrorHandler, 'httpResponseRequiresNewTransaction'); }); afterEach(() => { @@ -272,7 +272,7 @@ describe('ExecuteQuery', () => { describe('when JSON response is available', () => { describe('when login is not required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(false); + userManagementMock.isPgaLoginRequired.and.returnValue(false); response = {responseJSON: errorMessageJson}; networkMock.onGet('/sqleditor/query_tool/poll/123').reply(401, response); @@ -336,7 +336,7 @@ describe('ExecuteQuery', () => { it('should not login is displayed', (done) => { setTimeout( () => { - expect(userManagementMock.pga_login).not + expect(userManagementMock.pgaLogin).not .toHaveBeenCalled(); done(); }, 0); @@ -345,7 +345,7 @@ describe('ExecuteQuery', () => { describe('when login is required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(true); + userManagementMock.isPgaLoginRequired.and.returnValue(true); response = {responseJSON: errorMessageJson}; networkMock.onGet('/sqleditor/query_tool/poll/123').reply(401, response); @@ -409,7 +409,7 @@ describe('ExecuteQuery', () => { it('should login is displayed', (done) => { setTimeout( () => { - expect(userManagementMock.pga_login) + expect(userManagementMock.pgaLogin) .toHaveBeenCalled(); done(); }, 0); @@ -420,7 +420,7 @@ describe('ExecuteQuery', () => { describe('when no JSON response is available', () => { describe('when login is not required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(false); + userManagementMock.isPgaLoginRequired.and.returnValue(false); response = { errormsg: errorMessageText, }; @@ -486,7 +486,7 @@ describe('ExecuteQuery', () => { it('should login is not displayed', (done) => { setTimeout( () => { - expect(userManagementMock.pga_login).not + expect(userManagementMock.pgaLogin).not .toHaveBeenCalled(); done(); }, 0); @@ -495,7 +495,7 @@ describe('ExecuteQuery', () => { describe('when login is required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(true); + userManagementMock.isPgaLoginRequired.and.returnValue(true); response = { errormsg: errorMessageText, }; @@ -561,7 +561,7 @@ describe('ExecuteQuery', () => { it('should login is displayed', (done) => { setTimeout( () => { - expect(userManagementMock.pga_login) + expect(userManagementMock.pgaLogin) .toHaveBeenCalled(); done(); }, 0); @@ -633,7 +633,7 @@ describe('ExecuteQuery', () => { it('should login is not displayed', (done) => { setTimeout( () => { - expect(userManagementMock.pga_login).not + expect(userManagementMock.pgaLogin).not .toHaveBeenCalled(); done(); }, 0); @@ -1366,7 +1366,7 @@ describe('ExecuteQuery', () => { describe('when error is returned by the server', () => { describe('when login is not required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(false); + userManagementMock.isPgaLoginRequired.and.returnValue(false); response.errormsg = 'some error message'; networkMock.onAny('/sqleditor/query_tool/start/123').reply(500, response); @@ -1422,19 +1422,19 @@ describe('ExecuteQuery', () => { it('should not save the state', () => { setTimeout(() => { - expect(sqlEditorMock.save_state).not.toHaveBeenCalled(); + expect(sqlEditorMock.saveState).not.toHaveBeenCalled(); }, 0); }); it('should not display pga login', () => { setTimeout(() => { - expect(userManagementMock.pga_login).not.toHaveBeenCalled(); + expect(userManagementMock.pgaLogin).not.toHaveBeenCalled(); }, 0); }); }); describe('when login is required', () => { beforeEach(() => { - userManagementMock.is_pga_login_required.and.returnValue(true); + userManagementMock.isPgaLoginRequired.and.returnValue(true); response.errormsg = 'some error message'; networkMock.onAny('/sqleditor/query_tool/start/123').reply(500, response); @@ -1490,7 +1490,7 @@ describe('ExecuteQuery', () => { it('should save the state', () => { setTimeout(() => { - expect(sqlEditorMock.save_state).toHaveBeenCalledWith( + expect(sqlEditorMock.saveState).toHaveBeenCalledWith( 'execute', [''] ); @@ -1499,7 +1499,7 @@ describe('ExecuteQuery', () => { it('should display pga login', () => { setTimeout(() => { - expect(userManagementMock.pga_login).toHaveBeenCalled(); + expect(userManagementMock.pgaLogin).toHaveBeenCalled(); }, 0); }); }); @@ -1561,19 +1561,19 @@ describe('ExecuteQuery', () => { it('should not save the state', () => { setTimeout(() => { - expect(sqlEditorMock.save_state).not.toHaveBeenCalled(); + expect(sqlEditorMock.saveState).not.toHaveBeenCalled(); }, 0); }); it('should not display pga login', () => { setTimeout(() => { - expect(userManagementMock.pga_login).not.toHaveBeenCalled(); + expect(userManagementMock.pgaLogin).not.toHaveBeenCalled(); }, 0); }); it('should not initialize a new transaction', () => { setTimeout(() => { - expect(sqlEditorMock.init_transaction).not.toHaveBeenCalled(); + expect(sqlEditorMock.initTransaction).not.toHaveBeenCalled(); }, 0); }); }); @@ -1635,7 +1635,7 @@ describe('ExecuteQuery', () => { it('should save the state', () => { setTimeout(() => { - expect(sqlEditorMock.save_state).toHaveBeenCalledWith( + expect(sqlEditorMock.saveState).toHaveBeenCalledWith( 'execute', [''] ); @@ -1644,13 +1644,13 @@ describe('ExecuteQuery', () => { it('should not display pga login', () => { setTimeout(() => { - expect(userManagementMock.pga_login).not.toHaveBeenCalled(); + expect(userManagementMock.pgaLogin).not.toHaveBeenCalled(); }, 0); }); it('should initialize a new transaction', () => { setTimeout(() => { - expect(sqlEditorMock.init_transaction).toHaveBeenCalled(); + expect(sqlEditorMock.initTransaction).toHaveBeenCalled(); }, 0); }); }); @@ -1665,7 +1665,7 @@ describe('ExecuteQuery', () => { it('saves state', () => { setTimeout(() => { - expect(sqlEditorMock.save_state).toHaveBeenCalledWith( + expect(sqlEditorMock.saveState).toHaveBeenCalledWith( 'execute', [''] ); diff --git a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js b/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js deleted file mode 100644 index 97d1bc5..0000000 --- a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js +++ /dev/null @@ -1,65 +0,0 @@ -////////////////////////////////////////////////////////////////////////// -// -// pgAdmin 4 - PostgreSQL Tools -// -// Copyright (C) 2013 - 2018, The pgAdmin Development Team -// This software is released under the PostgreSQL Licence -// -////////////////////////////////////////////////////////////////////////// - -import {is_new_transaction_required} from '../../../pgadmin/static/js/sqleditor/is_new_transaction_required'; - -describe('#is_new_transaction_required', () => { - describe('when status is not 404', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 300, - })).toBe(false); - }); - }); - - describe('when status is 404', () => { - describe('when data is not present', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - })).toBeFalsy(); - }); - }); - - describe('when data is present', () => { - describe('when info is not present inside data', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: {}, - })).toBeFalsy(); - }); - }); - - describe('when info is present inside data', () => { - describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: { - info: 'some information', - }, - })).toBe(false); - }); - }); - - describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => { - it('should return false', () => { - expect(is_new_transaction_required({ - status: 404, - data: { - info: 'DATAGRID_TRANSACTION_REQUIRED', - }, - })).toBe(true); - }); - }); - }); - }); - }); -}); diff --git a/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js b/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js new file mode 100644 index 0000000..2128152 --- /dev/null +++ b/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js @@ -0,0 +1,191 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2018, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// + +import { + httpResponseRequiresNewTransaction, + handleQueryToolAjaxError +} from '../../../pgadmin/static/js/sqleditor/query_tool_http_error_handler'; + +describe('#httpResponseRequiresNewTransaction', () => { + describe('when status is not 404', () => { + it('should return false', () => { + expect(httpResponseRequiresNewTransaction({ + status: 300, + })).toBe(false); + }); + }); + + describe('when status is 404', () => { + describe('when data is not present', () => { + it('should return false', () => { + expect(httpResponseRequiresNewTransaction({ + status: 404, + })).toBeFalsy(); + }); + }); + + describe('when data is present', () => { + describe('when info is not present inside data', () => { + it('should return false', () => { + expect(httpResponseRequiresNewTransaction({ + status: 404, + data: {}, + })).toBeFalsy(); + }); + }); + + describe('when info is present inside data', () => { + describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => { + it('should return false', () => { + expect(httpResponseRequiresNewTransaction({ + status: 404, + data: { + info: 'some information', + }, + })).toBe(false); + }); + }); + + describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => { + it('should return false', () => { + expect(httpResponseRequiresNewTransaction({ + status: 404, + data: { + info: 'DATAGRID_TRANSACTION_REQUIRED', + }, + })).toBe(true); + }); + }); + }); + }); + }); +}); + + +describe('#handleQueryToolAjaxError', () => { + let sqlEditorHandler, + exceptionSpy, stateToSave, + stateParameters, checkTransaction, UserManagementMock, + pgBrowserMock; + + beforeEach(() => { + stateToSave = 'testState'; + stateParameters = []; + checkTransaction = false; + sqlEditorHandler = jasmine.createSpyObj( + 'handler', ['initTransaction', 'saveState', 'handle_connection_lost'] + ); + exceptionSpy = { + readyState: 0, + status: 404, + data: { + info: 'CONNECTION_LOST', + }, + }; + pgBrowserMock = { + 'Browser': { + 'UserManagement': jasmine.createSpyObj('UserManagement', ['isPgaLoginRequired', 'pgaLogin']) + } + }; + }); + + describe('when ready state is 0', () => { + it('should return connection', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('Not connected to the server or the connection to the server has been closed.'); + }); + }); + + describe('when there is an ajax error due to login is required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 401; + exceptionSpy.data.info = 'PGADMIN_LOGIN_REQUIRED'; + pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(true); + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(pgBrowserMock.Browser.UserManagement.pgaLogin).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error and new transaction initialization required', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 404; + exceptionSpy.data.info = 'DATAGRID_TRANSACTION_REQUIRED'; + pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false); + checkTransaction = true; + }); + + it('should save the current state and call login handler', () => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pgaLogin).not.toHaveBeenCalled(); + expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.initTransaction).toHaveBeenCalled(); + }); + }); + + describe('when there is an ajax error due to database connection has been lost', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 503; + exceptionSpy.responseJSON = { + 'info': 'CONNECTION_LOST' + }; + pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false); + checkTransaction = false; + }); + + it('should save the current state and call connection lost handler', (done) => { + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ); + expect(pgBrowserMock.Browser.UserManagement.pgaLogin).not.toHaveBeenCalled(); + setTimeout(() => { + expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters); + expect(sqlEditorHandler.handle_connection_lost).toHaveBeenCalledWith(false, exceptionSpy); + done(); + }, 0); + }); + }); + + describe('when there is an ajax error due to unknown reason', () => { + beforeEach(() => { + exceptionSpy.readyState = 1; + exceptionSpy.status = 803; + exceptionSpy.responseText = 'ajax failed with unknown reason'; + pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false); + checkTransaction = false; + }); + + it('should return proper error message from ajax exception', () => { + expect( + handleQueryToolAjaxError( + pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave, + stateParameters, checkTransaction + ) + ).toBe('ajax failed with unknown reason'); + }); + }); + +}); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [pgAdmin4][RM#3235] Code refactoring in Query tool @ 2018-04-04 10:21 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Dave Page @ 2018-04-04 10:21 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Thanks, applied. On Wed, Apr 4, 2018 at 9:43 AM, Murtuza Zabuawala < [email protected]> wrote: > Hi, > > Thank you Victoria & Joao for reviewing. > PFA updated patch. > > On Wed, Apr 4, 2018 at 12:26 AM, Joao De Almeida Pereira < > [email protected]> wrote: > >> Hi Murtuza >> >> It is really good to see other patches that are just refactoring code. >> >> We have some suggestions: >> - We are trying to use === instead of == because === ensure that the type >> is also checked (https://developer.mozilla.org >> /en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness) >> > Done > > > - Now that we are refactoring some code, maybe we should keep some >> consistency on the way we name functions and variables. >> > We should use camelCase for names instead of snake_case. In general, if >> you see a function or variable name that doesn't fit the desired syntax or >> if a block of code seems confusing, it is better to refactor it. >> > Done, I have also changed other variables. > BTW I'm using camelCase convention from last few patches :) > > - By the name of the function is_new_transaction_required, it describes >> what the function represents but doesn't seem to explain the full scope of >> the function. What do you think about the name: httpResponseRequiresNewT >> ransaction >> > Done > > > - The extraction doesn't look like it matches the code removed >> > >> - if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) >> { >> - self.save_state('_explain_timing', []); >> - return pgAdmin.Browser.UserManagement.pga_login(); >> - } >> - >> - if(transaction.is_new_transaction_required(e)) { >> - self.save_state('_explain_timing', []); >> - return self.init_transaction(); >> - } >> - >> - alertify.alert(gettext('Explain options error'), >> - gettext('Error occurred while setting timing option in >> explain.') >> + let msg = httpErrorHandler.handleQueryToolAjaxError( >> + pgAdmin, self, e, '_explain_timing', null, true >> ); >> + alertify.alert(gettext('Explain options error'), msg); >> In this case we are only saving state if the following conditions are >> true: >> isNotConnectedToThePythonServer and httpResponseJSONIsPresent and >> connectionLostToPostgresDatabase and shouldSaveState >> That is not the case on the removed code. >> > I think the *null* value got your attention b > ut actually I have a check in *handleQueryToolAjaxError *which will make > it array [] with respect to arguments for the state to save, Anyways I have > also changed it to pass [] instead of null for better clarity. > We have all those checks in our function so it check for those condition > and save the state only if those returns True. > > - The functions extracted when are called do not use all the parameters. >> This tells us that the function groups too much functionality that is not >> used in same cases. Maybe we should split this function into smaller >> functions that do each part. >> > We already had split up the function in two part. > > >> >> >> Thanks >> Victoria & Joao >> >> On Tue, Apr 3, 2018 at 11:38 AM Murtuza Zabuawala < >> [email protected]> wrote: >> >>> Hi Dave, >>> >>> PFA updated patch, I've renamed it to query_tool_http_error_handler.js >>> & query_tool_http_error_handler_spec.js respectively. >>> >>> -- >>> Regards, >>> Murtuza Zabuawala >>> EnterpriseDB: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> >>> On Tue, Apr 3, 2018 at 7:43 PM, Dave Page <[email protected]> wrote: >>> >>>> HI >>>> >>>> On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala < >>>> [email protected]> wrote: >>>> >>>>> Hi, >>>>> >>>>> PFA patch to extract the common code from query tool to handle ajax >>>>> errors & connection handling, Also added unit tests around extracted code. >>>>> >>>> >>>> Looks good to me, except, I wonder if we should rename >>>> is_new_transaction_required.js/is_new_transaction_required_spec.js to >>>> something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I >>>> like that though. >>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2018-04-04 10:21 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-04-03 11:27 [pgAdmin4][RM#3235] Code refactoring in Query tool Murtuza Zabuawala <[email protected]> 2018-04-03 14:13 ` Dave Page <[email protected]> 2018-04-03 15:38 ` Murtuza Zabuawala <[email protected]> 2018-04-03 18:56 ` Joao De Almeida Pereira <[email protected]> 2018-04-04 08:43 ` Murtuza Zabuawala <[email protected]> 2018-04-04 10:21 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox