Coderabbitai review bug and other fixes

 - Fix the conditional commas in the INSERT values list of chain.tasks
 - _init_ is not __init__ — this initializer never runs.
 - Prevent updating non-existent database_connection column
 - Another unguarded check in pg_timetable init
 - Change terminology from pgAgent Jobs/steps to pgTimetable Chain/tasks
 - Make task ordering less fragile
 - Fix some issues with parameter update when reordering

Author: robe2

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

@@ -170,14 +170,16 @@ def wrap(self, *args, **kwargs):
             self.template_path = 'pgt_chain/sql/default'
 
             if 'timetable' not in self.manager.db_info:
-                _, res = self.conn.execute_dict("""
+                status, res = self.conn.execute_dict("""
 SELECT EXISTS(
         SELECT 1 FROM information_schema.columns
         WHERE
             table_schema='timetable' AND table_name='task' AND
             column_name='database_connection'
     ) has_connstr""")
 
+                if not status:
+                    return internal_server_error(errormsg=res)
                 self.manager.db_info['timetable'] = res['rows'][0]
 
             return f(self, *args, **kwargs)
@@ -271,7 +273,7 @@ def properties(self, gid, sid, chain_id=None):
 
     @check_precondition
     def create(self, gid, sid):
-        """Create the pgAgent job."""
+        """Create the pgTimeTable chain."""
         required_args = [
             'chain_name'
         ]
@@ -492,21 +494,15 @@ def _process_ctasks(self, chain_id, ctasks):
     def _upsert_task_params(self, task_id, parameters):
         if not parameters:
             return True, None
-        if isinstance(parameters, dict):
-            parameters = parameters.get('added', []) + parameters.get('changed', [])
-        if not parameters:
-            return True, None
-        status, res = self.conn.execute_void(
-            "DELETE FROM timetable.parameter WHERE task_id = %s", (task_id,)
-        )
-        if not status:
-            return status, res
-        for idx, param in enumerate(parameters):
+
+        def _insert_param(idx, param):
             if not isinstance(param, dict):
                 param = {'order_id': idx + 1, 'value': str(param)}
+            param.pop('_t', None)
             order_id = param.get('order_id')
             if order_id is None:
                 order_id = idx + 1
+            order_id = int(order_id)
             val = param.get('value', '')
             if val is None:
                 val = ''
@@ -517,14 +513,28 @@ def _upsert_task_params(self, task_id, parameters):
             except (ValueError, TypeError):
                 sql = "INSERT INTO timetable.parameter(task_id, order_id, value) VALUES (%s, %s, to_jsonb(%s::text))"
                 params = (task_id, order_id, val)
-            status, res = self.conn.execute_void(sql, params)
+            return self.conn.execute_void(sql, params)
+
+        status, res = self.conn.execute_void(
+            "DELETE FROM timetable.parameter WHERE task_id = %s", (task_id,)
+        )
+        if not status:
+            return status, res
+
+        if isinstance(parameters, dict):
+            all_params = parameters.get('changed', []) + parameters.get('added', [])
+        else:
+            all_params = parameters
+
+        for idx, param in enumerate(all_params):
+            status, res = _insert_param(idx, param)
             if not status:
                 return status, res
         return True, None
 
     @check_precondition
     def delete(self, gid, sid, chain_id=None):
-        """Delete the pgAgent Job."""
+        """Delete the pgTimeTable chain."""
 
         if chain_id is None:
             data = request.form if request.form else json.loads(
@@ -646,8 +656,8 @@ def sql(self, gid, sid, chain_id):
     @check_precondition
     def run_now(self, gid, sid, chain_id):
         """
-        This function will set the next run to now, to inform the pgAgent to
-        run the job now.
+        This function will set the next run to now, to inform pgTimeTable to
+        run the chain now.
         """
         status, res = self.conn.execute_void(
             render_template(

web/pgadmin/browser/server_groups/servers/pg_timetable/static/js/pgt_chain.ui.js

@@ -109,8 +109,14 @@ export default class PgtChainSchema extends BaseUISchema {
         depChange: (state, source, topState, actionObj) => {
           if (actionObj.type === SCHEMA_STATE_ACTIONS.ADD_ROW && state?.ctasks) {
             const tasks = state.ctasks;
-            const lastOrder = tasks.reduce((max, t) => Math.max(max, t.task_order || 0), 0);
-            tasks[tasks.length - 1].task_order = lastOrder + 10;
+            const addedTask = actionObj.value?.cid
+              ? tasks.find(t => t.cid === actionObj.value.cid)
+              : tasks[tasks.length - 1];
+
+            if (addedTask && (addedTask.task_order === undefined || addedTask.task_order === null)) {
+              const lastOrder = tasks.reduce((max, t) => Math.max(max, parseInt(t.task_order, 10) || 0), 0);
+              addedTask.task_order = lastOrder + 10;
+            }
           }
           return state;
         },

web/pgadmin/browser/server_groups/servers/pg_timetable/tasks/__init__.py

@@ -7,7 +7,7 @@
 #
 ##########################################################################
 
-"""Implements pgAgent Job Step Node"""
+"""Implements pgTimeTable Chain Task Node"""
 
 import json
 from functools import wraps
@@ -107,7 +107,7 @@ class ChainTaskView(PGChildNodeView)
 
         A view class for ChainTask node derived from PGChildNodeView.
         This class is responsible for all the stuff related to view like
-        updating job step node, showing properties, showing sql in sql pane.
+        updating chain task, showing properties, showing sql in sql pane.
 
     Methods:
     -------
@@ -120,29 +120,29 @@ class ChainTaskView(PGChildNodeView)
         manager,conn & template_path properties to self
 
     * list()
-      - This function is used to list all the job step nodes within that
+      - This function is used to list all the chain tasks within that
       collection.
 
     * nodes()
       - This function will used to create all the child node within that
       collection.
-        Here it will create all the job step node.
+        Here it will create all the chain task nodes.
 
     * properties(gid, sid, chain_id, task_id)
-      - This function will show the properties of the selected job step node
+      - This function will show the properties of the selected chain task
 
     * update(gid, sid, chain_id, task_id)
-      - This function will update the data for the selected job step node
+      - This function will update the data for the selected chain task
 
     * msql(gid, sid, chain_id, task_id)
       - This function is used to return modified SQL for the selected
-      job step node
+      chain task
 
     * sql(gid, sid, chain_id, jscid)
       - Dummy response for sql panel
 
     * delete(gid, sid, chain_id, jscid)
-      - Drops job step
+      - Drops chain task
     """
 
     node_type = blueprint.node_type
@@ -168,7 +168,7 @@ class ChainTaskView(PGChildNodeView)
         'stats': [{'get': 'statistics'}]
     })
 
-    def _init_(self, **kwargs):
+    def __init__(self, **kwargs):
         """
         Method is used to initialize the ChainTaskView and its base view.
         Initialize all the variables create/used dynamically like conn,
@@ -220,13 +220,13 @@ def wrap(*args, **kwargs):
     @check_precondition
     def list(self, gid, sid, chain_id):
         """
-        This function is used to list all the job step nodes within
+        This function is used to list all the chain tasks within
         that collection.
 
         Args:
             gid: Server Group ID
             sid: Server ID
-            chain_id: Job ID
+            chain_id: Chain ID
         """
         sql = render_template(
             "/".join([self.template_path, self._PROPERTIES_SQL]),
@@ -253,12 +253,12 @@ def nodes(self, gid, sid, chain_id, task_id=None):
         """
         This function is used to create all the child nodes
         within the collection.
-        Here it will create all the job step nodes.
+        Here it will create all the chain tasks.
 
         Args:
             gid: Server Group ID
             sid: Server ID
-            chain_id: Job ID
+            chain_id: Chain ID
         """
         res = []
         sql = render_template(
@@ -311,12 +311,12 @@ def nodes(self, gid, sid, chain_id, task_id=None):
     @check_precondition
     def properties(self, gid, sid, chain_id, task_id):
         """
-        This function will show the properties of the selected job step node.
+        This function will show the properties of the selected chain task.
 
         Args:
             gid: Server Group ID
             sid: Server ID
-            chain_id: Job ID
+            chain_id: Chain ID
             task_id: ChainTask ID
         """
         sql = render_template(
@@ -346,27 +346,16 @@ def properties(self, gid, sid, chain_id, task_id):
     @check_precondition
     def create(self, gid, sid, chain_id):
         """
-        This function will update the data for the selected job step node.
-
-        Args:
-            gid: Server Group ID
-            sid: Server ID
-            chain_id: Job ID
+        This function will create the chain task.
         """
-        data = {}
-        if request.args:
-            for k, v in request.args.items():
-                try:
-                    data[k] = json.loads(
-                        v.decode('utf-8') if hasattr(v, 'decode') else v
-                    )
-                except ValueError:
-                    data[k] = v
-        else:
-            data = json.loads(request.data.decode())
+        data = request.form if request.form else json.loads(
+            request.data.decode('utf-8')
+        )
 
         if 'parameters' in data:
             params_raw = data.get('parameters', [])
+            if isinstance(params_raw, str):
+                params_raw = json.loads(params_raw)
             if isinstance(params_raw, dict):
                 params_raw = params_raw.get('added', []) + params_raw.get('changed', [])
             cleaned = []
@@ -409,7 +398,7 @@ def create(self, gid, sid, chain_id):
         if len(res['rows']) == 0:
             return gone(
                 errormsg=gettext(
-                    "Job step creation failed."
+                    "Chain task creation failed."
                 )
             )
         row = res['rows'][0]
@@ -425,12 +414,12 @@ def create(self, gid, sid, chain_id):
     @check_precondition
     def update(self, gid, sid, chain_id, task_id):
         """
-        This function will update the data for the selected job step node.
+        This function will update the data for the selected chain task.
 
         Args:
             gid: Server Group ID
             sid: Server ID
-            chain_id: Job ID
+            chain_id: Chain ID
             task_id: ChainTask ID
         """
         data = request.form if request.form else json.loads(
@@ -439,19 +428,26 @@ def update(self, gid, sid, chain_id, task_id):
 
         if 'parameters' in data:
             params_raw = data.get('parameters', [])
+            if isinstance(params_raw, str):
+                params_raw = json.loads(params_raw)
+
             if isinstance(params_raw, dict):
-                params_raw = params_raw.get('added', []) + params_raw.get('changed', [])
+                params_list = params_raw.get('changed', []) + params_raw.get('added', [])
+            else:
+                params_list = params_raw
+
             cleaned = []
-            for idx, param in enumerate(params_raw):
+            for idx, param in enumerate(params_list):
                 if not isinstance(param, dict):
-                    cleaned.append({'order_id': idx + 1, 'value': str(param), '_is_json': False})
-                else:
-                    try:
-                        json.loads(param.get('value', ''))
-                        param['_is_json'] = True
-                    except (ValueError, TypeError):
-                        param['_is_json'] = False
-                    cleaned.append(param)
+                    param = {'order_id': idx + 1, 'value': str(param)}
+                param.pop('_t', None)
+                param.setdefault('value', '')
+                try:
+                    json.loads(param['value'])
+                    param['_is_json'] = True
+                except (ValueError, TypeError):
+                    param['_is_json'] = False
+                cleaned.append(param)
             data['parameters'] = cleaned
 
         sql = render_template(
@@ -482,7 +478,7 @@ def update(self, gid, sid, chain_id, task_id):
         if len(res['rows']) == 0:
             return gone(
                 errormsg=gettext(
-                    "Job step update failed."
+                    "Chain task update failed."
                 )
             )
         row = res['rows'][0]
@@ -498,7 +494,7 @@ def update(self, gid, sid, chain_id, task_id):
 
     @check_precondition
     def delete(self, gid, sid, chain_id, task_id=None):
-        """Delete the Job step."""
+        """Delete the chain task."""
 
         if task_id is None:
             data = request.form if request.form else json.loads(
@@ -523,13 +519,13 @@ def delete(self, gid, sid, chain_id, task_id=None):
     def msql(self, gid, sid, chain_id, task_id=None):
         """
         This function is used to return modified SQL for the selected
-        job step node.
+        chain task.
 
         Args:
             gid: Server Group ID
             sid: Server ID
-            chain_id: Job ID
-            task_id: Job Step ID
+            chain_id: Chain ID
+            task_id: Chain Task ID
         """
         data = {}
         sql = ''
@@ -539,6 +535,30 @@ def msql(self, gid, sid, chain_id, task_id=None):
             except ValueError:
                 data[k] = v
 
+        if 'parameters' in data:
+            params_raw = data.get('parameters', [])
+            if isinstance(params_raw, str):
+                params_raw = json.loads(params_raw)
+
+            if isinstance(params_raw, dict):
+                params_list = params_raw.get('changed', []) + params_raw.get('added', [])
+            else:
+                params_list = params_raw
+
+            cleaned = []
+            for idx, param in enumerate(params_list):
+                if not isinstance(param, dict):
+                    param = {'order_id': idx + 1, 'value': str(param)}
+                param.pop('_t', None)
+                param.setdefault('value', '')
+                try:
+                    json.loads(param['value'])
+                    param['_is_json'] = True
+                except (ValueError, TypeError):
+                    param['_is_json'] = False
+                cleaned.append(param)
+            data['parameters'] = cleaned
+
         if task_id is None:
             sql = render_template(
                 "/".join([self.template_path, self._CREATE_SQL]),