From 68975572f37484004916abc704a02e0f5b1e2aa8 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 24 Apr 2018 09:23:08 -0400 Subject: [PATCH 1/2] do not update modified_by for system fields --- awx/main/models/base.py | 12 +++++++++--- awx/main/tests/functional/models/test_job.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/awx/main/models/base.py b/awx/main/models/base.py index 7639bc4548..fcca82474c 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -256,6 +256,7 @@ class PrimordialModel(CreatedModifiedModel): def save(self, *args, **kwargs): update_fields = kwargs.get('update_fields', []) + fields_are_specified = bool(update_fields) user = get_current_user() if user and not user.id: user = None @@ -263,9 +264,14 @@ class PrimordialModel(CreatedModifiedModel): self.created_by = user if 'created_by' not in update_fields: update_fields.append('created_by') - self.modified_by = user - if 'modified_by' not in update_fields: - update_fields.append('modified_by') + # Update modified_by if not called with update_fields, or if any + # editable fields are present in update_fields + if ( + (not fields_are_specified) or + any(getattr(self._meta.get_field(name), 'editable', True) for name in update_fields)): + self.modified_by = user + if 'modified_by' not in update_fields: + update_fields.append('modified_by') super(PrimordialModel, self).save(*args, **kwargs) def clean_description(self): diff --git a/awx/main/tests/functional/models/test_job.py b/awx/main/tests/functional/models/test_job.py index bc166fd77d..ec23045fea 100644 --- a/awx/main/tests/functional/models/test_job.py +++ b/awx/main/tests/functional/models/test_job.py @@ -1,6 +1,7 @@ import pytest from awx.main.models import JobTemplate, Job +from crum import impersonate @pytest.mark.django_db @@ -49,3 +50,18 @@ def test_awx_custom_virtualenv_without_jt(project): job = Job.objects.get(pk=job.id) assert job.ansible_virtualenv_path == '/venv/fancy-proj' + + +@pytest.mark.django_db +def test_update_parent_instance(job_template, alice): + # jobs are launched as a particular user, user not saved as modified_by + with impersonate(alice): + assert job_template.current_job is None + assert job_template.status == 'never updated' + assert job_template.modified_by is None + job = job_template.jobs.create(status='new') + job.status = 'pending' + job.save() + assert job_template.current_job == job + assert job_template.status == 'pending' + assert job_template.modified_by is None From 13550acb9105cdd224776430bb55222bfc2cddeb Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 24 Apr 2018 10:49:04 -0400 Subject: [PATCH 2/2] fix cross-talk between JT-proj due to arg mutability --- awx/main/models/unified_jobs.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index ab03389535..943956f7ac 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -263,14 +263,7 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio if field not in update_fields: update_fields.append(field) # Do the actual save. - try: - super(UnifiedJobTemplate, self).save(*args, **kwargs) - except ValueError: - # A fix for https://trello.com/c/S4rU1F21 - # Does not resolve the root cause. Tis merely a bandaid. - if 'scm_delete_on_next_update' in update_fields: - update_fields.remove('scm_delete_on_next_update') - super(UnifiedJobTemplate, self).save(*args, **kwargs) + super(UnifiedJobTemplate, self).save(*args, **kwargs) def _get_current_status(self): @@ -722,7 +715,10 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique def _get_parent_instance(self): return getattr(self, self._get_parent_field_name(), None) - def _update_parent_instance_no_save(self, parent_instance, update_fields=[]): + def _update_parent_instance_no_save(self, parent_instance, update_fields=None): + if update_fields is None: + update_fields = [] + def parent_instance_set(key, val): setattr(parent_instance, key, val) if key not in update_fields: