Fix logic around validation and highlighting of Sort/Filter in the Query Tool. Fixes #3607

docs/en_US/release_notes_3_4.rst

@@ -19,5 +19,6 @@ Bug fixes
 
 | `Bug #3576 <https://redmine.postgresql.org/issues/3576>`_ - Ensure queries are no longer executed when dashboards are closed.
 | `Bug #3596 <https://redmine.postgresql.org/issues/3596>`_ - Fix support for the CLOB datatype in EPAS.
+| `Bug #3607 <https://redmine.postgresql.org/issues/3607>`_ - Fix logic around validation and highlighting of Sort/Filter in the Query Tool.
 | `Bug #3630 <https://redmine.postgresql.org/issues/3630>`_ - Ensure auto-complete works for objects in schemas other than public and pg_catalog.
 

web/pgadmin/static/js/sqleditor/filter_dialog.js

@@ -215,11 +215,15 @@ let FilterDialog = {
                   'trans_id': handler.transId,
                 }),
                 filterCollectionModel
-              ).then(function () {
+              ).then(function (result) {
                 // Hide Progress ...
                 $(
                   self.showFilterProgress[0]
                 ).addClass('hidden');
+
+                let response = result.data.data;
+
+                if (response.status) {
                   setTimeout(
                     function() {
                       self.close(); // Close the dialog now
@@ -227,6 +231,12 @@ let FilterDialog = {
                       queryToolActions.executeQuery(handler);
                     }, 10
                   );
+                } else {
+                  Alertify.alert(
+                      gettext('Validation Error'),
+                      response.result
+                  );
+                }
 
               }).catch(function (error) {
                 // Hide Progress ...

web/pgadmin/tools/datagrid/static/js/datagrid.js

@@ -302,6 +302,8 @@ define('pgadmin.datagrid', [
                 );
 
                 this.setContent($content.get(0));
+                // Disable OK button
+                that.__internal.buttons[0].element.disabled = true;
 
                 // Apply CodeMirror to filter text area.
                 this.filter_obj = CodeMirror.fromTextArea($sql_filter.get(0), {
@@ -324,6 +326,14 @@ define('pgadmin.datagrid', [
                   that.filter_obj.refresh();
                   that.filter_obj.focus();
                 }, 500);
+
+                that.filter_obj.on('change', function() {
+                  if (that.filter_obj.getValue() !== '') {
+                    that.__internal.buttons[0].element.disabled = false;
+                  } else {
+                    that.__internal.buttons[0].element.disabled = true;
+                  }
+                });
               },
 
               callback: function(closeEvent) {
@@ -331,6 +341,7 @@ define('pgadmin.datagrid', [
                 if (closeEvent.button.text == gettext('OK')) {
                   var sql = this.filter_obj.getValue();
                   var that = this;
+                  closeEvent.cancel = true; // Do not close dialog
 
                   // Make ajax call to include the filter by selection
                   $.ajax({
@@ -344,6 +355,7 @@ define('pgadmin.datagrid', [
                     if (res.data.status) {
                       // Initialize the data grid.
                       self.create_transaction(that.baseUrl, null, 'false', parentData.server.server_type, '', grid_title, sql, false);
+                      that.close(); // Close the dialog
                     }
                     else {
                       alertify.alert(

web/pgadmin/tools/sqleditor/__init__.py

@@ -236,12 +236,13 @@ def start_view_data(trans_id):
        trans_obj is not None and session_obj is not None:
         # set fetched row count to 0 as we are executing query again.
         trans_obj.update_fetched_row_cnt(0)
-        session_obj['command_obj'] = pickle.dumps(trans_obj, -1)
 
         # Fetch the sql and primary_keys from the object
         sql = trans_obj.get_sql(default_conn)
         pk_names, primary_keys = trans_obj.get_primary_keys(default_conn)
 
+        session_obj['command_obj'] = pickle.dumps(trans_obj, -1)
+
         has_oids = False
         if trans_obj.object_type == 'table':
             # Fetch OIDs status

web/pgadmin/tools/sqleditor/command.py

@@ -166,6 +166,7 @@ class SQLFilter(object):
         self.obj_id = kwargs['obj_id']
         self.__row_filter = kwargs.get('sql_filter', None)
         self.__dara_sorting = kwargs.get('data_sorting', None)
+        self.__set_sorting_from_filter_dialog = False
 
         manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(self.sid)
         conn = manager.connection(did=self.did)
@@ -222,12 +223,17 @@ class SQLFilter(object):
             return self.__dara_sorting
         return None
 
-    def set_data_sorting(self, data_filter):
+    def set_data_sorting(self, data_filter, set_from_filter_dialog=False):
         """
         This function validates the filter and set the
         given filter to member variable.
         """
         self.__dara_sorting = data_filter['data_sorting']
+        self.__set_sorting_from_filter_dialog = set_from_filter_dialog
+
+    def is_sorting_set_from_filter_dialog(self):
+        """This function return whether sorting is set from filter dialog"""
+        return self.__set_sorting_from_filter_dialog
 
     def is_filter_applied(self):
         """
@@ -276,10 +282,9 @@ class SQLFilter(object):
         status = True
         result = None
 
-        if row_filter is None or row_filter == '':
+        if row_filter is not None and row_filter != '':
-            return False, gettext('Filter string is empty.')
+            manager = \
-
+                get_driver(PG_DEFAULT_DRIVER).connection_manager(self.sid)
-        manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(self.sid)
             conn = manager.connection(did=self.did)
 
             if conn.connected():
@@ -466,23 +471,35 @@ class TableCommand(GridCommand):
         sql_filter = self.get_filter()
         data_sorting = self.get_data_sorting()
 
+        # If data sorting is none and not reset from the filter dialog then
+        # set the data sorting in following conditions:
+        #   1. When command type is VIEW_FIRST_100_ROWS or VIEW_LAST_100_ROWS.
+        #   2. When command type is VIEW_ALL_ROWS and limit is greater than 0
+
+        if data_sorting is None and \
+            not self.is_sorting_set_from_filter_dialog() \
+            and (self.cmd_type in (VIEW_FIRST_100_ROWS, VIEW_LAST_100_ROWS) or
+                 (self.cmd_type == VIEW_ALL_ROWS and self.limit > 0)):
+            sorting = {'data_sorting': []}
+            for pk in primary_keys:
+                sorting['data_sorting'].append(
+                    {'name': pk, 'order': self.get_pk_order()})
+            self.set_data_sorting(sorting)
+            data_sorting = self.get_data_sorting()
+
         if sql_filter is None:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
                 object_name=self.object_name,
-                nsp_name=self.nsp_name, pk_names=pk_names,
+                nsp_name=self.nsp_name, limit=self.limit, has_oids=has_oids,
-                cmd_type=self.cmd_type, limit=self.limit,
-                primary_keys=primary_keys, has_oids=has_oids,
                 data_sorting=data_sorting
             )
         else:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
                 object_name=self.object_name,
-                nsp_name=self.nsp_name, pk_names=pk_names,
+                nsp_name=self.nsp_name, limit=self.limit, has_oids=has_oids,
-                cmd_type=self.cmd_type, sql_filter=sql_filter,
+                sql_filter=sql_filter, data_sorting=data_sorting
-                limit=self.limit, primary_keys=primary_keys,
-                has_oids=has_oids, data_sorting=data_sorting
             )
 
         return sql
@@ -563,12 +580,6 @@ class TableCommand(GridCommand):
 
         for row in result['rows']:
             all_columns.append(row['attname'])
-            all_sorted_columns.append(
-                {
-                    'name': row['attname'],
-                    'order': self.get_pk_order()
-                }
-            )
 
         # Fetch the rest of the column names
         query = render_template(
@@ -919,15 +930,13 @@ class ViewCommand(GridCommand):
         if sql_filter is None:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 limit=self.limit, data_sorting=data_sorting
             )
         else:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 sql_filter=sql_filter, limit=self.limit,
                 data_sorting=data_sorting
             )
@@ -982,15 +991,13 @@ class ForeignTableCommand(GridCommand):
         if sql_filter is None:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 limit=self.limit, data_sorting=data_sorting
             )
         else:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 sql_filter=sql_filter, limit=self.limit,
                 data_sorting=data_sorting
             )
@@ -1035,15 +1042,13 @@ class CatalogCommand(GridCommand):
         if sql_filter is None:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 limit=self.limit, data_sorting=data_sorting
             )
         else:
             sql = render_template(
                 "/".join([self.sql_path, 'objectquery.sql']),
-                object_name=self.object_name,
+                object_name=self.object_name, nsp_name=self.nsp_name,
-                nsp_name=self.nsp_name, cmd_type=self.cmd_type,
                 sql_filter=sql_filter, limit=self.limit,
                 data_sorting=data_sorting
             )

web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/objectquery.sql

@@ -7,9 +7,6 @@ WHERE {{ sql_filter }}
 ORDER BY {% for obj in data_sorting %}
 {{ conn|qtIdent(obj.name) }} {{ obj.order|upper }}{% if not loop.last %}, {% else %} {% endif %}
 {% endfor %}
-{% elif primary_keys %}
-ORDER BY {% for p in primary_keys %}{{conn|qtIdent(p)}}{% if cmd_type == 1 or cmd_type == 3 %} ASC{% elif cmd_type == 2 %} DESC{% endif %}
-{% if not loop.last %}, {% else %} {% endif %}{% endfor %}
 {% endif %}
 {% if limit > 0 %}
 LIMIT {{ limit }}

web/pgadmin/tools/sqleditor/utils/filter_dialog.py

@@ -86,8 +86,9 @@ class FilterDialog(object):
 
         if status and conn is not None and \
            trans_obj is not None and session_obj is not None:
-            trans_obj.set_data_sorting(data)
+            trans_obj.set_data_sorting(data, True)
-            trans_obj.set_filter(data.get('sql'))
+            status, res = trans_obj.set_filter(data.get('sql'))
+            if status:
                 # As we changed the transaction object we need to
                 # restore it and update the session variable.
                 session_obj['command_obj'] = pickle.dumps(trans_obj, -1)