More fixes addressing coderabbitai review

 - Address status failures on tuple updates in  _process_ctasks(), _upsert_task_params(), _upsert_task_params()
 - Do not return ok on partial chain-field UPDATE failures
 - Unguarded query status before indexing res['rows'][0] in check_precondition()
 - kind should not be represented as  boolean, should be cast to text
 - Only insert to database_connection column if has_connstr

Author: robe2

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

@@ -359,6 +359,10 @@ def update(self, gid, sid, chain_id):
             request.data.decode('utf-8')
         )
 
+        status, res = self.conn.execute_void('BEGIN')
+        if not status:
+            return internal_server_error(errormsg=res)
+
         chain_fields = {k: data[k] for k in ['chain_name', 'live', 'max_instances', 'timeout', 'self_destruct', 'exclusive_execution', 'client_name', 'on_error', 'run_at'] if k in data}
         if chain_fields:
             sets = []
@@ -374,9 +378,13 @@ def update(self, gid, sid, chain_id):
             sql = f"UPDATE timetable.chain SET {', '.join(sets)} WHERE chain_id = %s"
             status, res = self.conn.execute_void(sql, params)
             if not status:
+                self.conn.execute_void('ROLLBACK')
                 return internal_server_error(errormsg=res)
 
-        self._process_ctasks(chain_id, data.get('ctasks', {}))
+        status, res = self._process_ctasks(chain_id, data.get('ctasks', {}))
+        if not status:
+            self.conn.execute_void('ROLLBACK')
+            return internal_server_error(errormsg=res)
 
         status, res = self.conn.execute_dict(
             render_template(
@@ -386,8 +394,13 @@ def update(self, gid, sid, chain_id):
         )
 
         if not status:
+            self.conn.execute_void('ROLLBACK')
             return internal_server_error(errormsg=res)
 
+        status, commit_res = self.conn.execute_void('COMMIT')
+        if not status:
+            return internal_server_error(errormsg=commit_res)
+
         row = res['rows'][0]
 
         return jsonify(
@@ -403,15 +416,17 @@ def update(self, gid, sid, chain_id):
 
     def _process_ctasks(self, chain_id, ctasks):
         if not isinstance(ctasks, dict):
-            return
+            return True, None
 
         for task in ctasks.get('deleted', []):
             tid = task.get('task_id') if isinstance(task, dict) else task
             if tid:
-                self.conn.execute_void(
+                status, res = self.conn.execute_void(
                     "DELETE FROM timetable.task WHERE task_id = %s AND chain_id = %s",
                     (tid, chain_id)
                 )
+                if not status:
+                    return status, res
 
         has_connstr = self.manager.db_info['timetable']['has_connstr']
         for task in ctasks.get('changed', []):
@@ -440,9 +455,13 @@ def _process_ctasks(self, chain_id, ctasks):
             if sets:
                 params.extend([tid, chain_id])
                 sql = f"UPDATE timetable.task SET {', '.join(sets)} WHERE task_id = %s AND chain_id = %s"
-                self.conn.execute_void(sql, params)
+                status, res = self.conn.execute_void(sql, params)
+                if not status:
+                    return status, res
             if 'parameters' in task:
-                self._upsert_task_params(tid, task['parameters'])
+                status, res = self._upsert_task_params(tid, task['parameters'])
+                if not status:
+                    return status, res
 
         for task in ctasks.get('added', []):
             if not isinstance(task, dict):
@@ -461,19 +480,27 @@ def _process_ctasks(self, chain_id, ctasks):
             placeholders = ', '.join(['%s'] * len(values))
             sql = f"INSERT INTO timetable.task ({', '.join(fields)}) VALUES ({placeholders}) RETURNING task_id"
             status, tid = self.conn.execute_scalar(sql, values)
-            if status and tid:
-                self._upsert_task_params(tid, task.get('parameters', []))
+            if not status:
+                return status, tid
+            if tid:
+                status, res = self._upsert_task_params(tid, task.get('parameters', []))
+                if not status:
+                    return status, res
+
+        return True, None
 
     def _upsert_task_params(self, task_id, parameters):
         if not parameters:
-            return
+            return True, None
         if isinstance(parameters, dict):
             parameters = parameters.get('added', []) + parameters.get('changed', [])
         if not parameters:
-            return
-        self.conn.execute_void(
+            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):
             if not isinstance(param, dict):
                 param = {'order_id': idx + 1, 'value': str(param)}
@@ -490,7 +517,10 @@ 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)
-            self.conn.execute_void(sql, params)
+            status, res = self.conn.execute_void(sql, params)
+            if not status:
+                return status, res
+        return True, None
 
     @check_precondition
     def delete(self, gid, sid, chain_id=None):

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

@@ -201,14 +201,16 @@ def wrap(*args, **kwargs):
             self.template_path = 'pgt_chaintask/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(*args, **kwargs)

web/pgadmin/browser/server_groups/servers/pg_timetable/templates/macros/pgt_chaintask.macros

@@ -1,18 +1,17 @@
 {% macro INSERT(has_connstr, chain_id, data, conn) -%}
 INSERT INTO timetable.task(
-    chain_id, task_name, task_order, command, kind, database_connection
+    chain_id, task_name, task_order, command, kind{% if has_connstr %}, database_connection{% endif %}
     {% if data.ignore_error is defined %}, ignore_error{% endif %}
 ) VALUES (
     {{ chain_id|qtLiteral(conn) }}::integer,
     {{ data.task_name|qtLiteral(conn) }}::text,
     {{ data.task_order|qtLiteral(conn) }}::integer,
     {{ data.command|qtLiteral(conn) }}::text,
-    {{ data.kind|qtLiteral(conn) }}::timetable.command_kind
-    {% if has_connstr and data.database_connection %},
-    {{ data.database_connection|qtLiteral(conn) }}::text{% else %}NULL{% endif %}
+    {{ data.kind|qtLiteral(conn) }}::timetable.command_kind,
+    {% if has_connstr %}, {% if data.database_connection %}{{ data.database_connection|qtLiteral(conn) }}::text{% else %}NULL{% endif %}{% endif %}
     {% if data.ignore_error is defined %},
     {% if data.ignore_error %}true{% else %}false{% endif %}{% endif %}
-) RETURNING task_id;
+) RETURNING task_id
 {%- endmacro %}
 {% macro UPDATE(has_connstr, chain_id, task_id, data, conn) -%}
 {% set has_task_fields = 'task_name' in data or 'task_order' in data or 'command' in data or 'kind' in data or 'ignore_error' in data or 'database_connection' in data %}
@@ -47,20 +46,7 @@ SELECT
     t.task_id, t.chain_id, t.task_name, t.task_order,
     t.kind::text AS kind,
     t.command, t.database_connection, t.ignore_error,
-    COALESCE(
-        (
-            SELECT jsonb_agg(
-                jsonb_build_object(
-                    'order_id', p.order_id,
-                    'value', p.value #>> '{}'
-                )
-                ORDER BY p.order_id
-            )
-            FROM timetable.parameter p
-            WHERE p.task_id = t.task_id
-        ),
-        '[]'::jsonb
-    ) AS parameters
+    NULL AS parameters
 FROM
     timetable.task t
 WHERE

web/pgadmin/browser/server_groups/servers/pg_timetable/templates/pgt_chaintask/sql/default/nodes.sql

@@ -1,6 +1,6 @@
 SELECT
     task_id, chain_id, task_name, task_order,
-    CASE WHEN kind::text = 'SQL' THEN true ELSE false END AS kind
+    kind::text AS kind
 FROM
     timetable.task
 WHERE