public inbox for [email protected]  
help / color / mirror / Atom feed
[pgAdmin4][Patch]: RM 4428 and 4448
2+ messages / 2 participants
[nested] [flat]

* [pgAdmin4][Patch]: RM 4428 and 4448
@ 2019-07-15 12:32  Akshay Joshi <[email protected]>
  0 siblings, 1 reply; 2+ messages in thread

From: Akshay Joshi @ 2019-07-15 12:32 UTC (permalink / raw)
  To: pgadmin-hackers

Hi Hackers,

Attached is the patch to fix following issues in pgagent module:

   - RM 4428 "malformed array literal error when updating pgagent job".
   - RM 4448 "Error when updating connection string in pgagent Jobs."
   - When user create a schedule using Create->Schedule dialog browser tree
   is not showing newly created node.
   - Properties tab showing same properties for all the created schedule.
   - Added validation in "pga_jobstep.js", throws error on browser when we
   modify step from the pgagent dialog and select the same node.


Added regression test cases to add schedule and step from the pgagent job
itself.
Please review it.

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*


Attachments:

  [application/octet-stream] RM_4428_4448.patch (18.4K, 3-RM_4428_4448.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/__init__.py b/web/pgadmin/browser/server_groups/servers/pgagent/__init__.py
index 19309c9f..8cdab8f8 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/__init__.py
@@ -23,6 +23,8 @@ from pgadmin.utils.ajax import make_json_response, internal_server_error, \
     make_response as ajax_response, gone, success_return
 from pgadmin.utils.driver import get_driver
 from pgadmin.utils.preferences import Preferences
+from pgadmin.browser.server_groups.servers.pgagent.utils \
+    import format_schedule_data
 
 
 class JobModule(CollectionNodeModule):
@@ -333,6 +335,9 @@ SELECT EXISTS(
             request.data.decode('utf-8')
         )
 
+        # Format the schedule and step data
+        self.format_schedule_step_data(data)
+
         status, res = self.conn.execute_void(
             render_template(
                 "/".join([self.template_path, 'update.sql']),
@@ -404,6 +409,9 @@ SELECT EXISTS(
             except ValueError:
                 data[k] = v
 
+        # Format the schedule and step data
+        self.format_schedule_step_data(data)
+
         return make_json_response(
             data=render_template(
                 "/".join([
@@ -546,5 +554,49 @@ SELECT EXISTS(
             status=200
         )
 
+    def format_schedule_step_data(self, data):
+        """
+        This function is used to format the schedule and step data.
+        :param data:
+        :return:
+        """
+        # Format the schedule data. Convert the boolean array
+        if 'jschedules' in data:
+            if 'added' in data['jschedules']:
+                for added_schedule in data['jschedules']['added']:
+                    format_schedule_data(added_schedule)
+            if 'changed' in data['jschedules']:
+                for changed_schedule in data['jschedules']['changed']:
+                    format_schedule_data(changed_schedule)
+
+        has_connection_str = self.manager.db_info['pgAgent']['has_connstr']
+        if 'jsteps' in data and has_connection_str:
+            if 'changed' in data['jsteps']:
+                for changed_step in data['jsteps']['changed']:
+
+                    if 'jstconntype' not in changed_step and (
+                        'jstdbname' in changed_step or
+                            'jstconnstr' in changed_step):
+                        status, rset = self.conn.execute_dict(
+                            render_template(
+                                "/".join([self.template_path, 'steps.sql']),
+                                jid=data['jobid'],
+                                jstid=changed_step['jstid'],
+                                conn=self.conn,
+                                has_connstr=has_connection_str
+                            )
+                        )
+                        if not status:
+                            return internal_server_error(errormsg=rset)
+
+                        row = rset['rows'][0]
+                        changed_step['jstconntype'] = row['jstconntype']
+                        if row['jstconntype']:
+                            if not ('jstdbname' in changed_step):
+                                changed_step['jstdbname'] = row['jstdbname']
+                        else:
+                            if not ('jstconnstr' in changed_step):
+                                changed_step['jstconnstr'] = row['jstconnstr']
+
 
 JobView.register_node_view(blueprint)
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/__init__.py b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/__init__.py
index c1903188..7b6a5a71 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/__init__.py
@@ -19,6 +19,8 @@ from pgadmin.browser.utils import PGChildNodeView
 from pgadmin.utils.ajax import make_json_response, gone, \
     make_response as ajax_response, internal_server_error
 from pgadmin.utils.driver import get_driver
+from pgadmin.browser.server_groups.servers.pgagent.utils \
+    import format_schedule_data
 
 from config import PG_DEFAULT_DRIVER
 
@@ -294,20 +296,6 @@ class JobScheduleView(PGChildNodeView):
             status=200
         )
 
-    @staticmethod
-    def format_list_data(value):
-        """
-        Converts to proper array data for sql
-        Args:
-            value: data to be converted
-
-        Returns:
-            Converted data
-        """
-        if not isinstance(value, list):
-            return value.replace("[", "{").replace("]", "}")
-        return value
-
     @check_precondition
     def create(self, gid, sid, jid):
         """
@@ -330,27 +318,13 @@ class JobScheduleView(PGChildNodeView):
         else:
             data = json.loads(request.data.decode())
             # convert python list literal to postgres array literal.
-            data['jscminutes'] = JobScheduleView.format_list_data(
-                data['jscminutes']
-            )
-            data['jschours'] = JobScheduleView.format_list_data(
-                data['jschours']
-            )
-            data['jscweekdays'] = JobScheduleView.format_list_data(
-                data['jscweekdays']
-            )
-            data['jscmonthdays'] = JobScheduleView.format_list_data(
-                data['jscmonthdays']
-            )
-            data['jscmonths'] = JobScheduleView.format_list_data(
-                data['jscmonths']
-            )
+            format_schedule_data(data)
 
         sql = render_template(
             "/".join([self.template_path, 'create.sql']),
             jid=jid,
             data=data,
-            fetch_id=False
+            fetch_id=True
         )
 
         status, res = self.conn.execute_void('BEGIN')
@@ -358,7 +332,6 @@ class JobScheduleView(PGChildNodeView):
             return internal_server_error(errormsg=res)
 
         status, res = self.conn.execute_scalar(sql)
-
         if not status:
             if self.conn.connected():
                 self.conn.execute_void('END')
@@ -414,26 +387,7 @@ class JobScheduleView(PGChildNodeView):
         else:
             data = json.loads(request.data.decode())
             # convert python list literal to postgres array literal.
-            if 'jscminutes' in data and data['jscminutes'] is not None:
-                data['jscminutes'] = JobScheduleView.format_list_data(
-                    data['jscminutes']
-                )
-            if 'jschours' in data and data['jschours'] is not None:
-                data['jschours'] = JobScheduleView.format_list_data(
-                    data['jschours']
-                )
-            if 'jscweekdays' in data and data['jscweekdays'] is not None:
-                data['jscweekdays'] = JobScheduleView.format_list_data(
-                    data['jscweekdays']
-                )
-            if 'jscmonthdays' in data and data['jscmonthdays'] is not None:
-                data['jscmonthdays'] = JobScheduleView.format_list_data(
-                    data['jscmonthdays']
-                )
-            if 'jscmonths' in data and data['jscmonths'] is not None:
-                data['jscmonths'] = JobScheduleView.format_list_data(
-                    data['jscmonths']
-                )
+            format_schedule_data(data)
 
         sql = render_template(
             "/".join([self.template_path, 'update.sql']),
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
index a5480ac1..8f6cd285 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/schedules/static/js/pga_schedule.js
@@ -389,9 +389,8 @@ define('pgadmin.node.pga_schedule', [
                 'select2/selectAllAdapter'
               ),
             },
-            selector: weekdays,
             formatter: new BooleanArrayFormatter(weekdays, true),
-            options: BooleanArrayOptions,
+            selector: weekdays, options: BooleanArrayOptions,
           },{
             id: 'jscmonthdays', label: gettext('Month Days'), cell: 'select2',
             group: gettext('Days'), control: 'select2', type: 'array',
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/steps/static/js/pga_jobstep.js b/web/pgadmin/browser/server_groups/servers/pgagent/steps/static/js/pga_jobstep.js
index e0ead97a..fb0624af 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/steps/static/js/pga_jobstep.js
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/steps/static/js/pga_jobstep.js
@@ -123,10 +123,13 @@ define('pgadmin.node.pga_jobstep', [
             var args = arguments && arguments.length > 1 && arguments[1];
 
             if (args) {
-              this.set(
-                'jstdbname',
-                (args['node_info'] || args.collection.top['node_info'])['server']['db']
-              );
+              if (!_.isUndefined(args['node_info']) ||
+                  !_.isUndefined(args.collection.top['node_info'])) {
+                this.set(
+                  'jstdbname',
+                  (args['node_info'] || args.collection.top['node_info'])['server']['db']
+                );
+              }
             }
           }
         },
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_job/sql/pre3.4/update.sql b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_job/sql/pre3.4/update.sql
index 588d81ee..91c3797d 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_job/sql/pre3.4/update.sql
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_job/sql/pre3.4/update.sql
@@ -17,4 +17,14 @@ WHERE jobid = {{ jid }};
 
 {% if 'deleted' in data.jschedules %}{% for schedule in data.jschedules.deleted %}{{ SCHEDULE.DELETE(jid, schedule.jscid) }}{% endfor %}{% endif %}
 {% if 'changed' in data.jschedules %}{% for schedule in data.jschedules.changed %}{{ SCHEDULE.UPDATE(jid, schedule.jscid, schedule) }}{% endfor %}{% endif %}
-{% if 'added' in data.jschedules %}{% for schedule in data.jschedules.added %}{{ SCHEDULE.INSERT(jid, schedule) }}{% endfor %}{% endif %}{% endif %}
+{% if 'added' in data.jschedules %}
+
+DO $$
+DECLARE
+    scid integer;
+BEGIN
+{% for schedule in data.jschedules.added %}{{ SCHEDULE.INSERT(jid, schedule) }}{% endfor %}
+END
+$$;
+{% endif %}
+{% endif %}
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/create.sql b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/create.sql
index 56c3fdab..a4ddbc6f 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/create.sql
@@ -5,4 +5,6 @@ DECLARE
 BEGIN
 {{ SCHEDULE.INSERT(jid, data) }}
 END
-$$;
+$$;{% if fetch_id %}
+
+SELECT jscid FROM pgagent.pga_schedule WHERE xmin::text = (txid_current() % (2^32)::bigint)::text;{% endif %}
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/properties.sql b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/properties.sql
index 6cbd9e47..3700dcd4 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/properties.sql
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/templates/pga_schedule/sql/pre3.4/properties.sql
@@ -1,2 +1,2 @@
 {% import 'macros/pga_schedule.macros' as SCHEDULE %}
-{{ SCHEDULE.PROPERTIES(jid, jstid) }}
+{{ SCHEDULE.PROPERTIES(jid, jscid) }}
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_add.py b/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_add.py
index b7d3cd4e..08424981 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_add.py
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_add.py
@@ -64,7 +64,8 @@ class PgAgentAddTestCase(BaseTestGenerator):
                 'jscmonths': [False] * 12,
                 'jschours': [False] * 24,
                 'jscminutes': [False] * 60,
-                'jscexceptions': [],
+                'jscexceptions': [{'jexdate': '2050-01-01',
+                                   'jextime': '12:00'}],
             }],
         }
 
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_put.py b/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_put.py
index 721c73b7..ac46b6a2 100644
--- a/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_put.py
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/tests/test_pgagent_put.py
@@ -16,7 +16,73 @@ from . import utils as pgagent_utils
 class PgAgentPutTestCase(BaseTestGenerator):
     """This class will test the put pgAgent job API"""
     scenarios = [
-        ('Put pgAgent job', dict(url='/browser/pga_job/obj/'))
+        ('Update pgAgent job description', dict(
+            url='/browser/pga_job/obj/',
+            data={
+                "jobdesc": "This is a test comment",
+            })),
+        ('Update pgagent job add schedule', dict(
+            url='/browser/pga_job/obj/',
+            data={'jschedules': {
+                'added': [{
+                    'jscjobid': '',
+                    'jscenabled': True,
+                    'jscdesc': 'This is a test comment',
+                    'jscname': 'test_sc1',
+                    'jscexceptions': [{'jexdate': '2050-01-01',
+                                       'jextime': '12:30'}],
+                    'jscstart': '2050-01-01 12:14:21 +05:30',
+                    'jscend': '2050-03-01 12:14:21 +05:30',
+                    'jscminutes': [False] * 60,
+                    # Below format is added to test the malformed array
+                    # literal issue.
+                    'jscweekdays': '[false, true, false, true, false, '
+                                   'false, false]',
+                    'jscmonthdays': [False] * 32,
+                    'jschours': [True] * 24,
+                    # Below format is added to test the malformed array
+                    # literal issue.
+                    'jscmonths': '[true, false, false, true, false, false,'
+                                 'true, false, false, true, false, false]'
+                }]
+            }
+            })),
+        ('Update pgagent job add steps with local connection', dict(
+            url='/browser/pga_job/obj/',
+            data={'jsteps': {
+                'added': [{
+                    'jstjobid': '',
+                    'jstname': 'test_st1',
+                    'jstdesc': '',
+                    'jstenabled': True,
+                    'jstkind': True,
+                    'jstconntype': True,
+                    'jstcode': 'SELECT 1;',
+                    'jstconnstr': None,
+                    'jstdbname': 'postgres',
+                    'jstonerror': 'f',
+                    'jstnextrun': '',
+                }]
+            }
+            })),
+        ('Update pgagent job add steps with remote connection', dict(
+            url='/browser/pga_job/obj/',
+            data={'jsteps': {
+                'added': [{
+                    'jstjobid': '',
+                    'jstname': 'test_st1',
+                    'jstdesc': '',
+                    'jstenabled': True,
+                    'jstkind': True,
+                    'jstconntype': False,
+                    'jstcode': 'SELECT 1;',
+                    'jstconnstr': 'host=localhost port=5432 dbname=postgres',
+                    'jstdbname': '',
+                    'jstonerror': 'f',
+                    'jstnextrun': '',
+                }]
+            }
+            })),
     ]
 
     def setUp(self):
@@ -31,16 +97,19 @@ class PgAgentPutTestCase(BaseTestGenerator):
 
     def runTest(self):
         """This function will put pgAgent job"""
-        data = {
-            "jobdesc": "This is a test comment",
-        }
+
+        if 'jschedules' in self.data and 'added' in self.data['jschedules']:
+            self.data['jschedules']['added'][0]['jscjobid'] = self.job_id
+
+        if 'jsteps' in self.data and 'added' in self.data['jsteps']:
+            self.data['jsteps']['added'][0]['jstjobid'] = self.job_id
 
         response = self.tester.put(
             '{0}{1}/{2}/{3}'.format(
                 self.url, str(utils.SERVER_GROUP), str(self.server_id),
                 str(self.job_id)
             ),
-            data=json.dumps(data),
+            data=json.dumps(self.data),
             follow_redirects=True,
             content_type='html/json'
         )
diff --git a/web/pgadmin/browser/server_groups/servers/pgagent/utils.py b/web/pgadmin/browser/server_groups/servers/pgagent/utils.py
new file mode 100644
index 00000000..3fdcf7f9
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/pgagent/utils.py
@@ -0,0 +1,46 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2019, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+"""pgagent helper utilities"""
+
+
+def format_boolean_array(value):
+    """
+    Converts to proper array data for sql
+    Args:
+        value: data to be converted
+
+    Returns:
+        Converted data
+    """
+    if not isinstance(value, list):
+        return value.replace("[", "{").replace("]", "}")
+    return value
+
+
+def format_schedule_data(data):
+    """
+    This function is used to format the schedule data. If data is not an
+    instance of list then format
+    :param data:
+    :return:
+    """
+
+    if 'jscminutes' in data and data['jscminutes'] is not None:
+        data['jscminutes'] = format_boolean_array(data['jscminutes'])
+    if 'jschours' in data and data['jschours'] is not None:
+        data['jschours'] = format_boolean_array(data['jschours'])
+    if 'jscweekdays' in data and data['jscweekdays'] is not None:
+        data['jscweekdays'] = format_boolean_array(data['jscweekdays'])
+    if 'jscmonthdays' in data and data['jscmonthdays'] is not None:
+        data['jscmonthdays'] = format_boolean_array(data['jscmonthdays'])
+    if 'jscmonths' in data and data['jscmonths'] is not None:
+        data['jscmonths'] = format_boolean_array(data['jscmonths'])
+
+    return data


^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: [pgAdmin4][Patch]: RM 4428 and 4448
@ 2019-07-15 14:55  Dave Page <[email protected]>
  parent: Akshay Joshi <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Dave Page @ 2019-07-15 14:55 UTC (permalink / raw)
  To: Akshay Joshi <[email protected]>; +Cc: pgadmin-hackers

Thanks, applied.

On Mon, Jul 15, 2019 at 1:32 PM Akshay Joshi <[email protected]>
wrote:

> Hi Hackers,
>
> Attached is the patch to fix following issues in pgagent module:
>
>    - RM 4428 "malformed array literal error when updating pgagent job".
>    - RM 4448 "Error when updating connection string in pgagent Jobs."
>    - When user create a schedule using Create->Schedule dialog browser
>    tree is not showing newly created node.
>    - Properties tab showing same properties for all the created schedule.
>    - Added validation in "pga_jobstep.js", throws error on browser when
>    we modify step from the pgagent dialog and select the same node.
>
>
> Added regression test cases to add schedule and step from the pgagent job
> itself.
> Please review it.
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2019-07-15 14:55 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 12:32 [pgAdmin4][Patch]: RM 4428 and 4448 Akshay Joshi <[email protected]>
2019-07-15 14:55 ` Dave Page <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox