Merge pull request #3999 from ryanpetrello/fix-system-jobs-362

fix a few bugs with the session and oauth2 cleanup scheduled jobs

awx/api/serializers.py

@@ -4658,6 +4658,10 @@ class ScheduleSerializer(LaunchConfigurationBaseSerializer, SchedulePreviewSeria
 
     def get_summary_fields(self, obj):
         summary_fields = super(ScheduleSerializer, self).get_summary_fields(obj)
+
+        if isinstance(obj.unified_job_template, SystemJobTemplate):
+            summary_fields['unified_job_template']['job_type'] = obj.unified_job_template.job_type
+
         if 'inventory' in summary_fields:
             return summary_fields
 

awx/main/migrations/0078_v360_clear_sessions_tokens_jt.py

@@ -19,11 +19,11 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='systemjob',
             name='job_type',
-            field=models.CharField(blank=True, choices=[('cleanup_jobs', 'Remove jobs older than a certain number of days'), ('cleanup_activitystream', 'Remove activity stream entries older than a certain number of days'), ('clearsessions', 'Removes expired browser sessions from the database'), ('cleartokens', 'Removes expired OAuth 2 access tokens and refresh tokens')], default='', max_length=32),
+            field=models.CharField(blank=True, choices=[('cleanup_jobs', 'Remove jobs older than a certain number of days'), ('cleanup_activitystream', 'Remove activity stream entries older than a certain number of days'), ('cleanup_sessions', 'Removes expired browser sessions from the database'), ('cleanup_tokens', 'Removes expired OAuth 2 access tokens and refresh tokens')], default='', max_length=32),
         ),
         migrations.AlterField(
             model_name='systemjobtemplate',
             name='job_type',
-            field=models.CharField(blank=True, choices=[('cleanup_jobs', 'Remove jobs older than a certain number of days'), ('cleanup_activitystream', 'Remove activity stream entries older than a certain number of days'), ('clearsessions', 'Removes expired browser sessions from the database'), ('cleartokens', 'Removes expired OAuth 2 access tokens and refresh tokens')], default='', max_length=32),
+            field=models.CharField(blank=True, choices=[('cleanup_jobs', 'Remove jobs older than a certain number of days'), ('cleanup_activitystream', 'Remove activity stream entries older than a certain number of days'), ('cleanup_sessions', 'Removes expired browser sessions from the database'), ('cleanup_tokens', 'Removes expired OAuth 2 access tokens and refresh tokens')], default='', max_length=32),
         ),
     ]

awx/main/models/jobs.py

@@ -1103,8 +1103,8 @@ class SystemJobOptions(BaseModel):
     SYSTEM_JOB_TYPE = [
         ('cleanup_jobs', _('Remove jobs older than a certain number of days')),
         ('cleanup_activitystream', _('Remove activity stream entries older than a certain number of days')),
-        ('clearsessions', _('Removes expired browser sessions from the database')),
+        ('cleanup_sessions', _('Removes expired browser sessions from the database')),
-        ('cleartokens', _('Removes expired OAuth 2 access tokens and refresh tokens'))
+        ('cleanup_tokens', _('Removes expired OAuth 2 access tokens and refresh tokens'))
     ]
 
     class Meta:
@@ -1179,18 +1179,19 @@ class SystemJobTemplate(UnifiedJobTemplate, SystemJobOptions):
             for key in unallowed_vars:
                 rejected[key] = data.pop(key)
 
-        if 'days' in data:
+        if self.job_type in ('cleanup_jobs', 'cleanup_activitystream'):
-            try:
+            if 'days' in data:
-                if type(data['days']) is bool:
+                try:
-                    raise ValueError
+                    if isinstance(data['days'], (bool, type(None))):
-                if float(data['days']) != int(data['days']):
+                        raise ValueError
-                    raise ValueError
+                    if float(data['days']) != int(data['days']):
-                days = int(data['days'])
+                        raise ValueError
-                if days < 0:
+                    days = int(data['days'])
-                    raise ValueError
+                    if days < 0:
-            except ValueError:
+                        raise ValueError
-                errors_list.append(_("days must be a positive integer."))
+                except ValueError:
-                rejected['days'] = data.pop('days')
+                    errors_list.append(_("days must be a positive integer."))
+                    rejected['days'] = data.pop('days')
 
         if errors_list:
             errors['extra_vars'] = errors_list

awx/main/tasks.py

@@ -2761,10 +2761,11 @@ class RunSystemJob(BaseTask):
                 json_vars = {}
             else:
                 json_vars = json.loads(system_job.extra_vars)
-            if 'days' in json_vars:
+            if system_job.job_type in ('cleanup_jobs', 'cleanup_activitystream'):
-                args.extend(['--days', str(json_vars.get('days', 60))])
+                if 'days' in json_vars:
-            if 'dry_run' in json_vars and json_vars['dry_run']:
+                    args.extend(['--days', str(json_vars.get('days', 60))])
-                args.extend(['--dry-run'])
+                if 'dry_run' in json_vars and json_vars['dry_run']:
+                    args.extend(['--dry-run'])
             if system_job.job_type == 'cleanup_jobs':
                 args.extend(['--jobs', '--project-updates', '--inventory-updates',
                              '--management-jobs', '--ad-hoc-commands', '--workflow-jobs',

awx/main/tests/unit/models/test_system_jobs.py

@@ -12,7 +12,9 @@ from awx.main.models import SystemJobTemplate
     {"days": 13435},
 ])
 def test_valid__clean_extra_data_system_jobs(extra_data):
-    accepted, rejected, errors = SystemJobTemplate().accept_or_ignore_variables(extra_data)
+    accepted, rejected, errors = SystemJobTemplate(
+        job_type='cleanup_jobs'
+    ).accept_or_ignore_variables(extra_data)
     assert not rejected
     assert not errors
 
@@ -32,12 +34,14 @@ def test_valid__clean_extra_data_system_jobs(extra_data):
     {"days": "foobar"},
 ])
 def test_invalid__extra_data_system_jobs(extra_data):
-    accepted, rejected, errors = SystemJobTemplate().accept_or_ignore_variables(extra_data)
+    accepted, rejected, errors = SystemJobTemplate(
+        job_type='cleanup_jobs'
+    ).accept_or_ignore_variables(extra_data)
     assert str(errors['extra_vars'][0]) == u'days must be a positive integer.'
 
 
 def test_unallowed_system_job_data():
-    sjt = SystemJobTemplate()
+    sjt = SystemJobTemplate(job_type='cleanup_jobs')
     accepted, ignored, errors = sjt.accept_or_ignore_variables({
         'days': 34,
         'foobar': 'baz'
@@ -54,7 +58,7 @@ def test_reject_other_prommpts():
 
 
 def test_reject_some_accept_some():
-    sjt = SystemJobTemplate()
+    sjt = SystemJobTemplate(job_type='cleanup_jobs')
     accepted, ignored, errors = sjt._accept_or_ignore_job_kwargs(limit="", extra_vars={
         'days': 34,
         'foobar': 'baz'

awx/ui/client/src/management-jobs/card/card.controller.js

@@ -31,7 +31,6 @@ export default
                 };
                 getManagementJobs();
 
-                $scope.cleanupJob = true;
                 // This handles the case where the user refreshes the management job notifications page.
                 if($state.current.name === 'managementJobsList.notifications') {
                     $scope.activeCard = parseInt($state.params.management_id);

awx/ui/client/src/management-jobs/scheduler/schedulerForm.partial.html

@@ -164,7 +164,7 @@
                     ng-show="sheduler_frequency_error">
                 </div>
             </div>
-            <div class="form-group SchedulerForm-formGroup" ng-if="cleanupJob">
+            <div class="form-group SchedulerForm-formGroup" ng-if="askDaysToKeep">
                 <label class="Form-inputLabel">
                     <span class="red-text">*</span>
                     <span translate>Days of data to keep</span>

awx/ui/client/src/scheduler/factories/schedule-post.factory.js

@@ -18,7 +18,7 @@ export default
                 scheduleData.rrule = RRuleToAPI(rrule.toString(), scope);
                 scheduleData.description = (/error/.test(rrule.toText())) ? '' : rrule.toText();
 
-                if (scope.cleanupJob) {
+                if (scope.askDaysToKeep) {
                     extra_vars = {
                         "days" : scope.scheduler_form.schedulerPurgeDays.$viewValue
                     };

awx/ui/client/src/scheduler/schedulerAdd.controller.js

@@ -347,7 +347,10 @@ export default ['$filter', '$state', '$stateParams', '$http', 'Wait',
         }
     } else if (base === 'system_job_templates') {
         schedule_url = GetBasePath(base) + $stateParams.id + '/schedules/';
-        $scope.cleanupJob = true;
+        let parentJobType = _.get($scope.parentObject, 'job_type');
+        if (parentJobType !== 'cleanup_tokens' && parentJobType !== 'cleanup_sessions') {
+            $scope.askDaysToKeep = true;
+        }
     }
 
     Wait('start');

awx/ui/client/src/scheduler/schedulerEdit.controller.js

@@ -186,7 +186,10 @@ function($filter, $state, $stateParams, Wait, $scope, moment,
 
         if (_.has(schedule, 'summary_fields.unified_job_template.unified_job_type') &&
             schedule.summary_fields.unified_job_template.unified_job_type === 'system_job'){
-            $scope.cleanupJob = true;
+            let scheduleJobType = _.get(schedule.summary_fields.unified_job_template, 'job_type');
+            if (scheduleJobType !== 'cleanup_tokens' && scheduleJobType !== 'cleanup_sessions') {
+                $scope.askDaysToKeep = true;
+            }
         }
 
         $scope.schedule_obj = scheduleResolve;
@@ -256,7 +259,7 @@ function($filter, $state, $stateParams, Wait, $scope, moment,
         $scope.noVars = true;
         scheduler.scope.timeZones = timezonesResolve;
         scheduler.scope.schedulerTimeZone = scheduleResolve.timezone;
-        if ($scope.cleanupJob){
+        if ($scope.askDaysToKeep){
             $scope.schedulerPurgeDays = Number(schedule.extra_data.days);
         }