Please find an updated patch attached. On Fri, Aug 23, 2019 at 7:57 AM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote:
> Hi Yosry, > > This breaks the reconnect for query tool. Open a query tool, execute some > query and then restart the python server. Go to the query tool and click > execute. It will show a warning, on continuing it should connect again. It > throws exception in browser console: > Uncaught TypeError: Cannot read property 'apply' of undefined > at Object.eval (VM69935 sqleditor.js:1769) > at Object.callback (alertify.js:3347) > at triggerCallback (alertify.js:1220) > at Object.buttonsClickHandler (alertify.js:1241) > at HTMLDivElement.eval (alertify.js:299) > > On Thu, Aug 22, 2019 at 11:44 PM Yosry Muhammad <yosry...@gmail.com> > wrote: > >> Please find an updated patch attached. >> >> On Mon, Aug 19, 2019 at 9:54 AM Yosry Muhammad <yosry...@gmail.com> >> wrote: >> >>> Jasmine tests passed on my machine, I will take another look once I have >>> access to my machine. >>> >>> On Mon, Aug 19, 2019, 7:57 AM Akshay Joshi < >>> akshay.jo...@enterprisedb.com> wrote: >>> >>>> Hi Yosry >>>> >>>> Jasmine tests are failing, can you please fix those and resend the >>>> patch. >>>> >>>> On Fri, Aug 16, 2019 at 11:23 PM Yosry Muhammad <yosry...@gmail.com> >>>> wrote: >>>> >>>>> Hi hackers, >>>>> >>>>> Please find attached a patch with minimal refactoring of: >>>>> web/pgadmin/tools/sqleditor/static/js/sqleditor.js >>>>> >>>>> This includes merging 2 redundant functions into one and renaming some >>>>> functions to have more expressive and consistent names. >>>>> >>>>> Please review ! >>>>> Thanks. >>>>> -- >>>>> *Yosry Muhammad Yosry* >>>>> >>>>> Computer Engineering student, >>>>> The Faculty of Engineering, >>>>> Cairo University (2021). >>>>> Class representative of CMP 2021. >>>>> https://www.linkedin.com/in/yosrym93/ >>>>> >>>> >>>> >>>> -- >>>> *Thanks & Regards* >>>> *Akshay Joshi* >>>> >>>> *Sr. Software Architect* >>>> *EnterpriseDB Software India Private Limited* >>>> *Mobile: +91 976-788-8246* >>>> >>> >> >> -- >> *Yosry Muhammad Yosry* >> >> Computer Engineering student, >> The Faculty of Engineering, >> Cairo University (2021). >> Class representative of CMP 2021. >> https://www.linkedin.com/in/yosrym93/ >> > > > -- > Thanks and Regards, > Aditya Toshniwal > Software Engineer | EnterpriseDB India | Pune > "Don't Complain about Heat, Plant a TREE" > -- *Yosry Muhammad Yosry* Computer Engineering student, The Faculty of Engineering, Cairo University (2021). Class representative of CMP 2021. https://www.linkedin.com/in/yosrym93/
diff --git a/docs/en_US/release_notes_4_13.rst b/docs/en_US/release_notes_4_13.rst index bc21dfeca..a0a3a38ef 100644 --- a/docs/en_US/release_notes_4_13.rst +++ b/docs/en_US/release_notes_4_13.rst @@ -22,5 +22,4 @@ Bug fixes | `Issue #2706 <https://redmine.postgresql.org/issues/2706>`_ - Added ProjectSet icon for explain module. | `Issue #2828 <https://redmine.postgresql.org/issues/2828>`_ - Added Gather Merge, Named Tuple Store Scan and Table Function Scan icon for explain module. | `Issue #4643 <https://redmine.postgresql.org/issues/4643>`_ - Fix Truncate option deselect issue for compound triggers. -| `Issue #4644 <https://redmine.postgresql.org/issues/4644>`_ - Fix length and precision enable/disable issue when changing the data type for Domain node. -| `Issue #4650 <https://redmine.postgresql.org/issues/4650>`_ - Fix SQL tab issue for Views. It's a regression of compound triggers. \ No newline at end of file +| `Issue #4644 <https://redmine.postgresql.org/issues/4644>`_ - Fix length and precision enable/disable issue when changing the data type for Domain node. \ No newline at end of file diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py index 4bc357c83..cfefa5314 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/views/__init__.py @@ -879,7 +879,6 @@ class ViewNode(PGChildNodeView, VacuumSettings): Get all compound trigger nodes associated with view node, generate their sql and render into sql tab """ - SQL_data = '' if self.manager.server_type == 'ppas' \ and self.manager.version >= 120000: @@ -889,6 +888,7 @@ class ViewNode(PGChildNodeView, VacuumSettings): # Define template path self.ct_trigger_temp_path = 'compound_triggers' + SQL_data = '' SQL = render_template("/".join( [self.ct_trigger_temp_path, 'sql/{0}/#{1}#/nodes.sql'.format( @@ -949,7 +949,7 @@ class ViewNode(PGChildNodeView, VacuumSettings): SQL_data += '\n' SQL_data += SQL - return SQL_data + return SQL_data def get_trigger_sql(self, vid): """ diff --git a/web/pgadmin/static/js/sqleditor/execute_query.js b/web/pgadmin/static/js/sqleditor/execute_query.js index 26da69148..b8baa410d 100644 --- a/web/pgadmin/static/js/sqleditor/execute_query.js +++ b/web/pgadmin/static/js/sqleditor/execute_query.js @@ -224,22 +224,22 @@ class ExecuteQuery { } if (this.userManagement.isPgaLoginRequired(httpMessage.response)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.userManagement.pgaLogin(); } if (httpErrorHandler.httpResponseRequiresNewTransaction(httpMessage.response)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.initTransaction(); } if (this.wasDatabaseConnectionLost(httpMessage)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.handle_connection_lost(false, httpMessage); } if(this.isCryptKeyMissing(httpMessage)) { - this.sqlServerObject.saveState('execute', [this.explainPlan]); + this.sqlServerObject.saveState('check_data_changes_to_execute_query', [this.explainPlan]); this.sqlServerObject.handle_cryptkey_missing(); return; } diff --git a/web/pgadmin/static/js/sqleditor/query_tool_actions.js b/web/pgadmin/static/js/sqleditor/query_tool_actions.js index 18e15ecbe..34b6827ad 100644 --- a/web/pgadmin/static/js/sqleditor/query_tool_actions.js +++ b/web/pgadmin/static/js/sqleditor/query_tool_actions.js @@ -39,13 +39,8 @@ let queryToolActions = { }, executeQuery: function (sqlEditorController) { - if(sqlEditorController.is_query_tool) { - this._clearMessageTab(); - sqlEditorController.execute(); - } else { - this._clearMessageTab(); - sqlEditorController.execute_data_query(); - } + this._clearMessageTab(); + sqlEditorController.check_data_changes_to_execute_query(); }, explainAnalyze: function (sqlEditorController) { @@ -60,7 +55,7 @@ let queryToolActions = { settings: this._settings(), }; this._clearMessageTab(); - sqlEditorController.execute(explainObject); + sqlEditorController.check_data_changes_to_execute_query(explainObject); }, explain: function (sqlEditorController) { @@ -76,7 +71,7 @@ let queryToolActions = { settings: this._settings(), }; this._clearMessageTab(); - sqlEditorController.execute(explainObject); + sqlEditorController.check_data_changes_to_execute_query(explainObject); }, download: function (sqlEditorController) { diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js index 2a956a3d7..61d33ce78 100644 --- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js @@ -2224,7 +2224,7 @@ define('tools.querytool', [ cm.className += ' bg-gray-lighter opacity-5 hide-cursor-workaround'; } self.disable_tool_buttons(true); - self.execute_data_query(); + self.check_data_changes_to_execute_query(); } }, @@ -2265,9 +2265,8 @@ define('tools.querytool', [ self.on('pgadmin-sqleditor:unindent_selected_code', self._unindent_selected_code, self); }, - // Checks if there is any dirty data in the grid before - // it executes the sql query in View Data mode - execute_data_query: function() { + // Checks if there is any dirty data in the grid before executing a query + check_data_changes_to_execute_query: function(explain_prefix=null, shouldReconnect=false) { var self = this; // Check if the data grid has any changes before running query @@ -2279,8 +2278,13 @@ define('tools.querytool', [ alertify.confirm(gettext('Unsaved changes'), gettext('The data has been modified, but not saved. Are you sure you wish to discard the changes?'), function() { - // Do nothing as user do not want to save, just continue - self._run_query(); + // The user does not want to save, just continue + if(self.is_query_tool) { + self._execute_sql_query(explain_prefix, shouldReconnect); + } + else { + self._execute_view_data_query(); + } }, function() { // Stop, User wants to save @@ -2291,12 +2295,17 @@ define('tools.querytool', [ cancel: gettext('No'), }); } else { - self._run_query(); + if(self.is_query_tool) { + self._execute_sql_query(explain_prefix, shouldReconnect); + } + else { + self._execute_view_data_query(); + } } }, - // This function makes the ajax call to execute the sql query in View Data mode - _run_query: function() { + // Makes the ajax call to execute the sql query in View Data mode + _execute_view_data_query: function() { var self = this, url = url_for('sqleditor.view_data_start', { 'trans_id': self.transId, @@ -2373,12 +2382,36 @@ define('tools.querytool', [ .fail(function(e) { self.trigger('pgadmin-sqleditor:loading-icon:hide'); let msg = httpErrorHandler.handleQueryToolAjaxError( - pgAdmin, self, e, '_run_query', [], true + pgAdmin, self, e, '_execute_view_data_query', [], true ); self.update_msg_history(false, msg); }); }, + // Executes sql query in the editor in Query Tool mode + _execute_sql_query: function(explain_prefix, shouldReconnect) { + var self = this, sql = ''; + + self.has_more_rows = false; + self.fetching_rows = false; + + if (!_.isUndefined(self.special_sql)) { + sql = self.special_sql; + } else { + /* If code is selected in the code mirror then execute + * the selected part else execute the complete code. + */ + var selected_code = self.gridView.query_tool_obj.getSelection(); + if (selected_code.length > 0) + sql = selected_code; + else + sql = self.gridView.query_tool_obj.getValue(); + } + + const executeQuery = new ExecuteQuery.ExecuteQuery(this, pgAdmin.Browser.UserManagement); + executeQuery.execute(sql, explain_prefix, shouldReconnect); + }, + // This is a wrapper to call_render function // We need this because we have separated columns route & result route // We need to combine both result here in wrapper before rendering grid @@ -3711,60 +3744,6 @@ define('tools.querytool', [ } }, - // Checks if there is any dirty data in the grid before - // it executes the sql query in Query Tool mode - execute: function(explain_prefix, shouldReconnect=false) { - var self = this; - - // Check if the data grid has any changes before running query - if (self.can_edit && _.has(self, 'data_store') && - (_.size(self.data_store.added) || - _.size(self.data_store.updated) || - _.size(self.data_store.deleted)) - ) { - alertify.confirm(gettext('Unsaved changes'), - gettext('The data has been modified, but not saved. Are you sure you wish to discard the changes?'), - function() { - // Do nothing as user do not want to save, just continue - self._execute_sql_query(explain_prefix, shouldReconnect); - }, - function() { - // Stop, User wants to save - return true; - } - ).set('labels', { - ok: gettext('Yes'), - cancel: gettext('No'), - }); - } else { - self._execute_sql_query(explain_prefix, shouldReconnect); - } - }, - - // Executes sql query in the editor in Query Tool mode - _execute_sql_query: function(explain_prefix, shouldReconnect) { - var self = this, sql = ''; - - self.has_more_rows = false; - self.fetching_rows = false; - - if (!_.isUndefined(self.special_sql)) { - sql = self.special_sql; - } else { - /* If code is selected in the code mirror then execute - * the selected part else execute the complete code. - */ - var selected_code = self.gridView.query_tool_obj.getSelection(); - if (selected_code.length > 0) - sql = selected_code; - else - sql = self.gridView.query_tool_obj.getValue(); - } - - const executeQuery = new ExecuteQuery.ExecuteQuery(this, pgAdmin.Browser.UserManagement); - executeQuery.execute(sql, explain_prefix, shouldReconnect); - }, - /* This function is used to highlight the error line and * underlining for the error word. */ diff --git a/web/regression/javascript/sqleditor/query_tool_actions_spec.js b/web/regression/javascript/sqleditor/query_tool_actions_spec.js index e5e486572..487bbb4eb 100644 --- a/web/regression/javascript/sqleditor/query_tool_actions_spec.js +++ b/web/regression/javascript/sqleditor/query_tool_actions_spec.js @@ -30,7 +30,7 @@ describe('queryToolActions', () => { it('calls the execute function on the sqlEditorController', () => { queryToolActions.executeQuery(sqlEditorController); - expect(sqlEditorController.execute).toHaveBeenCalled(); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalled(); }); }); describe('when the command is being run from the view data view', () => { @@ -39,10 +39,10 @@ describe('queryToolActions', () => { sqlEditorController.is_query_tool = false; }); - it('it calls the execute_data_query function on the sqlEditorController', () => { + it('it calls the check_data_changes_to_execute_query function on the sqlEditorController', () => { queryToolActions.executeQuery(sqlEditorController); - expect(sqlEditorController.execute_data_query).toHaveBeenCalled(); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalled(); }); }); }); @@ -74,7 +74,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -100,7 +100,7 @@ describe('queryToolActions', () => { summary: true, settings: true, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -128,7 +128,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -156,7 +156,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -184,7 +184,7 @@ describe('queryToolActions', () => { settings: true, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); }); @@ -213,7 +213,7 @@ describe('queryToolActions', () => { summary: false, settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -239,7 +239,7 @@ describe('queryToolActions', () => { settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); @@ -264,7 +264,7 @@ describe('queryToolActions', () => { summary: false, settings: false, }; - expect(sqlEditorController.execute).toHaveBeenCalledWith(explainObject); + expect(sqlEditorController.check_data_changes_to_execute_query).toHaveBeenCalledWith(explainObject); }); }); }); @@ -621,8 +621,7 @@ describe('queryToolActions', () => { trigger: jasmine.createSpy('trigger'), table_name: 'iAmATable', is_query_tool: true, - execute: jasmine.createSpy('execute'), - execute_data_query: jasmine.createSpy('execute_data_query'), + check_data_changes_to_execute_query: jasmine.createSpy('check_data_changes_to_execute_query'), }; } });
