Fixed the following code smells:

1. Use concise character class syntax
2. Add a "break" statement or remove this "else" clause.
3. Replace this generic exception class with a more specific one.
4. Use a regular expression literal instead of the 'RegExp' constructor.
5. Use the opposite operator ("not in") instead.

web/pgadmin/browser/server_groups/servers/pgagent/__init__.py

@@ -173,7 +173,7 @@ class JobView(PGChildNodeView):
             # Set the template path for the sql scripts.
             self.template_path = 'pga_job/sql/pre3.4'
 
-            if not ('pgAgent' in self.manager.db_info):
+            if 'pgAgent'not in self.manager.db_info:
                 status, res = self.conn.execute_dict("""
 SELECT EXISTS(
         SELECT 1 FROM information_schema.columns

web/pgadmin/browser/server_groups/servers/pgagent/steps/__init__.py

@@ -200,7 +200,7 @@ class JobStepView(PGChildNodeView):
 
             self.template_path = 'pga_jobstep/sql/pre3.4'
 
-            if not ('pgAgent' in self.manager.db_info):
+            if 'pgAgent' not in self.manager.db_info:
                 status, res = self.conn.execute_dict("""
 SELECT EXISTS(
         SELECT 1 FROM information_schema.columns

web/pgadmin/browser/server_groups/servers/roles/__init__.py

@@ -21,6 +21,7 @@ from pgadmin.utils.ajax import make_json_response, \
     internal_server_error, forbidden, success_return, gone
 from pgadmin.utils.driver import get_driver
 from pgadmin.utils.constants import ERROR_FETCHING_ROLE_INFORMATION
+from pgadmin.utils.exception import ExecuteError
 
 from config import PG_DEFAULT_DRIVER
 from flask_babel import gettext
@@ -1293,6 +1294,9 @@ WHERE
         )
         status, res = conn.execute_scalar(SQL)
 
+        if not status:
+            raise ExecuteError(res)
+
         return status, res
 
     @check_precondition()
@@ -1404,9 +1408,6 @@ WHERE
 
             status, old_role_name = self._execute_role_reassign(conn, rid)
 
-            if not status:
-                raise Exception(old_role_name)
-
             data['old_role_name'] = old_role_name
 
             is_reassign = True if data['role_op'] == 'reassign' else False
@@ -1421,15 +1422,9 @@ WHERE
                 status, new_role_name = \
                     self._execute_role_reassign(conn, data['new_role_id'])
 
-                if not status:
-                    raise Exception(new_role_name)
-
                 data['new_role_name'] = new_role_name
 
-            status, res = self._execute_role_reassign(conn, None, data)
+            self._execute_role_reassign(conn, None, data)
-
-            if not status:
-                raise Exception(res)
 
             if is_already_connected is False and can_disconn:
                 manager.release(did=did)

web/pgadmin/browser/static/js/datamodel.js

@@ -859,15 +859,13 @@ define([
       return null;
     },
     number_validate: function(value, field) {
-      let pattern = new RegExp('^-?[0-9]+(\.?[0-9]*)?$');
+      if (!/^-?[0-9]+(\.?[0-9]*)?$/.test(value)) {
-      if (!pattern.test(value)) {
         return pgadminUtils.sprintf(pgAdmin.Browser.messages.MUST_BE_NUM, field.label);
       }
       return this.check_min_max(value, field);
     },
     integer_validate: function(value, field) {
-      let pattern = new RegExp('^-?[0-9]*$');
+      if (!/^-?[0-9]*$/.test(value)) {
-      if (!pattern.test(value)) {
         return pgadminUtils.sprintf(pgAdmin.Browser.messages.MUST_BE_INT, field.label);
       }
       return this.check_min_max(value, field);

web/pgadmin/static/js/components/CodeMirror.jsx

@@ -423,8 +423,7 @@ export default function CodeMirror({currEditor, name, value, options, events, re
     let pref = pgWindow?.pgAdmin?.Browser?.get_preferences_for_module('sqleditor') || {};
     if (autocomplete && pref.autocomplete_on_key_press) {
       editor.current.on('keyup', (cm, event)=>{
-        let pattern = new RegExp('^[ -~]{1}$');
+        if (!cm.state.completionActive && (event.key == 'Backspace' || /^[ -~]{1}$'/.test(event.key))) {
-        if (!cm.state.completionActive && (event.key == 'Backspace' || pattern.test(event.key))) {
           OrigCodeMirror.commands.autocomplete(cm, null, {completeSingle: false});
         }
       });

web/pgadmin/static/js/pgadmin.js

@@ -76,7 +76,7 @@ define([], function() {
   pgAdmin.natural_sort = function(a, b, options) {
     options = options || {};
 
-    let re = /(^-?[0-9]+(\.?[0-9]*)[df]?e?[0-9]?$|^0x[0-9a-f]+$|[0-9]+)/gi,
+    let re = /(^-?\d+(\.?\d*)[df]?e?\d?$|^0x[0-9a-f]+$|\d+)/gi,
       sre = /(^[ ]*|[ ]*$)/g,
       dre = /(^([\w ]+,?[\w ]+)?[\w ]+,?[\w ]+\d+:\d+(:\d+)?[\w ]?|^\d{1,4}[\/\-]\d{1,4}[\/\-]\d{1,4}|^\w+, \w+ \d+, \d{4})/,
       hre = /^0x[0-9a-f]+$/i,

web/pgadmin/static/js/validators.js

@@ -27,8 +27,7 @@ export function minMaxValidator(label, value, minValue, maxValue) {
 export function numberValidator(label, value) {
   if((_.isUndefined(value) || _.isNull(value) || String(value) === ''))
     return null;
-  let pattern = new RegExp('^-?[0-9]+(\.?[0-9]*)?$');
+  if (!/^-?[0-9]+(\.?[0-9]*)?$/.test(value)) {
-  if (!pattern.test(value)) {
     return sprintf(pgAdmin.Browser.messages.MUST_BE_NUM, label);
   }
   return null;
@@ -38,8 +37,7 @@ export function numberValidator(label, value) {
 export function integerValidator(label, value) {
   if((_.isUndefined(value) || _.isNull(value) || String(value) === ''))
     return null;
-  let pattern = new RegExp('^-?[0-9]*$');
+  if (!/^-?[0-9]*$/.test(value)) {
-  if (!pattern.test(value)) {
     return sprintf(pgAdmin.Browser.messages.MUST_BE_INT, label);
   }
   return null;

web/pgadmin/tools/search_objects/static/js/SearchObjects.jsx

@@ -189,7 +189,7 @@ const translateSearchObjectsPath = (nodeData, path, catalog_level)=> {
   /* add the slash to match regex, remove it from display path later */
   path = '/' + path;
   /* the below regex will match all /:schema.2200:/ */
-  let new_path = path.replace(/\/:[a-zA-Z_]+\.[0-9]+:\//g, (token)=>{
+  let new_path = path.replace(/\/:[a-zA-Z_]+\.\d+:\//g, (token)=>{
     let orig_token = token;
     /* remove the slash and colon */
     token = token.slice(2, -2);

web/pgadmin/utils/sqlautocomplete/parseutils/ctes.py

@@ -119,7 +119,7 @@ def extract_column_names(parsed):
     if tok_val in ("insert", "update", "delete"):
         # Jump ahead to the RETURNING clause where the list of column names is
         idx, tok = parsed.token_next_by(idx, (Keyword, "returning"))
-    elif not tok_val == "select":
+    elif tok_val != "select":
         # Must be invalid CTE
         return ()
 

web/pgadmin/utils/sqlautocomplete/parseutils/tables.py

@@ -54,7 +54,7 @@ def extract_from_part(parsed, stop_at_punctuation=True):
             # INNER JOIN, FULL OUTER JOIN, etc.
             elif (
                 item.ttype is Keyword and
-                (not item.value.upper() == "FROM") and
+                item.value.upper() != "FROM" and
                 (not item.value.upper().endswith("JOIN"))
             ):
                 tbl_prefix_seen = False

web/regression/feature_tests/pg_datatype_validation_test.py

@@ -183,12 +183,6 @@ class PGDataypeFeatureTest(BaseFeatureTest):
     def after(self):
         self.page.remove_server(self.server)
 
-    # TODO - To be remove
-    def _schema_node_expandable(self):
-        self.page.expand_database_node("Server", self.server['name'],
-                                       self.server['db_password'],
-                                       self.test_db)
-
     def _check_datatype(self):
         # Slick grid does not render all the column if viewport is not enough
         # wide. So execute test as batch of queries.

web/regression/feature_tests/query_tool_journey_test.py

@@ -492,7 +492,7 @@ class QueryToolJourneyTest(BaseFeatureTest):
                       str(retry), file=sys.stderr)
                 retry -= 1
                 if retry == 0:
-                    raise Exception(e)
+                    raise TimeoutError(e)
 
     def _check_can_add_row(self):
         return self.page.check_if_element_exist_by_xpath(

web/regression/feature_utils/pgadmin_page.py

@@ -611,31 +611,6 @@ class PgadminPage:
             print("The databases/previous nodes not expanded", file=sys.stderr)
         return database_expanded
 
-    # TODO - We might need this method
-    # def click_to_expand_database_node(self, database_name, database_node):
-    #     """
-    #     Method clicks on specified database name from expanded databases node
-    #     of server.
-    #     :param sub_nodes_of_databases_node:
-    #     :param index_of_required_db_node:
-    #     :param name_of_database:
-    #     :return: True if particular database click is successful & expanded
-    #     """
-    #     database_expanded = False
-    #     if self.check_if_element_exist_by_xpath(
-    #     TreeAreaLocators.database_node_exp_status(database_name), 2):
-    #         database_expanded = True
-    #     else:
-    #         # TODO - This is bug 6962
-    #         webdriver.ActionChains(self.driver).click(database_node).perform()
-    #         if self.check_if_element_exist_by_xpath(
-    #         TreeAreaLocators.database_node_exp_status(database_name)):
-    #             database_expanded = True
-    #     print("click_to_expand_database_node> db_node_expanded_status - ",
-    #     database_expanded)
-    #     return database_expanded
-    #
-
     def expand_database_child_node(self, server_group_name, server_name,
                                    server_password, database_name,
                                    database_child_node_name):
@@ -1150,6 +1125,8 @@ class PgadminPage:
                         webdriver.ActionChains(self.driver).move_to_element(
                             top_el).perform()
                         r_scroll -= 1
+                else:
+                    break
         else:
             print("check_if_element_exists_with_scroll > Element NOT found",
                   xpath, file=sys.stderr)